Skip to content

Implement onParametersSet for handling only analyzers node parameters#551

Merged
ct2034 merged 2 commits into
ros:ros2from
Noel215:fix/update-on-new-parameters
May 19, 2026
Merged

Implement onParametersSet for handling only analyzers node parameters#551
ct2034 merged 2 commits into
ros:ros2from
Noel215:fix/update-on-new-parameters

Conversation

@Noel215
Copy link
Copy Markdown
Contributor

@Noel215 Noel215 commented Mar 5, 2026

The previous implementation is using a callback that is invoked for all the parameter events of all ROS 2 nodes and it filters by node name. It could lead to race conditions when setting parameters depending on the depth size of the callback.

With this new approach, the callback is only triggered when setting parameters for the analyzers node, avoiding possible race conditions when setting the parameters.

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.
@mergify mergify Bot added the ros2 PR tackling a ROS2 branch label Mar 5, 2026
@Noel215
Copy link
Copy Markdown
Contributor Author

Noel215 commented May 15, 2026

@ct2034 could you review/merge these changes?

Copy link
Copy Markdown
Collaborator

@ct2034 ct2034 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. This is a lot cleaner than before :)

@ct2034 ct2034 added the backport Mergify will try to backport this. (WIP / Testing the functionality) label May 19, 2026
@ct2034 ct2034 merged commit bfbbf22 into ros:ros2 May 19, 2026
3 checks passed
mergify Bot added a commit that referenced this pull request May 19, 2026
Implement onParametersSet for handling only analyzers node parameters (backport #551)
mergify Bot added a commit that referenced this pull request May 19, 2026
Implement onParametersSet for handling only analyzers node parameters (backport #551)
mergify Bot added a commit that referenced this pull request May 19, 2026
Implement onParametersSet for handling only analyzers node parameters (backport #551)
ct2034 added a commit to redvinaa/diagnostics that referenced this pull request May 20, 2026
…ros#551)

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.

Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
ct2034 added a commit that referenced this pull request May 20, 2026
* Add starting_up_state parameter to Updater

* Lint

* Change to unsigned char

* Remove condition

* Fix example relative path (#550)

* Implement onParametersSet for handling only analyzers node parameters (#551)

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.

Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>

* Get rid of deprecated rclcpp::spin_some() (#563)

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>

* -Wreorder

Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>

---------

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Noel Jiménez García <noel.jimenez.gar@gmail.com>
Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Maurice Alexander Purnawan <mauricepurnawan@gmail.com>
ct2034 added a commit that referenced this pull request May 20, 2026
* Add starting_up_state parameter to Updater

* Lint

* Change to unsigned char

* Remove condition

* Fix example relative path (#550)

* Implement onParametersSet for handling only analyzers node parameters (#551)

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.



* Get rid of deprecated rclcpp::spin_some() (#563)



* -Wreorder



---------






(cherry picked from commit 60232a7)

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Vince Reda <60265874+redvinaa@users.noreply.github.com>
Co-authored-by: Noel Jiménez García <noel.jimenez.gar@gmail.com>
Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Maurice Alexander Purnawan <mauricepurnawan@gmail.com>
ct2034 added a commit that referenced this pull request May 20, 2026
* Add starting_up_state parameter to Updater

* Lint

* Change to unsigned char

* Remove condition

* Fix example relative path (#550)

* Implement onParametersSet for handling only analyzers node parameters (#551)

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.



* Get rid of deprecated rclcpp::spin_some() (#563)



* -Wreorder



---------






(cherry picked from commit 60232a7)

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Vince Reda <60265874+redvinaa@users.noreply.github.com>
Co-authored-by: Noel Jiménez García <noel.jimenez.gar@gmail.com>
Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Maurice Alexander Purnawan <mauricepurnawan@gmail.com>
ct2034 added a commit that referenced this pull request May 20, 2026
* Add starting_up_state parameter to Updater

* Lint

* Change to unsigned char

* Remove condition

* Fix example relative path (#550)

* Implement onParametersSet for handling only analyzers node parameters (#551)

The previous implementation is using a callback that is invoked for all
the parameter events of all ROS 2 nodes and it filters by node name.
It could lead to race conditions when setting parameters depending on
the depth size of the callback.

With this new approach, the callback is only triggered when setting
parameters for the analyzers node, avoiding possible race conditions
when setting the parameters.



* Get rid of deprecated rclcpp::spin_some() (#563)



* -Wreorder



---------






(cherry picked from commit 60232a7)

Signed-off-by: mini-1235 <mauricepurnawan@gmail.com>
Signed-off-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Vince Reda <60265874+redvinaa@users.noreply.github.com>
Co-authored-by: Noel Jiménez García <noel.jimenez.gar@gmail.com>
Co-authored-by: Christian Henkel <christian.henkel2@de.bosch.com>
Co-authored-by: Maurice Alexander Purnawan <mauricepurnawan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Mergify will try to backport this. (WIP / Testing the functionality) ros2 PR tackling a ROS2 branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants