Skip to content

Commit

Permalink
Derive ROS parameter description from field doc comments
Browse files Browse the repository at this point in the history
With this change, adding doc comments to fields of structures used
with `#[derive(RosParams)]` results in those comments to be used as
parameter description.

See r2r/examples/parameters_derive.rs for how to use and test this
feature.

The advantage of this implementation is that it should be backward
compatible with all current users of r2r crate. However, it has also
some disadvantages:

- There is no API for setting parameter description without using
  RosParams trait. Older apps should either be ported to use RosParams
  or they cannot benefit from this.
- The implementation could be more efficient, if the compatibility is
  broken. Currently, we have two HashMaps - one for ParameterValues
  and another for ParameterDescriptors. It might be better to change
  Node::params to a HashMap whose values contain both ParameterValue
  and ParameterDescriptior. This way, descriptors would be also
  available to applications, which might (or might not) be useful.
  • Loading branch information
wentasah committed Sep 26, 2023
1 parent 48423f0 commit f0d4da3
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 21 deletions.
14 changes: 14 additions & 0 deletions r2r/examples/parameters_derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ use std::sync::{Arc, Mutex};
// par1: 5.1
// par2: 0
//
// ros2 param describe /demo/my_node par1 nested.nested2.par5
// Prints:
// Parameter name: par1
// Type: double
// Description: Parameter description
// Constraints:
// Parameter name: nested.nested2.par5
// Type: integer
// Description: Small parameter
// Constraints:

// Error handling:
// cargo run --example parameters_derive -- --ros-args -p nested.par4:=xxx

Expand All @@ -31,7 +42,9 @@ use std::sync::{Arc, Mutex};

