Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

[rostest] Refactor hztestX.test#862

Merged
dirk-thomas merged 3 commits intoros:kinetic-develfrom
wkentaro:refactor-hztest-test
Aug 24, 2016
Merged

[rostest] Refactor hztestX.test#862
dirk-thomas merged 3 commits intoros:kinetic-develfrom
wkentaro:refactor-hztest-test

Conversation

@wkentaro
Copy link
Copy Markdown
Contributor

@wkentaro wkentaro commented Aug 11, 2016

Why?

The files hztestX.test are the samples for ROS users for testing nodes.
This PR simplify the test xml to demonstrate how to use <rosparam> for private params.

@wkentaro wkentaro force-pushed the refactor-hztest-test branch from 2a01baa to 47db21c Compare August 11, 2016 18:21
@dirk-thomas
Copy link
Copy Markdown
Member

Please add more information to the pull request describing why you want this to be changed.

@wkentaro
Copy link
Copy Markdown
Contributor Author

Updated the information.

@dirk-thomas
Copy link
Copy Markdown
Member

For new ROS users I would expect the previous way of setting the parameters to be easier and much less error prone. Each parameter is very simple and separate from the others. Using <rosparam> for this requires writting valid yaml which some users might not be familiar with. So I think this changes raises the bar to understand the example which I don't think it a good idea.

@wkentaro
Copy link
Copy Markdown
Contributor Author

Your comments are reasonable, but in the first look, I thought the <rosparam> does not work for hztest test node. So can I add a comment about the another style of code using rosparam?

@wkentaro wkentaro force-pushed the refactor-hztest-test branch from 7c13ea7 to 3549e78 Compare August 24, 2016 08:48
@wkentaro
Copy link
Copy Markdown
Contributor Author

Fixed.

@dirk-thomas
Copy link
Copy Markdown
Member

Looks good to me. Thanks!.

@dirk-thomas dirk-thomas merged commit 75b1397 into ros:kinetic-devel Aug 24, 2016
@wkentaro wkentaro deleted the refactor-hztest-test branch August 24, 2016 19:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants