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

Ros2 Diagnostics - Fix for Frame_id naming #640

Merged
merged 6 commits into from
Dec 20, 2024
Merged

Conversation

dwffls
Copy link
Contributor

@dwffls dwffls commented Dec 5, 2024

Overview

Author: Daan Wijffels

Changes

ROS distro: Tested on Iron
List of changes:

  • Ros2 Diagnostics uses stats to provide additional information, i fixed so that it conforms to the standards
  • When using the camera node to publish tf frames, the frame_id's used in the messages we're not correctly adjusted. this pr fixes this and adds the base_name to the frameNames

Testing

Hardware used: OAK-D-SR

@dwffls
Copy link
Contributor Author

dwffls commented Dec 6, 2024

I know these should have probably been two seperate PR's but pushed by accident to the same branch.
Hope these can still be merged, otherwise al make a seperate PR

@dwffls dwffls changed the title Fixed ros2 diagnostics syntax Ros2 Diagnostics - Fix for Frame_id naming Dec 6, 2024
Copy link
Collaborator

@Serafadam Serafadam left a comment

Choose a reason for hiding this comment

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

Hi, regarding formatting it looks okay, but regarding the base frame I would prefer not to include CameraParamHandler in BaseNode but instead follow similar logic to how rsCompatibilityMode is obtained

@dwffls
Copy link
Contributor Author

dwffls commented Dec 11, 2024

Created a tfPrefix function in sensors_helpers to fix it.
If anything else needs changing let met know!

@Serafadam Serafadam merged commit fe5285a into luxonis:humble Dec 20, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants