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

Update "fetching_messages" example #115

Merged

Conversation

bryfox
Copy link
Contributor

@bryfox bryfox commented Dec 16, 2024

Changelog

None

Docs

None

Description

This updates the "fetching_messages" example to remove a deprecated method call in favor of the streaming iter_messages. Instead of printing the length, it prints the timestamp, channel, and topic of each message; for example:

1616425609698483083 | /gps/navheading          | sensor_msgs/Imu
1616425609700292327 | /gps/fix_velocity        | geometry_msgs/TwistWithCovarianceStamped

Copy link

linear bot commented Dec 16, 2024

@@ -9,7 +9,7 @@ on:

jobs:
python:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Staying on 22 for now to avoid build errors: https://github.com/foxglove/foxglove-python/actions/runs/12356809802/job/34535840957. Runners have been rolling out 24.04 as "latest" this month.

@@ -9,10 +9,9 @@

# Make sure you've imported either the mcap-ros1-support or mcap-protobuf-support
# libraries before making this call in order to get decoded messages.
messages = client.get_messages(
for schema, channel, message, decoded_message in client.iter_messages(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we do anything with decoded_message in this example?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started to, but doing anything useful seemed dependent on the data and implementation (per comment above). I could prefix with an underscore to indicate it's unused?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I think it's fine to leave it as is to show it's there

Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested, works for me

FYI, the first line import datetime can be deleted - it was showing a lint error for me


print(f"downloaded {len(messages)} messages")
):
print(f"[{message.log_time}] Channel: {channel.topic} Schema: {schema.name}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: formatting of the labels feels a bit hard to read for me:

[1479512770391847576] Channel: /imu/data Schema: sensor_msgs/Imu
[1479512770391861165] Channel: /time_reference Schema: sensor_msgs/TimeReference
[1479512770391922289] Channel: /time_reference Schema: sensor_msgs/TimeReference
[1479512770391937993] Channel: /pressure Schema: sensor_msgs/FluidPressure

Maybe something simpler?

Suggested change
print(f"[{message.log_time}] Channel: {channel.topic} Schema: {schema.name}")
print(f"{message.log_time} | {channel.topic} \t| {schema.name}")

i.e.

1479512810369905916 | /fix	| sensor_msgs/NavSatFix
1479512810369936841 | /ecef/	| geometry_msgs/PointStamped
1479512810370480491 | /time_reference	| sensor_msgs/TimeReference
1479512810370530865 | /time_reference	| sensor_msgs/TimeReference
1479512810370682589 | /imu/data	| sensor_msgs/Imu
1479512810370709498 | /time_reference	| sensor_msgs/TimeReference
1479512810370745760 | /fix	| sensor_msgs/NavSatFix

@bryfox
Copy link
Contributor Author

bryfox commented Dec 17, 2024

FYI, the first line import datetime can be deleted - it was showing a lint error for me

Thanks; CI now lints the examples as well.

@bryfox bryfox merged commit 72fdbeb into main Dec 17, 2024
1 check passed
@bryfox bryfox deleted the bryan/fg-9176-fetching_messagespy-example-uses-deprecated-method branch December 17, 2024 17:29
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.

3 participants