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.

Sign up for GitHub

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

Jump to bottom

QoS - Liveliness, Deadline, and Lifespan C++ Demos #320

Merged
merged 6 commits into from
May 17, 2019
Merged

QoS - Liveliness, Deadline, and Lifespan C++ Demos #320

merged 6 commits into from
May 17, 2019

Conversation

emersonknapp
Copy link
Contributor

@emersonknapp emersonknapp commented Mar 29, 2019 •
edited
Loading

depends on ros2/rmw#171
depends on ros2/rclcpp#673

Add dedicated C++ demos for each of the newly-introduced Quality of Service policies.

@tfoote tfoote added the in progress Actively being worked on (Kanban column) label Mar 29, 2019
@nburek nburek mentioned this pull request Apr 1, 2019
Closed
29 tasks
@emersonknapp emersonknapp changed the title Liveliness, Deadline, and Lifespan QoS Demos QoS - Liveliness, Deadline, and Lifespan Demos Apr 11, 2019
@emersonknapp emersonknapp changed the title QoS - Liveliness, Deadline, and Lifespan Demos QoS - Liveliness, Deadline, and Lifespan C++ Demos Apr 16, 2019
@wjwwood wjwwood added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 16, 2019
wjwwood
wjwwood reviewed Apr 29, 2019
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

early review

quality_of_service_demo/README.md Outdated Show resolved Hide resolved
quality_of_service_demo/README.md Outdated Show resolved Hide resolved
quality_of_service_demo/README.md Show resolved Hide resolved
quality_of_service_demo/README.md Outdated Show resolved Hide resolved
quality_of_service_demo/README.md Outdated Show resolved Hide resolved
quality_of_service_demo/src/common_nodes.cpp Outdated Show resolved Hide resolved
quality_of_service_demo/src/deadline.cpp Outdated Show resolved Hide resolved
quality_of_service_demo/src/deadline.cpp Outdated Show resolved Hide resolved
quality_of_service_demo/src/deadline.cpp Show resolved Hide resolved
quality_of_service_demo/src/lifespan.cpp Outdated Show resolved Hide resolved
Emerson Knapp and others added 5 commits May 13, 2019 14:27
@mm318
Add demos for liveliness, lifespan, and deadline QoS policies
9d0af6d 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@mm318
Comment out portions that require non-merged work
882cce9 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@mm318
Uncomment functionality, fix build, fix docs, clarify CLI options
3e0c641 
Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@mm318
update to be compatible with latest rclcpp API
895c1a2 
Signed-off-by: Miaofei <miaofei@amazon.com>
Address remaining PR feedback. Mostly code comments and documentation…
8ea63da 
…. Some code restructure and extra variable name verbosity.

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Contributor Author

Thanks for the feedback - I've uploaded a new version. Since this doesn't fall under feature freeze, I totally understand if we leave it until after that to review.

@dirk-thomas
Copy link
Member

@thomas-moulard Please run CI for this.

@dirk-thomas
Copy link
Member

Please run CI for this.

@thomas-moulard Friendly ping.

@thomas-moulard
Copy link

Ack.

@emersonknapp
Copy link
Contributor Author

emersonknapp commented May 17, 2019 •
edited
Loading

@thomas-moulard
Gist file: https://gist.githubusercontent.com/emersonknapp/bd3fe42d8b5d2e9bb6cd980a19b21ded/raw/b03011985fe0587349389b73900b45c96f3f8738/ros2.repos
Test flags: --packages-select quality_of_service_demo
Build flags: --packages-up-to quality_of_service_demo
Ci: ci_launcher

@thomas-moulard
Copy link

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

wjwwood
wjwwood requested changes May 17, 2019
View reviewed changes
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

I'd like to see the examples be made consistent with each other before merging, but otherwise only minor comments.

quality_of_service_demo/src/lifespan.cpp Outdated Show resolved Hide resolved
quality_of_service_demo/src/lifespan.cpp Show resolved Hide resolved
quality_of_service_demo/src/liveliness.cpp Outdated Show resolved Hide resolved
quality_of_service_demo/src/liveliness.cpp Outdated Show resolved Hide resolved
@thomas-moulard
Copy link

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Unwrap liveliness demo, fix argument usage messaging, make code order…
70cbb1a 
…ing consistent across all demos

Signed-off-by: Emerson Knapp <eknapp@amazon.com>
@emersonknapp
Copy link
Contributor Author

@thomas-moulard let's re-run with comments addressed, and the windows warning fixed
Gist file: https://gist.githubusercontent.com/emersonknapp/700c8e974ee963f7aeface33c1d70b45/raw/b03011985fe0587349389b73900b45c96f3f8738/ros2.repos
Build flags: --packages-up-to quality_of_service_demo
Test flags: --packages-select quality_of_service_demo
Job: ci_launcher

@dirk-thomas dirk-thomas requested a review from wjwwood May 17, 2019 18:49
@thomas-moulard
Copy link

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@dirk-thomas dirk-thomas merged commit 20a0d13 into ros2:master May 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wjwwood wjwwood wjwwood approved these changes

Assignees
No one assigned
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@emersonknapp @dirk-thomas @thomas-moulard @wjwwood @tfoote @mm318