Refactor first_rclrs_node example for improved clarity and structure

Author: roboticswithjulia

examples/minimal_pub_sub/src/first_rclrs_node.rs

@@ -5,12 +5,11 @@
 //!
 //! ## Create a package
 //! In ROS 2, `ros2 pkg create` is the standard way of creating packages. However, the functionality to create Rust packages with this tool is not yet implemented, so they need to be created manually.
-//! 
+//!
 //! You can use the current package that has already been created, or you can create a new package using `cargo` as usual:
 //! ```console
 //! cargo new republisher_node && cd republisher_node
 //! ```
-
 //! In the `Cargo.toml` file, add dependencies for `rclrs = "*"` and `std_msgs = "*"`, if they are not already included.
 //!
 //! Additionally, a new `package.xml` needs to be created if you want your node to be buildable with `colcon` and make sure to change the build type to `ament_cargo` and to include the two packages mentioned above in the dependencies, as such:
@@ -62,23 +61,19 @@
 //!            _data,
 //!        })
 //!    }
-//!}
-//! ```
-
+//!}```
 //! Next, add a main function to launch it:
 
-//!```rust
+//! ```rust
 //! fn main() -> Result<(), rclrs::RclrsError> {
-//!    let context = Context::default_from_env()?;
-//!    let mut executor = context.create_basic_executor();
-//!    let _republisher = RepublisherNode::new(&executor)?;
-//!    executor
-//!        .spin(SpinOptions::default())
-//!        .first_error()
-//!        .map_err(|err| err.into())
-//!}
-//! ```
-
+//!     let context = Context::default_from_env()?;
+//!     let mut executor = context.create_basic_executor();
+//!     let _republisher = RepublisherNode::new(&executor)?;
+//!     executor
+//!         .spin(SpinOptions::default())
+//!         .first_error()
+//!         .map_err(|err| err.into())
+//! }```
 
 //! ## Run the node
 //! You should now be able to run this node with `cargo build` and then `cargo run`. However, the subscription callback still has a `todo!` in it, so it will exit with an error when it receives a message.
@@ -89,17 +84,15 @@
 //! ```rust
 //! |msg: StringMsg| {
 //!     data = Some(msg);
-//!},
-//!```
-
-
+//! }
+//! ```
 //! This is a standard pattern in C++, but doesn't work in Rust. Why not?
 //!
-//! Written like this, `data` is *borrowed* by the callback, but `data` is a local variable which only exists in its current form until the end of `RepublisherNode::new()`. 
+//! Written like this, `data` is *borrowed* by the callback, but `data` is a local variable which only exists in its current form until the end of `RepublisherNode::new()`.
 //! The subscription callback is required to not borrow any variables because the subscription, and therefore the callback, could live indefinitely.
-//!  > 💡 As an aside, this requirement is expressed by the `'static` bound on the generic parameter `F` for the callback in `Node::create_subscription()`.
+//! > 💡 As an aside, this requirement is expressed by the `'static` bound on the generic parameter `F` for the callback in `Node::create_subscription()`.
 //!
-//! You might think "I don't want to borrow from the local variable `data` anyway, I want to borrow from the `data` field in `RepublisherNode`!" and you would be right. 
+//! You might think "I don't want to borrow from the local variable `data` anyway, I want to borrow from the `data` field in `RepublisherNode`!" and you would be right.
 //! That variable lives considerably longer, but also not forever, so it wouldn't be enough. A secondary problem is that it would be a *self-referential struct*, which is not allowed in Rust.
 //!
 //! The solution is _shared ownership_ of the data by the callback and the node. The `Arc` type provides shared ownership, but since it only gives out shared references to its data,
@@ -109,7 +102,7 @@
 //! 1. Import `Mutex`
 //! 2. Adjust the type of the `data` field
 //! 3. Create two pointers to the same data (wrapped in a `Mutex`)
