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 domain_id to node/init options #206

Open
nnmm opened this issue Jun 23, 2022 · 4 comments
Open

Add domain_id to node/init options #206

nnmm opened this issue Jun 23, 2022 · 4 comments

Comments

@nnmm
Copy link
Contributor

nnmm commented Jun 23, 2022

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 #186 (comment)

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 nnmm changed the title Add domain_id to node options Add domain_id to node/init options Jun 23, 2022
@jhdcs
Copy link
Collaborator

jhdcs commented Jun 23, 2022

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.

@nnmm
Copy link
Contributor Author

nnmm commented Jun 23, 2022

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

@Soya-Onishi
Copy link
Contributor

@nnmm Sure! Please assign to @DS3a

@DS3a
Copy link
Contributor

DS3a commented Jun 23, 2022

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 added a commit to DS3a/ros2_rust that referenced this issue Jul 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants