Merge branch 'luca/add_time_source' into luca/parameter_services

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

examples/minimal_pub_sub/src/minimal_subscriber.rs

@@ -6,7 +6,6 @@ fn main() -> Result<(), Error> {
     let context = rclrs::Context::new(env::args())?;
 
     let node = rclrs::create_node(&context, "minimal_subscriber")?;
-    let inner_node = node.clone();
 
     let mut num_messages: usize = 0;
 
@@ -17,8 +16,6 @@ fn main() -> Result<(), Error> {
             num_messages += 1;
             println!("I heard: '{}'", msg.data);
             println!("(Got {} messages so far)", num_messages);
-            let now = inner_node.get_clock().now();
-            dbg!(now);
         },
     )?;
 

rclrs/src/clock.rs

@@ -90,7 +90,7 @@ impl Clock {
         let mut clock = self._rcl_clock.lock().unwrap();
         let mut time_point: i64 = 0;
         unsafe {
-            // SAFETY: No preconditions for his function
+            // SAFETY: No preconditions for this function
             rcl_clock_get_now(&mut *clock, &mut time_point);
         }
         Time {
@@ -99,11 +99,9 @@ impl Clock {
         }
     }
 
-    /// Helper function to initialize a default clock, same behavior as `rcl_init_generic_clock`.
-    /// Needed because functions that initialize a clock take as an input a mutable reference
-    /// to a clock and don't actually return one, so we need a function to generate one. Doing this
-    /// instead of a `Default` implementation allows the function to be private and avoids
-    /// exposing a public API to create an invalid clock
+    /// Helper function to privately initialize a default clock, with the same behavior as
+    /// `rcl_init_generic_clock`. By defining a private function instead of implementing
+    /// `Default`,  we avoid exposing a public API to create an invalid clock.
     // SAFETY: Getting a default value is always safe.
     unsafe fn init_generic_clock() -> rcl_clock_t {
         let allocator = rcutils_get_default_allocator();

rclrs/src/lib.rs

@@ -44,7 +44,7 @@ pub use rcl_bindings::rmw_request_id_t;
 pub use service::*;
 pub use subscription::*;
 pub use time::*;
-pub use time_source::*;
+use time_source::*;
 pub use wait::*;
 
 /// Polls the node for new messages and executes the corresponding callbacks.

rclrs/src/node/builder.rs

@@ -2,7 +2,10 @@ use std::ffi::CString;
 use std::sync::{Arc, Mutex};
 
 use crate::rcl_bindings::*;
-use crate::{ClockType, Context, Node, ParameterInterface, RclrsError, TimeSource, ToResult};
+use crate::{
+    ClockType, Context, Node, ParameterInterface, QoSProfile, RclrsError, TimeSource, ToResult,
+    QOS_PROFILE_CLOCK,
+};
 
 /// A builder for creating a [`Node`][1].
 ///
@@ -15,6 +18,7 @@ use crate::{ClockType, Context, Node, ParameterInterface, RclrsError, TimeSource
 /// - `arguments: []`
 /// - `enable_rosout: true`
 /// - `clock_type: ClockType::RosTime`
+/// - `clock_qos: QOS_PROFILE_CLOCK`
 ///
 /// # Example
 /// ```
@@ -45,6 +49,7 @@ pub struct NodeBuilder {
     arguments: Vec<String>,
     enable_rosout: bool,
     clock_type: ClockType,
+    clock_qos: QoSProfile,
 }
 
 impl NodeBuilder {
@@ -92,6 +97,7 @@ impl NodeBuilder {
             arguments: vec![],
             enable_rosout: true,
             clock_type: ClockType::RosTime,
+            clock_qos: QOS_PROFILE_CLOCK,
         }
     }
 
@@ -230,6 +236,12 @@ impl NodeBuilder {
         self
     }
 
+    /// Sets the QoSProfile for the clock subscription.
+    pub fn clock_qos(mut self, clock_qos: QoSProfile) -> Self {
+        self.clock_qos = clock_qos;
+        self
+    }
+
     /// Builds the node instance.
     ///
     /// Node name and namespace validation is performed in this method.
@@ -281,7 +293,10 @@ impl NodeBuilder {
             guard_conditions_mtx: Mutex::new(vec![]),
             services_mtx: Mutex::new(vec![]),
             subscriptions_mtx: Mutex::new(vec![]),
-            _time_source: TimeSource::new(weak.clone(), self.clock_type),
+            _time_source: TimeSource::builder(weak.clone(), self.clock_type)
+                .clock_qos(self.clock_qos)
+                .build()
+                .unwrap(),
             _parameter,
         }))
     }

