Catch parameter changes done in INACTIVE#6194
Conversation
aeb7bbd to
c27a4b1
Compare
c27a4b1 to
f3a94eb
Compare
|
This seems to cause a test to fail now: |
|
Also, does this cover all of the potential cases? Maybe I'm mistaken, but it seems to me that there are many more nodes that this may need to be added to? Maybe this should be a nav2_ros_common node utility so its more abstracted so its not so much copy/pasted boilerplate. |
Let's see with the latest commit.
Switching to draft to think more about it and review the code base. I agree that if it is needed in more place, this probably need a utility / refactor. |
|
Sounds good! That now passes |
|
Turns out it is a bit of a can of worms, I will have to go case by case. |
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
Signed-off-by: Guillaume Doisy <guillaume@dexory.com>
7fa4367 to
0a89a0d
Compare
Basic Info
Description of contribution in a few bullet points
Many Nav2 lifecycle nodes register the on-set parameter callback only in
activate()/on_activate(). As a result, anyset_parameterscall made while a node is INACTIVE updates the parameter store but leaves the cached members stale The value isn't picked up until that same parameter is set again after the node is active. Symptom:ros2 param getreports the new value while the node keeps running on the old one.Fix
After re-registering the callbacks on activation, replay the current parameter store through the node's existing
updateParametersCallbackto resync the cache. Declared but uninitialized parameters (PARAMETER_NOT_SET) are skipped, since a bulk get on those throwsParameterUninitializedException.Description of documentation updates required from your changes
Description of how this change was tested
I first made the change to nav2_util/parameter_handler and velocity_smoother.cpp for the behavior I needed to fix. And I extensively tested these on a real robot running continuously for the last weeks.
Then I did the same for the other nodes that could be affected. And for these last ones, I did not test as extensively, so a proper second pair of eyes (and a CI run) would be appreciated.
Future work that may be required in bullet points
For Maintainers:
backport-*.