fix(opcua): accept severity as an alias in event_alarms#501
Open
mfaferek93 wants to merge 2 commits into
Open
Conversation
…ently dropped event_alarms only read severity_override, so a severity: key was silently dropped and the fault fell back to the live event Severity. Accept severity as an alias (source-level and per-mapping), validated like the threshold path; warn on unknown keys; extract severity resolution into a testable helper.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes OPC UA event_alarms configuration parsing so a severity: key is no longer silently ignored, aligning behavior with the threshold alarm: block and the acceptance criteria from issue #500. It also extracts the event-alarm severity resolution into a small helper to make the precedence/band logic unit-testable.
Changes:
- Accept
severity:as an alias forseverity_override:inevent_alarms(source-level and per-mapping), validated via the existing severity validation path, withseverity_overridetaking precedence when both are present. - Warn on unknown keys in
event_alarmsentries and theirmappingsto prevent future typos from being silently dropped. - Extract live-severity band mapping + override precedence into
OpcuaPlugin::map_severity(...)and add unit/regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/ros2_medkit_plugins/ros2_medkit_opcua/src/node_map.cpp |
Parses severity: as an alias for severity_override: in event_alarms and warns on unknown keys. |
src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp |
Extracts event-alarm severity resolution into map_severity() and uses it in on_event_alarm(). |
src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp |
Declares the new map_severity() helper for unit testing. |
src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp |
Adds regression tests ensuring severity: sets the override and precedence behaves as intended. |
src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp |
Adds unit tests covering override-wins behavior and live-severity band boundary mapping. |
bburda
reviewed
Jul 4, 2026
bburda
left a comment
Collaborator
There was a problem hiding this comment.
Additional notes outside the inline comments:
- Behaviour change for existing configs: routing
severity_overridethroughvalidate_severitymeans a config with a non-canonical string (e.g.severity_override: high) no longer reaches the fault verbatim. It becomesERRORwith a warning. This matches the thresholdalarm:path and is an improvement, but it changes the emitted severity for such configs. Worth a line in the PR description or CHANGELOG. README.mdevent_alarmssection documentsseverity_overridebut not the newseverityalias. A one-line note thatevent_alarmsalso acceptsseverity:would match the fix.
…wins yaml-cpp reports a present-but-null severity_override: key as defined, so a blank override shadowed a valid severity: sibling and logged a spurious non-string warning. Gate on a real value (IsDefined and not IsNull). Add regression tests for the null-override fallback and invalid-bucket default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
event_alarmssilently dropping aseverity:key.node_map.cpp: theevent_alarmsloader now acceptsseverity:as an alias forseverity_override:(source-level and per-mapping), routed throughvalidate_severitylike the thresholdalarm:path, and warns on unknown keys in the block.event_alarmsentry withseverity: ERRORresolves to ERROR; the helper covers override-wins and the band boundaries.Closes #500.