Skip to content

Add Node API method for setting the parameters_callback.#228

Merged
nuclearsandwich merged 2 commits intomasterfrom
parameters-callback
Aug 24, 2018
Merged

Add Node API method for setting the parameters_callback.#228
nuclearsandwich merged 2 commits intomasterfrom
parameters-callback

Conversation

@nuclearsandwich
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
YuanYuYuan pushed a commit to YuanYuYuan/rclpy that referenced this pull request Nov 12, 2025
Async Service Examples and Backend Fixes
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