-//! 4. Make the closure `move`, and inside it, lock the `Mutex` and store the message  
+//! 4. Make the closure `move`, and inside it, lock the `Mutex` and store the message
 
 //! ```rust
 //! use std::sync::{Arc, Mutex};  // (1)
@@ -140,20 +133,17 @@
 //!        })
 //!    }
 //! }
-
 //! ```
 //!
-
 //! If that seems needlessly complicated – maybe it is, in the sense that `rclrs` could potentially introduce new abstractions to improve the ergonomics of this use case. This is to be discussed.
-//! 
+//!
 //! If you couldn't follow the explanation involving borrowing, closures etc. above, an explanation of these concepts is unfortunately out of scope of this tutorial.
 //! There are many good Rust books and tutorials that can help you understand these crucial features. The online book [*The Rust Programming Language*](https://doc.rust-lang.org/book/) is a good place to start for most topics.
-
-
+//!
 //! ## Periodically run a republishing function
-//! 
+//!
 //! The node still doesn't republish the received messages. First, let's add a publisher to the node:
-
+//!
 //! ```rust
 //! // Add this new field to the RepublisherNode struct, after the subscription:
 //! _publisher: Arc<rclrs::Publisher<StringMsg>>,
@@ -167,7 +157,6 @@
 //!     _data,
 //! })
 //! ```
-
 //! Then, let's add a `republish()` function to the `RepublisherNode` struct that periodically republishes the last message received, or does nothing if none was received:
 //!
 //! ```rust
@@ -178,36 +167,34 @@
 //!     Ok(())
 //! }
 //! ```
-
 //! What's left to do is to call this function every second. `rclrs` doesn't yet have ROS timers, which run a function at a fixed interval,
 //! but it's easy enough to achieve with a thread, a loop, and the sleep function. Change your main function to spawn a separate thread:
 //!
 //! ```rust
 //! fn main() -> Result<(), rclrs::RclrsError> {
-//!    let context = Context::default_from_env()?;
-//!    let mut executor = context.create_basic_executor();
-//!    let _republisher = RepublisherNode::new(&executor)?;
-//!    std::thread::spawn(|| -> Result<(), rclrs::RclrsError> {
-//!        loop {
-//!            use std::time::Duration;
-//!            std::thread::sleep(Duration::from_millis(1000));
-//!            _republisher.republish()?;
-//!        }
-//!    });
-//!    executor
-//!       .spin(SpinOptions::default())
-//!       .first_error()
-//!       .map_err(|err| err.into())
-//!}
-//!```
-
-
-//! But wait, this doesn't work – there is an error about the thread closure needing to outlive `'static`. 
-//! That's again the same issue as above: Rust doesn't allow borrowing variables in this closure, 
+//!     let context = Context::default_from_env()?;
+//!     let mut executor = context.create_basic_executor();
+//!     let _republisher = RepublisherNode::new(&executor)?;
+//!     std::thread::spawn(move || -> Result<(), rclrs::RclrsError> {
+//!         loop {
+//!             use std::time::Duration;
+//!             std::thread::sleep(Duration::from_millis(1000));
+//!             _republisher.republish()?;
+//!         }
+//!     });
+//!     executor
+//!         .spin(SpinOptions::default())
+//!         .first_error()
+//!         .map_err(|err| err.into())
+//! }
+//! ```
+//!
+//! But wait, this doesn't work – there is an error about the thread closure needing to outlive `'static`.
+//! That's again the same issue as above: Rust doesn't allow borrowing variables in this closure,
 //! because the function that the variable is coming from might return before the thread that borrows the variable ends.
 //! > 💡 Of course, you could argue that this cannot really happen here, because returning from `main()` will also terminate the other threads, but Rust isn't that smart.
-//! 
-//! The solution is also the same as above: Shared ownership with `Arc`. Only this time, `Mutex` isn't needed since both the `rclcpp::spin()` 
+//!
+//! The solution is also the same as above: Shared ownership with `Arc`. Only this time, `Mutex` isn't needed since both the `rclcpp::spin()`
 //! and the `republish()` function only require a shared reference:
 //! ```rust
 //! fn main() -> Result<(), rclrs::RclrsError> {
