ros2 / rcl_interfaces

A repository for messages and services used by the ROS client libraries
Apache License 2.0
38 stars 42 forks source link

Update the comments for SetParametersResult to reflect reality. #159

Closed clalancette closed 1 year ago

clalancette commented 1 year ago

One of the ways that parameter setting can fail is if a user callback (registered with add_on_set_parameters_callback()) rejects it. These callbacks are chained in a Last-In-First-Called manner.

Another piece of information to be aware of is that the call to "set_parameters_atomically" has a single SetParametersResult return value.

Combining the above two pieces of information, then when "set_parameters_atomically" is called and one of the callbacks rejects it, it can easily set the reason string and return that to the user since no further callbacks are going to be called. However, if a callback accepts it but expects to set a reason code, one of the later callbacks can easily overwrite that return code.

Because of this, change the documentation to reflect the reality that a 'reason' can only be specified for the failure case. On success, 'reason' is undefined.

This should fix https://github.com/ros2/rclcpp/issues/2269

clalancette commented 1 year ago

CI:

clalancette commented 1 year ago

I'm going to go ahead and merge this one in; we can always change it again based on further feedback.