Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add publisher with member function example #261

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AlexKaravaev
Copy link
Contributor

I thought that it can be quite useful to have this in examples. It's like a mix between one of the rclcpp examples and republisher from tutorial.

What I don't like is that it's still not 100% member function, ideally we would have something like:

    fn init(&mut self) {
        thread::spawn(move || loop {
            self.publish();
        });
    }
    
    fn publish(&self){
            thread::sleep(Duration::from_secs(1));

            let mut msg = std_msgs::msg::String::default();

            msg.data = format!("Hello, world! {}", publish_count.lock().unwrap());

            println!("Publishing: [{}]", msg.data);

            publisher.lock().unwrap().publish(msg).unwrap();
            let mut num_count = publish_count.lock().unwrap();
            *num_count += 1;
    }

But then I didn't came up with an elegant solution to that, without any errors on self lifetime.

@nnmm
Copy link
Contributor

nnmm commented Sep 20, 2022

@AlexKaravaev
Copy link
Contributor Author

@nnmm Exactly. For me it was quite tricky actually to write something like that in Rust with ros2, so I thought that having example with that could be helpful to someone else.


use anyhow::Result;

struct Publisher {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you name this differently? It'd make it clear that it's not the Publsher from rclrs. For example,MinimalPublisher from the similar rclcpp example.

@esteve
Copy link
Collaborator

esteve commented Sep 21, 2022

@AlexKaravaev thanks a lot for the example! From reading this code, I do wonder how we can make rclrs more suitable for a usecase like this. Perhaps a trait that has a default implementation that standardizes the Node initialization, it'd be similar to how Nodes are created in C++ and that can be used as components.

Something like this:

pub struct Node {}

pub trait ComposableNode {
    fn new(node_name: &str) -> Self;

    fn init_node(node: &str) -> Node {
        // Common initialization
        Node {}
    }
}

pub struct MinimalPublisher {
    node: Node,
}

impl ComposableNode for MinimalPublisher {
    fn new(node_name: &str) -> Self {
        let composable_node = Self {
            node: Self::init_node(&node_name),
        };
        // do stuff with composable_node
        composable_node
    }
}

fn main() {
    let minimal_publisher = MinimalPublisher::new("mynode");
}

Do you think the addition of a ComposableNode in the rclrs would help here?

Comment on lines +1 to +59
use std::{
env,
sync::{Arc, Mutex},
thread,
time::Duration,
};

use anyhow::Result;

struct Publisher {
publisher: Arc<Mutex<rclrs::Publisher<std_msgs::msg::String>>>,
publish_count: Arc<Mutex<u32>>,
}

unsafe impl Send for Publisher {}

impl Publisher {
pub fn new(context: &rclrs::Context) -> Self {
let node = rclrs::create_node(context, "publisher").unwrap();

let publisher = node
.create_publisher::<std_msgs::msg::String>("topic", rclrs::QOS_PROFILE_DEFAULT)
.unwrap();

Self {
publisher: Arc::new(Mutex::new(publisher)),
publish_count: Arc::new(Mutex::new(0)),
}
}

fn init(&mut self) {
let publish_count = self.publish_count.clone();
let publisher = self.publisher.clone();

thread::spawn(move || loop {
thread::sleep(Duration::from_secs(1));

let msg = std_msgs::msg::String {
data: format!("Hello, world! {}", publish_count.lock().unwrap()),
};

println!("Publishing: [{}]", msg.data);

publisher.lock().unwrap().publish(msg).unwrap();
let mut num_count = publish_count.lock().unwrap();
*num_count += 1;
});
}
}

fn main() -> Result<(), rclrs::RclrsError> {
let context = rclrs::Context::new(env::args())?;
let mut publisher = Publisher::new(&context);
publisher.init();
while context.ok() {
std::thread::sleep(std::time::Duration::from_millis(100));
}
Ok(())
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I would change:

  • add a timer_callback() like in the original rclcpp version
  • use an atomic variable instead of a Mutex for the counter
  • make all fallible methods return Result
  • remove unneeded lines (e.g. unsafe impl Send for Publisher {} and use anyhow::Result)
Suggested change
use std::{
env,
sync::{Arc, Mutex},
thread,
time::Duration,
};
use anyhow::Result;
struct Publisher {
publisher: Arc<Mutex<rclrs::Publisher<std_msgs::msg::String>>>,
publish_count: Arc<Mutex<u32>>,
}
unsafe impl Send for Publisher {}
impl Publisher {
pub fn new(context: &rclrs::Context) -> Self {
let node = rclrs::create_node(context, "publisher").unwrap();
let publisher = node
.create_publisher::<std_msgs::msg::String>("topic", rclrs::QOS_PROFILE_DEFAULT)
.unwrap();
Self {
publisher: Arc::new(Mutex::new(publisher)),
publish_count: Arc::new(Mutex::new(0)),
}
}
fn init(&mut self) {
let publish_count = self.publish_count.clone();
let publisher = self.publisher.clone();
thread::spawn(move || loop {
thread::sleep(Duration::from_secs(1));
let msg = std_msgs::msg::String {
data: format!("Hello, world! {}", publish_count.lock().unwrap()),
};
println!("Publishing: [{}]", msg.data);
publisher.lock().unwrap().publish(msg).unwrap();
let mut num_count = publish_count.lock().unwrap();
*num_count += 1;
});
}
}
fn main() -> Result<(), rclrs::RclrsError> {
let context = rclrs::Context::new(env::args())?;
let mut publisher = Publisher::new(&context);
publisher.init();
while context.ok() {
std::thread::sleep(std::time::Duration::from_millis(100));
}
Ok(())
}
use std::{
env,
sync::atomic::{AtomicU32, Ordering},
sync::{Arc, Mutex},
thread,
time::Duration,
};
use rclrs::RclrsError;
struct MinimalPublisher {
publisher: Arc<Mutex<rclrs::Publisher<std_msgs::msg::String>>>,
publish_count: AtomicU32,
}
impl MinimalPublisher {
pub fn new(context: &rclrs::Context) -> Result<Self, RclrsError> {
let node = rclrs::create_node(context, "publisher")?;
let publisher =
node.create_publisher::<std_msgs::msg::String>("topic", rclrs::QOS_PROFILE_DEFAULT)?;
Ok(Self {
publisher: Arc::new(Mutex::new(publisher)),
publish_count: AtomicU32::new(0),
})
}
fn timer_callback(&self) -> Result<(), RclrsError> {
let cur_publish_count = self.publish_count.fetch_add(1, Ordering::Relaxed);
let msg = std_msgs::msg::String {
data: format!("Hello, world! {}", cur_publish_count),
};
println!("Publishing: [{}]", msg.data);
self.publisher.lock().unwrap().publish(msg)
}
// Simulates a timer
fn run(&self) -> Result<(), RclrsError> {
loop {
thread::sleep(Duration::from_secs(1));
self.timer_callback()?;
}
}
}
fn main() -> Result<(), rclrs::RclrsError> {
let context = rclrs::Context::new(env::args())?;
let publisher = MinimalPublisher::new(&context)?;
publisher.run()
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, I'm on the fence on whether adding this example makes sense before we have timers. With timers, it may look very different.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants