Make the order of parameters deterministic

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.
This commit is contained in:
Michal Sojka 2024-04-28 19:30:44 +02:00 committed by Martin Dahl
parent b46e3744b2
commit e9f375e8f6
5 changed files with 13 additions and 9 deletions

View File

@ -27,6 +27,7 @@ uuid = { version = "1.2.2", features = ["serde", "v4"] }
futures = "0.3.25"
log = "0.4.18"
phf = "0.11.1"
indexmap = "2.2.6"
[dev-dependencies]
serde_json = "1.0.89"

View File

@ -71,8 +71,9 @@
//! }
//! ```
// otherwise crates using r2r needs to specify the same version of uuid as
// otherwise crates using r2r needs to specify the same version of indexmap and uuid as
// this crate depend on, which seem like bad user experience.
pub extern crate indexmap;
pub extern crate uuid;
mod error;

View File

@ -3,6 +3,7 @@ use futures::{
future::{self, join_all, FutureExt, TryFutureExt},
stream::{Stream, StreamExt},
};
use indexmap::IndexMap;
use std::{
collections::HashMap,
ffi::{CStr, CString},
@ -43,7 +44,7 @@ use crate::{
pub struct Node {
context: Context,
/// ROS parameters.
pub params: Arc<Mutex<HashMap<String, Parameter>>>,
pub params: Arc<Mutex<IndexMap<String, Parameter>>>,
pub(crate) node_handle: Box<rcl_node_t>,
// the node owns the subscribers
pub(crate) subscribers: Vec<Box<dyn Subscriber_>>,
@ -198,7 +199,7 @@ impl Node {
};
let mut node = Node {
params: Arc::new(Mutex::new(HashMap::new())),
params: Arc::new(Mutex::new(IndexMap::new())),
context: ctx,
node_handle,
subscribers: Vec::new(),
@ -460,7 +461,7 @@ impl Node {
fn handle_list_parameters(
req: ServiceRequest<rcl_interfaces::srv::ListParameters::Service>,
params: &Arc<Mutex<HashMap<String, Parameter>>>,
params: &Arc<Mutex<IndexMap<String, Parameter>>>,
) -> future::Ready<()> {
use rcl_interfaces::srv::ListParameters;
@ -503,7 +504,7 @@ impl Node {
fn handle_desc_parameters(
req: ServiceRequest<rcl_interfaces::srv::DescribeParameters::Service>,
params: &Arc<Mutex<HashMap<String, Parameter>>>,
params: &Arc<Mutex<IndexMap<String, Parameter>>>,
) -> future::Ready<()> {
use rcl_interfaces::{msg::ParameterDescriptor, srv::DescribeParameters};
let mut descriptors = Vec::<ParameterDescriptor>::new();

View File

@ -1,7 +1,8 @@
use crate::{Error, Result};
use std::{collections::HashMap, ffi::CStr};
use std::ffi::CStr;
use crate::msg_types::generated_msgs::rcl_interfaces;
use indexmap::IndexMap;
use r2r_rcl::*;
/// ROS parameter value.
@ -190,7 +191,7 @@ impl Parameter {
/// `parameters_derive.rs` example.
pub trait RosParams {
fn register_parameters(
&mut self, prefix: &str, param: Option<Parameter>, params: &mut HashMap<String, Parameter>,
&mut self, prefix: &str, param: Option<Parameter>, params: &mut IndexMap<String, Parameter>,
) -> Result<()>;
fn get_parameter(&mut self, param_name: &str) -> Result<ParameterValue>;
fn set_parameter(&mut self, param_name: &str, param_val: &ParameterValue) -> Result<()>;
@ -202,7 +203,7 @@ macro_rules! impl_ros_params {
impl RosParams for $type {
fn register_parameters(
&mut self, prefix: &str, param: Option<Parameter>,
params: &mut HashMap<String, Parameter>,
params: &mut IndexMap<String, Parameter>,
) -> Result<()> {
if let Some(cli_param) = params.get(prefix) {
// Apply parameter value if set from command line or launch file

View File

@ -26,7 +26,7 @@ pub fn derive_r2r_params(input: proc_macro::TokenStream) -> proc_macro::TokenStr
&mut self,
prefix: &str,
desc: ::std::option::Option<::r2r::Parameter>,
params: &mut ::std::collections::hash_map::HashMap<String, ::r2r::Parameter>,
params: &mut ::r2r::indexmap::IndexMap<String, ::r2r::Parameter>,
) -> ::r2r::Result<()> {
let prefix = if prefix.is_empty() {
String::from("")