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

Add optional namespace to /set_camera_info service in CameraInfoManager #324

Merged
merged 4 commits into from
Sep 25, 2024

Conversation

jasiex01
Copy link
Contributor

@jasiex01 jasiex01 commented Sep 24, 2024

Hi,
In the quite common case that one node serves two cameras and where each camera has its own CameraInfoManager, there will only be one /set_camera_info service that will update both .yaml files which is not ideal.
Up until Jazzy this could have been remedied by using a sub-node with its custom namespace, however since updating the code to work with lifecycle nodes this is not possible anymore.

I have added a ns parameter to CameraInfoManager constructors that will be added as a prefix to the /set_camera_info service during its creation. As the parameter is optional, without specyfing it CameraInfoManager will act as before - the string passed during the creation of the service will be ~/set_camera_info. This ensures backwards compatibility and would not require anybody to modify their code and this would allow bigger control for users over the name of the service.

This would be great to backport to Jazzy

Copy link
Collaborator

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

We cannot backport this PR as it is, the PR breaks ABI, the signature of the constructor has changed

@jasiex01
Copy link
Contributor Author

Fixed the attribute order

@ahcorde
Copy link
Collaborator

ahcorde commented Sep 25, 2024

Pulls: #324
Gist: https://gist.githubusercontent.com/ahcorde/5b8d0bdeb61f02dae474e0e360299b85/raw/3986349c5c747dbb3422ad5cc8b00f3dcad84368/ros2.repos
BUILD args: --packages-above-and-dependencies camera_info_manager --packages-above-and-dependencies camera_info_manager
TEST args: --packages-above camera_info_manager --packages-above camera_info_manager
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14612

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit 3b0a1b6 into ros-perception:rolling Sep 25, 2024
2 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