ros2_rust icon indicating copy to clipboard operation
ros2_rust copied to clipboard

Add domain_id to node/init options

Open nnmm opened this issue 2 years ago • 4 comments

The domain_id is a useful option. In ROS distros up to Foxy, it is part of the node options. Afterwards it was moved to the context, or more precisely, the init options. See https://github.com/ros2-rust/ros2_rust/pull/186#discussion_r885689072

We should use the "ros_distro" cfg attribute to add it in the correct place for each distro. I.e. for Foxy, the node builder structure (with a default value of RMW_DEFAULT_DOMAIN_ID), and in for Galactic and up, it's open. There is no context builder struct yet, but we could add one. Alternatively, we could make Context::new() accept an Option<RosDomainId>.

Additionally, a domain_id() getter could be added to the context (Galactic and up) or node (Foxy) that calls the respective rcl function.

nnmm avatar Jun 23 '22 11:06 nnmm

A ContextBuilder may not be a bad idea - if we go the avenue of tying signal handling to contexts within #188, a builder would make that process easier.

It would also help if we ever wanted to add anything else to contexts in future.

jhdcs avatar Jun 23 '22 12:06 jhdcs

@Soya-Onishi You haven't yet started with this, correct? Would you be fine with it if I assign this issue to @DS3a?

nnmm avatar Jun 23 '22 12:06 nnmm

@nnmm Sure! Please assign to @DS3a

Soya-Onishi avatar Jun 23 '22 12:06 Soya-Onishi

Thank you for explaining the issue... I will start with writing a context builder... And then proceed to add the domain_id functionality based on the distro.

DS3a avatar Jun 23 '22 13:06 DS3a