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

Provide for local callbacks on parameter events in parameter node API #609

Closed
tfoote opened this issue Dec 11, 2018 · 6 comments
Closed
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@tfoote
Copy link
Contributor

tfoote commented Dec 11, 2018

This is a follow up to discussions in #595 and #608 and somewhat related to #296 but for the post change callbacks not validation stage which would be a 3rd callback type proposed.

It would be valuable to not rely on parameter events being published but use the internal node interface API to get the events in a callback for different interfaces. Along with that change if there was a more fully featured API for registering callbacks for parameter changes locally, the parameter event publishing could also be split out into a separate component that could obviate the need for a parameter to turn that functionality on and off in the constructor of the parameter, and avoid the dependencies on the clock and topics interface for the core parameter API.

@ros-discourse
Copy link

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/security-ramifications-of-global-parameter-events/15924/8

@clalancette
Copy link
Contributor

This was actually completed for Iron, so closing this out.

@jrutgeer
Copy link
Contributor

@clalancette
It seems this issue is not fully complete, see this TODO item in the code.

I came across this due to someone asking about disabling /parameter_events subscribers on Robotics Stack Exchange.

So I think this should be reopened.

@clalancette
Copy link
Contributor

It seems this issue is not fully complete, see this TODO item in the code.

This issue is indeed complete; we now provide a local callback that can be used. If you'd like to track changing the code in time_source.cpp to use that, please feel free to open a new issue.

@jrutgeer
Copy link
Contributor

If you'd like to track changing the code in time_source.cpp to use that, please feel free to open a new issue.

Ok.

However, @fujitatomoya a few lines above in this thread there's a reference to a commit where you seem to already have implemented this?

fujitatomoya added a commit to fujitatomoya/rclcpp that referenced this issue on Oct 1, 2019
fujitatomoya@5f21f03

@fujitatomoya
Copy link
Collaborator

@jrutgeer to be honest, i do not remember... probably i was working on the experimental patch? bad memory, sorry 😓

created tracking issue for this, #2370

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants