Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mcap info prints channel metadata #1000

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

b-camacho
Copy link
Contributor

@b-camacho b-camacho commented Oct 27, 2023

Public-Facing Changes

mcap info did not show channel metadata. Now it will print (up to 50 characters of) json-marshalled channel metadata.

Description

Example output after this change on tests/conformance/data/TenMessages/TenMessages-rch-rsh-st.mcap:

library:
profile:
messages:  10
duration:  9ns
start:     0.000000000
end:       0.000000009
channels:
	(1) example  10 msgs (1111111111.11 Hz)  {"foo":"bar"}   : Example [c]
attachments: 0
metadata: 0

Before this change:

library:
profile:
messages:  10
duration:  9ns
start:     0.000000000
end:       0.000000009
channels:
	(1) example  10 msgs (1111111111.11 Hz)  : Example [c]
attachments: 0
metadata: 0

For channels that don't include metadata, there's no change in behavior

Motivation

At work, we use channel metadata to record which physical device a channel was recorded on. This means multiple channels can exist in the same mcap file with the same topic name. It's hard to distinguish between them without the channel metadata.

@b-camacho
Copy link
Contributor Author

I really care about channel metadata being shown somehow in mcap info, but I'm not very opinionated on how - happy to change the format to be shown after the channel listing, or hide it behind a flag. I'm pretty happy with the current impl though

@CLAassistant
Copy link

CLAassistant commented Oct 27, 2023

CLA assistant check
All committers have signed the CLA.

@james-rms
Copy link
Collaborator

