Refactor Node parse() function. #73
Conversation
Make use of ignore support in ExecuteProcess. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
Depends on ros2/launch#336 |
| kwargs['arguments'] = super()._parse_cmdline(args, parser) | ||
| node_name = entity.get_attr('node_name', optional=True) | ||
| if node_name is None and 'name' in kwargs: | ||
| node_name = kwargs['name'] |
There was a problem hiding this comment.
This is quite strange. So, if you just pass name you're setting the node_name. But if you pass node_name and name, the first send the node name and the second the executable name.
What if someone wants to set the executable name without remapping the node name?
I originally avoided to expose the executable name, because I didn't like <node ... node_name="..." .../>.
But if we want to add it, I think it should look like: <node ... name="..." exec-name="..." .../>.
There was a problem hiding this comment.
Yeap, that change makes absolutely no sense. Thanks for pointing it out. But I'll simply stop doing creative renaming in the parsing function. It's certainly not where we'd want to be. See a4be9d0.
There was a problem hiding this comment.
Sounds good. We should probably revisit naming of attributes in other actions ...
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
|
CI is in ros2/launch#336. |
delete-merged-branch
bot
deleted the
hidmic/execute-process-parse-extension
branch
|
@hidmic Can you update the tutorial and the design document that specifies the XML format? |
Precisely what the title says: ignore
cmdfrom ExecuteProcess and parseargsproperly.