Add VDA5050 Safety State Broadcaster #1958

YaraShahin · 2025-10-08T16:41:23Z

This Pull Request introduces the VDA5050 Safety State Broadcaster, a controller for publishing safety state information in ROS 2. The broadcaster reads safety-related state interfaces from hardware and exposes them in standardized ROS messages, as defined by the VDA5050 standard, for easy integration with VDA5050-based fleet management systems, safety monitoring, and higher-level decision-making nodes.

Dependency: This PR depends on control_msgs#266 which introduces the VDA5050SafetyState message.

Features

Standardized Safety Output: Publishes a control_msgs::msg::VDA5050SafetyState message representing the current safety state of the system: field violation and E-stop status.
Flexible Interface Support: Reads from configurable state interfaces covering field violation, manual E-stop, remote E-stop, and auto-acknowledged E-stop, ensuring full compatibility with the VDA5050 schema.
Parameterization: Fully configurable through YAML using generate_parameter_library.

Published Topic

~/vda5050_safety_state (control_msgs::msg::VDA5050SafetyState) — publishes the combined safety state: field violation and prioritized E-stop status.

Contributions via pull requests are much appreciated. Before sending us a pull request, please ensure that:

Limited scope. Your PR should do one thing or one set of things. Avoid adding “random fixes” to PRs. Put those on separate PRs.
Give your PR a descriptive title. Add a short summary, if required.
Make sure the pipeline is green.
Don’t be afraid to request reviews from maintainers.
New code = new tests. If you are adding new functionality, always make sure to add some tests exercising the code and serving as live documentation of your original intention.

To send us a pull request, please:

Fork the repository.
Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
Commit to your fork using clear commit messages.
Send a pull request, answering any default questions in the pull request interface.
Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

christophfroehlich

First (very superficially) review

vda5050_safety_state_broadcaster/test/vda5050_safety_state_broadcaster_params.yaml

vda5050_safety_state_broadcaster/CMakeLists.txt

vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.xml

ros2_controllers.rolling.repos

Co-authored-by: Christoph Fröhlich <[email protected]>

christophfroehlich

CI fails because of the API breaking changes of the realtime_publisher.

I have a general question, shouldn't we use data_type bool for state_interfaces here? Or at least support both?

vda5050_safety_state_broadcaster/package.xml

...te_broadcaster/include/vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.hpp

christophfroehlich · 2025-12-31T09:46:55Z

...te_broadcaster/include/vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.hpp

+  /**
+   * @brief Safely converts a double value to bool, treating NaN as false.
+   * @param value The double value to convert.
+   * @return true if value is not NaN and not zero, false otherwise.
+   */
+  bool safe_double_to_bool(double value) const
+  {
+    if (std::isnan(value))
+    {
+      return false;
+    }
+    return value != 0.0;
+  }
+};


if we stay with double values, should we place this somewhere else? controller_interface/helpers for example?

Here, I would rather use the logic to support boolean interfaces too based on the get_data_type method

I added bool support with the get_data_type method as @saikishor recommended.

I’ve kept safe_double_to_bool for now because it's still necessary to handle NaN cases and the conversion logic for existing double-based interfaces. Regarding moving it to controller_interface/helpers: I agree it could be a useful utility for other broadcasters. I’m happy to move it there (or to a common header) if you think it's generic enough for the wider framework.

@saikishor where would this fit best?

Co-authored-by: Christoph Fröhlich <[email protected]>

…broadcaster/vda5050_safety_state_broadcaster.hpp Co-authored-by: Christoph Fröhlich <[email protected]>

saikishor

Overall looking pretty good. Some nitpicks

saikishor · 2026-01-03T02:49:04Z

vda5050_safety_state_broadcaster/src/vda5050_safety_state_broadcaster.yaml

