Skip to content

Commit

Permalink
Remove leading underscore from private fields (#354)
Browse files Browse the repository at this point in the history 
* 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>
  • Loading branch information
@luca-della-vedova
luca-della-vedova authored Dec 5, 2023
1 parent f29144b commit 9082816
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 72 deletions.
38 changes: 19 additions & 19 deletions rclrs/src/clock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
12 changes: 6 additions & 6 deletions rclrs/src/node.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
}
}

Expand Down
8 changes: 4 additions & 4 deletions rclrs/src/node/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
}

Expand Down
26 changes: 13 additions & 13 deletions rclrs/src/parameter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
}
Expand Down Expand Up @@ -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(),
})
}
Expand Down Expand Up @@ -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(),
})
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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) => {
Expand Down Expand Up @@ -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.
Expand All @@ -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(),
})
}
Expand Down Expand Up @@ -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()
Expand All @@ -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() {
Expand Down Expand Up @@ -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,
Expand Down
60 changes: 30 additions & 30 deletions rclrs/src/time_source.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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].
Expand Down Expand Up @@ -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),
}
}
}
Expand All @@ -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
Expand All @@ -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;
}
}
}
Expand All @@ -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);
Expand Down

0 comments on commit 9082816

Please sign in to comment.