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

Support for pre-set and post-set parameter callback. #966

Merged
merged 23 commits into from
Jul 25, 2022
Merged

Support for pre-set and post-set parameter callback. #966

merged 23 commits into from
Jul 25, 2022

Conversation

deepanshubansal01
Copy link
Contributor

@deepanshubansal01 deepanshubansal01 commented Jul 11, 2022 •
edited
Loading

This PR supports adding new parameters registered callbacks namely pre_set_parameter and post_set_parameter callback to node API. Full description can be found in rclcpp version of the PR here ros2/rclcpp#1947

Design document: https://github.com/ros2/rclcpp/blob/deepanshu/local-param-changed-callback-support/rclcpp/doc/proposed_node_parameter_callbacks.md
Demo node: ros2/demos#565
rclcpp ticket: ros2/rclcpp#1947
Related discussion: ros2/rclcpp#1190, ros2/rclcpp#609

Signed-off-by: deepanshu deepanshubansal01@gmail.com

@deepanshubansal01 deepanshubansal01 marked this pull request as draft July 11, 2022 15:43
@deepanshubansal01 deepanshubansal01 self-assigned this Jul 11, 2022
@deepanshubansal01 deepanshubansal01 mentioned this pull request Jul 11, 2022
Merged
@deepanshubansal01 deepanshubansal01 marked this pull request as ready for review July 12, 2022 20:24
@deepanshubansal01 deepanshubansal01 mentioned this pull request Jul 13, 2022
Merged
@deepanshubansal01
add support for pre and post param registered callback
83920cf 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
Revert "add support for pre and post param registered callback"
cb3d3f0 
This reverts commit 999628b.

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
add support for pre and post param registered callback
5bec0da 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
add some documentation and testing of pre callback
f95d822 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
simplify params API
049411e 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
simplify params API
2b66e89 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
simplify params API
b4d300f 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
added some TODO's
38fbb00 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
correct refactored API's
73b3194 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
Remove redundant _set_parameters func
3cc949d 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
_call_pre_set_parameters_callback func
7866af9 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
return modified list from pre-set param callback
76714f6 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
pre-set param callback logic change
822a6a1 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
change function defination of remove_pre_set_param_callback
0ad9943 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
added unit test for pre_set_param_callback
161031b 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
Add a separate funct for post_set_param handling
4070088 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
added more unit tests for pre set and post set param callback
18846e4 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
updated documentation of Node API's based on pre and post set param c…
3afa6ee 
…allbacks

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
minor change in documentation for set_param API
4dd60b4 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
ament_flake8 and ament_pyflakes
ab800cf 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01 deepanshubansal01 force-pushed the deepanshu/local-parameter-callback-support branch from 0570f70 to ab800cf Compare July 13, 2022 21:36
@deepanshubansal01
ament_pep257
d4274e4 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
update API documentation
ddc0b36 
Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
fujitatomoya
fujitatomoya approved these changes Jul 18, 2022
View reviewed changes
Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

lgtm

@adityapande-1995
Copy link

adityapande-1995 commented Jul 18, 2022 •
edited
Loading

Full CI :

build : --packages-above-and-dependencies rclpy
test : --packages-above rclpy

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

@deepanshubansal01
add unit test for empty pre param callback and made some changes afte…
37f023f 
…r CI run failure

Signed-off-by: deepanshu <deepanshubansal01@gmail.com>
@deepanshubansal01
Copy link
Contributor Author

@adityapande-1995 we can try running the CI again now:)

@adityapande-1995
Copy link

adityapande-1995 commented Jul 18, 2022 •
edited
Loading

CI round 2 :

build : --packages-above-and-dependencies rclpy
test : --packages-above rclpy

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

@adityapande-1995
Copy link

CI round 3 :

build : --packages-above-and-dependencies rclpy
test : --packages-above rclpy

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

@adityapande-1995 adityapande-1995 merged commit 7685ee2 into rolling Jul 25, 2022
@delete-merged-branch delete-merged-branch bot deleted the deepanshu/local-parameter-callback-support branch July 25, 2022 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adityapande-1995 adityapande-1995 adityapande-1995 approved these changes

@fujitatomoya fujitatomoya fujitatomoya approved these changes

@jacobperron jacobperron Awaiting requested review from jacobperron

Assignees

@deepanshubansal01 deepanshubansal01

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@deepanshubansal01 @adityapande-1995 @fujitatomoya