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 configurability #635

Merged
merged 29 commits into from
Nov 17, 2020
Merged

Qos configurability #635

merged 29 commits into from
Nov 17, 2020

Conversation

ivanpauno
Copy link
Member

@ivanpauno ivanpauno commented Oct 28, 2020 •
edited
Loading

Depends on #636. (update:merged)
Equivalent to ros2/rclcpp#1408.

It adds all the needed helper function and classes to enable qos configurability.
I haven't yet modified Node.create_publisher and Node.create_subscription. (update: done)

@ivanpauno ivanpauno added the enhancement New feature or request label Oct 28, 2020
@ivanpauno ivanpauno self-assigned this Oct 28, 2020
@ivanpauno ivanpauno mentioned this pull request Oct 28, 2020
Merged
@ivanpauno ivanpauno force-pushed the ivanpauno/qos-configurability branch from 8d7b21c to ef72cbd Compare October 28, 2020 19:28
@ivanpauno ivanpauno marked this pull request as ready for review October 28, 2020 20:53
jacobperron
jacobperron reviewed Oct 29, 2020
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

Looking good!

rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Show resolved Hide resolved
rclpy/rclpy/qos_overriding_options.py Outdated Show resolved Hide resolved
ivanpauno and others added 22 commits October 30, 2020 17:02
@ivanpauno
Add QoSOverridingOptions
fdad6e3 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Fix Node.get_parameters_by_prefix() type annotation
3300d10 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Add helper classmethod for using default policies
67ac124 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Add test cases
80781c9 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Please linters + improve test setup and teardown
5fbc30c 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Declare qos overrides parameters in create_publisher/create_subscript…
5e6235b 
…ion + fixes

Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
b18d0ba 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
ef1e7bb 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
fa434f9 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
2e5297f 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
9b06ffa 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
style
b34a04c 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
51bddf4 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
fad4467 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
grammar
85a8cb7 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
589d2a2 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
bug
0c5e7ce 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
17c834a 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno
Fix _get_qos_policy_parameter type annotation
50700ac 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno @jacobperron
qos -> QoS
523dd59 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>

Co-authored-by: Jacob Perron <[email protected]>
@ivanpauno
_declare_qos_parameteres -> _declare_qos_parameters
812d44f 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Fix after rebasing
c4f109b 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno ivanpauno force-pushed the ivanpauno/qos-configurability branch from 3f2abbc to c4f109b Compare October 30, 2020 20:17
@ivanpauno
Copy link
Member Author

Force-pushed to resolve conflicts with master.

jacobperron
jacobperron approved these changes Nov 2, 2020
View reviewed changes
Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

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

LGTM

rclpy/rclpy/node.py Outdated Show resolved Hide resolved
@ivanpauno
Handle exception gracefully
751a601 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
jacobperron
jacobperron reviewed Nov 3, 2020
View reviewed changes
rclpy/rclpy/node.py Outdated Show resolved Hide resolved
@ivanpauno
Suppress exception context
59951a3 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
flake8
9fce94b 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
mjeronimo
mjeronimo approved these changes Nov 3, 2020
View reviewed changes
Copy link

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

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

Looks good.

@ivanpauno
Add test to CMakeLists
f8ba331 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Use declare_or_get logic
0446d24 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Improve testing
a9fc10f 
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
@ivanpauno
Copy link
Member Author

CI can be found here.

This was referenced Nov 9, 2020
Merged
Merged
@ivanpauno
Copy link
Member Author

@ros-pull-request-builder retest this please

@ivanpauno
Copy link
Member Author

ivanpauno commented Nov 17, 2020 •
edited
Loading

CI:

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

@ivanpauno ivanpauno merged commit ff56a94 into master Nov 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the ivanpauno/qos-configurability branch November 17, 2020 20:43
@jacobperron jacobperron mentioned this pull request Jan 13, 2021
Closed
@jacobperron jacobperron mentioned this pull request Jan 21, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacobperron jacobperron jacobperron approved these changes

@mjeronimo mjeronimo mjeronimo approved these changes

Assignees

@ivanpauno ivanpauno

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ivanpauno @mjeronimo @jacobperron