Make the order of parameters deterministic #94
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GUI tools for working with parameters (at least Foxglove Studio, rqt and rig_reconfigure) show the parameters in the same order as returned by the ListParameters service. r2r stores the parameters in a HashMap, which iterates keys and values in arbitrary order. The result is that the GUI tools show the parameters in different order after every node invocation, which can be quite annoying. To make the order deterministic, we change the parameter storage from HashMap to IndexMap, which iterates the map in insertion order. According to the indexmap documentation, IndexMap is a drop-in replacement of HashMap so this change should not require code changes in applications using r2r. At least r2r examples and my projects needed no changes.
The previous commit made the order of parameters deterministic (defined by so called insertion order), but the order still depends on whether or not some parameters were set from the command line. Command line parameters are inserted to the IndexMap at Node creation time, while other RosParams-derived parameters later, during the call to make_derived_parameter_handler(). This commit ensures that parameters processed by make_derived_parameter_handler() are always inserted in the same order, even if some of them were already inserted before due to CLI arguments.
|
Makes sense and looks good, merging. Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GUI tools for working with parameters (at least Foxglove Studio, rqt and rig_reconfigure) show the parameters in the same order as returned by the ListParameters service. r2r stores the parameters in a HashMap, which iterates keys and values in arbitrary order. The result is that the GUI tools show the parameters in different order after every node invocation, which can be quite annoying.
To make the order deterministic, we change the parameter storage from HashMap to IndexMap, which iterates the map in insertion order. According to the indexmap documentation, IndexMap is a drop-in replacement of HashMap so this change should not require code changes in applications using r2r. At least r2r examples and my projects needed no changes.