#[derive(RosParams, Default, Debug)]
struct Params {
/// Parameter description
par1: f64,
/// Dummy parameter [m/s]
par2: i32,
nested: NestedParams,
}
Expand All @@ -45,6 +58,7 @@ struct NestedParams {

#[derive(RosParams, Default, Debug)]
struct NestedParams2 {
/// Small parameter
par5: i8,
}

Expand Down
2 changes: 1 addition & 1 deletion r2r/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ mod context;
pub use context::Context;

mod parameters;
pub use parameters::{ParameterValue, RosParams};
pub use parameters::{ParameterDescriptor, ParameterValue, RosParams};

mod clocks;
pub use clocks::{Clock, ClockType};
Expand Down
41 changes: 22 additions & 19 deletions r2r/src/nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,15 @@ impl Node {
&mut self, params_struct: Option<Arc<Mutex<dyn RosParams + Send>>>,
) -> Result<(impl Future<Output = ()> + Send, impl Stream<Item = (String, ParameterValue)>)>
{
let mut descs = HashMap::new();
if let Some(ps) = &params_struct {
// register all parameters
ps.lock()
.unwrap()
.register_parameters("", &mut self.params.lock().unwrap())?;
ps.lock().unwrap().register_parameters(
"",
&mut self.params.lock().unwrap(),
ParameterDescriptor::default(),
&mut descs,
)?;
}
let mut handlers: Vec<std::pin::Pin<Box<dyn Future<Output = ()> + Send>>> = Vec::new();
let (mut event_tx, event_rx) = mpsc::channel::<(String, ParameterValue)>(10);
Expand Down Expand Up @@ -363,7 +367,7 @@ impl Node {
let params = self.params.clone();
let desc_params_future = desc_params_request_stream.for_each(
move |req: ServiceRequest<DescribeParameters::Service>| {
Self::handle_desc_parameters(req, &params)
Self::handle_desc_parameters(req, &params, &descs)
},
);

Expand Down Expand Up @@ -446,25 +450,24 @@ impl Node {
fn handle_desc_parameters(
req: ServiceRequest<rcl_interfaces::srv::DescribeParameters::Service>,
params: &Arc<Mutex<HashMap<String, ParameterValue>>>,
descs: &HashMap<String, ParameterDescriptor>,
) -> future::Ready<()> {
use rcl_interfaces::msg::ParameterDescriptor;
use rcl_interfaces::msg;
use rcl_interfaces::srv::DescribeParameters;
let mut descriptors = Vec::<ParameterDescriptor>::new();
let mut descriptors = Vec::<msg::ParameterDescriptor>::new();
let params = params.lock().unwrap();
for name in &req.message.names {
if let Some(pv) = params.get(name) {
descriptors.push(ParameterDescriptor {
name: name.clone(),
type_: pv.into_parameter_type(),
..Default::default()
});
} else {
// parameter not found, but undeclared allowed, so return empty
descriptors.push(ParameterDescriptor {
name: name.clone(),
..Default::default()
});
}
let default = ParameterDescriptor::default();
let d = descs.get(name).unwrap_or(&default);
descriptors.push(msg::ParameterDescriptor {
name: name.clone(),
type_: params
.get(name)
.unwrap_or(&ParameterValue::NotSet)
.into_parameter_type(),
description: d.description.to_string(),
..Default::default()
});
}
req.respond(DescribeParameters::Response { descriptors })
.expect("could not send reply to describe parameters request");
Expand Down
11 changes: 11 additions & 0 deletions r2r/src/parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ impl ParameterValue {
}
}

/// ROS parameter descriptor.
#[derive(Default)]
pub struct ParameterDescriptor {
pub description: &'static str,
// TODO: Add other fields like min, max, step. Use field
// attributes for defining them.
}

/// Trait for use it with
/// [`Node::make_derived_parameter_handler()`](crate::Node::make_derived_parameter_handler()).
///
Expand All @@ -168,6 +176,7 @@ impl ParameterValue {
pub trait RosParams {
fn register_parameters(
&mut self, prefix: &str, params: &mut HashMap<String, ParameterValue>,
desc: ParameterDescriptor, descs: &mut HashMap<String, ParameterDescriptor>,
) -> Result<()>;
fn get_parameter(&mut self, param_name: &str) -> Result<ParameterValue>;
fn set_parameter(&mut self, param_name: &str, param_val: &ParameterValue) -> Result<()>;
Expand All @@ -179,6 +188,7 @@ macro_rules! impl_ros_params {
impl RosParams for $type {
fn register_parameters(
&mut self, prefix: &str, params: &mut HashMap<String, ParameterValue>,
desc: ParameterDescriptor, descs: &mut HashMap<String, ParameterDescriptor>,
) -> Result<()> {
if let Some(param_val) = params.get(prefix) {
// Apply parameter value if set from command line or launch file
Expand All @@ -188,6 +198,7 @@ macro_rules! impl_ros_params {
// Insert missing parameter with its default value
params.insert(prefix.to_owned(), $param_value_type($to_param_conv(self)?));
}
descs.insert(prefix.to_owned(), desc);
Ok(())
}

Expand Down
26 changes: 25 additions & 1 deletion r2r_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub fn derive_r2r_params(input: proc_macro::TokenStream) -> proc_macro::TokenStr
&mut self,
prefix: &str,
params: &mut ::std::collections::hash_map::HashMap<String, ::r2r::ParameterValue>,
desc: ::r2r::ParameterDescriptor,
descs: &mut ::std::collections::hash_map::HashMap<String, ::r2r::ParameterDescriptor>,
) -> ::r2r::Result<()> {
let prefix = if prefix.is_empty() {
String::from("")
Expand Down Expand Up @@ -80,9 +82,13 @@ fn get_register_calls(data: &Data) -> TokenStream {
let field_matches = fields.named.iter().map(|f| {
let name = &f.ident;
let format_str = format!("{{prefix}}{}", name.as_ref().unwrap());
let desc = get_field_doc(f);
quote_spanned! {
f.span() =>
self.#name.register_parameters(&format!(#format_str), params)?;
let desc = ::r2r::ParameterDescriptor {
description: #desc,
};
self.#name.register_parameters(&format!(#format_str), params, desc, descs)?;
}
});
quote! {
Expand All @@ -95,6 +101,24 @@ fn get_register_calls(data: &Data) -> TokenStream {
}
}

fn get_field_doc(f: &syn::Field) -> String {
if let Some(doc) = f
.attrs
.iter()
.find(|&attr| attr.path().get_ident().is_some_and(|id| id == "doc"))
{
match &doc.meta.require_name_value().unwrap().value {
::syn::Expr::Lit(exprlit) => match &exprlit.lit {
::syn::Lit::Str(s) => s.value().trim().to_owned(),
_ => unimplemented!(),
},
_ => unimplemented!(),
}
} else {
"".to_string()
}
}

// Generate match arms for RosParams::update_parameters()
fn param_matches_for(call: TokenStream, data: &Data) -> TokenStream {
match *data {
Expand Down

0 comments on commit f0d4da3

Please sign in to comment.