+  fieldViolation_interfaces: {
+    type: string_array,
+    default_value: [],
+    description: "names of interfaces that are used to acknowledge field violation events by setting the interface to 1.0.",
+  }
+  eStop_manual_interfaces: {
+    type: string_array,
+    default_value: [],
+    description: "names of interfaces that are used to manually acknowledge eStop events by setting the interface to 1.0.",
+  }
+  eStop_remote_interfaces: {
+    type: string_array,
+    default_value: [],
+    description: "names of interfaces that are used to remotely acknowledge eStop events by setting the interface to 1.0.",
+  }
+  eStop_autoack_interfaces: {
+    type: string_array,
+    default_value: [],
+    description: "names of interfaces that are used to autoacknowledge eStop events by setting the interface to 1.0.",
+  }


let's place then under the namesapce interfaces or eStop_Interfaces or emergency_stop_interfaces and remove the interfaces prefixes in the naming

I have moved the three E-Stop types under a nested eStop_interfaces namespace (e.g., eStop_interfaces.manual, eStop_interfaces.remote, etc.) and removed the repetitive prefixes.

I’ve kept fieldViolation_interfaces at the top level because it is logically distinct from the Emergency Stop handling in the VDA5050 specification. What do you think?

Nice. Thanks for the change. Regarding your comment, does it make sense like interfaces.fieldViolation and then interfaces.eStop.* ?

What do you think?. Just throwing an idea.

Personally, I'm not a big fan of deep nesting because it makes the C++ access verbose, but I agree that grouping them under a unified interfaces namespace looks much cleaner in the YAML. I've implemented it as interfaces.field_violation and interfaces.e_stop.*. Thanks for the suggestion!

vda5050_safety_state_broadcaster/CMakeLists.txt

vda5050_safety_state_broadcaster/doc/userdoc.rst

saikishor · 2026-01-03T02:55:27Z

...te_broadcaster/include/vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.hpp

+  /**
+   * @brief Safely converts a double value to bool, treating NaN as false.
+   * @param value The double value to convert.
+   * @return true if value is not NaN and not zero, false otherwise.
+   */
+  bool safe_double_to_bool(double value) const
+  {
+    if (std::isnan(value))
+    {
+      return false;
+    }
+    return value != 0.0;
+  }
+};


Here, I would rather use the logic to support boolean interfaces too based on the get_data_type method

vda5050_safety_state_broadcaster/package.xml

vda5050_safety_state_broadcaster/CMakeLists.txt

vda5050_safety_state_broadcaster/src/vda5050_safety_state_broadcaster.cpp

saikishor · 2026-01-03T03:01:36Z

vda5050_safety_state_broadcaster/src/vda5050_safety_state_broadcaster.cpp

+    if (safe_double_to_bool(
+          state_interfaces_[itf_idx].get_optional().value_or(kUninitializedValue)))
+    {
+      fieldViolation_value = true;
+      break;
+    }
+  }


It would be nice to also support boolean interfaces here based on the type you get from get_data_type

vda5050_safety_state_broadcaster/src/vda5050_safety_state_broadcaster.cpp

YaraShahin · 2026-01-03T14:24:10Z

CI fails because of the API breaking changes of the realtime_publisher.

I updated the APIs for realtime_publisher and LoanedStateInterface. I ran the tests locally on jazzy and rolling.

I have a general question, shouldn't we use data_type bool for state_interfaces here? Or at least support both?

Yes, thanks @christophfroehlich for the suggestion. I added bool support with the get_data_type method as @saikishor recommended and added a testcases for the new bool interfaces.

Co-authored-by: Sai Kishor Kothakota <[email protected]>

saikishor · 2026-01-03T14:43:24Z

Thank you @YaraShahin . I'll take a look.

…dcaster.cpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

christophfroehlich

Thanks for the updates. Some comments in the code, and please consider failing tests, like that from clang

  /home/runner/work/ros2_controllers/ros2_controllers/.work/target_ws/src/ros2_controllers/vda5050_safety_state_broadcaster/src/vda5050_safety_state_broadcaster.cpp:211:27: error: implicit conversion changes signedness: 'int' to 'size_type' (aka 'unsigned long') [-Werror,-Wsign-conversion]
    211 |         state_interfaces_[itf_idx].get_name().c_str());
        |         ~~~~~~~~~~~~~~~~~ ^~~~~~~

