Remove leading underscore from private fields (#354)

* Remove leading underscore from private fields

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>

* Revert renaming to suppress unused warning

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>

---------

Signed-off-by: Luca Della Vedova <lucadv@intrinsic.ai>

Author: luca-della-vedova

rclrs/src/clock.rs

@@ -28,15 +28,15 @@ impl From<ClockType> 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<Mutex<rcl_clock_t>>,
+    kind: ClockType,
+    rcl_clock: Arc<Mutex<rcl_clock_t>>,
     // 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<Mutex<rcl_clock_t>>,
+    rcl_clock: Arc<Mutex<rcl_clock_t>>,
 }
 
 impl Clock {
@@ -54,52 +54,52 @@ 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<ClockSource>) {
-        let clock = Self::make(type_);
+    pub fn new(kind: ClockType) -> (Self, Option<ClockSource>) {
+        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.
             rcl_clock = Self::init_generic_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
             rcl_clock_get_now(&mut *clock, &mut time_point);
         }
         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,16 +147,16 @@ impl ClockSource {
         }
     }
 
-    fn new(clock: Arc<Mutex<rcl_clock_t>>) -> Self {
-        let source = Self { _rcl_clock: clock };
+    fn new(rcl_clock: Arc<Mutex<rcl_clock_t>>) -> Self {
+        let source = Self { rcl_clock };
         source.set_ros_time_enable(true);
         source
     }
 
     /// 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

rclrs/src/node.rs

@@ -71,8 +71,8 @@ pub struct Node {
     pub(crate) guard_conditions_mtx: Mutex<Vec<Weak<GuardCondition>>>,
     pub(crate) services_mtx: Mutex<Vec<Weak<dyn ServiceBase>>>,
     pub(crate) subscriptions_mtx: Mutex<Vec<Weak<dyn SubscriptionBase>>>,
-    _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<Arc<str>>,
     ) -> 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,
         }
     }
 

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)
     }
 

rclrs/src/parameter.rs

@@ -430,7 +430,7 @@ impl<T: ParameterVariant> TryFrom<ParameterBuilder<'_, T>> 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<ParameterBuilder<'a, T>> 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<ParameterBuilder<'a, T>> 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<T: ParameterVariant>(&self, name: &str) -> Option<T> {
-        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<Arc<str>>,
         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<str> = 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<Mutex<ParameterMap>>,
-    _override_map: ParameterOverrideMap,
+    parameter_map: Arc<Mutex<ParameterMap>>,
+    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<Self, RclrsError> {
         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<T> = 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,

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<Weak<Node>>,
-    _clock: RwLock<Clock>,
-    _clock_source: Arc<Mutex<Option<ClockSource>>>,
-    _requested_clock_type: ClockType,
-    _clock_qos: QoSProfile,
-    _clock_subscription: Mutex<Option<Arc<Subscription<ClockMsg>>>>,
-    _last_received_time: Arc<Mutex<Option<i64>>>,
-    _use_sim_time: Mutex<Option<MandatoryParameter<bool>>>,
+    node: Mutex<Weak<Node>>,
+    clock: RwLock<Clock>,
+    clock_source: Arc<Mutex<Option<ClockSource>>>,
+    requested_clock_type: ClockType,
+    clock_qos: QoSProfile,
+    clock_subscription: Mutex<Option<Arc<Subscription<ClockMsg>>>>,
+    last_received_time: Arc<Mutex<Option<i64>>>,
+    use_sim_time: Mutex<Option<MandatoryParameter<bool>>>,
 }
 
 /// 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<Subscription<ClockMsg>> {
-        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::<ClockMsg, _>("/clock", self._clock_qos, move |msg: ClockMsg| {
+            .create_subscription::<ClockMsg, _>("/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);