From 77615d41b8fa02ab37a96cbfc47e494418ff076b Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 5 Dec 2023 13:43:02 +0800 Subject: [PATCH 1/2] Remove leading underscore from private fields Signed-off-by: Luca Della Vedova --- rclrs/src/clock.rs | 38 ++++++++++++------------- rclrs/src/node.rs | 12 ++++---- rclrs/src/node/builder.rs | 8 +++--- rclrs/src/parameter.rs | 34 +++++++++++----------- rclrs/src/time_source.rs | 60 +++++++++++++++++++-------------------- 5 files changed, 76 insertions(+), 76 deletions(-) diff --git a/rclrs/src/clock.rs b/rclrs/src/clock.rs index c89705d9d..018c6f5b7 100644 --- a/rclrs/src/clock.rs +++ b/rclrs/src/clock.rs @@ -28,15 +28,15 @@ impl From for rcl_clock_type_t { /// Struct that implements a Clock and wraps `rcl_clock_t`. #[derive(Clone, Debug)] pub struct Clock { - _type: ClockType, - _rcl_clock: Arc>, + kind: ClockType, + rcl_clock: Arc>, // TODO(luca) Implement jump callbacks } /// A clock source that can be used to drive the contained clock. Created when a clock of type /// `ClockType::RosTime` is constructed pub struct ClockSource { - _rcl_clock: Arc>, + rcl_clock: Arc>, } impl Clock { @@ -54,19 +54,19 @@ impl Clock { /// to update it pub fn with_source() -> (Self, ClockSource) { let clock = Self::make(ClockType::RosTime); - let clock_source = ClockSource::new(clock._rcl_clock.clone()); + let clock_source = ClockSource::new(clock.rcl_clock.clone()); (clock, clock_source) } /// Creates a new clock of the given `ClockType`. - pub fn new(type_: ClockType) -> (Self, Option) { - let clock = Self::make(type_); + pub fn new(kind: ClockType) -> (Self, Option) { + let clock = Self::make(kind); let clock_source = - matches!(type_, ClockType::RosTime).then(|| ClockSource::new(clock._rcl_clock.clone())); + matches!(kind, ClockType::RosTime).then(|| ClockSource::new(clock.rcl_clock.clone())); (clock, clock_source) } - fn make(type_: ClockType) -> Self { + fn make(kind: ClockType) -> Self { let mut rcl_clock; unsafe { // SAFETY: Getting a default value is always safe. @@ -74,24 +74,24 @@ impl Clock { let mut allocator = rcutils_get_default_allocator(); // Function will return Err(_) only if there isn't enough memory to allocate a clock // object. - rcl_clock_init(type_.into(), &mut rcl_clock, &mut allocator) + rcl_clock_init(kind.into(), &mut rcl_clock, &mut allocator) .ok() .unwrap(); } Self { - _type: type_, - _rcl_clock: Arc::new(Mutex::new(rcl_clock)), + kind, + rcl_clock: Arc::new(Mutex::new(rcl_clock)), } } /// Returns the clock's `ClockType`. pub fn clock_type(&self) -> ClockType { - self._type + self.kind } /// Returns the current clock's timestamp. pub fn now(&self) -> Time { - let mut clock = self._rcl_clock.lock().unwrap(); + let mut clock = self.rcl_clock.lock().unwrap(); let mut time_point: i64 = 0; unsafe { // SAFETY: No preconditions for this function @@ -99,7 +99,7 @@ impl Clock { } Time { nsec: time_point, - clock: Arc::downgrade(&self._rcl_clock), + clock: Arc::downgrade(&self.rcl_clock), } } @@ -128,14 +128,14 @@ impl Drop for ClockSource { impl PartialEq for ClockSource { fn eq(&self, other: &Self) -> bool { - Arc::ptr_eq(&self._rcl_clock, &other._rcl_clock) + Arc::ptr_eq(&self.rcl_clock, &other.rcl_clock) } } impl ClockSource { /// Sets the value of the current ROS time. pub fn set_ros_time_override(&self, nanoseconds: i64) { - let mut clock = self._rcl_clock.lock().unwrap(); + let mut clock = self.rcl_clock.lock().unwrap(); // SAFETY: Safe if clock jump callbacks are not edited, which is guaranteed // by the mutex unsafe { @@ -147,8 +147,8 @@ impl ClockSource { } } - fn new(clock: Arc>) -> Self { - let source = Self { _rcl_clock: clock }; + fn new(rcl_clock: Arc>) -> Self { + let source = Self { rcl_clock }; source.set_ros_time_enable(true); source } @@ -156,7 +156,7 @@ impl ClockSource { /// Sets the clock to use ROS Time, if enabled the clock will report the last value set through /// `Clock::set_ros_time_override(nanoseconds: i64)`. fn set_ros_time_enable(&self, enable: bool) { - let mut clock = self._rcl_clock.lock().unwrap(); + let mut clock = self.rcl_clock.lock().unwrap(); if enable { // SAFETY: Safe if clock jump callbacks are not edited, which is guaranteed // by the mutex diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index b91e16276..f1792c1e6 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -71,8 +71,8 @@ pub struct Node { pub(crate) guard_conditions_mtx: Mutex>>, pub(crate) services_mtx: Mutex>>, pub(crate) subscriptions_mtx: Mutex>>, - _time_source: TimeSource, - _parameter: ParameterInterface, + time_source: TimeSource, + parameter: ParameterInterface, } impl Eq for Node {} @@ -102,7 +102,7 @@ impl Node { /// Returns the clock associated with this node. pub fn get_clock(&self) -> Clock { - self._time_source.get_clock() + self.time_source.get_clock() } /// Returns the name of the node. @@ -398,16 +398,16 @@ impl Node { &'a self, name: impl Into>, ) -> ParameterBuilder<'a, T> { - self._parameter.declare(name.into()) + self.parameter.declare(name.into()) } /// Enables usage of undeclared parameters for this node. /// /// Returns a [`Parameters`] struct that can be used to get and set all parameters. pub fn use_undeclared_parameters(&self) -> Parameters { - self._parameter.allow_undeclared(); + self.parameter.allow_undeclared(); Parameters { - interface: &self._parameter, + interface: &self.parameter, } } diff --git a/rclrs/src/node/builder.rs b/rclrs/src/node/builder.rs index 543bd50d2..2dd82a79c 100644 --- a/rclrs/src/node/builder.rs +++ b/rclrs/src/node/builder.rs @@ -281,7 +281,7 @@ impl NodeBuilder { }; let rcl_node_mtx = Arc::new(Mutex::new(rcl_node)); - let _parameter = ParameterInterface::new( + let parameter = ParameterInterface::new( &rcl_node_mtx, &rcl_node_options.arguments, &rcl_context.global_arguments, @@ -293,12 +293,12 @@ impl NodeBuilder { guard_conditions_mtx: Mutex::new(vec![]), services_mtx: Mutex::new(vec![]), subscriptions_mtx: Mutex::new(vec![]), - _time_source: TimeSource::builder(self.clock_type) + time_source: TimeSource::builder(self.clock_type) .clock_qos(self.clock_qos) .build(), - _parameter, + parameter, }); - node._time_source.attach_node(&node); + node.time_source.attach_node(&node); Ok(node) } diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index d04e36623..b2de21783 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -36,16 +36,16 @@ use std::sync::{ #[derive(Clone, Debug)] struct ParameterOptionsStorage { - _description: Arc, - _constraints: Arc, + description: Arc, + constraints: Arc, ranges: ParameterRanges, } impl From> for ParameterOptionsStorage { fn from(opts: ParameterOptions) -> Self { Self { - _description: opts.description, - _constraints: opts.constraints, + description: opts.description, + constraints: opts.constraints, ranges: opts.ranges.into(), } } @@ -430,7 +430,7 @@ impl TryFrom> for OptionalParameter name: builder.name, value, ranges, - map: Arc::downgrade(&builder.interface._parameter_map), + map: Arc::downgrade(&builder.interface.parameter_map), _marker: Default::default(), }) } @@ -494,7 +494,7 @@ impl<'a, T: ParameterVariant + 'a> TryFrom> for Mandator name: builder.name, value, ranges, - map: Arc::downgrade(&builder.interface._parameter_map), + map: Arc::downgrade(&builder.interface.parameter_map), _marker: Default::default(), }) } @@ -586,7 +586,7 @@ impl<'a, T: ParameterVariant + 'a> TryFrom> for ReadOnly Ok(ReadOnlyParameter { name: builder.name, value, - map: Arc::downgrade(&builder.interface._parameter_map), + map: Arc::downgrade(&builder.interface.parameter_map), _marker: Default::default(), }) } @@ -703,7 +703,7 @@ impl<'a> Parameters<'a> { /// /// Returns `Some(T)` if a parameter of the requested type exists, `None` otherwise. pub fn get(&self, name: &str) -> Option { - let storage = &self.interface._parameter_map.lock().unwrap().storage; + let storage = &self.interface.parameter_map.lock().unwrap().storage; let storage = storage.get(name)?; match storage { ParameterStorage::Declared(storage) => match &storage.value { @@ -728,7 +728,7 @@ impl<'a> Parameters<'a> { name: impl Into>, value: T, ) -> Result<(), ParameterValueError> { - let mut map = self.interface._parameter_map.lock().unwrap(); + let mut map = self.interface.parameter_map.lock().unwrap(); let name: Arc = name.into(); match map.storage.entry(name) { Entry::Occupied(mut entry) => { @@ -767,8 +767,8 @@ impl<'a> Parameters<'a> { } pub(crate) struct ParameterInterface { - _parameter_map: Arc>, - _override_map: ParameterOverrideMap, + parameter_map: Arc>, + override_map: ParameterOverrideMap, allow_undeclared: AtomicBool, // NOTE(luca-della-vedova) add a ParameterService field to this struct to add support for // services. @@ -781,14 +781,14 @@ impl ParameterInterface { global_arguments: &rcl_arguments_t, ) -> Result { let rcl_node = rcl_node_mtx.lock().unwrap(); - let _override_map = unsafe { + let override_map = unsafe { let fqn = call_string_getter_with_handle(&rcl_node, rcl_node_get_fully_qualified_name); resolve_parameter_overrides(&fqn, node_arguments, global_arguments)? }; Ok(ParameterInterface { - _parameter_map: Default::default(), - _override_map, + parameter_map: Default::default(), + override_map, allow_undeclared: Default::default(), }) } @@ -820,7 +820,7 @@ impl ParameterInterface { ranges.validate()?; let override_value: Option = if ignore_override { None - } else if let Some(override_value) = self._override_map.get(name).cloned() { + } else if let Some(override_value) = self.override_map.get(name).cloned() { Some( override_value .try_into() @@ -831,7 +831,7 @@ impl ParameterInterface { }; let prior_value = - if let Some(prior_value) = self._parameter_map.lock().unwrap().storage.get(name) { + if let Some(prior_value) = self.parameter_map.lock().unwrap().storage.get(name) { match prior_value { ParameterStorage::Declared(_) => return Err(DeclarationError::AlreadyDeclared), ParameterStorage::Undeclared(param) => match param.clone().try_into() { @@ -869,7 +869,7 @@ impl ParameterInterface { value: DeclaredValue, options: ParameterOptionsStorage, ) { - self._parameter_map.lock().unwrap().storage.insert( + self.parameter_map.lock().unwrap().storage.insert( name, ParameterStorage::Declared(DeclaredStorage { options, diff --git a/rclrs/src/time_source.rs b/rclrs/src/time_source.rs index 102cdbd08..19274fa9e 100644 --- a/rclrs/src/time_source.rs +++ b/rclrs/src/time_source.rs @@ -7,14 +7,14 @@ use std::sync::{Arc, Mutex, RwLock, Weak}; /// If the node's `use_sim_time` parameter is set to `true`, the `TimeSource` will subscribe /// to the `/clock` topic and drive the attached clock pub(crate) struct TimeSource { - _node: Mutex>, - _clock: RwLock, - _clock_source: Arc>>, - _requested_clock_type: ClockType, - _clock_qos: QoSProfile, - _clock_subscription: Mutex>>>, - _last_received_time: Arc>>, - _use_sim_time: Mutex>>, + node: Mutex>, + clock: RwLock, + clock_source: Arc>>, + requested_clock_type: ClockType, + clock_qos: QoSProfile, + clock_subscription: Mutex>>>, + last_received_time: Arc>>, + use_sim_time: Mutex>>, } /// A builder for creating a [`TimeSource`][1]. @@ -56,14 +56,14 @@ impl TimeSourceBuilder { ClockType::SteadyTime => Clock::steady(), }; TimeSource { - _node: Mutex::new(Weak::new()), - _clock: RwLock::new(clock), - _clock_source: Arc::new(Mutex::new(None)), - _requested_clock_type: self.clock_type, - _clock_qos: self.clock_qos, - _clock_subscription: Mutex::new(None), - _last_received_time: Arc::new(Mutex::new(None)), - _use_sim_time: Mutex::new(None), + node: Mutex::new(Weak::new()), + clock: RwLock::new(clock), + clock_source: Arc::new(Mutex::new(None)), + requested_clock_type: self.clock_type, + clock_qos: self.clock_qos, + clock_subscription: Mutex::new(None), + last_received_time: Arc::new(Mutex::new(None)), + use_sim_time: Mutex::new(None), } } } @@ -76,7 +76,7 @@ impl TimeSource { /// Returns the clock that this TimeSource is controlling. pub(crate) fn get_clock(&self) -> Clock { - self._clock.read().unwrap().clone() + self.clock.read().unwrap().clone() } /// Attaches the given node to to the `TimeSource`, using its interface to read the @@ -89,27 +89,27 @@ impl TimeSource { .default(false) .mandatory() .unwrap(); - *self._node.lock().unwrap() = Arc::downgrade(node); + *self.node.lock().unwrap() = Arc::downgrade(node); self.set_ros_time_enable(param.get()); - *self._use_sim_time.lock().unwrap() = Some(param); + *self.use_sim_time.lock().unwrap() = Some(param); } fn set_ros_time_enable(&self, enable: bool) { - if matches!(self._requested_clock_type, ClockType::RosTime) { - let mut clock = self._clock.write().unwrap(); + if matches!(self.requested_clock_type, ClockType::RosTime) { + let mut clock = self.clock.write().unwrap(); if enable && matches!(clock.clock_type(), ClockType::SystemTime) { let (new_clock, mut clock_source) = Clock::with_source(); - if let Some(last_received_time) = *self._last_received_time.lock().unwrap() { + if let Some(last_received_time) = *self.last_received_time.lock().unwrap() { Self::update_clock(&mut clock_source, last_received_time); } *clock = new_clock; - *self._clock_source.lock().unwrap() = Some(clock_source); - *self._clock_subscription.lock().unwrap() = Some(self.create_clock_sub()); + *self.clock_source.lock().unwrap() = Some(clock_source); + *self.clock_subscription.lock().unwrap() = Some(self.create_clock_sub()); } if !enable && matches!(clock.clock_type(), ClockType::RosTime) { *clock = Clock::system(); - *self._clock_source.lock().unwrap() = None; - *self._clock_subscription.lock().unwrap() = None; + *self.clock_source.lock().unwrap() = None; + *self.clock_subscription.lock().unwrap() = None; } } } @@ -119,15 +119,15 @@ impl TimeSource { } fn create_clock_sub(&self) -> Arc> { - let clock = self._clock_source.clone(); - let last_received_time = self._last_received_time.clone(); + let clock = self.clock_source.clone(); + let last_received_time = self.last_received_time.clone(); // Safe to unwrap since the function will only fail if invalid arguments are provided - self._node + self.node .lock() .unwrap() .upgrade() .unwrap() - .create_subscription::("/clock", self._clock_qos, move |msg: ClockMsg| { + .create_subscription::("/clock", self.clock_qos, move |msg: ClockMsg| { let nanoseconds: i64 = (msg.clock.sec as i64 * 1_000_000_000) + msg.clock.nanosec as i64; *last_received_time.lock().unwrap() = Some(nanoseconds); From ce2c50242a5fdfe31fb433f5e6dc355f956780c9 Mon Sep 17 00:00:00 2001 From: Luca Della Vedova Date: Tue, 5 Dec 2023 13:48:31 +0800 Subject: [PATCH 2/2] Revert renaming to suppress unused warning Signed-off-by: Luca Della Vedova --- rclrs/src/parameter.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rclrs/src/parameter.rs b/rclrs/src/parameter.rs index b2de21783..75256d869 100644 --- a/rclrs/src/parameter.rs +++ b/rclrs/src/parameter.rs @@ -36,16 +36,16 @@ use std::sync::{ #[derive(Clone, Debug)] struct ParameterOptionsStorage { - description: Arc, - constraints: Arc, + _description: Arc, + _constraints: Arc, ranges: ParameterRanges, } impl From> for ParameterOptionsStorage { fn from(opts: ParameterOptions) -> Self { Self { - description: opts.description, - constraints: opts.constraints, + _description: opts.description, + _constraints: opts.constraints, ranges: opts.ranges.into(), } }