rclrs/src/time_source.rs

@@ -6,14 +6,12 @@ use std::sync::{Arc, Mutex, RwLock, Weak};
 /// Time source for a node that drives the attached clock.
 /// 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 struct TimeSource {
+pub(crate) struct TimeSource {
     _node: Weak<Node>,
     _clock: RwLock<Clock>,
     _clock_source: Arc<Mutex<Option<ClockSource>>>,
     _requested_clock_type: ClockType,
     _clock_qos: QoSProfile,
-    // TODO(luca) implement clock threads, for now will run in main thread
-    _use_clock_thread: bool,
     _clock_subscription: Option<Arc<Subscription<ClockMsg>>>,
     _last_received_time: Arc<Mutex<Option<i64>>>,
 }
@@ -25,47 +23,35 @@ pub struct TimeSource {
 ///
 /// The default values for optional fields are:
 /// - `clock_qos: QOS_PROFILE_CLOCK`[3]
-/// - `use_clock_thread: true`
 ///
 ///
 /// [1]: crate::TimeSource
 /// [2]: crate::TimeSource::builder
 /// [3]: crate::QOS_PROFILE_CLOCK
-pub struct TimeSourceBuilder {
+pub(crate) struct TimeSourceBuilder {
     node: Weak<Node>,
     clock_qos: QoSProfile,
-    use_clock_thread: bool,
     clock_type: ClockType,
 }
 
 impl TimeSourceBuilder {
     /// Creates a builder for a time source that drives the given clock for the given node.
-    pub fn new(node: Weak<Node>, clock_type: ClockType) -> Self {
+    pub(crate) fn new(node: Weak<Node>, clock_type: ClockType) -> Self {
         Self {
             node,
             clock_qos: QOS_PROFILE_CLOCK,
-            use_clock_thread: true,
             clock_type,
         }
     }
 
     /// Sets the QoS for the `/clock` topic.
-    pub fn clock_qos(mut self, clock_qos: QoSProfile) -> Self {
+    pub(crate) fn clock_qos(mut self, clock_qos: QoSProfile) -> Self {
         self.clock_qos = clock_qos;
         self
     }
 
-    /// Sets use_clock_thread.
-    ///
-    /// If set to `true`, the clock callbacks will run in a separate thread.
-    /// NOTE: Currently unimplemented
-    pub fn use_clock_thread(mut self, use_clock_thread: bool) -> Self {
-        self.use_clock_thread = use_clock_thread;
-        self
-    }
-
     /// Builds the `TimeSource` and attaches the provided `Node` and `Clock`.
-    pub fn build(self) -> Result<TimeSource, RclrsError> {
+    pub(crate) fn build(self) -> Result<TimeSource, RclrsError> {
         let clock = match self.clock_type {
             ClockType::RosTime | ClockType::SystemTime => Clock::system(),
             ClockType::SteadyTime => Clock::steady(),
@@ -76,7 +62,6 @@ impl TimeSourceBuilder {
             _clock_source: Arc::new(Mutex::new(None)),
             _requested_clock_type: self.clock_type,
             _clock_qos: self.clock_qos,
-            _use_clock_thread: self.use_clock_thread,
             _clock_subscription: None,
             _last_received_time: Arc::new(Mutex::new(None)),
         };
@@ -86,24 +71,19 @@ impl TimeSourceBuilder {
 }
 
 impl TimeSource {
-    /// Creates a new `TimeSource` with default parameters.
-    pub fn new(node: Weak<Node>, clock_type: ClockType) -> Self {
-        TimeSourceBuilder::new(node, clock_type).build().unwrap()
-    }
-
     /// Creates a new `TimeSourceBuilder` with default parameters.
-    pub fn builder(node: Weak<Node>, clock_type: ClockType) -> TimeSourceBuilder {
+    pub(crate) fn builder(node: Weak<Node>, clock_type: ClockType) -> TimeSourceBuilder {
         TimeSourceBuilder::new(node, clock_type)
     }
 
     /// Returns the clock that this TimeSource is controlling.
-    pub fn get_clock(&self) -> Clock {
+    pub(crate) fn get_clock(&self) -> Clock {
         self._clock.read().unwrap().clone()
     }
 
     /// Attaches the given node to to the `TimeSource`, using its interface to read the
     /// `use_sim_time` parameter and create the clock subscription.
-    pub fn attach_node(&mut self, node: Weak<Node>) {
+    fn attach_node(&mut self, node: Weak<Node>) {
         self._node = node;
 
         // TODO(luca) register a parameter callback, hold the parameter handle