From cb0ef75472546a2d0d0d7504c982f665c0afe75a Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Sun, 5 Jul 2026 21:49:53 +0200 Subject: [PATCH 1/4] feat(opcua): raise PLC_COMMS_LOST fault on connection loss The poller raises a single component-scoped PLC_COMMS_LOST fault (severity ERROR) once the OPC UA connection has been continuously down for comms_lost_debounce_ms (default 5000, debouncing reconnect blips), and clears it on the next successful reconnect. Knobs: comms_lost_fault_enabled / comms_lost_debounce_ms / comms_lost_severity. --- .../ros2_medkit_opcua/README.md | 3 ++ .../ros2_medkit_opcua/opcua_poller.hpp | 28 ++++++++++++ .../ros2_medkit_opcua/src/opcua_plugin.cpp | 18 ++++++++ .../ros2_medkit_opcua/src/opcua_poller.cpp | 43 +++++++++++++++++++ .../test/test_opcua_plugin.cpp | 27 ++++++++++++ 5 files changed, 119 insertions(+) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index 83538090..0ee9daa8 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -367,6 +367,9 @@ ros2_medkit_gateway: | `subscription_interval_ms` | `500` | Publishing interval for OPC-UA subscriptions when `prefer_subscriptions: true` | | `condition_replay_strategy` | `auto` | Active-condition replay on reconnect: `method`, `read`, `auto`, `off` (see below) | | `require_confirm_for_clear` | `true` | Require both Acknowledge AND Confirm before a native alarm auto-clears. Set `false` for Confirm-less servers (e.g. Siemens S7-1500) so alarms clear on Acknowledge alone (see below) | +| `comms_lost_fault_enabled` | `true` | Raise a component-scoped `PLC_COMMS_LOST` fault when the connection stays down (issue #496) | +| `comms_lost_debounce_ms` | `5000` | Continuous down time before `PLC_COMMS_LOST` is raised (debounces reconnect blips) | +| `comms_lost_severity` | `ERROR` | SOVD severity bucket for the `PLC_COMMS_LOST` fault | ### OPC-UA client security (SecurityPolicy, certificates, user auth) 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 83b502a0..4bb4b962 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 @@ -114,6 +114,14 @@ struct PollerConfig { /// forever. Default true preserves the spec-strict behaviour. Issue #478; /// needs real-S7-1500 validation. bool require_confirm_for_clear{true}; + /// Issue #496: raise a single component-scoped ``PLC_COMMS_LOST`` fault when + /// the OPC-UA connection stays down for ``comms_lost_debounce`` continuously + /// (so a brief blip during a normal reconnect does not flap a fault), and + /// clear it on the next successful reconnect. ``enabled`` default true; + /// ``severity`` is the SOVD severity bucket for the fault. + bool comms_lost_fault_enabled{true}; + std::chrono::milliseconds comms_lost_debounce{5000}; + std::string comms_lost_severity{"ERROR"}; /// Optional warn-level log sink for operator-visible failures inside the /// poll thread. Set by the plugin owning the poller to its log_warn /// helper so events like ``ConditionRefresh failed`` reach the ROS 2 log @@ -215,9 +223,22 @@ class OpcuaPoller { static bool should_clear_after_refresh(SovdAlarmStatus last_status, const std::string & condition_id, const std::set & seen); + /// Decide whether the debounced comms-lost fault should be raised now + /// (issue #496). Raised only when the fault is enabled, is not already + /// raised, and the connection has been continuously down since + /// ``down_since`` for at least ``debounce``. Pure and static so the + /// debounce gate is unit-testable without a live connection. + static bool comms_lost_should_raise(bool enabled, bool already_raised, + std::chrono::steady_clock::time_point down_since, + std::chrono::steady_clock::time_point now, std::chrono::milliseconds debounce); + private: void poll_loop(); void do_poll(); + /// Issue #496: emit the component-scoped comms-lost raise/clear edge through + /// the alarm callback (fault_code ``PLC_COMMS_LOST``, scoped to the node + /// map's root component). + void emit_comms_lost(bool active); void setup_subscriptions(); void evaluate_alarms(); void on_data_change(const std::string & node_id, const OpcuaValue & value); @@ -337,6 +358,13 @@ class OpcuaPoller { // later) gets one log per connect, not one per re-subscribe attempt. bool condition_refresh_warned_{false}; + // Issue #496: comms-lost debounce state, touched only on the poll thread. + // ``comms_down_since_`` is set the first poll iteration the connection is + // observed down and cleared on reconnect; ``comms_lost_raised_`` guards the + // one-shot raise / matching clear so the fault is idempotent. + std::optional comms_down_since_; + bool comms_lost_raised_{false}; + // Thread safety: must be set via set_poll_callback() before start(). // Not modified after start(), so safe to read from the poll thread without a mutex. PollCallback poll_callback_; diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp index f00977e6..aa5c152b 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp @@ -232,6 +232,24 @@ void OpcuaPlugin::configure(const nlohmann::json & config) { poller_config_.require_confirm_for_clear = !(v == "0" || v == "false" || v == "no" || v == "off"); } + // Issue #496: comms-lost fault knobs. + if (config.contains("comms_lost_fault_enabled")) { + poller_config_.comms_lost_fault_enabled = config["comms_lost_fault_enabled"].get(); + } + if (auto * env = std::getenv("OPCUA_COMMS_LOST_ENABLED")) { + const std::string v = env; + poller_config_.comms_lost_fault_enabled = !(v == "0" || v == "false" || v == "no" || v == "off"); + } + if (config.contains("comms_lost_debounce_ms")) { + poller_config_.comms_lost_debounce = std::chrono::milliseconds(config["comms_lost_debounce_ms"].get()); + } + if (auto * env = std::getenv("OPCUA_COMMS_LOST_DEBOUNCE_MS")) { + poller_config_.comms_lost_debounce = std::chrono::milliseconds(std::atoi(env)); + } + if (config.contains("comms_lost_severity")) { + poller_config_.comms_lost_severity = config["comms_lost_severity"].get(); + } + // Environment variables override YAML config (for Docker) if (auto * env = std::getenv("OPCUA_ENDPOINT_URL")) { client_config_.endpoint_url = env; 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 f1fc291c..feb17c64 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 @@ -841,6 +841,29 @@ void OpcuaPoller::on_data_change(const std::string & node_id, const OpcuaValue & evaluate_alarms(); } +bool OpcuaPoller::comms_lost_should_raise(bool enabled, bool already_raised, + std::chrono::steady_clock::time_point down_since, + std::chrono::steady_clock::time_point now, + std::chrono::milliseconds debounce) { + if (!enabled || already_raised) { + return false; + } + return (now - down_since) >= debounce; +} + +void OpcuaPoller::emit_comms_lost(bool active) { + ros2_medkit::fault_detection::FaultSignal signal; + signal.fault_code = "PLC_COMMS_LOST"; + signal.severity = config_.comms_lost_severity; + signal.message = active ? ("OPC-UA connection lost to " + client_.endpoint_url()) + : ("OPC-UA connection restored to " + client_.endpoint_url()); + signal.active = active; + std::lock_guard alarm_lock(alarm_mutex_); + if (alarm_callback_) { + alarm_callback_(node_map_.component_id(), signal); + } +} + void OpcuaPoller::poll_loop() { auto reconnect_wait = config_.reconnect_interval; constexpr auto max_reconnect_wait = std::chrono::milliseconds(60000); @@ -853,9 +876,22 @@ void OpcuaPoller::poll_loop() { snapshot_.connected = false; } + // Issue #496: remember when the connection first went down so the + // comms-lost fault only fires after a continuous debounce window. + if (!comms_down_since_) { + comms_down_since_ = std::chrono::steady_clock::now(); + } + // Attempt reconnect with original config (preserves timeout, etc.) if (client_.connect(client_.current_config())) { reconnect_wait = config_.reconnect_interval; // reset on success + // Issue #496: connection restored - clear the comms-lost fault if it + // was raised, then reset the debounce timer. + if (comms_lost_raised_) { + emit_comms_lost(/*active=*/false); + comms_lost_raised_ = false; + } + comms_down_since_.reset(); if (config_.prefer_subscriptions) { setup_subscriptions(); } @@ -873,6 +909,13 @@ void OpcuaPoller::poll_loop() { setup_event_subscriptions(); } } else { + // Issue #496: still down - raise the component-scoped comms-lost fault + // once the debounce window has elapsed (idempotent via the raised flag). + if (comms_lost_should_raise(config_.comms_lost_fault_enabled, comms_lost_raised_, *comms_down_since_, + std::chrono::steady_clock::now(), config_.comms_lost_debounce)) { + emit_comms_lost(/*active=*/true); + comms_lost_raised_ = true; + } // Exponential backoff capped at 60s. condition_variable so stop() wakes immediately. { std::unique_lock lock(stop_mutex_); diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp index 49a9087d..af397e48 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp @@ -386,6 +386,33 @@ TEST(ReconcileShouldClearCondition, SkipsClearWhenSourceScanFailed) { modeled_sources)); } +// Issue #496: comms-lost debounce gate. +TEST(CommsLostShouldRaise, RaisesAfterDebounceElapsed) { + const auto t0 = std::chrono::steady_clock::time_point{}; + const auto debounce = std::chrono::milliseconds(5000); + // Continuously down for the full window -> raise. + EXPECT_TRUE(OpcuaPoller::comms_lost_should_raise(/*enabled=*/true, /*already_raised=*/false, t0, + t0 + std::chrono::milliseconds(5000), debounce)); + EXPECT_TRUE(OpcuaPoller::comms_lost_should_raise(true, false, t0, t0 + std::chrono::seconds(9), debounce)); +} + +TEST(CommsLostShouldRaise, HoldsDuringDebounceWindow) { + const auto t0 = std::chrono::steady_clock::time_point{}; + const auto debounce = std::chrono::milliseconds(5000); + // A blip shorter than the window must not raise. + EXPECT_FALSE(OpcuaPoller::comms_lost_should_raise(true, false, t0, t0 + std::chrono::milliseconds(4999), debounce)); +} + +TEST(CommsLostShouldRaise, IdempotentAndDisabled) { + const auto t0 = std::chrono::steady_clock::time_point{}; + const auto debounce = std::chrono::milliseconds(5000); + const auto late = t0 + std::chrono::seconds(30); + // Already raised -> do not raise again (one-shot). + EXPECT_FALSE(OpcuaPoller::comms_lost_should_raise(true, /*already_raised=*/true, t0, late, debounce)); + // Disabled -> never raise. + EXPECT_FALSE(OpcuaPoller::comms_lost_should_raise(/*enabled=*/false, false, t0, late, debounce)); +} + // Issue #478 safety-gate: an empty scan from a source that has NEVER yielded a // condition instance node (EventNotifier-only server, e.g. S7-1500) must NOT // clear the still-active tracked fault. This is the single most important From 60d092b55f69e62ca766287fa17c7fa4b77cfb26 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Sun, 5 Jul 2026 22:03:26 +0200 Subject: [PATCH 2/4] fix(opcua): validate comms-lost debounce and severity config Clamp comms_lost_debounce_ms to >= 0 and keep the existing value on a non-numeric/negative OPCUA_COMMS_LOST_DEBOUNCE_MS (atoi parsed it as 0, silently disabling the debounce). Validate comms_lost_severity against the SOVD buckets and default to ERROR on a typo so the fault is not downgraded to INFO. Document the comms-lost env overrides. --- .../ros2_medkit_opcua/README.md | 2 ++ .../ros2_medkit_opcua/src/opcua_plugin.cpp | 24 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index 0ee9daa8..4b42c08c 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -491,6 +491,8 @@ Write operations use the `set_` prefix convention: | `OPCUA_USER_CERT` | X.509 user-token cert (DER) | | `OPCUA_CONDITION_REPLAY` | `method` / `read` / `auto` / `off` | | `OPCUA_REQUIRE_CONFIRM_FOR_CLEAR` | `0`/`false`/`no`/`off` to clear native alarms on Acknowledge alone (Confirm-less servers) | +| `OPCUA_COMMS_LOST_ENABLED` | `0`/`false`/`no`/`off` to disable the `PLC_COMMS_LOST` fault | +| `OPCUA_COMMS_LOST_DEBOUNCE_MS` | Continuous down time (ms) before `PLC_COMMS_LOST` is raised | ## Hardware Deployment diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp index aa5c152b..c22089ef 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp @@ -241,14 +241,34 @@ void OpcuaPlugin::configure(const nlohmann::json & config) { poller_config_.comms_lost_fault_enabled = !(v == "0" || v == "false" || v == "no" || v == "off"); } if (config.contains("comms_lost_debounce_ms")) { - poller_config_.comms_lost_debounce = std::chrono::milliseconds(config["comms_lost_debounce_ms"].get()); + const int ms = config["comms_lost_debounce_ms"].get(); + poller_config_.comms_lost_debounce = std::chrono::milliseconds(ms < 0 ? 0 : ms); } if (auto * env = std::getenv("OPCUA_COMMS_LOST_DEBOUNCE_MS")) { - poller_config_.comms_lost_debounce = std::chrono::milliseconds(std::atoi(env)); + // Keep the existing value on a non-numeric / negative override so a typo + // does not silently disable the debounce (atoi would parse it as 0). + char * end = nullptr; + const long ms = std::strtol(env, &end, 10); + if (end != env && *end == '\0' && ms >= 0) { + poller_config_.comms_lost_debounce = std::chrono::milliseconds(ms); + } else { + log_warn(std::string("Ignoring invalid OPCUA_COMMS_LOST_DEBOUNCE_MS='") + env + + "' (want a non-negative integer)"); + } } if (config.contains("comms_lost_severity")) { poller_config_.comms_lost_severity = config["comms_lost_severity"].get(); } + { + // Validate against the SOVD severity buckets and default to ERROR on a typo, + // so the comms-lost fault is not silently downgraded to INFO by the severity + // map (same guard as the node-map severity validation). + const std::string & s = poller_config_.comms_lost_severity; + if (s != "INFO" && s != "WARNING" && s != "ERROR" && s != "CRITICAL") { + log_warn("Unknown comms_lost_severity '" + s + "' - defaulting to ERROR"); + poller_config_.comms_lost_severity = "ERROR"; + } + } // Environment variables override YAML config (for Docker) if (auto * env = std::getenv("OPCUA_ENDPOINT_URL")) { From cd80298d1d103939eccd4e4e62061ffba1adde96 Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Mon, 6 Jul 2026 10:47:47 +0200 Subject: [PATCH 3/4] fix(opcua): bound the comms-lost debounce override Reset errno and reject ERANGE so an overflowing OPCUA_COMMS_LOST_DEBOUNCE_MS no longer saturates to LONG_MAX and silently disables the fault, and clamp both the YAML and env paths to [0, 3600000] ms. --- .../ros2_medkit_opcua/README.md | 4 +-- .../ros2_medkit_opcua/src/opcua_plugin.cpp | 32 +++++++++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index 4b42c08c..ed679c11 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -368,7 +368,7 @@ ros2_medkit_gateway: | `condition_replay_strategy` | `auto` | Active-condition replay on reconnect: `method`, `read`, `auto`, `off` (see below) | | `require_confirm_for_clear` | `true` | Require both Acknowledge AND Confirm before a native alarm auto-clears. Set `false` for Confirm-less servers (e.g. Siemens S7-1500) so alarms clear on Acknowledge alone (see below) | | `comms_lost_fault_enabled` | `true` | Raise a component-scoped `PLC_COMMS_LOST` fault when the connection stays down (issue #496) | -| `comms_lost_debounce_ms` | `5000` | Continuous down time before `PLC_COMMS_LOST` is raised (debounces reconnect blips) | +| `comms_lost_debounce_ms` | `5000` | Continuous down time before `PLC_COMMS_LOST` is raised (debounces reconnect blips; clamped to [0, 3600000] ms) | | `comms_lost_severity` | `ERROR` | SOVD severity bucket for the `PLC_COMMS_LOST` fault | ### OPC-UA client security (SecurityPolicy, certificates, user auth) @@ -492,7 +492,7 @@ Write operations use the `set_` prefix convention: | `OPCUA_CONDITION_REPLAY` | `method` / `read` / `auto` / `off` | | `OPCUA_REQUIRE_CONFIRM_FOR_CLEAR` | `0`/`false`/`no`/`off` to clear native alarms on Acknowledge alone (Confirm-less servers) | | `OPCUA_COMMS_LOST_ENABLED` | `0`/`false`/`no`/`off` to disable the `PLC_COMMS_LOST` fault | -| `OPCUA_COMMS_LOST_DEBOUNCE_MS` | Continuous down time (ms) before `PLC_COMMS_LOST` is raised | +| `OPCUA_COMMS_LOST_DEBOUNCE_MS` | Continuous down time (ms) before `PLC_COMMS_LOST` is raised (clamped to [0, 3600000] ms; non-numeric / out-of-range keeps the existing value) | ## Hardware Deployment diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp index c22089ef..c7a4cc65 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -240,20 +241,37 @@ void OpcuaPlugin::configure(const nlohmann::json & config) { const std::string v = env; poller_config_.comms_lost_fault_enabled = !(v == "0" || v == "false" || v == "no" || v == "off"); } + // Clamp the comms-lost debounce to [0, 1h], shared by the YAML and env paths. + // Without an upper bound an overflowing or huge value pushes the gate so far + // out it never fires, silently disabling the fault; 0 keeps the immediate + // report behaviour. + constexpr long kMaxCommsLostDebounceMs = 3600000; // 1 hour + const auto clamp_debounce_ms = [](long long ms) -> long { + if (ms < 0) { + return 0; + } + if (ms > kMaxCommsLostDebounceMs) { + return kMaxCommsLostDebounceMs; + } + return static_cast(ms); + }; if (config.contains("comms_lost_debounce_ms")) { - const int ms = config["comms_lost_debounce_ms"].get(); - poller_config_.comms_lost_debounce = std::chrono::milliseconds(ms < 0 ? 0 : ms); + const long long ms = config["comms_lost_debounce_ms"].get(); + poller_config_.comms_lost_debounce = std::chrono::milliseconds(clamp_debounce_ms(ms)); } if (auto * env = std::getenv("OPCUA_COMMS_LOST_DEBOUNCE_MS")) { - // Keep the existing value on a non-numeric / negative override so a typo - // does not silently disable the debounce (atoi would parse it as 0). + // Keep the existing value on a non-numeric / out-of-range / negative + // override so a typo does not silently disable the debounce (atoi would + // parse it as 0). errno + ERANGE rejects an overflowing run of digits that + // strtol would otherwise saturate to LONG_MAX and slip past the >= 0 guard. + errno = 0; char * end = nullptr; const long ms = std::strtol(env, &end, 10); - if (end != env && *end == '\0' && ms >= 0) { - poller_config_.comms_lost_debounce = std::chrono::milliseconds(ms); + if (end != env && *end == '\0' && errno != ERANGE && ms >= 0) { + poller_config_.comms_lost_debounce = std::chrono::milliseconds(clamp_debounce_ms(ms)); } else { log_warn(std::string("Ignoring invalid OPCUA_COMMS_LOST_DEBOUNCE_MS='") + env + - "' (want a non-negative integer)"); + "' (want a non-negative integer <= 3600000 ms)"); } } if (config.contains("comms_lost_severity")) { From 897785e546f433a69646089b837f091f3db83a4e Mon Sep 17 00:00:00 2001 From: mfaferek93 Date: Tue, 7 Jul 2026 17:02:42 +0200 Subject: [PATCH 4/4] fix(opcua): retry comms-lost report until the fault sink is matched Gate the comms_lost_raised_ latch on the ReportFault service being discovered, so a fire-and-forget report raised before DDS match is not dropped and then permanently suppressed by the raised flag. The poll arm retries on each cycle until the sink appears. --- .../include/ros2_medkit_opcua/opcua_poller.hpp | 5 +++++ .../ros2_medkit_opcua/src/opcua_plugin.cpp | 5 +++++ .../ros2_medkit_opcua/src/opcua_poller.cpp | 11 +++++++++-- 3 files changed, 19 insertions(+), 2 deletions(-) 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 4bb4b962..f9dcab78 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 @@ -128,6 +128,11 @@ struct PollerConfig { /// instead of stderr only. Empty by default - the poller falls back to /// stderr in that case. std::function log_warn; + /// Optional readiness probe for the fault sink (ReportFault service). The poll + /// thread only latches the comms-lost fault once this returns true, so a + /// fire-and-forget report is never dropped-and-forgotten while the sink is + /// unmatched - it retries on the next poll instead. Empty => assume ready. + std::function report_sink_ready; }; /// Manages OPC-UA data collection via subscriptions (preferred) or polling diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp index c7a4cc65..39f50396 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp @@ -423,6 +423,11 @@ void OpcuaPlugin::set_context(PluginContext & context) { // dispatch and flush_pending_reports (run on every poll) drains it once the // service appears, so a late sink still receives the alarm - without stalling // startup or capping recovery at a fixed timeout. + // Issue #496: on top of that, gate the comms-lost latch on the fault sink + // being discovered, so the raised flag is not set before the report can land. + poller_config_.report_sink_ready = [this]() { + return fault_clients_->report && fault_clients_->report->service_is_ready(); + }; poller_->start(poller_config_); log_info("OPC-UA poller started (mode: " + std::string(poller_->using_subscriptions() ? "subscription" : "poll") + ")"); 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 feb17c64..64081d23 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 @@ -913,8 +913,15 @@ void OpcuaPoller::poll_loop() { // once the debounce window has elapsed (idempotent via the raised flag). if (comms_lost_should_raise(config_.comms_lost_fault_enabled, comms_lost_raised_, *comms_down_since_, std::chrono::steady_clock::now(), config_.comms_lost_debounce)) { - emit_comms_lost(/*active=*/true); - comms_lost_raised_ = true; + // Issue #496: only latch once the fault sink can actually receive the + // report. ReportFault is fire-and-forget, so a report sent before the + // service is matched is dropped; latching regardless would then + // suppress every retry and lose the fault. Leaving it unlatched re-runs + // this arm on the next poll until the sink is discovered. + if (!config_.report_sink_ready || config_.report_sink_ready()) { + emit_comms_lost(/*active=*/true); + comms_lost_raised_ = true; + } } // Exponential backoff capped at 60s. condition_variable so stop() wakes immediately. {