Feat: add user callback #250
Conversation
|
ping @pac48 |
1 similar comment
|
ping @pac48 |
...ameter_library_py/generate_parameter_library_py/jinja_templates/cpp/parameter_library_header
Outdated
Show resolved
Hide resolved
| updated_params.__stamp = clock_.now(); | ||
| update_internal_params(updated_params); | ||
| if (user_callback_) { | ||
| user_callback_(updated_params); |
There was a problem hiding this comment.
The variable is called update_params. But I believe it contains all parameters, not just the ones that got updated right?
Would it be an idea to only supply the actually updated parameters? Otherwise the logging statements (like in the example) would be really verbose if you have multiple parameters. And from a log-reader standpoint you still don't have a clue what actually changed.
There was a problem hiding this comment.
That is a good point. The user callback signature could be changed to const std::vector<rclcpp::Parameter> ¶meters, which is only the updated parameters. The variable parameters is in scope. So we could change it to
if (user_callback_) {
user_callback_(parameters);
}
@YannickdeHoop Does this seem reasonable? The one thing I like about having all of the parameters is that dependencies between parameters could be considered in the callback, e.g. the user sets parameter A, but B was not set, so don't trigger some action.
There was a problem hiding this comment.
I get the point and I understand that this is a valid use case. But in my implementation I'd like to have all parameters not only the one that i updated. So I suggest to create an issue to implement another user_callback function that returns only the updated params.
There was a problem hiding this comment.
Sure, can do!
One last question: is setUserCallback the right name here to differentiate these two use cases?
Because once this is in, it's harder to rename because of backward compatibility etc.
|
@YannickdeHoop Can you run pre-commit to fix the CI failure |
3 participants
Hi,
I added functionality to bind a custom callback. This callback will be fired when the parameters are updated, and those updated parameters are then directly available. I think this covers #6