diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index e4dc4120b..0381ef765 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -334,8 +334,13 @@ only when all of them match the observed event; declaration order breaks ties. `condition_name` / `source_node` / `event_type` are equality matches; `match_message` is a case-sensitive substring match on the event Message. An event matching no mapping uses the source-level `fault_code`; if that is also -empty the event is ignored. A mapping-level `severity_override` / `message` -overrides the source-level one; otherwise the source-level value is inherited. +empty the event is ignored. Exception: a non-condition event (a plain BaseEvent +/ system message whose `ConditionId` resolves to null - e.g. a SIMATIC "CPU not +in RUN" arriving on a `ns=0;i=2253` catch-all) is dropped before mapping +resolution runs, so it never falls through to the source-level `fault_code` and +that catch-all code is never raised for it (see "System messages" under +`auto_alarms` below). A mapping-level `severity_override` / `message` overrides +the source-level one; otherwise the source-level value is inherited. The plugin auto-registers `acknowledge_fault` and `confirm_fault` operations on every entity that has at least one `event_alarms` entry. Invoke them with: @@ -493,8 +498,12 @@ no node-map file required. EventNotifier as real alarms - a Siemens Server object also emits SIMATIC system messages (e.g. `"CPU not in RUN"`) over `i=2253`. Per OPC-UA Part 9 §5.5.2.13 only a true Condition instance carries a ConditionId; a system -message resolves it to `NodeId.Null`, and `auto_alarms` drops those events -before deriving a fault. Use `exclude` for anything that still needs +message resolves it to `NodeId.Null`. The bridge drops those events for every +alarm source - explicit `event_alarms` (including a source-level `fault_code` +catch-all) and `auto_alarms` alike - before any mapping resolution or +auto-derivation, so they never surface as a fault. On an explicit source it also +logs a one-time warning (a real alarm that genuinely lacks a ConditionId would +otherwise be dropped silently). Use `exclude` for anything that still needs filtering by text (e.g. a server that does set a ConditionId on system events). diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst b/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst index 994d49280..8ee173e2e 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst @@ -220,6 +220,8 @@ State machine Inputs from each event payload (positional ``EventFilter`` select clauses): +- ``ConditionId`` - NodeId; null means the notification is not an A&C Condition + (a plain BaseEvent / system message), gated by rule 0 below before the rest - ``EnabledState.Id`` - bool - ``ShelvingState.CurrentState.Id`` - NodeId; non-Unshelved => suppressed - ``ActiveState.Id`` - bool @@ -232,6 +234,10 @@ Decision order, first match wins: +-----+--------------------------------------+---------------------------------------+ | # | Condition | Outcome | +=====+======================================+=======================================+ +| 0 | ``ConditionId == null`` | not a Condition: dropped for every | +| | (non-condition event, e.g. a | alarm source - explicit and auto - | +| | system message) | before the state machine runs | ++-----+--------------------------------------+---------------------------------------+ | 1 | ``BranchId != null`` | history-only (no SOVD update) | +-----+--------------------------------------+---------------------------------------+ | 2 | ``EnabledState == false`` | clear if was active, else no-op | @@ -254,6 +260,13 @@ Decision order, first match wins: | | not ``require_confirm_for_clear``) | | +-----+--------------------------------------+---------------------------------------+ +Rule 0 is enforced in the poller's ``on_event`` (via ``is_condition_event``) +before any state-machine input is built, and applies to every alarm source - +explicit ``event_alarms`` (including a source-level ``fault_code`` catch-all on +``ns=0;i=2253``) and zero-config ``auto_alarms`` alike. A drop on an explicit +source is additionally logged once as an operator warning, since a real alarm +that genuinely lacks a ConditionId would otherwise vanish without a trace. + ``require_confirm_for_clear`` (default ``true``) gates rule 5b on both Acknowledge and Confirm. Some servers do not implement the optional ``Confirm`` transition - Siemens S7-1500 supports ``Acknowledge`` / ``ConditionRefresh`` / diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp index 13081aca5..d6b7eee61 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp @@ -266,9 +266,11 @@ class OpcuaPoller { /// ConditionId SAO resolves to a non-null NodeId only for AlarmConditionType /// (and subtype) instances; a plain BaseEvent / SystemEvent notification - /// e.g. a Siemens Server-object "CPU not in RUN" system message delivered on - /// the same EventNotifier (i=2253) auto_alarms subscribes to - carries no - /// ConditionId and must not be auto-derived into a fault. Pure and static so - /// the system-message filter is unit-testable without a server. + /// the same EventNotifier (i=2253) an alarm source subscribes to - carries no + /// ConditionId. ``on_event`` drops such an event for EVERY alarm source, + /// explicit ``event_alarms`` and zero-config ``auto_alarms`` alike, before the + /// mapping/auto-derivation split, so it never becomes a fault. Pure and static + /// so the guard is unit-testable without a server. static bool is_condition_event(const opcua::NodeId & condition_id); private: @@ -390,6 +392,10 @@ class OpcuaPoller { std::set read_modeled_sources_; // Sources already warned about as read-fallback-unsupported (warn once each). std::set read_unsupported_warned_sources_; + // Configured alarm sources already warned that a non-condition (null + // ConditionId) event was dropped despite their explicit fault_code/mappings + // (warn once each; see the guard in on_event). + std::set noncondition_drop_warned_sources_; // ConditionRefresh bracketing state. open62541 sends the buffered historical // condition burst between RefreshStartEvent and RefreshEndEvent; we apply each diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp index b02ba80e3..566cd7ddb 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp @@ -752,6 +752,51 @@ void OpcuaPoller::on_event(const AlarmEventConfig & cfg, const std::vector PLC_ALARM), latched at Healed: + // (a) should_clear_after_refresh counts Healed as active, and the null cid + // is never replayed inside a ConditionRefresh burst, so the next + // RefreshEnd emits a spurious ClearFault carrying that fault_code - + // clearing a genuine alarm that shares the code; + // (b) that null entry can win lookup_condition's linear (entity_id, + // fault_code) scan, so a later Acknowledge / Confirm is misrouted to a + // null ConditionId (call_method on runtime->condition_id). + // Dropping the event up front stops the entry from ever being created. + if (!is_condition_event(condition_id)) { + // On an EXPLICIT source (operator supplied a fault_code and/or mappings) a + // real alarm can legitimately arrive with a null ConditionId - an edge + // open62541 server built without UA_ENABLE_SUBSCRIPTIONS_ALARMS_CONDITIONS, + // or an AlarmConditionType fired via UA_Server_triggerEvent rather than + // createCondition. Dropping it with only the DEBUG line below would regress + // that path silently (pre-PR it still raised), so warn the operator once per + // source. The synthetic auto source carries no fault_code/mappings + // (effective_alarm_sources leaves them empty), so this predicate excludes + // it and the auto/synthetic path keeps only the DEBUG line. + if ((!cfg.fault_code.empty() || !cfg.mappings.empty()) && + noncondition_drop_warned_sources_.insert(cfg.source_node_id_str).second) { + const std::string code_desc = + cfg.fault_code.empty() ? std::string("its mapped fault codes") : "fault_code '" + cfg.fault_code + "'"; + warn_operator("OPC-UA non-condition event on configured alarm source '" + cfg.source_node_id_str + "' (" + + code_desc + + ") carries a null ConditionId and was dropped; if this source's alarms carry no ConditionId they " + "will not surface as faults."); + } + RCLCPP_DEBUG_STREAM(opcua_poller_logger(), "on_event: non-condition event (null ConditionId, type=" + << event_type.toString() << ") - ignoring"); + return; + } + // A condition (re)reported inside a refresh burst is still active; remember it // so RefreshEnd does not clear it. Recorded before the field-count bail below // so even a partial-field replay counts as "seen". @@ -863,20 +908,9 @@ void OpcuaPoller::on_event(const AlarmEventConfig & cfg, const std::vector("Time")), &now, + &UA_TYPES[UA_TYPES_DATETIME]); + UA_UInt16 severity = 500; + UA_Server_writeObjectProperty_scalar(server, event_node, UA_QUALIFIEDNAME(0, const_cast("Severity")), + &severity, &UA_TYPES[UA_TYPES_UINT16]); + UA_LocalizedText msg = + UA_LOCALIZEDTEXT(const_cast("en"), const_cast("Session state changed to Created")); + UA_Server_writeObjectProperty_scalar(server, event_node, UA_QUALIFIEDNAME(0, const_cast("Message")), &msg, + &UA_TYPES[UA_TYPES_LOCALIZEDTEXT]); + // originId = Server object (i=2253); delete the one-shot event node after. + return UA_Server_triggerEvent(server, event_node, UA_NODEID_NUMERIC(0, UA_NS0ID_SERVER), nullptr, UA_TRUE); +} + UA_StatusCode set_shelving(UA_Server * server, const Condition & c, bool shelved) { // ShelvingState is a ShelvedStateMachineType sub-object on the condition // (Part 9). open62541's experimental A&C does not implement the TimedShelve @@ -612,6 +639,18 @@ void cli_loop(UA_Server * server, UA_UInt16 ns) { } continue; } + // ``sysevent`` fires a non-condition BaseEventType on the Server object + // (i=2253); it has no and is not in g_conditions, so handle it + // before the condition lookup (like ``quit`` / ``set``). + if (cmd == "sysevent") { + UA_StatusCode rc = handle_sysevent(server); + if (rc == UA_STATUSCODE_GOOD) { + std::cout << "OK sysevent" << std::endl; + } else { + std::cout << "ERR sysevent:" << UA_StatusCode_name(rc) << std::endl; + } + continue; + } auto it = g_conditions.find(name); if (cmd != "quit" && it == g_conditions.end()) { std::cout << "ERR unknown_condition:" << name << std::endl; diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py index 3498ea552..2beecbedc 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_secured.test.py @@ -69,6 +69,9 @@ ROS_DOMAIN_ID = '228' ALARM_CODE = 'PLC_OVERPRESSURE' +# Source-level catch-all code a non-condition event would be mis-mapped to if the +# poller's ConditionId guard regressed. +NONCOND_CODE = 'PLC_ALARM' def require_flag(): @@ -174,6 +177,32 @@ def write_node_map(path): ) +def write_catchall_node_map(path): + """ + Node map with a single explicit event_alarms catch-all on the Server object. + + Every real AlarmConditionType event on i=2253 maps to PLC_ALARM via the + source-level fault_code. A non-condition BaseEventType on the same + EventNotifier resolves its ConditionId to null and must be dropped by the + poller's guard rather than mapped to PLC_ALARM. Only this one source is + mapped, so a real condition (which is also a notifier of i=2253) surfaces + exactly one PLC_ALARM fault - no second monitored item, no fault_code race. + """ + path.write_text( + 'area_id: plc_systems\n' + 'component_id: alarm_test_runtime\n' + 'nodes:\n' + ' - node_id: "ns=2;s=Tank.Level"\n' + ' entity_id: tank_process\n' + ' data_name: tank_level\n' + ' data_type: float\n' + 'event_alarms:\n' + ' - alarm_source: "ns=0;i=2253"\n' + ' entity_id: tank_process\n' + ' fault_code: PLC_ALARM\n' + ) + + def write_gateway_params(path, *, port, plugin, server_port, node_map, manifest, certs, password): """Render a gateway params file pointing the opcua plugin at the secure server.""" @@ -261,7 +290,7 @@ def main(): server_log = workdir / 'server.log' env = dict(os.environ, ROS_DOMAIN_ID=ROS_DOMAIN_ID) - server = fault_mgr = gw_good = gw_bad = None + server = fault_mgr = gw_good = gw_bad = gw_catch = None try: # --- Certificates (regenerated every run, never committed) --------- subprocess.run(['bash', str(gen_certs), str(certs)], check=True, @@ -376,6 +405,75 @@ def main(): terminate(gw_good) gw_good = None + # === NON-CONDITION GUARD: explicit event_alarms catch-all on i=2253 = + # A catch-all on the Server object maps every real condition to + # PLC_ALARM. A non-condition BaseEventType on the SAME EventNotifier + # (the fixture's 'sysevent' command) carries a null ConditionId and must + # be dropped by the poller's guard - it must never surface as PLC_ALARM, + # while a real condition on the catch-all still faults. Reset the still- + # active Overpressure condition first so its ConditionRefresh replay does + # not pre-map PLC_ALARM before the sysevent is even fired. + server.stdin.write('clear Overpressure\n') + server.stdin.flush() + + catch_node_map = workdir / 'catchall_nodes.yaml' + write_catchall_node_map(catch_node_map) + catch_port = free_port() + catch_params = workdir / 'gateway_catch.yaml' + write_gateway_params(catch_params, port=catch_port, plugin=plugin, + server_port=server_port, node_map=catch_node_map, + manifest=manifest, certs=certs, password=PASSWORD) + catch_log = workdir / 'gateway_catch.log' + gw_catch = start_gateway(catch_params, catch_log, env) + catch_base = f'http://127.0.0.1:{catch_port}/api/v1' + catch_status = wait_json( + f'{catch_base}/components/alarm_test_runtime/x-plc-status', + lambda j: j.get('connected') is True, deadline=70) + if not (catch_status and catch_status.get('connected') is True): + print('FAIL: catch-all gateway did not connect over the secure channel', + file=sys.stderr) + print(catch_log.read_text(errors='replace')[-3000:], file=sys.stderr) + return 1 + + # Fire the non-condition system event; the guard must drop it. + server.stdin.write('sysevent\n') + server.stdin.flush() + leaked = wait_json( + f'{catch_base}/faults', + lambda j: any(i.get('fault_code') == NONCOND_CODE for i in j.get('items', [])), + deadline=20, period=2.0) + if leaked and any(i.get('fault_code') == NONCOND_CODE for i in leaked.get('items', [])): + print('FAIL: non-condition event surfaced as PLC_ALARM (guard regressed)', + file=sys.stderr) + print(json.dumps(leaked), file=sys.stderr) + print('server log:\n' + Path(server_log).read_text(errors='replace'), + file=sys.stderr) + return 1 + print(' OK non-condition guard: sysevent did NOT raise PLC_ALARM') + + # A real condition on the catch-all source still faults (proves the + # pipeline can produce PLC_ALARM, so the absence above is meaningful). + server.stdin.write('fire Overpressure 750\n') + server.stdin.flush() + catch_faults = wait_json( + f'{catch_base}/faults', + lambda j: any(i.get('fault_code') == NONCOND_CODE and i.get('status') == 'CONFIRMED' + for i in j.get('items', [])), deadline=120) + got_alarm = catch_faults and any( + i.get('fault_code') == NONCOND_CODE and i.get('status') == 'CONFIRMED' + for i in catch_faults.get('items', [])) + if not got_alarm: + print('FAIL: real condition on the i=2253 catch-all did not surface as PLC_ALARM', + file=sys.stderr) + print(json.dumps(catch_faults), file=sys.stderr) + print('server log:\n' + Path(server_log).read_text(errors='replace'), + file=sys.stderr) + return 1 + print(' OK catch-all: real condition surfaced as PLC_ALARM CONFIRMED') + + terminate(gw_catch) + gw_catch = None + # === NEGATIVE: wrong password must fail closed ===================== bad_params = workdir / 'gateway_bad.yaml' write_gateway_params(bad_params, port=bad_port, plugin=plugin, @@ -413,6 +511,7 @@ def main(): return 0 finally: terminate(gw_good) + terminate(gw_catch) terminate(gw_bad) terminate(fault_mgr) if server is not None: diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_poller.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_poller.cpp index 541f26290..9e9cf5575 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_poller.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_poller.cpp @@ -153,7 +153,8 @@ TEST(NodeIdsEquivalentTest, UnparseableSpellingsFallBackToRawEquality) { TEST(IsConditionEventTest, NullConditionIdIsRejected) { // Part 9 §5.5.2.13: a non-condition event (e.g. a Siemens Server-object // system message such as "CPU not in RUN") resolves the ConditionId SAO - // to NodeId.Null - the system-message filter for auto_alarms. + // to NodeId.Null. on_event's guard drops such events for every alarm + // source - explicit event_alarms and auto_alarms alike. EXPECT_FALSE(OpcuaPoller::is_condition_event(opcua::NodeId())); }