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

Add Node API method for setting the parameters_callback. #228

Merged
merged 2 commits into from
Aug 24, 2018

Conversation

nuclearsandwich
Copy link
Member

In #214 I wired up support for invoking a callback function to accept or reject a set of parameters but did not add the API to set that callback or tests for that functionality.

This pull request adds a set_parameters_callback method to rclpy.Node and some unit tests verifying that the callbacks are used and that the parameter values are set or not appropriately.

Every test case I used has been put into the unit tests below.

Connects to #202

@nuclearsandwich nuclearsandwich added the in review Waiting for review (Kanban column) label Aug 20, 2018
@nuclearsandwich nuclearsandwich self-assigned this Aug 20, 2018
@dirk-thomas dirk-thomas mentioned this pull request Aug 20, 2018
6 tasks
Copy link
Member

@mikaelarguedas mikaelarguedas left a comment

Choose a reason for hiding this comment

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

lgtm with one inline question

Is there CI for this PR ?

self.node.destroy_node()

def test_set_callback_accepting_all(self):
global callback_called
Copy link
Member

Choose a reason for hiding this comment

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

is it necessary for this to be global? could it be local to each test case and nonlocal to the callbacks?

Copy link
Member Author

Choose a reason for hiding this comment

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

TIL about nonlocal That's precisely what I needed. Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

resolved by 73f6cf2

@nuclearsandwich
Copy link
Member Author

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

@nuclearsandwich nuclearsandwich merged commit e7b2932 into master Aug 24, 2018
@nuclearsandwich nuclearsandwich deleted the parameters-callback branch August 24, 2018 17:50
@nuclearsandwich nuclearsandwich removed the in review Waiting for review (Kanban column) label Aug 24, 2018
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.

3 participants