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

DMW-Thesis #15

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Odin-byte
Copy link

No description provided.

@Odin-byte Odin-byte force-pushed the feature/prefix_for_multi_robot branch from ea5eb73 to b5c1539 Compare August 2, 2024 14:24
Comment on lines +766 to +771
parser.add_argument(
"-n",
"--number",
help="the number to append to each session name within the config file. used for multi robot setups. Can also be used within sessions as a env_variable. this is used as a unique ros domain id",
default="",
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit hesitant to just call this number
Maybe we should rather call this session_id_no or session_prefix_no or something

We should maybe also parse this argument as an integer explicitly to have some type checking

It is later used as ROS_DOMAIN_ID - is this really guaranteed to be unique?

Copy link
Contributor

@fmessmer fmessmer Aug 20, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe uuid?

Copy link
Author

Choose a reason for hiding this comment

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

I would lean towards session_prefix_no

Copy link
Contributor

Choose a reason for hiding this comment

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

instance_id it is - as discussed with @Odin-byte

@fmessmer fmessmer marked this pull request as ready for review August 20, 2024 13:36
@fmessmer
Copy link
Contributor

fmessmer commented Aug 21, 2024

the (merge-able) version is provided via #19
in order to not conflict with @Odin-byte thesis work

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.

2 participants