feat(opcua): route alarms to distinct faults by message substring#506
feat(opcua): route alarms to distinct faults by message substring#506mfaferek93 wants to merge 4 commits into
Conversation
event_alarms mappings gained a match_message field: a case-sensitive substring match on the event Message, AND-combined with the existing condition_name/source_node/event_type matchers. Lets one OPC UA source route alarms that share an EventType and SourceNode (e.g. Program_Alarms from one Siemens S7-1500 FB) to their own SOVD fault by text.
There was a problem hiding this comment.
Pull request overview
Adds OPC UA alarm-routing support to disambiguate alarms that share condition_name / source_node / event_type by additionally matching on a case-sensitive substring of the event Message, enabling distinct SOVD fault codes for same-source PLC alarms (per #505).
Changes:
- Extend
AlarmMappingwithmatch_messageand updateNodeMap::resolve_alarm(...)to AND-combine substring matching with existing equality matchers. - Plumb the live/snapshot event message through
OpcuaPollerinto alarm resolution. - Update plugin documentation and add unit coverage for message-substring routing and fallback behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp | Updates existing resolve-alarm tests for new signature and adds MatchByMessageSubstring coverage. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp | Passes event/snapshot message into NodeMap::resolve_alarm for runtime + read-replay paths. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/node_map.cpp | Parses match_message from YAML and applies substring routing in resolve_alarm. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/README.md | Documents match_message behavior and clarifies matcher semantics. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/node_map.hpp | Adds match_message to AlarmMapping and updates the resolve-alarm API surface. |
match_message is a substring match, not equality like the other match fields; the struct doc said all fields must equal the observed event.
bburda
left a comment
bburda
left a comment
There was a problem hiding this comment.
Additional findings outside the diff (both in ros2_medkit_opcua/README.md):
- The
event_alarmsexample usesPLC_OVERPRESSUREtwice: once on the simple-form source (entity_id: tank_process) and once in the first mapping of theline_1source. The loader rejects a fault code reused across sources, which the comment just above this example states, so this example fails to load if copied as-is. It predates this PR, but since you are editing this block, renaming onePLC_OVERPRESSUREwould make the documented example loadable. - The multi-alarm form comment still says "the first whose non-empty match fields all
equalthe observed event wins". Withmatch_messagebeing a substring match, this is no longer accurate. You updated the precedence paragraph below (equal to match), so this inline comment can get the same fix.
|
|
||
| // Empty event message never matches a non-empty match_message -> catch-all. | ||
| auto r4 = NodeMap::resolve_alarm(cfg, "", "ns=3;i=1845", "ns=3;i=1805", ""); | ||
| EXPECT_EQ(r4.fault_code, "PLC_PROGRAM_ALARM"); |
There was a problem hiding this comment.
This test does not exercise the case-sensitivity that the struct, the code comment, and the README all document: every case here uses an exact-case substring, so replacing find with a case-insensitive search would keep all assertions green. Consider one more case that pins it, e.g. match_message "Overtemp" against message "overtemp fault" should fall through to the catch-all (PLC_PROGRAM_ALARM).
There was a problem hiding this comment.
Fixed in 1abf553. Added r5/r6 to MatchByMessageSubstring: "overtemp fault" against match_message: "Overtemp" must fall through to the catch-all PLC_PROGRAM_ALARM, and the exact-case "Overtemp fault" routes to the distinct fault. Confirmed with a negative control that flipping find to a case-insensitive search turns r5 red. test_node_map green.
Rename the duplicated PLC_OVERPRESSURE so the example config loads (fault codes are globally unique) and correct the comment to say match_message is a substring match rather than an equality.
Add a case where a case-mismatched event message falls to the catch-all and the exact case routes to the mapping, so a case-insensitive find would regress red.
Adds a
match_messagefield toevent_alarms[].mappings- a case-sensitive substring match on the event Message, AND-combined with the existingcondition_name/source_node/event_typematchers. This lets one OPC UA source route alarms that share an EventType and SourceNode (e.g. Program_Alarms from one Siemens S7-1500 FB) to distinct SOVD faults by their text.AlarmMappinggainsmatch_message;resolve_alarmtakes the event Message and skips a mapping whose non-emptymatch_messageis not a substring of it.condition_name/source_node/event_typestay equality matches.mappings[].match_message; the poller passes the live event Message (and the read-path snapshot message).MatchByMessageSubstring) covering a match, the catch-all fallback, and an empty message.Closes #505