-
Notifications
You must be signed in to change notification settings - Fork 195
Add ROS 2 command line arguments design document #245
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
Conversation
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
I think we can (and probably should) augment this document to encompass all ROS 2 nodes command line features. Also, I wonder now that we're introducing options, if we should move some features away from the double leading underscore syntax e.g. |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
articles/150_ros_command_line.md
Outdated
Parameter assignment may be achieved using the `--param`/`-p` option. | ||
This option takes a single `name:=value` assignment statement, where `value` is in YAML format. | ||
|
||
As an example, to assign an integer value of `2` to an `int_param` upon running `some_node`, one may execute: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if integer
is a good example here. That also opens the question what makes this a parameter of type since there needs to be some magic to convert the type solely based on its value which has problems on its own.
Therefore I would suggest to simply use a string value here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I agree with using a string value to simplify the example but I also think that parameter value syntax cannot be left out of a command line specification. Here, just like when parameter files are parsed in rcl
and when launch
substitutions occur in Node
parameters, we delegate on YAML format rules for type inference based on values. Whether that's a good call or not is likely worth a separate discussion.
articles/150_ros_command_line.md
Outdated
--- | ||
|
||
- This will become a table of contents (this text will be scraped). | ||
{:toc} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a short section about the implementation. Basically where the extraction from argc
/ argv
happens and that all remaining arguments are still available for the user code to process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. It also raises an interesting question. Assuming extraction happens in rcl
, should we support arguments specific to upper layers e.g. rclcpp
or rclpy
? Those would have to be available unparsed, separate from user defined arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, but they could live in between the --ros-args
and --
, and rcl
could just ignore them and leave them in a "left over args" output. Then rclcpp
and/or rclpy
can use those arguments or make warnings/errors because there are unknown parameters left over.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@dirk-thomas PTAL |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@dirk-thomas @wjwwood PTAL at c079dc7 so we can call it done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Alright, going in! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
late lgtm too :)
Follow-up PR after the discussions that took place in #242.