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

Adding 'doc' attribute to roslaunch <arg> tags #379

Merged
merged 1 commit into from
Mar 14, 2014

Conversation

jbohren
Copy link
Member

@jbohren jbohren commented Mar 14, 2014

This is implemented as described in #294 (comment)

Additionally, it brings arg_dump.py into python3 compliance with the print() function.

This should also be a viable modification for release into hydro.

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

The output for the following roslaunch file is shown below:

<launch>
  <arg name="foo" default="true" doc="I pity the foo'."/>
  <arg name="bar" doc="Someone walks into this."/>
  <arg name="baz" default="false"/>
  <arg name="nop"/>
</launch>
[moldy-crow:...mm/test/test_roslaunch/test]$ roslaunch --ros-args ros_args.launch
  bar: (default: None) Someone walks into this.
  baz: (default: "false") 
  foo: (default: "true") I pity the foo'.
  nop: (default: None) 

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

Actually, I just had another idea on the output, so hold off on this PR.

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

With the revised commit, it outputs the following:

[moldy-crow:...mm/test/test_roslaunch/test]$ roslaunch --ros-args ros_args.launch
Required Arguments:
  bar: Someone walks into this.
  nop: None
Optional Arguments:
  baz (default "false"): None
  foo (default "true"): I pity the foo'.

@dirk-thomas
Copy link
Member

Please remove all Python related changes from the PR. There is a separate PR pending for Python 3 compatibility (#358).

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

Will do.

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

Updated to remove python3 future stuff.

@@ -45,7 +45,8 @@
def get_args(roslaunch_files):
loader = roslaunch.xmlloader.XmlLoader(resolve_anon=False)
config = load_config_default(roslaunch_files, None, loader=loader, verbose=False, assign_machines=False)
return loader.root_context.resolve_dict['arg']
resolve_dict = loader.root_context.resolve_dict
return resolve_dict['arg_doc'] if 'arg_doc' in resolve_dict else dict()
Copy link
Member

Choose a reason for hiding this comment

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

Please use the following instead:

return getattr(loader.root_context.resolve_dict, 'arg_doc', {})

Copy link
Member Author

Choose a reason for hiding this comment

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

Replaced with return loader.root_context.resolve_dict.get('arg_doc', {})

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

The current PR has the following behavior:

<launch>
  <!-- ros_args.launch -->
  <arg name="foo" default="true" doc="I pity the foo'."/>
  <arg name="bar" doc="Someone walks into this."/>
  <arg name="baz" default="false"/>
  <arg name="nop"/>
  <arg name="fix" value="true"/>
</launch>
$> roslaunch --ros-args ros_args.launch
Required Arguments:
  bar: Someone walks into this.
  nop: undocumented
Optional Arguments:
  baz (default "false"): undocumented
  foo (default "true"): I pity the foo'.

@dirk-thomas
Copy link
Member

Could you also create a simple test which invoked the command line / API on the example launch file you already added? That would ensure that the functionality will keep working in the future.

@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

Unit test added.

@dirk-thomas
Copy link
Member

Thank you for that great PR. The CI job passes and the output of --ros-args is now much more helpful.

One minor thing before the merge, can please also update the test script to use spaces after commas as well as after colons in a dict (to follow PEP8).

* This adds a 'doc' attribute to the roslaunch <arg> tag
* This tag is read in and stored in the resolve_dict under 'arg_doc'
* The keys in resolve_dict['arg_doc'] are the argument names, and the
  values are tuples where the first element is None or the doc string,
  and the second element is None or the default value.
* The arg_dump.get_args function still returns a dict, so the return
  value's keys can still be used for a list of arg names.
* A unit test has been added to test_roslaunch/test/roslaunch.test which
  loads the ros_args.launch launchfile and compares the output of
  arg_dump.get_args to the expected output.
@jbohren
Copy link
Member Author

jbohren commented Mar 14, 2014

One minor thing before the merge, can please also update the test script to use spaces after commas as well as after colons in a dict (to follow PEP8).

Done.

@130s
Copy link
Member

130s commented Oct 28, 2015

Wiki page is updated for this change. Please add any correction if necessary.

rsinnet pushed a commit to MisoRobotics/ros_comm that referenced this pull request Jun 19, 2017
Adding 'doc' attribute to roslaunch <arg> tags
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.

3 participants