christophfroehlich · 2026-01-07T06:58:12Z

...te_broadcaster/include/vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.hpp

  std::shared_ptr<vda5050_safety_state_broadcaster::ParamListener> param_listener_;
  std::shared_ptr<rclcpp::Publisher<control_msgs::msg::VDA5050SafetyState>>
    vda5050_safety_state_publisher_;
+  control_msgs::msg::VDA5050SafetyState safety_state_msg;


nitpick, but we typically mark member variables as such

Suggested change

control_msgs::msg::VDA5050SafetyState safety_state_msg;

control_msgs::msg::VDA5050SafetyState safety_state_msg_;

Thank you. I did the change in the latest commit along with the necessary corresponding changes in cpp.

christophfroehlich · 2026-01-07T06:59:55Z

...te_broadcaster/include/vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.hpp

+  /**
+   * @brief Safely converts a double value to bool, treating NaN as false.
+   * @param value The double value to convert.
+   * @return true if value is not NaN and not zero, false otherwise.
+   */
+  bool safe_double_to_bool(double value) const
+  {
+    if (std::isnan(value))
+    {
+      return false;
+    }
+    return value != 0.0;
+  }
+};


@saikishor where would this fit best?

vda5050_safety_state_broadcaster/doc/userdoc.rst

codecov · 2026-01-07T07:28:52Z

Codecov Report

❌ Patch coverage is 90.69767% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.88%. Comparing base (c703235) to head (6847ecc).

Files with missing lines	Patch %	Lines
...oadcaster/src/vda5050_safety_state_broadcaster.cpp	78.04%	14 Missing and 4 partials ⚠️
...ter/test/test_vda5050_safety_state_broadcaster.hpp	95.65%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1958      +/-   ##
==========================================
+ Coverage   84.79%   84.88%   +0.08%     
==========================================
  Files         151      156       +5     
  Lines       14607    14822     +215     
  Branches     1266     1285      +19     
==========================================
+ Hits        12386    12581     +195     
- Misses       1763     1777      +14     
- Partials      458      464       +6