IMO:

  1. lets hide this behind a flag, once this is done you can set the behavior your want permanently in $HOME/.mcap.yaml. I suspect a lot of ros 2 users will only see {"offered_qos_profiles": \"{\"history\":\"unknown\"...., they won't want this by default.
  2. I think you can afford to take up more space to show the metadata, especially if it's opt-in behavior. My first guess would be to show the metadata indented on new lines below the channel, and let long keys / values wrap.

@wkalt
Copy link
Contributor

wkalt commented Oct 30, 2023

@b-camacho the mcap list channels command shows the full metadata - have you found that insufficient?

I'm kind of skeptical this is a good feature for "info" - metadata can contain weird stuff like escaped yaml strings, not just simple key/values, that will make the output much worse unless you're asking for it. And seems like if you're asking for it, you need all of it.

I wonder if maybe a better direction would be support for,

  • --format json, applied to info, list channels, list schemas, list chunks, etc. This would improve the experience for users trying to consume these programmatically.
  • --extended flag on the info command, which effectively dumps all of the above into a single JSON document to summarize the file. Users could then interrogate the data, store the document in databases, render it on webpages, etc.

Seems like the combination of those would give you what you want (single call with all the info) in a more programmatically-consumable way - though maybe list channels gives you enough already. What do you think?

@wkalt
Copy link
Contributor

wkalt commented Oct 30, 2023

for ease of discussion, here's the "list channels" output for a converted ros1 bag, which contains all the stuff required in channel info for the ros1 mcap profile:

$ mcap list channels ~/data/bags/cal_loop.mcap
id  schemaId  topic                                             messageEncoding  metadata
0   1         /can_bus_dbw/can_rx                               ros1             {"callerid":"/can_node","latching":"0","md5sum":"33747cb98e223cafb806d7e94cb4071f","topic":"/can_bus_dbw/can_rx"}
1   2         /center_camera/camera_info                        ros1             {"callerid":"/center_camera/center_camera_nodelet_manager","latching":"0","md5sum":"c9a58c1b0b154e0e6da7578cb991d214","topic":"/center_camera/camera_info"}
2   3         /imu/data                                         ros1             {"callerid":"/xsens_driver","latching":"0","md5sum":"6a62c6daae103f4ff57a132d6f95cec2","topic":"/imu/data"}
3   4         /ecef/                                            ros1             {"callerid":"/xsens_driver","latching":"0","md5sum":"c63aecb41bfdfd6b7e1fac37c7cbe7bf","topic":"/ecef"}
4   5         /fix                                              ros1             {"callerid":"/xsens_driver","latching":"0","md5sum":"2d3a8cd499b9b4a0249fb98fd05cfa48","topic":"/fix"}
5   6         /vehicle/dbw_enabled                              ros1             {"callerid":"/vehicle/dbw_node","latching":"1","md5sum":"8b94c1b53db61fb6aed406028ad6332a","topic":"/vehicle/dbw_enabled"}
6   7         /vehicle/filtered_accel                           ros1             {"callerid":"/vehicle/twist_controller","latching":"0","md5sum":"fdb28210bfa9d7c91146260178d9a584","topic":"/vehicle/filtered_accel"}
7   8         /vehicle/joint_states                             ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"3066dcd76a6cfaef579bd0f34173e9fd","topic":"/vehicle/joint_states"}
8   9         /time_reference                                   ros1             {"callerid":"/xsens_driver","latching":"0","md5sum":"fded64a0265108ba86c3d38fb11c0c16","topic":"/time_reference"}
9   10        /center_camera/image_color/compressed             ros1             {"callerid":"/center_camera/center_camera_nodelet_manager","latching":"0","md5sum":"8f7a12909da2c9d3332d540a0977563f","topic":"/center_camera/image_color/compressed"}
10  11        /pressure                                         ros1             {"callerid":"/xsens_driver","latching":"0","md5sum":"804dc5cea1c5306d6a2eb80b9833befe","topic":"/pressure"}
11  12        /vehicle/twist_controller/parameter_descriptions  ros1             {"callerid":"/vehicle/twist_controller","latching":"1","md5sum":"757ce9d44ba8ddd801bb30bc456f946f","topic":"/vehicle/twist_controller/parameter_descriptions"}
12  13        /vehicle/twist_controller/parameter_updates       ros1             {"callerid":"/vehicle/twist_controller","latching":"1","md5sum":"958f16a05573709014982821e6822580","topic":"/vehicle/twist_controller/parameter_updates"}
13  14        /vehicle/gear_report                              ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"f33342dfeb80c29d8fe4b31e22519594","topic":"/vehicle/gear_report"}
14  15        /vehicle/wheel_speed_report                       ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"a2c91f746e5d8bec139c834f92ac7468","topic":"/vehicle/wheel_speed_report"}
15  16        /vehicle/throttle_info_report                     ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"8255d20d2bbc661ad39074024259c71a","topic":"/vehicle/throttle_info_report"}
16  17        /vehicle/brake_report                             ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"a306c167d365176ae6159e3c4e3f3197","topic":"/vehicle/brake_report"}
17  18        /vehicle/imu/data_raw                             ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"6a62c6daae103f4ff57a132d6f95cec2","topic":"/vehicle/imu/data_raw"}
18  19        /vehicle/throttle_report                          ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"a7fd7b93c8549e83c319e38a18f6dbdc","topic":"/vehicle/throttle_report"}
19  20        /right_camera/camera_info                         ros1             {"callerid":"/right_camera/right_camera_nodelet_manager","latching":"0","md5sum":"c9a58c1b0b154e0e6da7578cb991d214","topic":"/right_camera/camera_info"}
20  21        /right_camera/image_color/compressed              ros1             {"callerid":"/right_camera/right_camera_nodelet_manager","latching":"0","md5sum":"8f7a12909da2c9d3332d540a0977563f","topic":"/right_camera/image_color/compressed"}
21  22        /vehicle/misc_1_report                            ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"9ecd16fb81815b3e46e0550feea1da2f","topic":"/vehicle/misc_1_report"}
22  23        /vehicle/brake_info_report                        ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"fc88af128b5b3213ea25ab325a9b3bbb","topic":"/vehicle/brake_info_report"}
23  24        /left_camera/camera_info                          ros1             {"callerid":"/left_camera/left_camera_nodelet_manager","latching":"0","md5sum":"c9a58c1b0b154e0e6da7578cb991d214","topic":"/left_camera/camera_info"}
24  25        /vehicle/steering_report                          ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"25bf2c220d904531d8bc16ab5271325d","topic":"/vehicle/steering_report"}
25  26        /left_camera/image_color/compressed               ros1             {"callerid":"/left_camera/left_camera_nodelet_manager","latching":"0","md5sum":"8f7a12909da2c9d3332d540a0977563f","topic":"/left_camera/image_color/compressed"}
26  27        /vehicle/suspension_report                        ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"a2c91f746e5d8bec139c834f92ac7468","topic":"/vehicle/suspension_report"}
27  28        /vehicle/fuel_level_report                        ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"f5ec1964dbda02fda82785b8035744e4","topic":"/vehicle/fuel_level_report"}
28  29        /velodyne_packets                                 ros1             {"callerid":"/velodyne_nodelet_manager","latching":"0","md5sum":"50804fc9533a0e579e6322c04ae70566","topic":"/velodyne_packets"}
29  30        /vehicle/sonar_cloud                              ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"1158d486dd51d683ce2f1be655c3c181","topic":"/vehicle/sonar_cloud"}
30  31        /vehicle/surround_report                          ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"17a8c9ed72da4f55d44d6d71483cf0e3","topic":"/vehicle/surround_report"}
31  32        /vehicle/gps/fix                                  ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"2d3a8cd499b9b4a0249fb98fd05cfa48","topic":"/vehicle/gps/fix"}
32  33        /vehicle/gps/time                                 ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"fded64a0265108ba86c3d38fb11c0c16","topic":"/vehicle/gps/time"}
33  34        /vehicle/gps/vel                                  ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"98d34b0043a2093cf9d9345ab6eef12e","topic":"/vehicle/gps/vel"}
34  35        /vehicle/tire_pressure_report                     ros1             {"callerid":"/vehicle/dbw_node","latching":"0","md5sum":"a2c91f746e5d8bec139c834f92ac7468","topic":"/vehicle/tire_pressure_report"}
35  36        /diagnostics                                      ros1             {"callerid":"/center_camera/center_camera_nodelet_manager","latching":"0","md5sum":"60810da900de1dd6ddd437c3503511da","topic":"/diagnostics"}

@b-camacho
Copy link
Contributor Author

@wkalt

I didn't know about list channels! It's very helpful to have the channel metadata available.

To make the discussion more concrete, consider this use case:
An mcap file from a malfunctioning system contains data from 3 robots. As a sanity check, a user runs mcap info to see if all robots are publishing poses at the expected rate.
Currently, since the origin info is stored in channel metadata, users would have to join mcap list channels output with mcap info.

I like the solution you proposed where the existing --json flag also works on mcap info and mcap list, as a new --extended flag makes mcap info output cover channel metadata. I'm gonna assume mcap info --extended without json should print the entire contents of channel metadata inline. I'll make the changes on this PR once I get some more time

@defunctzombie
Copy link
Contributor

@james-rms what decisions are left on this PR to get this feature merged? seems like we need to decide on the feature flag. Might I propose --show-channel-metadata?

@b-camacho
Copy link
Contributor Author

Ah, I think we decided to add a bigger rework of the --json flag to this PR, I haven't gotten the chance to do it yet, but I'll try to get something in tomorrow - then the maintainers can take another look

@defunctzombie
Copy link
Contributor

Seems like this work has gone stale? Do we still want to keep it open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants