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

Cannot create subscription/publisher with QoS override parameter via parameter callback. #2741

Open
fujitatomoya opened this issue Feb 12, 2025 · 4 comments · May be fixed by #2742
Open

Cannot create subscription/publisher with QoS override parameter via parameter callback. #2741

fujitatomoya opened this issue Feb 12, 2025 · 4 comments · May be fixed by #2742
Assignees

Comments

@fujitatomoya
Copy link
Collaborator

Description

it is not uncommon to create or destroy either subscription or publisher when the parameter is updated.
for example, having /enable_data_generator parameter in sensor node, and sensor node watches parameter activity with post set parameter callback using add_post_set_parameters_callback. and once that is enabled, sensor node creates the publisher to start publishing sensing data. (or vice-verse)

there is no problem creating publisher or subscription via parameter callback without QoS override parameters for publisher or subscription. but with QoS override parameters, this does not allow us to create publisher or subscription. because parameter set or declare operation is protected by ParameterMutationRecursionGuard.

bool parameter_modification_enabled_{true};

this is blocking user application to create either publisher or subscription via parameter callback.

Reproducible Sample

https://github.com/fujitatomoya/ros2_test_prover/blob/master/prover_rclcpp/src/create_sub_via_param.cpp

### this nodes waits until the parameter update, and then tries to create the subscription during parameter post set callback
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 run prover_rclcpp create_sub_via_param
terminate called after throwing an instance of 'rclcpp::exceptions::ParameterModifiedInCallbackException'
  what():  cannot set or declare a parameter, or change the callback from within set callback
[ros2run]: Aborted

### this command never comes back, since it actually kills the server node
root@tomoyafujita:~/ros2_ws/colcon_ws# ros2 param set /test_node create_sub true

Additional Information

related to https://github.com/ros2/rclcpp/pull/2378/files#r1411471264

this issue is blocking the enhancement PR #2378,
that using NodeParameterInterface instead of /parameter_event provides significant efficiency especially ROS 2 system has many nodes in the network. with this fix, we do not need to receive all the events and filter the node and use_sim_time parameter from /parameter_event topic.

@ros-discourse
Copy link

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-pmc-meeting-minutes-2025-01-18/42127/1

@SteveMacenski
Copy link
Collaborator

Interestingly enough, we just hit this today in Nav2 with a community PR ros-navigation/navigation2#4968

@fujitatomoya
Copy link
Collaborator Author

@SteveMacenski do you think #2742 makes sense for your use case?

@SteveMacenski
Copy link
Collaborator

I believe so!

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 a pull request may close this issue.

3 participants