Flag	Coverage Δ
unittests	`84.88% <90.69%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...e_broadcaster/vda5050_safety_state_broadcaster.hpp	`100.00% <100.00%> (ø)`
...est/test_load_vda5050_safety_state_broadcaster.cpp	`100.00% <100.00%> (ø)`
...ter/test/test_vda5050_safety_state_broadcaster.cpp	`100.00% <100.00%> (ø)`
...ter/test/test_vda5050_safety_state_broadcaster.hpp	`95.65% <95.65%> (ø)`
...oadcaster/src/vda5050_safety_state_broadcaster.cpp	`78.04% <78.04%> (ø)`

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions bot requested review from christophfroehlich, erickisos, homalozoa, olivier-stasse, rosterloh, saikishor and xguay October 8, 2025 16:41

christophfroehlich requested changes Oct 8, 2025

View reviewed changes

vda5050_safety_state_broadcaster/test/vda5050_safety_state_broadcaster_params.yaml Show resolved Hide resolved

vda5050_safety_state_broadcaster/CMakeLists.txt Outdated Show resolved Hide resolved

vda5050_safety_state_broadcaster/vda5050_safety_state_broadcaster.xml Show resolved Hide resolved

YaraShahin requested a review from christophfroehlich October 9, 2025 13:42

christophfroehlich added this to Review triage Nov 15, 2025

github-project-automation bot moved this to WIP in Review triage Nov 15, 2025

christophfroehlich moved this from WIP to Needs review in Review triage Nov 15, 2025

christophfroehlich reviewed Nov 15, 2025

View reviewed changes

ros2_controllers.rolling.repos Outdated Show resolved Hide resolved

Yara Shahin and others added 4 commits November 27, 2025 17:19

add vda5050 safety state broadcaster

67304ae

remove license from yaml

9018a98

fix build options

f05cdf0

TEMP: Add custom control_msgs repo

317e7ea

YaraShahin force-pushed the add-vda5050-safety-state-broadcaster branch from 50ca35f to 317e7ea Compare November 27, 2025 15:20

remove license from xml file

76bf882

YaraShahin requested a review from christophfroehlich November 27, 2025 15:26

YaraShahin and others added 2 commits December 29, 2025 11:28

Update ros2_controllers.rolling.repos

9e82216

Co-authored-by: Christoph Fröhlich <[email protected]>

Merge branch 'master' into add-vda5050-safety-state-broadcaster

1dbf823

christophfroehlich requested changes Dec 31, 2025

View reviewed changes

github-project-automation bot moved this from Needs review to WIP in Review triage Dec 31, 2025

YaraShahin and others added 6 commits January 2, 2026 15:10

Update vda5050_safety_state_broadcaster/package.xml

ae14bcb

Co-authored-by: Christoph Fröhlich <[email protected]>

Update vda5050_safety_state_broadcaster/package.xml

71e86a9

Co-authored-by: Christoph Fröhlich <[email protected]>

Update vda5050_safety_state_broadcaster/package.xml

6afd8c8

Co-authored-by: Christoph Fröhlich <[email protected]>

Update vda5050_safety_state_broadcaster/include/vda5050_safety_state_…

c3382a4

…broadcaster/vda5050_safety_state_broadcaster.hpp Co-authored-by: Christoph Fröhlich <[email protected]>

add parameter description

896bfd7

update realtime API

7f37f5e

update LoanedStateInterface API

494bbe4

saikishor requested changes Jan 3, 2026

View reviewed changes

add bool support

8017b34

YaraShahin and others added 4 commits January 3, 2026 16:25

Update vda5050_safety_state_broadcaster/CMakeLists.txt

108f7cb

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update vda5050_safety_state_broadcaster/CMakeLists.txt

bf2d8e5

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update vda5050_safety_state_broadcaster/package.xml

8a99a78

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update vda5050_safety_state_broadcaster/CMakeLists.txt

664b661

Co-authored-by: Sai Kishor Kothakota <[email protected]>

YaraShahin and others added 4 commits January 3, 2026 16:44

Update vda5050_safety_state_broadcaster/src/vda5050_safety_state_broa…

14b147a

…dcaster.cpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

change parameter interface suffix naming

979eab2

remove rolling private member msg_

620f71d

resolve rolling warnings

4c470a4

YaraShahin requested review from christophfroehlich and saikishor January 3, 2026 15:51

Merge branch 'master' into add-vda5050-safety-state-broadcaster

fa04ac2

christophfroehlich mentioned this pull request Jan 7, 2026

Boolean data type signals are not supported by GPIO Controller #1970

Open

christophfroehlich requested changes Jan 7, 2026

View reviewed changes

Yara Shahin added 4 commits January 7, 2026 11:21

fix docs job warning

a6cd37a

fix clang type errors

77d25b9

yaml param structure

09c6ccf

member variable naming

6847ecc

YaraShahin requested a review from christophfroehlich January 7, 2026 10:23

	control_msgs::msg::VDA5050SafetyState safety_state_msg;
	control_msgs::msg::VDA5050SafetyState safety_state_msg_;

Add VDA5050 Safety State Broadcaster #1958

Are you sure you want to change the base?

Add VDA5050 Safety State Broadcaster #1958

Uh oh!

Conversation

YaraShahin commented Oct 8, 2025

Features

Published Topic

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

YaraShahin commented Jan 3, 2026

Uh oh!

saikishor commented Jan 3, 2026

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

codecov bot commented Jan 7, 2026 •

edited

Loading