Skip to content

Commit

Permalink
Cleanup of bridge args and code (clippy warnings)
Browse files Browse the repository at this point in the history
  • Loading branch information
JEnoch committed Oct 11, 2023
1 parent b087607 commit cbbb068
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 165 deletions.
4 changes: 2 additions & 2 deletions DEFAULT_CONFIG.json5
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
//// nodename: A ROS node name to be used by this bridge.
//// Default: "zenoh_bridge_ros2dds"
////
// namespace: "zenoh_bridge_ros2dds",
// nodename: "zenoh_bridge_ros2dds",

////
//// domain: The DDS Domain ID. By default set to 0, or to "$ROS_DOMAIN_ID" is this environment variable is defined.
Expand Down Expand Up @@ -76,7 +76,7 @@
// },

////
//// pub_max_frequencies: Specifies a list of maximum frequency of messages routing over zenoh for a set of topics.
//// pub_max_frequencies: Specifies a list of maximum frequency of publications routing over zenoh for a set of Publishers.
//// The strings must have the format "<regex>=<float>":
//// - "regex" is a regular expression matching a Publisher interface name
//// - "float" is the maximum frequency in Hertz;
Expand Down
62 changes: 18 additions & 44 deletions zenoh-bridge-ros2dds/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use async_liveliness_monitor::LivelinessMonitor;
//
// Copyright (c) 2022 ZettaScale Technology
//
Expand All @@ -12,6 +11,7 @@ use async_liveliness_monitor::LivelinessMonitor;
// Contributors:
// ZettaScale Zenoh Team, <[email protected]>
//
use async_liveliness_monitor::LivelinessMonitor;
use clap::{App, Arg};
use std::time::{Duration, SystemTime};
use zenoh::config::{Config, ModeDependentValue};
Expand Down Expand Up @@ -47,41 +47,41 @@ fn parse_args() -> (Config, Option<f32>) {
// zenoh related arguments:
//
.arg(Arg::from_usage(
r#"-i, --id=[HEX_STRING] \
r"-i, --id=[HEX_STRING] \
'The identifier (as an hexadecimal string, with odd number of chars - e.g.: 0A0B23...) that zenohd must use.
WARNING: this identifier must be unique in the system and must be 16 bytes maximum (32 chars)!
If not set, a random UUIDv4 will be used.'"#,
If not set, a random UUIDv4 will be used.'"
))
.arg(Arg::from_usage(
r#"-m, --mode=[MODE] 'The zenoh session mode.'"#)
r"-m, --mode=[MODE] 'The zenoh session mode.'")
.possible_values(["peer", "client"])
.default_value("peer")
)
.arg(Arg::from_usage(
r#"-c, --config=[FILE] \
'The configuration file. Currently, this file must be a valid JSON5 file.'"#,
r"-c, --config=[FILE] \
'The configuration file. Currently, this file must be a valid JSON5 file.'"
))
.arg(Arg::from_usage(
r#"-l, --listen=[ENDPOINT]... \
r"-l, --listen=[ENDPOINT]... \
'A locator on which this router will listen for incoming sessions.
Repeat this option to open several listeners.'"#,
Repeat this option to open several listeners.'"
),
)
.arg(Arg::from_usage(
r#"-e, --connect=[ENDPOINT]... \
r"-e, --connect=[ENDPOINT]... \
'A peer locator this router will try to connect to.
Repeat this option to connect to several peers.'"#,
Repeat this option to connect to several peers.'"
))
.arg(Arg::from_usage(
r#"--no-multicast-scouting \
r"--no-multicast-scouting \
'By default the zenoh bridge listens and replies to UDP multicast scouting messages for being discovered by peers and routers.
This option disables this feature.'"#
This option disables this feature.'"
))
.arg(Arg::from_usage(
r#"--rest-http-port=[PORT | IP:PORT] \
r"--rest-http-port=[PORT | IP:PORT] \
'Configures HTTP interface for the REST API (disabled by default, setting this option enables it). Accepted values:'
- a port number
- a string with format `<local_ip>:<port_number>` (to bind the HTTP server to a specific interface)."#
- a string with format `<local_ip>:<port_number>` (to bind the HTTP server to a specific interface)."
))
//
// DDS related arguments:
Expand Down Expand Up @@ -111,35 +111,13 @@ This option is not active by default, unless the "ROS_LOCALHOST_ONLY" environmen

app = app
.arg(Arg::from_usage(
r#"-a, --allow=[String]... 'A regular expression matching the set of 'partition/topic-name' that must be routed via zenoh. By default, all partitions and topics are allowed.
If both '--allow' and '--deny' are set a partition and/or topic will be allowed if it matches only the 'allow' expression.
Repeat this option to configure several topic expressions. These expressions are concatenated with '|'.
Examples of expressions: '.*/TopicA', 'Partition-?/.*', 'cmd_vel|rosout'...'"#
))
.arg(Arg::from_usage(
r#"--deny=[String]... 'A regular expression matching the set of 'partition/topic-name' that must not be routed via zenoh. By default, no partitions and no topics are denied.
If both '--allow' and '--deny' are set a partition and/or topic will be allowed if it matches only the 'allow' expression.
Repeat this option to configure several topic expressions. These expressions are concatenated with '|'.
Examples of expressions: '.*/TopicA', 'Partition-?/.*', 'cmd_vel|rosout'...'"#
))
.arg(Arg::from_usage(
r#"--max-frequency=[String]... 'Specifies a maximum frequency of data routing over zenoh for a set of topics. The string must have the format "<regex>=<float>":
- "regex" is a regular expression matching the set of 'partition/topic-name' (same syntax than --allow option)
for which the data (per DDS instance) must be routed at no higher rate than the specified max frequency.
r#"--pub-max-frequency=[String]... 'Specifies a maximum frequency of publications routing over zenoh for a set of Publishers.
The string must have the format "<regex>=<float>":
- "regex" is a regular expression matching a Publisher interface name
- "float" is the maximum frequency in Hertz; if publication rate is higher, downsampling will occur when routing.
Repeat this option to configure several topics expressions with a max frequency.'"#
))
.arg(Arg::from_usage(
r#"-r, --generalise-sub=[String]... 'A list of key expression to use for generalising subscriptions (usable multiple times).'"#
))
.arg(Arg::from_usage(
r#"-w, --generalise-pub=[String]... 'A list of key expression to use for generalising publications (usable multiple times).'"#
))
.arg(Arg::from_usage(
r#"-f, --fwd-discovery 'When set, rather than creating a local route when discovering a local DDS entity, this discovery info is forwarded to the remote plugins/bridges. Those will create the routes, including a replica of the discovered entity.'"#
).alias("forward-discovery")
)
.arg(Arg::from_usage(
r#"--queries-timeout=[float]... 'A float in seconds (default: 5.0 sec) that will be used as a timeout when the bridge
queries any other remote bridge for discovery information and for historical data for TRANSIENT_LOCAL DDS Readers it serves
(i.e. if the query to the remote bridge exceed the timeout, some historical samples might be not routed to the Readers, but the route will not be blocked forever)."#
Expand Down Expand Up @@ -201,11 +179,7 @@ r#"--watchdog=[PERIOD] 'Experimental!! Run a watchdog thread that monitors the
{
insert_json5!(config, args, "plugins/ros2/shm_enabled", if "dds-enable-shm");
}
insert_json5!(config, args, "plugins/ros2/allow", for "allow", .collect::<Vec<_>>());
insert_json5!(config, args, "plugins/ros2/deny", for "deny", .collect::<Vec::<_>>());
insert_json5!(config, args, "plugins/ros2/max_frequencies", for "max-frequency", .collect::<Vec<_>>());
insert_json5!(config, args, "plugins/ros2/generalise_pubs", for "generalise-pub", .collect::<Vec<_>>());
insert_json5!(config, args, "plugins/ros2/generalise_subs", for "generalise-sub", .collect::<Vec<_>>());
insert_json5!(config, args, "plugins/ros2/pub_max_frequencies", for "pub-max-frequency", .collect::<Vec<_>>());
insert_json5!(config, args, "plugins/ros2/queries_timeout", if "queries-timeout", .parse::<f64>().unwrap());

let watchdog_period = if args.is_present("watchdog") {
Expand Down
15 changes: 5 additions & 10 deletions zenoh-plugin-ros2dds/src/dds_discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use serde::{Deserialize, Serialize};
use std::ffi::CStr;
use std::fmt;
use std::mem::MaybeUninit;
use std::os::raw;
use std::sync::Arc;

use crate::dds_types::TypeInfo;
Expand Down Expand Up @@ -80,14 +79,14 @@ unsafe extern "C" fn on_data(dr: dds_entity_t, arg: *mut std::os::raw::c_void) {
let _ = dds_get_instance_handle(dp, &mut dpih);

#[allow(clippy::uninit_assumed_init)]
let mut si = MaybeUninit::<[dds_sample_info_t; MAX_SAMPLES as usize]>::uninit();
let mut samples: [*mut ::std::os::raw::c_void; MAX_SAMPLES as usize] =
[std::ptr::null_mut(); MAX_SAMPLES as usize];
let mut si = MaybeUninit::<[dds_sample_info_t; MAX_SAMPLES]>::uninit();
let mut samples: [*mut ::std::os::raw::c_void; MAX_SAMPLES] =
[std::ptr::null_mut(); MAX_SAMPLES];
samples[0] = std::ptr::null_mut();

let n = dds_take(
dr,
samples.as_mut_ptr() as *mut *mut raw::c_void,
samples.as_mut_ptr(),
si.as_mut_ptr() as *mut dds_sample_info_t,
MAX_SAMPLES,
MAX_SAMPLES as u32,
Expand Down Expand Up @@ -237,11 +236,7 @@ unsafe extern "C" fn on_data(dr: dds_entity_t, arg: *mut std::os::raw::c_void) {
}
}
}
dds_return_loan(
dr,
samples.as_mut_ptr() as *mut *mut raw::c_void,
MAX_SAMPLES as i32,
);
dds_return_loan(dr, samples.as_mut_ptr(), MAX_SAMPLES as i32);
Box::into_raw(btx);
}

Expand Down
5 changes: 3 additions & 2 deletions zenoh-plugin-ros2dds/src/dds_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ pub fn dds_write(data_writer: dds_entity_t, data: Vec<u8>) -> Result<(), String>

unsafe extern "C" fn listener_to_callback<F>(dr: dds_entity_t, arg: *mut std::os::raw::c_void)
where
F: Fn(&DDSRawSample) -> (),
F: Fn(&DDSRawSample),
{
let callback = arg as *mut F;
let mut zp: *mut ddsi_serdata = std::ptr::null_mut();
Expand All @@ -237,6 +237,7 @@ where
}
}

#[allow(clippy::too_many_arguments)]
pub fn create_dds_reader<F>(
dp: dds_entity_t,
topic_name: String,
Expand All @@ -248,7 +249,7 @@ pub fn create_dds_reader<F>(
callback: F,
) -> Result<dds_entity_t, String>
where
F: Fn(&DDSRawSample) -> () + std::marker::Send + 'static,
F: Fn(&DDSRawSample) + std::marker::Send + 'static,
{
unsafe {
let t = create_topic(dp, &topic_name, &type_name, type_info, keyless);
Expand Down
47 changes: 25 additions & 22 deletions zenoh-plugin-ros2dds/src/discovered_entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,26 @@ pub struct DiscoveredEntities {

impl Debug for DiscoveredEntities {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
writeln!(
f,
"participants: {:?}\n",
"participants: {:?}",
self.participants.keys().collect::<Vec<&Gid>>()
)?;
write!(
writeln!(
f,
"writers: {:?}\n",
"writers: {:?}",
self.writers.keys().collect::<Vec<&Gid>>()
)?;
write!(
writeln!(
f,
"readers: {:?}\n",
"readers: {:?}",
self.readers.keys().collect::<Vec<&Gid>>()
)?;
write!(f, "ros_participant_info: {:?}\n", self.ros_participant_info)?;
write!(f, "nodes_info: {:?}\n", self.nodes_info)?;
write!(
writeln!(f, "ros_participant_info: {:?}", self.ros_participant_info)?;
writeln!(f, "nodes_info: {:?}", self.nodes_info)?;
writeln!(
f,
"admin_space: {:?}\n",
"admin_space: {:?}",
self.admin_space.keys().collect::<Vec<&OwnedKeyExpr>>()
)
}
Expand Down Expand Up @@ -126,8 +126,8 @@ impl DiscoveredEntities {

// Check if this Writer is present in some NodeInfo.undiscovered_writer list
let mut event: Option<ROS2DiscoveryEvent> = None;
for (_, nodes_map) in &mut self.nodes_info {
for (_, node) in nodes_map {
for nodes_map in self.nodes_info.values_mut() {
for node in nodes_map.values_mut() {
if let Some(i) = node
.undiscovered_writer
.iter()
Expand Down Expand Up @@ -168,8 +168,8 @@ impl DiscoveredEntities {
);

// Remove the Writer from any NodeInfo that might use it, possibly leading to a UndiscoveredX event
for (_, nodes_map) in &mut self.nodes_info {
for (_, node) in nodes_map {
for nodes_map in self.nodes_info.values_mut() {
for node in nodes_map.values_mut() {
if let Some(e) = node.remove_writer(gid) {
// A Reader can be used by only 1 Node, no need to go on with loops
return Some(e);
Expand All @@ -196,8 +196,8 @@ impl DiscoveredEntities {

// Check if this Reader is present in some NodeInfo.undiscovered_reader list
let mut event = None;
for (_, nodes_map) in &mut self.nodes_info {
for (_, node) in nodes_map {
for nodes_map in self.nodes_info.values_mut() {
for node in nodes_map.values_mut() {
if let Some(i) = node
.undiscovered_reader
.iter()
Expand Down Expand Up @@ -238,8 +238,8 @@ impl DiscoveredEntities {
);

// Remove the Reader from any NodeInfo that might use it, possibly leading to a UndiscoveredX event
for (_, nodes_map) in &mut self.nodes_info {
for (_, node) in nodes_map {
for nodes_map in self.nodes_info.values_mut() {
for node in nodes_map.values_mut() {
if let Some(e) = node.remove_reader(gid) {
// A Reader can be used by only 1 Node, no need to go on with loops
return Some(e);
Expand Down Expand Up @@ -337,7 +337,9 @@ impl DiscoveredEntities {
"ROS Node {ros_node_info} declares Reader on {}",
entity.topic_name
);
node.update_with_reader(entity).map(|e| events.push(e));
if let Some(e) = node.update_with_reader(entity) {
events.push(e)
};
} else {
log::debug!(
"ROS Node {ros_node_info} declares a not yet discovered DDS Reader: {rgid}"
Expand All @@ -352,7 +354,9 @@ impl DiscoveredEntities {
"ROS Node {ros_node_info} declares Writer on {}",
entity.topic_name
);
node.update_with_writer(entity).map(|e| events.push(e));
if let Some(e) = node.update_with_writer(entity) {
events.push(e)
};
} else {
log::debug!(
"ROS Node {ros_node_info} declares a not yet discovered DDS Writer: {wgid}"
Expand Down Expand Up @@ -389,8 +393,7 @@ impl DiscoveredEntities {
EntityRef::Node(gid, name) => self
.nodes_info
.get(gid)
.map(|map| map.get(name))
.flatten()
.and_then(|map| map.get(name))
.map(serde_json::to_value)
.transpose(),
}
Expand Down
2 changes: 1 addition & 1 deletion zenoh-plugin-ros2dds/src/gid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl fmt::Debug for Gid {
if self == &Gid::NOT_DISCOVERED {
write!(f, "NOT_DISCOVERED")
} else {
let s = hex::encode(&self.0);
let s = hex::encode(self.0);
write!(f, "{s}")
}
}
Expand Down
2 changes: 1 addition & 1 deletion zenoh-plugin-ros2dds/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ pub struct ROS2PluginRuntime<'a> {
participant: dds_entity_t,
_member: LivelinessToken<'a>,
plugin_id: OwnedKeyExpr,
// admin space: index is the admin_keyexpr (relative to admin_prefix)
// admin space: index is the admin_keyexpr
// value is the JSon string to return to queries.
admin_space: HashMap<OwnedKeyExpr, AdminRef>,
}
Expand Down
Loading

0 comments on commit cbbb068

Please sign in to comment.