@@ -231,43 +218,46 @@
 //! ## Try it out
 //! In separate terminals, run `cargo run --bin first_rclrs_node` if running the current node or `cargo run` otherwise and `ros2 topic echo /out_topic`. Nothing will be shown yet, since our node hasn't received any data yet.
 //!
-//! In another terminal, publish a single message with `ros2 topic pub /in_topic std_msgs/msg/String '{data: "Bonjour"}' -1`. 
+//! In another terminal, publish a single message with `ros2 topic pub /in_topic std_msgs/msg/String '{data: "Bonjour"}' -1`.
 //! The terminal with `ros2 topic echo` should now receive a new `Bonjour` message every second.
 //!
 //! Now publish another message, e.g. `ros2 topic pub /in_topic std_msgs/msg/String '{data: "Servus"}' -1` and observe the `ros2 topic echo` terminal receiving that message from that point forward.
 
-use std::sync::{Arc, Mutex};  // (1)
-use std_msgs::msg::String as StringMsg;
 use rclrs::*;
+use std::sync::{Arc, Mutex};
+use std_msgs::msg::String as StringMsg;
+
 struct RepublisherNode {
-   _node: Arc<rclrs::Node>,
-   _subscription: Arc<rclrs::Subscription<StringMsg>>,
-   _publisher: Arc<rclrs::Publisher<StringMsg>>,
-   _data: Arc<Mutex<Option<StringMsg>>>,  // (2)
+    _node: Arc<rclrs::Node>,
+    _subscription: Arc<rclrs::Subscription<StringMsg>>,
+    _publisher: Arc<rclrs::Publisher<StringMsg>>,
+    _data: Arc<Mutex<Option<StringMsg>>>,
 }
+
 impl RepublisherNode {
-   fn new(executor: &rclrs::Executor) -> Result<Self, rclrs::RclrsError> {
-       let _node = executor.create_node("republisher")?;
-       let _data = Arc::new(Mutex::new(None));  // (3)
-       let data_cb = Arc::clone(&_data);
-       let _subscription = _node.create_subscription(
-           "in_topic".keep_last(10).transient_local(),  // (4)
-           move |msg: StringMsg| {
-               *data_cb.lock().unwrap() = Some(msg);
-           },
-       )?;
-       let _publisher = _node.create_publisher::<std_msgs::msg::String>("out_topic")?;
-       Ok(Self {
-           _node,
-           _subscription,
-           _publisher,
-           _data,
-       })
-   }
-   fn republish(&self) -> Result<(), rclrs::RclrsError> {
+    fn new(executor: &rclrs::Executor) -> Result<Self, rclrs::RclrsError> {
+        let _node = executor.create_node("republisher")?;
+        let _data = Arc::new(Mutex::new(None));
+        let data_cb = Arc::clone(&_data);
+        let _subscription = _node.create_subscription(
+            "in_topic".keep_last(10).transient_local(),
+            move |msg: StringMsg| {
+                *data_cb.lock().unwrap() = Some(msg);
+            },
+        )?;
+        let _publisher = _node.create_publisher::<std_msgs::msg::String>("out_topic")?;
+        Ok(Self {
+            _node,
+            _subscription,
+            _publisher,
+            _data,
+        })
+    }
+
+    fn republish(&self) -> Result<(), rclrs::RclrsError> {
         if let Some(s) = &*self._data.lock().unwrap() {
             self._publisher.publish(s)?;
-                                                }
+        }
         Ok(())
     }
 }
@@ -283,7 +273,7 @@ fn main() -> Result<(), rclrs::RclrsError> {
         }
     });
     executor
-       .spin(SpinOptions::default())
-       .first_error()
-       .map_err(|err| err.into())
- }
+        .spin(SpinOptions::default())
+        .first_error()
+        .map_err(|err| err.into())
+}