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

[rcl_interfaces] Homogenize iface definition #23

Merged
merged 4 commits into from
Nov 3, 2017

Conversation

mikaelarguedas
Copy link
Member

This is just cleanup

  • first commit make msg/srv definition consistent 27920fa
  • second commit moves the repo's README.md to the right folder 89da210
  • thiird commit creates a generic README for the entire repo 5cd70c2

README.md Outdated
* List the parameters on this node matching the filters.
* `set_parameters`: `SetParameters`
* Set parameters on this node.
A set of packages which contain interface files (.msg and .srv) used for the internal implementation and testing of the client libraries.
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't consider those "internal". Most of them define public interfaces, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

"used for the implementation and testing" then ?

Copy link
Member

Choose a reason for hiding this comment

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

"defining the interface across client libraries"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm not sure because as you said it is user facing and currently most of them are not used to communicate across client libraries given that they're used mostly/(only?) in rclcpp.

@ros2/team maybe some native speakers have some insights ?

Copy link
Member

Choose a reason for hiding this comment

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

They're definitely public and I would say their primary use is implementation of rcl concepts rather than testing, though they could be used for that too I guess.

"This repository contains a set of packages which themselves contain interface files (i.e. .msg and .srv files) used to implement client library concepts like parameters."

Copy link
Member Author

Choose a reason for hiding this comment

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

my goal here is to give a description of the content of the repository and not only describe the rcl_interfaces package, that's why I talked about testing (given that this repo also contains test_msgs).

I'm fine with "This repository contains a set of packages which themselves contain interface files (i.e. .msg and .srv files) used to implement client library concepts" I just wanted to clarify the scope

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I was confused by the diff and I didn't know there were test messages here. In that case I agree, something like:

"This repository contains a set of packages which themselves contain interface files (i.e. .msg and .srv files) which are used both to implement client library concepts and in testing."

Or something like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've updated it with approximately the verbiage of above.

@dhood dhood added the in review Waiting for review (Kanban column) label Nov 2, 2017
@mikaelarguedas
Copy link
Member Author

Thanks @tfoote
@ros2/team reay for review

@mikaelarguedas mikaelarguedas merged commit 7710246 into master Nov 3, 2017
@mikaelarguedas mikaelarguedas deleted the homogenize_iface_definition branch November 3, 2017 21:12
@mikaelarguedas mikaelarguedas removed the in review Waiting for review (Kanban column) label Nov 3, 2017
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.

None yet

5 participants