diff --git a/docs/api/rest.rst b/docs/api/rest.rst index 0321f22fc..590414c62 100644 --- a/docs/api/rest.rst +++ b/docs/api/rest.rst @@ -1501,13 +1501,19 @@ Query and manage faults. - ``freeze_frame``: Data captured at fault confirmation. Entity frames for faults that were already confirmed when the gateway started are captured at gateway start instead and carry ``"capture_origin": "startup"`` in - their ``x-medkit`` block. For a plugin-backed entity that reports its - link down, the values are the plugin's last known ones and may predate - the confirmation by the length of the outage; such entries carry - ``connected`` (the payload's link flag, ``false`` for the loss-of-comms - case) and ``source_timestamp`` (the payload's own timestamp, verbatim) - in ``x-medkit``, both only when the plugin's payload reports them. - ``captured_at`` always dates the capture, not the values. + their ``x-medkit`` block. An entity frame also carries ``source`` in + ``x-medkit``, naming the path that read the values + (``plugin_data_provider`` for the owning plugin's DataProvider, + ``plugin_x_plc_data_route`` for its ``x-plc-data`` route). These values + are not a ROS message, so ``topic`` and ``message_type`` are empty and + ``source`` is the only field saying where the numbers came from. For a + plugin-backed entity that reports its link down, the values are the + plugin's last known ones and may predate the confirmation by the length of + the outage. Such entries carry ``connected`` (the payload's link flag, + ``false`` for the loss-of-comms case) and ``source_timestamp`` (the + payload's own timestamp, verbatim) in ``x-medkit``, both only when the + plugin's payload reports them. ``captured_at`` always dates the capture, + not the values. - ``rosbag``: Recording file available via bulk-data endpoint **Response codes:** diff --git a/docs/tutorials/snapshots.rst b/docs/tutorials/snapshots.rst index f17d55d7e..c23e845ef 100644 --- a/docs/tutorials/snapshots.rst +++ b/docs/tutorials/snapshots.rst @@ -216,6 +216,26 @@ with: ros2 run ros2_medkit_gateway gateway_node --ros-args \ -p entity_freeze_frame.enabled:=false +A plugin entity's values are not a ROS message, so ``topic`` and +``message_type`` are empty on these frames. ``x-medkit.source`` names the +capture path instead, so a consumer can still tell where the values came +from: + +.. list-table:: + :header-rows: 1 + :widths: 30 70 + + * - ``x-medkit.source`` + - Meaning + * - ``plugin_data_provider`` + - Read through the owning plugin's ``DataProvider::list_data``. + * - ``plugin_x_plc_data_route`` + - Read by dispatching the owning plugin's own ``x-plc-data`` route + in-process (plugins that export no ``DataProvider``). + +The field is absent on freeze-frames captured by the fault manager from a ROS +topic. Those carry a real ``topic`` and ``message_type`` instead. + Example plugin-entity freeze-frame in the fault response: .. code-block:: json @@ -227,6 +247,7 @@ Example plugin-entity freeze-frame in the fault response: "x-medkit": { "topic": "", "message_type": "", + "source": "plugin_x_plc_data_route", "full_data": {"tank_level": 87.5, "pump_running": true}, "captured_at": "2026-07-14T12:00:00.000Z" } diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/entity_freeze_frame_capture.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/entity_freeze_frame_capture.hpp index 7dc081dc0..d3c041ee4 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/entity_freeze_frame_capture.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/entity_freeze_frame_capture.hpp @@ -60,6 +60,13 @@ namespace ros2_medkit_gateway { */ class EntityFreezeFrameCapture { public: + /// Capture-path identifiers stored in Frame::source and served as + /// ``x-medkit.source``. The plugin's own DataProvider, and the in-process + /// dispatch of the plugin's `x-plc-data` route for plugins that export no + /// DataProvider. + static constexpr const char * kSourceDataProvider = "plugin_data_provider"; + static constexpr const char * kSourceXPlcDataRoute = "plugin_x_plc_data_route"; + /// One captured frame: the entity's data values at fault-confirm time. /// captured_at_ns dates the capture, not the values - a disconnected entity /// serves its last known values, whose age is bounded only by the outage. @@ -72,6 +79,13 @@ class EntityFreezeFrameCapture { bool startup_catchup{false}; std::optional connected; ///< payload's top-level link flag, when reported nlohmann::json source_timestamp; ///< payload's own "timestamp" field verbatim (null when absent) + /// Which capture path read the values (kSourceDataProvider / + /// kSourceXPlcDataRoute), served as ``x-medkit.source``. These values are + /// entity data, not a ROS message, so ``topic`` and ``message_type`` are + /// empty on the wire and would otherwise leave a consumer with nothing at + /// all saying where the numbers came from. Empty when the caller named no + /// path. + std::string source; }; /// Resolves an entity id to its owning plugin's DataProvider (nullptr when @@ -187,9 +201,10 @@ class EntityFreezeFrameCapture { bool capture_for_event(const ros2_medkit_msgs::msg::FaultEvent & event, bool startup_catchup = false); /// Build a frame from list-data-shaped content, enforcing the shared - /// no-row-of-nulls invariant on both capture paths. + /// no-row-of-nulls invariant on both capture paths. @p source names the path + /// that read the content and is stored verbatim in Frame::source. std::optional frame_from_content(const std::string & entity_id, const std::string & fault_code, - const nlohmann::json & content); + const nlohmann::json & content, const std::string & source); /// Capture via the plugin's own x-plc-data route (no DataProvider exported). /// Returns nullopt when the route yields nothing usable. diff --git a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp index a46a11bf5..89a153d1b 100644 --- a/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp +++ b/src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp @@ -497,6 +497,27 @@ class GatewayNode : public rclcpp::Node { std::unique_ptr server_thread_; }; +/** + * @brief Is this node FQN the gateway's own, rather than a diagnosable peer? + * + * True for the gateway node itself and for the helper nodes it creates inside + * its own process: the subscription executor's `_sub`, the fault-service + * transport's `_fault_clients`, and the lifecycle reader's + * `_lifecycle_state_reader`. None of these begins with '_', so the ROS 2 + * hidden-node convention does not cover them and the gateway would otherwise + * count them as peers and list them as diagnosable Apps - reporting on itself. + * + * A fault_manager node sharing the process is NOT ours: it is a separate, + * diagnosable component and stays visible. + * + * Exact matches only. A prefix test would also claim a genuine peer named + * `_monitor` or `2`, and dropping a real node is the worse error. + * + * @param node_fqn Fully qualified node name to test ("/ns/node") + * @param self_fqn The gateway node's own FQN. An empty value matches nothing + */ +bool is_own_gateway_node(const std::string & node_fqn, const std::string & self_fqn); + /** * @brief Filter ROS 2 internal nodes from an app list * @@ -505,11 +526,18 @@ class GatewayNode : public rclcpp::Node { * before checking for the underscore prefix, using the routing table for precise * prefix detection. * + * Also removes local apps bound to one of the gateway's own nodes + * (is_own_gateway_node), which the underscore rule cannot see. The test is on + * the bound node FQN, and only for apps with no routing-table entry: a peer's + * helper nodes are the peer's business and are left to the peer's own filter. + * * @param apps App vector to filter in place * @param peer_routing_table Maps entity_id -> peer_name for remote entities + * @param self_fqn The gateway node's own FQN. Empty disables the self check * @return Number of apps removed */ size_t filter_internal_node_apps(std::vector & apps, - const std::unordered_map & peer_routing_table); + const std::unordered_map & peer_routing_table, + const std::string & self_fqn); } // namespace ros2_medkit_gateway diff --git a/src/ros2_medkit_gateway/src/entity_freeze_frame_capture.cpp b/src/ros2_medkit_gateway/src/entity_freeze_frame_capture.cpp index fe060826c..01bde9674 100644 --- a/src/ros2_medkit_gateway/src/entity_freeze_frame_capture.cpp +++ b/src/ros2_medkit_gateway/src/entity_freeze_frame_capture.cpp @@ -188,13 +188,14 @@ EntityFreezeFrameCapture::standing_faults_from_list_reply(const nlohmann::json & std::optional EntityFreezeFrameCapture::frame_from_content(const std::string & entity_id, const std::string & fault_code, - const nlohmann::json & content) { + const nlohmann::json & content, const std::string & source) { if (!content_has_live_data(content)) { log_fallback_failure_once(fault_code, "entity '" + entity_id + "' returned no data items"); return std::nullopt; } Frame frame; frame.entity_id = entity_id; + frame.source = source; frame.values = values_from_list_content(content); if (!values_have_data(frame.values)) { // Items present but nothing usable in them (all-null values, or no usable @@ -228,7 +229,7 @@ EntityFreezeFrameCapture::capture_via_route(const std::string & entity_id, const if (!content) { return std::nullopt; // not plugin-owned, no x-plc-data route, or handler error } - return frame_from_content(entity_id, fault_code, *content); + return frame_from_content(entity_id, fault_code, *content, kSourceXPlcDataRoute); } void EntityFreezeFrameCapture::log_fallback_failure_once(const std::string & fault_code, const std::string & message) { @@ -410,7 +411,7 @@ bool EntityFreezeFrameCapture::capture_for_event(const ros2_medkit_msgs::msg::Fa log_fallback_failure_once(fault_code, "list_data('" + source + "') failed: " + result.error().message); continue; } - if (auto frame = frame_from_content(source, fault_code, result->content)) { + if (auto frame = frame_from_content(source, fault_code, result->content, kSourceDataProvider)) { frames.push_back(std::move(*frame)); } } catch (const std::exception & e) { diff --git a/src/ros2_medkit_gateway/src/gateway_node.cpp b/src/ros2_medkit_gateway/src/gateway_node.cpp index a8f32986e..0d4e8c5ff 100644 --- a/src/ros2_medkit_gateway/src/gateway_node.cpp +++ b/src/ros2_medkit_gateway/src/gateway_node.cpp @@ -15,6 +15,7 @@ #include "ros2_medkit_gateway/gateway_node.hpp" #include +#include #include #include #include @@ -1596,6 +1597,27 @@ GatewayNode::GatewayNode(const rclcpp::NodeOptions & options) : Node("ros2_medki }); } +bool is_own_gateway_node(const std::string & node_fqn, const std::string & self_fqn) { + if (self_fqn.empty() || node_fqn.empty()) { + return false; + } + if (node_fqn == self_fqn) { + return true; + } + // The helper nodes the gateway creates inside its own process, each named + // after this node plus a fixed suffix. Where each one is set: + // "_sub" Ros2SubscriptionExecutor::Config + // (subscription_node_name_suffix) + // "_fault_clients" Ros2FaultServiceTransport + // "_lifecycle_state_reader" Ros2LifecycleStateReader + // Exact matches only: a prefix test would also claim a genuine peer named + // "_monitor" or "2", and hiding a real node is the worse error. + static constexpr std::array kHelperSuffixes{"_sub", "_fault_clients", "_lifecycle_state_reader"}; + return std::any_of(kHelperSuffixes.begin(), kHelperSuffixes.end(), [&](const char * suffix) { + return node_fqn == self_fqn + suffix; + }); +} + size_t GatewayNode::count_peer_nodes(const std::vector> & nodes_and_namespaces, const std::string & self_fqn) { size_t count = 0; @@ -1608,10 +1630,7 @@ size_t GatewayNode::count_peer_nodes(const std::vector_monitor" or "2"). - if (fqn == self_fqn || fqn == self_fqn + "_sub" || fqn == self_fqn + "_fault_clients") { + if (is_own_gateway_node(fqn, self_fqn)) { continue; } ++count; @@ -2457,14 +2476,15 @@ void GatewayNode::refresh_cache() { } } - // Filter ROS 2 internal nodes (underscore prefix convention). + // Filter ROS 2 internal nodes (underscore prefix convention) and this + // gateway's own helper nodes. // Controlled by discovery.runtime.filter_internal_nodes parameter (default: true). // Covers local heuristic apps (which bypass the merge pipeline orphan filter // in runtime_only mode) and any peer apps that slipped through fetch_entities. if (filter_internal_nodes_) { - auto removed = filter_internal_node_apps(apps, peer_routing_table); + auto removed = filter_internal_node_apps(apps, peer_routing_table, get_fully_qualified_name()); if (removed > 0) { - RCLCPP_DEBUG(get_logger(), "Filtered %zu internal node apps (_ prefix)", removed); + RCLCPP_DEBUG(get_logger(), "Filtered %zu internal node apps (_ prefix or own helper node)", removed); } } @@ -2561,9 +2581,10 @@ void GatewayNode::stop_rest_server() { } size_t filter_internal_node_apps(std::vector & apps, - const std::unordered_map & peer_routing_table) { + const std::unordered_map & peer_routing_table, + const std::string & self_fqn) { auto before = apps.size(); - auto end = std::remove_if(apps.begin(), apps.end(), [&peer_routing_table](const App & app) { + auto end = std::remove_if(apps.begin(), apps.end(), [&peer_routing_table, &self_fqn](const App & app) { std::string original_id = app.id; auto rt_it = peer_routing_table.find(app.id); if (rt_it != peer_routing_table.end()) { @@ -2573,6 +2594,14 @@ size_t filter_internal_node_apps(std::vector & apps, if (original_id.size() > prefix.size() && original_id.compare(0, prefix.size(), prefix) == 0) { original_id = original_id.substr(prefix.size()); } + } else if (is_own_gateway_node(app.effective_fqn(), self_fqn)) { + // A local app bound to one of this gateway's own nodes. Those names do + // not start with '_' ("_sub", "_fault_clients", ...), + // so only the FQN test catches them, and without it the gateway + // advertises its own plumbing as diagnosable apps. Remote entities are + // skipped deliberately: a peer's helper nodes carry the same FQNs and are + // the peer's own filter's business. + return true; } // ROS 2 internal nodes use _ prefix convention return !original_id.empty() && original_id[0] == '_'; diff --git a/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp b/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp index b235acc53..c7282fd7b 100644 --- a/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp +++ b/src/ros2_medkit_gateway/src/http/handlers/fault_handlers.cpp @@ -263,6 +263,13 @@ json FaultHandlers::merge_entity_freeze_frames(json env_data, snap["topic"] = ""; // entity data values, not a ROS topic snap["message_type"] = ""; snap["captured_at_ns"] = frame.captured_at_ns; + // Capture provenance. topic/message_type stay empty because these values + // are not a ROS message, which leaves "source" as the only field naming + // where the numbers came from - so carry it whenever the capture named a + // path. + if (!frame.source.empty()) { + snap["source"] = frame.source; + } if (frame.startup_catchup) { // Values were read at gateway start, not when the fault confirmed; // absent marker = captured on the confirm edge. @@ -344,6 +351,12 @@ dto::FaultDetail FaultHandlers::build_sovd_fault_response(const json & fault_jso snap["x-medkit"]["capture_origin"] = s["capture_origin"]; } // Entity-frame provenance (merge_entity_freeze_frames), only when known. + // "source" names the capture path (a plugin DataProvider or the + // plugin's x-plc-data route). A consumer reads it instead of the + // empty topic/message_type an entity frame necessarily carries. + if (s.contains("source") && s["source"].is_string()) { + snap["x-medkit"]["source"] = s["source"]; + } if (s.contains("connected") && s["connected"].is_boolean()) { snap["x-medkit"]["connected"] = s["connected"]; } diff --git a/src/ros2_medkit_gateway/test/test_entity_freeze_frame_capture.cpp b/src/ros2_medkit_gateway/test/test_entity_freeze_frame_capture.cpp index b9a44b2e0..b97372530 100644 --- a/src/ros2_medkit_gateway/test/test_entity_freeze_frame_capture.cpp +++ b/src/ros2_medkit_gateway/test/test_entity_freeze_frame_capture.cpp @@ -573,6 +573,15 @@ TEST_F(EntityFreezeFrameCaptureTest, DisconnectedEntityWithLastKnownValuesIsCapt ASSERT_TRUE(frames[0].connected.has_value()); EXPECT_FALSE(*frames[0].connected); EXPECT_EQ(frames[0].source_timestamp, 1234567890); + // Which path read the values. This capture had no DataProvider and went + // through the route fallback, so the frame must name that path. The + // DataProvider flavour of the same case asserts the other constant, which is + // what stops the two from being swapped at their call sites unnoticed. + EXPECT_EQ(frames[0].source, EntityFreezeFrameCapture::kSourceXPlcDataRoute); + // The wire value itself, not just the symbol: swapping what the two constants + // hold is an API break for every consumer of x-medkit.source, and comparing + // symbol against symbol would not see it. + EXPECT_EQ(frames[0].source, "plugin_x_plc_data_route"); } /// @verifies REQ_INTEROP_088 @@ -642,6 +651,11 @@ TEST_F(EntityFreezeFrameCaptureTest, DisconnectedDataProviderWithLastKnownValues ASSERT_TRUE(frames[0].connected.has_value()); EXPECT_FALSE(*frames[0].connected); EXPECT_TRUE(frames[0].source_timestamp.is_null()); // provider content has no timestamp field + // The provider path names itself, and the route path (same case, above) names + // the other constant: the pair is what makes a swap of the two call sites + // visible. The literal pins the wire value the API reference documents. + EXPECT_EQ(frames[0].source, EntityFreezeFrameCapture::kSourceDataProvider); + EXPECT_EQ(frames[0].source, "plugin_data_provider"); } /// @verifies REQ_INTEROP_088 @@ -787,6 +801,40 @@ TEST(MergeEntityFreezeFrames, AppendsWhenNoConfiguredFreezeFrame) { EXPECT_FALSE(snap.contains("capture_origin")); // confirm-edge frames carry no marker } +TEST(MergeEntityFreezeFrames, CarriesCapturePathAsSource) { + // An entity frame has no ROS topic, so topic/message_type are necessarily + // empty, so "source" is the only field left saying where they came from. + json env_data = {{"snapshots", json::array()}}; + EntityFreezeFrameCapture::Frame frame; + frame.entity_id = "plc_app"; + frame.values = {{"temperature", 42.5}}; + frame.captured_at_ns = 1234; + frame.source = EntityFreezeFrameCapture::kSourceXPlcDataRoute; + + auto merged = FaultHandlers::merge_entity_freeze_frames(env_data, {frame}); + ASSERT_EQ(merged["snapshots"].size(), 1u); + const auto & snap = merged["snapshots"][0]; + EXPECT_EQ(snap["source"], EntityFreezeFrameCapture::kSourceXPlcDataRoute); + EXPECT_EQ(snap["topic"], ""); + EXPECT_EQ(snap["message_type"], ""); +} + +TEST(MergeEntityFreezeFrames, OmitsSourceForAFrameThatNamesNoPath) { + // A merge-helper contract, not a control for the capture tests: both capture + // paths always name themselves (asserted from real captures in + // Disconnected{Entity,DataProvider}WithLastKnownValuesIsCaptured), so this + // frame is one only a caller can build. The helper must then leave the key + // out rather than invent a provenance the wire consumer would trust. + json env_data = {{"snapshots", json::array()}}; + EntityFreezeFrameCapture::Frame frame; + frame.entity_id = "plc_app"; + frame.values = {{"temperature", 42.5}}; + + auto merged = FaultHandlers::merge_entity_freeze_frames(env_data, {frame}); + ASSERT_EQ(merged["snapshots"].size(), 1u); + EXPECT_FALSE(merged["snapshots"][0].contains("source")); +} + TEST(MergeEntityFreezeFrames, StartupCatchUpFrameCarriesCaptureOrigin) { json env_data = {{"snapshots", json::array()}}; EntityFreezeFrameCapture::Frame frame; diff --git a/src/ros2_medkit_gateway/test/test_fault_handlers.cpp b/src/ros2_medkit_gateway/test/test_fault_handlers.cpp index 9c7f08a91..29b53ef22 100644 --- a/src/ros2_medkit_gateway/test/test_fault_handlers.cpp +++ b/src/ros2_medkit_gateway/test/test_fault_handlers.cpp @@ -142,6 +142,54 @@ TEST_F(FaultHandlersTest, BuildSovdFaultResponsePropagatesCaptureOrigin) { EXPECT_EQ(snap["x-medkit"]["capture_origin"], "startup"); } +TEST_F(FaultHandlersTest, BuildSovdFaultResponseServesEntityFrameSource) { + // A plugin-captured entity frame reaches the wire with an empty topic and + // message_type (the values are not a ROS message) plus x-medkit.source + // naming the capture path that read them. + ros2_medkit_msgs::msg::Fault fault; + fault.fault_code = "PLC_ALARM"; + + json env_data = {{"snapshots", json::array({{{"type", "freeze_frame"}, + {"snapshot_type", "freeze_frame"}, + {"name", "plc_app"}, + {"data", R"({"tank_level": 87.5})"}, + {"topic", ""}, + {"message_type", ""}, + {"captured_at_ns", 1234}, + {"source", "plugin_x_plc_data_route"}}})}}; + + auto response = to_json(FaultHandlers::build_sovd_fault_response(fault_json(fault), env_data, "/apps/plc_app")); + + auto & snap = response["environment_data"]["snapshots"][0]; + EXPECT_EQ(snap["x-medkit"]["source"], "plugin_x_plc_data_route"); + EXPECT_EQ(snap["x-medkit"]["topic"], ""); + EXPECT_EQ(snap["x-medkit"]["message_type"], ""); +} + +TEST_F(FaultHandlersTest, BuildSovdFaultResponseOmitsSourceWhenTheSnapshotHasNone) { + // Absence control for the test above, on the same harness: a topic-captured + // freeze frame (the fault_manager's own) carries no source, and none is + // invented for it. + ros2_medkit_msgs::msg::Fault fault; + fault.fault_code = "TEMP_FAULT"; + + ros2_medkit_msgs::msg::EnvironmentData env_data; + ros2_medkit_msgs::msg::Snapshot freeze_frame; + freeze_frame.type = "freeze_frame"; + freeze_frame.name = "temperature"; + freeze_frame.data = R"({"temperature": 85.5})"; + freeze_frame.topic = "/motor/temperature"; + freeze_frame.message_type = "sensor_msgs/msg/Temperature"; + env_data.snapshots.push_back(freeze_frame); + + auto response = + to_json(FaultHandlers::build_sovd_fault_response(fault_json(fault), env_json(env_data), "/apps/motor")); + + auto & snap = response["environment_data"]["snapshots"][0]; + EXPECT_FALSE(snap["x-medkit"].contains("source")); + EXPECT_EQ(snap["x-medkit"]["topic"], "/motor/temperature"); +} + // Conversion layer must emit an explicit "snapshot_type" discriminator so // downstream consumers (handler, SSE, MCP) can dispatch on a single key // regardless of which optional payload fields are present. diff --git a/src/ros2_medkit_gateway/test/test_gateway_node.cpp b/src/ros2_medkit_gateway/test/test_gateway_node.cpp index 2f81dbc6c..557aef176 100644 --- a/src/ros2_medkit_gateway/test/test_gateway_node.cpp +++ b/src/ros2_medkit_gateway/test/test_gateway_node.cpp @@ -1058,10 +1058,14 @@ TEST(GatewayStartupSummary, CountPeerNodesExcludesOwnAndHidden) { } TEST(GatewayStartupSummary, CountPeerNodesZeroWhenOnlyOwnNodes) { + // Every helper the gateway creates inside its own process. A gateway alone on + // the graph must report zero peers, so each helper has to be recognized - + // including the lifecycle reader, which the list previously omitted. const std::vector> nodes = { {"ros2_medkit_gateway", "/"}, {"ros2_medkit_gateway_sub", "/"}, {"ros2_medkit_gateway_fault_clients", "/"}, + {"ros2_medkit_gateway_lifecycle_state_reader", "/"}, }; // Zero peers is the condition that triggers the empty-graph warning. EXPECT_EQ(ros2_medkit_gateway::GatewayNode::count_peer_nodes(nodes, "/ros2_medkit_gateway"), 0u); diff --git a/src/ros2_medkit_gateway/test/test_handler_context.cpp b/src/ros2_medkit_gateway/test/test_handler_context.cpp index 968f29469..573c356c0 100644 --- a/src/ros2_medkit_gateway/test/test_handler_context.cpp +++ b/src/ros2_medkit_gateway/test/test_handler_context.cpp @@ -874,7 +874,7 @@ TEST(FilterInternalNodeAppsTest, FiltersLocalInternalNodes) { apps.push_back(another_internal); std::unordered_map routing; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 2u); ASSERT_EQ(apps.size(), 1u); @@ -896,7 +896,7 @@ TEST(FilterInternalNodeAppsTest, PreservesAllNormalNodes) { apps.push_back(a3); std::unordered_map routing; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 0u); EXPECT_EQ(apps.size(), 3u); @@ -921,7 +921,7 @@ TEST(FilterInternalNodeAppsTest, FiltersPeerPrefixedInternalNodes) { routing["peer_subsystem___ros2cli_daemon"] = "peer_subsystem"; routing["peer_subsystem__lidar_driver"] = "peer_subsystem"; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 1u); ASSERT_EQ(apps.size(), 1u); @@ -939,7 +939,7 @@ TEST(FilterInternalNodeAppsTest, DoesNotStripPrefixWithoutRoutingEntry) { apps.push_back(ambiguous); std::unordered_map routing; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 0u); ASSERT_EQ(apps.size(), 1u); @@ -950,7 +950,7 @@ TEST(FilterInternalNodeAppsTest, HandlesEmptyAppList) { std::vector apps; std::unordered_map routing; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 0u); EXPECT_TRUE(apps.empty()); @@ -981,7 +981,7 @@ TEST(FilterInternalNodeAppsTest, MixedLocalAndRemoteInternalNodes) { routing["sub_b__actuator"] = "sub_b"; routing["sub_b___parameter_bridge"] = "sub_b"; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 2u); ASSERT_EQ(apps.size(), 2u); @@ -1006,12 +1006,88 @@ TEST(FilterInternalNodeAppsTest, PeerPrefixMatchMustBeExact) { std::unordered_map routing; routing["my_peer__sensor"] = "my_peer"; - auto removed = filter_internal_node_apps(apps, routing); + auto removed = filter_internal_node_apps(apps, routing, "/ros2_medkit_gateway"); EXPECT_EQ(removed, 0u); ASSERT_EQ(apps.size(), 1u); } +namespace { + +App bound_app(const std::string & id, const std::string & fqn) { + App app; + app.id = id; + app.name = id; + app.bound_fqn = fqn; + return app; +} + +} // namespace + +TEST(FilterInternalNodeAppsTest, DropsTheGatewaysOwnHelperNodes) { + // The gateway creates "_sub", "_fault_clients" and + // "_lifecycle_state_reader" in its own process. None starts with '_', + // so runtime introspection returns them as ordinary apps and the gateway ends + // up listing its own plumbing as diagnosable. + const std::string self_fqn = "/ros2_medkit_gateway"; + std::vector apps{ + bound_app("ros2_medkit_gateway", self_fqn), + bound_app("ros2_medkit_gateway_sub", self_fqn + "_sub"), + bound_app("ros2_medkit_gateway_fault_clients", self_fqn + "_fault_clients"), + bound_app("ros2_medkit_gateway_lifecycle_state_reader", self_fqn + "_lifecycle_state_reader"), + // Positive controls on the same harness: a similarly suffixed FOREIGN + // node, a node whose name merely extends the gateway's, and the + // fault_manager, which is a separate diagnosable component even when it + // shares the process. + bound_app("other_gateway_sub", "/other_gateway_sub"), + bound_app("ros2_medkit_gateway_monitor", self_fqn + "_monitor"), + bound_app("fault_manager", "/fault_manager"), + }; + + std::unordered_map routing; + auto removed = filter_internal_node_apps(apps, routing, self_fqn); + + EXPECT_EQ(removed, 4u); + std::set remaining; + for (const auto & app : apps) { + remaining.insert(app.id); + } + EXPECT_EQ(remaining, (std::set{"other_gateway_sub", "ros2_medkit_gateway_monitor", "fault_manager"})); +} + +TEST(FilterInternalNodeAppsTest, LeavesPeerHelperNodesToThePeer) { + // A remote entity carrying the same FQN belongs to the peer that reported + // it, so this gateway must not reach across and filter it. + const std::string self_fqn = "/ros2_medkit_gateway"; + std::vector apps{bound_app("sub_b__ros2_medkit_gateway_sub", self_fqn + "_sub")}; + + std::unordered_map routing; + routing["sub_b__ros2_medkit_gateway_sub"] = "sub_b"; + + auto removed = filter_internal_node_apps(apps, routing, self_fqn); + + EXPECT_EQ(removed, 0u); + ASSERT_EQ(apps.size(), 1u); +} + +TEST(IsOwnGatewayNodeTest, MatchesSelfAndHelpersExactlyAndNothingElse) { + const std::string self_fqn = "/ros2_medkit_gateway"; + EXPECT_TRUE(is_own_gateway_node(self_fqn, self_fqn)); + EXPECT_TRUE(is_own_gateway_node(self_fqn + "_sub", self_fqn)); + EXPECT_TRUE(is_own_gateway_node(self_fqn + "_fault_clients", self_fqn)); + EXPECT_TRUE(is_own_gateway_node(self_fqn + "_lifecycle_state_reader", self_fqn)); + + // Prefix neighbours are genuine peers, not ours. + EXPECT_FALSE(is_own_gateway_node(self_fqn + "_monitor", self_fqn)); + EXPECT_FALSE(is_own_gateway_node(self_fqn + "2", self_fqn)); + EXPECT_FALSE(is_own_gateway_node("/other" + self_fqn + "_sub", self_fqn)); + EXPECT_FALSE(is_own_gateway_node("/fault_manager", self_fqn)); + + // An unknown self FQN must claim nothing rather than everything. + EXPECT_FALSE(is_own_gateway_node(self_fqn, "")); + EXPECT_FALSE(is_own_gateway_node("", self_fqn)); +} + // ============================================================================= // Area fault/log aggregation handler tests (via REST API) // ============================================================================= diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/CHANGELOG.rst b/src/ros2_medkit_plugins/ros2_medkit_opcua/CHANGELOG.rst index 9f37ccc7f..1db5561be 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/CHANGELOG.rst +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/CHANGELOG.rst @@ -2,6 +2,16 @@ Changelog for package ros2_medkit_opcua ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +Forthcoming +----------- +* **Breaking:** ``discovery.interval_s`` now distinguishes unset from an explicit ``0``. Leaving the key out (and leaving ``OPCUA_DISCOVERY_INTERVAL_S`` unset) keeps the built-in 30 s re-scan cadence, while an explicit ``0`` means "discovery on, start-up scan only" instead of selecting that same default. A deployment that wrote ``interval_s: 0`` meaning "use the default" stops re-scanning, and dropping the key restores the previous behaviour. A negative value is refused with a warning and leaves the cadence unset +* While no OPC UA session is established, the reconnect loop re-scans on that cadence and adopts a server that appeared after start-up, so a gateway that booted before its PLC finds it without a restart. The cadence is measured from the end of the previous sweep, the reconnect backoff is capped at the cadence so the documented interval is the real one, and a sweep is cancellable (``SIGINT`` / ``SIGTERM`` during start-up, the plugin's ``shutdown()`` for a re-scan) instead of having to run to completion. A re-scan whose outcome has not changed is reported at DEBUG rather than repeating the whole report every interval +* With no node map, the SOVD component identity is re-derived from the device on the first session after such an adoption, so a component named after the fallback endpoint (because nothing answered at start-up) stops being served under that placeholder once the real PLC is adopted +* ``PLC_COMMS_LOST`` is cleared on every successful connect, including the first, so a fault ``fault_manager`` persisted before a restart does not stand against a healthy link. That clear, and the per-entity ``DELETE /{entity}/faults/{code}`` route when it is served by this plugin, set ``skip_correlation_auto_clear``: neither is an operator resolving a root cause, so neither may cascade-clear correlated symptom faults reported by apps in other entities. A clear reported by the device itself still cascades +* Fault dispatches buffered while ``fault_manager`` is unreachable no longer lose one-shot events. Only the link-state ``PLC_COMMS_LOST`` clear is re-derivable (the next reconnect sends it again), so it is what the bounded buffer gives up first. Alarm reports, a device alarm's inactive edge and an operator's scoped clear age out oldest-first as before, and at most one clear per fault code is pending at a time +* Gateway-side changes that land on this plugin's entities: a freeze-frame captured from a plugin entity now names the path that read the values in ``x-medkit.source`` (``plugin_data_provider`` or ``plugin_x_plc_data_route``, the only provenance an entity frame carries since it has no ROS topic), and the gateway no longer lists its own in-process helper nodes among the discovered apps +* Contributors: @bburda + 0.7.0 (2026-08-27) ------------------ * Config-less discovery. A read-only network scan finds the OPC UA server instead of requiring its endpoint up front (`#509 `_), ``auto_browse`` walks the address space recursively and builds the SOVD tree from it (`#510 `_), and identity, writability and fault triggers are read from the device itself rather than declared in a node map (`#544 `_) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index 0381ef765..7b6abdde0 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -548,7 +548,7 @@ 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_fault_enabled` | `true` | Raise a component-scoped `PLC_COMMS_LOST` fault when the connection stays down, and clear it on every successful connect (issue #496) | | `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 | | `discovery.enabled` | `false` | Opt-in read-only PLC network discovery (auto endpoint). See below | @@ -604,12 +604,18 @@ plugins.opcua.discovery: connect_timeout_ms: 600 # per-port TCP connect timeout scan_concurrency: 100 # bounded, polite concurrent connect count identify_timeout_ms: 6000 # per GetEndpoints identify - interval_s: 0 # 0 = one-shot at startup (periodic re-scan: TODO) + # re-scan cadence while disconnected. Omit the key for the built-in 30 s, + # or set it to 0 to keep discovery on but never re-scan (start-up scan only). + interval_s: 30 anonymous_none_only: true # only auto-connect None/Anonymous servers ``` Environment overrides (Docker / appliance): `OPCUA_DISCOVERY_ENABLED`, `OPCUA_DISCOVERY_SUBNETS` (comma-separated CIDRs), `OPCUA_DISCOVERY_INTERVAL_S`. +Leaving `interval_s` (and `OPCUA_DISCOVERY_INTERVAL_S`) unset means "no cadence +stated" and takes the 30 s default. An explicit `0` is honoured as written and +turns the recurring sweep off. A negative value is refused with a warning and +leaves the cadence unset. How it works: 1. Bounded concurrent TCP connect sweep of the configured ports across the @@ -625,24 +631,52 @@ How it works: auto-selects the best None/Anonymous data server (deterministic, lowest ip:port) and connects to the **scanned ip:port** - not the advertised EndpointUrl, which a server may report as a non-resolvable hostname. +5. While no session is established, the reconnect loop scans again every + `interval_s` (default 30 s), measured from the END of the previous sweep, and + adopts a newly found server for its next connect attempt, logging the swap at + INFO. The re-scan is consulted once per reconnect attempt, and those are + spaced by an exponential backoff, so the backoff ceiling is capped at + `interval_s` while discovery is re-scanning - otherwise the real cadence + would be `max(interval_s, backoff)` rather than the stated one. This covers + the common race where the gateway and the PLC boot together: the startup scan + finds nothing because the PLC is still coming up, and without a re-scan the + plugin would retry the fallback endpoint until someone restarted it. +6. On the first session after such an adoption, a config-less deployment (no + node map) re-derives the SOVD component identity from the device itself, so + the component stops being served under the provisional `opcua-` name it + got when nothing answered. The change is logged at INFO. + +Re-scanning stops as soon as a session is up, and never starts at all when an +`endpoint_url` is configured. Safety / OT posture: - Everything is read-only: TCP connect + `GetEndpoints` only. No writes, no subscriptions, no second long-lived session. +- The scan is NOT one-shot: while the plugin has no session it repeats every + `interval_s` (default 30 s) for as long as it stays disconnected. Set + `interval_s: 0` (or `OPCUA_DISCOVERY_INTERVAL_S=0`) to keep discovery on with + the start-up scan only, or `enabled: false` to switch it off entirely. +- A sweep is cancellable, so a stop does not have to wait out a subnet the size + of a /16. The start-up sweep runs while the gateway node is still being + constructed, so what ends it is `SIGINT` / `SIGTERM`, which the plugin sees + through `rclcpp::ok()`. A re-scan sweep runs on the poll thread and is ended + by either that or the plugin's own `shutdown()`. - An explicitly configured `endpoint_url` (or `OPCUA_ENDPOINT_URL`) always wins; discovery then does nothing, so it never opens a second session on a PLC the plugin already polls. -- Secured-only servers (no None/Anonymous endpoint) are surfaced in the startup - log as leads requiring operator credentials - never auto-connected or probed. +- Secured-only servers (no None/Anonymous endpoint) are surfaced in the log as + leads requiring operator credentials - never auto-connected or probed. A + re-scan whose outcome has not changed reports at DEBUG instead of repeating + the whole report, so a recurring sweep does not bury the rest of the log. - The scan is bounded (short connect timeout, capped concurrency) and CIDRs wider than /16 are rejected to prevent an accidental broad sweep. Note on passive discovery: a stock Siemens S7-1500 neither multicast-announces (mDNS `_opcua-tcp._tcp`) nor registers with an OPC-UA LDS, so passive sources find nothing there; the active scan is what discovers it. Passive mDNS / LDS -`FindServers` sources (useful on Kepware / Prosys / GDS estates) and periodic -re-scan + multi-endpoint registration are planned follow-ups; this iteration -delivers the active-scan core and single "auto endpoint" mode. +`FindServers` sources (useful on Kepware / Prosys / GDS estates) and +multi-endpoint registration are planned follow-ups. This iteration delivers the +active-scan core and a single "auto endpoint" mode. ### Active-condition replay on reconnect (issue #389/#478) @@ -694,6 +728,20 @@ so the alarm clears on `Acknowledge` alone. The default (`true`) is unchanged and spec-strict; the relaxed path still requires acknowledgement and needs real-S7-1500 validation. +### Connection loss and `PLC_COMMS_LOST` (issue #496) + +When the OPC-UA connection stays down for `comms_lost_debounce_ms` continuously, +the plugin raises one component-scoped `PLC_COMMS_LOST` fault (a shorter blip +during a normal reconnect does not flap it). + +The fault is cleared on **every** successful connect, both the initial one and +every later reconnect, whether or not this process was the one that raised it. +The fault manager keys faults by fault code and persists them, so a fault raised +before a gateway restart is still standing while the new process has no memory +of it. Clearing only what the running process remembered left exactly that fault +CONFIRMED for good. The clear is fire-and-forget, so a clear for a fault that is +not there is harmless. + Node map entries also support an optional `ros2_topic` field to override the auto-generated ROS 2 topic name for the PLC value bridge: ```yaml @@ -827,6 +875,16 @@ bash scripts/run_integration_tests.sh bash scripts/stop.sh ``` +A separate scenario covers the config-less discovery start-up race, which the +suite above cannot see because it pins `OPCUA_ENDPOINT_URL` and so +short-circuits discovery. It starts the gateway before any server, with +discovery on and no endpoint configured, then brings a server up and asserts +the gateway adopts it without a restart: + +```bash +bash src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_discovery_race_test.sh +``` + ### Test Coverage | Category | Tests | What it validates | diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_discovery_race_test.sh b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_discovery_race_test.sh new file mode 100755 index 000000000..5b807472b --- /dev/null +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_discovery_race_test.sh @@ -0,0 +1,321 @@ +#!/usr/bin/env bash +# Copyright 2026 mfaferek93 +# +# Integration test for the config-less discovery start-up race. +# +# The field failure this reproduces: the gateway and the PLC power on +# together, the gateway's start-up scan runs while the PLC is still booting +# and finds nothing, and without a re-scan the plugin retries its fallback +# endpoint for as long as it runs. Only a restart ever found the PLC. +# +# So this scenario starts the gateway FIRST, with discovery on and no +# OPCUA_ENDPOINT_URL, asserts it settled on the fallback endpoint with no +# session, then starts an OPC-UA server and asserts the gateway adopts it +# within two re-scan intervals without being restarted. +# +# It then repeats the race with NO node map at all (the config-less deployment). +# There the component identity is derived from the device, and a gateway that +# scanned before its PLC existed can only name it after the fallback endpoint - +# so the second pass asserts the SOVD component stops being served under that +# provisional name once the PLC is adopted. +# +# Every other opcua docker scenario pins OPCUA_ENDPOINT_URL, which +# short-circuits discovery, so none of them can see this. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "$0")/../../../../.." && pwd)" +NET_NAME=discovery-race-net +NET_SUBNET=172.31.77.0/24 +SERVER_NAME=discovery-race-server +GATEWAY_NAME=discovery-race-gateway +SERVER_PORT=4840 +GATEWAY_PORT=8089 +RESCAN_INTERVAL_S=5 +CONFIG_DIR=/tmp/discovery_race_config + +# The fallback the plugin keeps when a scan selects nothing (OpcuaClientConfig). +FALLBACK_ENDPOINT="opc.tcp://localhost:4840" +# Config-less naming: with no node map the component id is derived from the +# device. Before any server exists that can only be the fallback endpoint's +# host. After adoption it is the test server's DI nameplate (Manufacturer "SelfPatch +# Devices" + Model "SPX-1000"), slugified. +FALLBACK_COMPONENT_ID="opcua-localhost" +DEVICE_COMPONENT_ID="selfpatch_devices_spx_1000" + +cleanup() { + local rc=$? + if [[ ${rc} -ne 0 ]]; then + for c in "${SERVER_NAME}" "${GATEWAY_NAME}"; do + echo "=== ${c} logs (cleanup trap) ===" >&2 + docker logs "${c}" >&2 2>&1 || true + done + fi + docker rm -f "${SERVER_NAME}" "${GATEWAY_NAME}" >/dev/null 2>&1 || true + docker network rm "${NET_NAME}" >/dev/null 2>&1 || true + rm -rf "${CONFIG_DIR}" +} +trap cleanup EXIT + +fail() { + echo " FAIL: $*" >&2 + exit 1 +} + +# x-plc-status of a named component. The node-map pass pins the id, the +# config-less pass has to look it up first. +status_json_for() { + curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/components/$1/x-plc-status" || echo '{}' +} + +status_json() { + status_json_for discovery_race_runtime +} + +status_field() { + status_json | python3 -c "import json,sys; print(json.load(sys.stdin).get('$1', ''))" +} + +status_field_for() { + status_json_for "$1" | python3 -c "import json,sys; print(json.load(sys.stdin).get('$2', ''))" +} + +component_ids() { + curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/components" 2>/dev/null | + python3 -c " +import json,sys +try: + print(' '.join(c.get('id','') for c in json.load(sys.stdin).get('items', []))) +except Exception: + print('') +" +} + +wait_for_rest_api() { + for _ in $(seq 1 60); do + if curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/components" >/dev/null 2>&1; then + return 0 + fi + sleep 1 + done + curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/components" >/dev/null \ + || fail "gateway REST API never came up" +} + +cd "${REPO_ROOT}" + +# Idempotent teardown of anything a hard-killed earlier run left behind. +docker rm -f "${SERVER_NAME}" "${GATEWAY_NAME}" >/dev/null 2>&1 || true +docker network rm "${NET_NAME}" >/dev/null 2>&1 || true + +echo "[1/9] Build images" +docker build --network=host \ + -f src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile \ + -t ros2_medkit_alarm_test_server:dev . >/dev/null +docker build --network=host \ + -f src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway \ + -t gateway-opcua:discovery-race . >/dev/null + +# A /24 keeps the read-only sweep to 254 hosts, so a scan finishes in seconds. +# Discovery rejects anything wider than /16 outright. +docker network create --subnet "${NET_SUBNET}" "${NET_NAME}" >/dev/null + +echo "[2/9] Start the gateway BEFORE any server, discovery on, no endpoint pinned" +mkdir -p "${CONFIG_DIR}" +cat >"${CONFIG_DIR}/discovery_nodes.yaml" <<'EOF' +area_id: plc_systems +component_id: discovery_race_runtime +# One node is enough: the assertion is on the session, not on any value. The +# node id need not resolve on the server, since a failed read does not drop +# the connection. +nodes: + - node_id: "ns=2;s=StatusWord" + entity_id: tank_process + data_name: status_word + data_type: int +EOF +cat >"${CONFIG_DIR}/manifest.yaml" <<'EOF' +manifest_version: "1.0" +EOF +cp src/ros2_medkit_plugins/ros2_medkit_opcua/docker/gateway_params.yaml \ + "${CONFIG_DIR}/gateway_params.yaml" + +docker run -d --name "${GATEWAY_NAME}" --network "${NET_NAME}" \ + -p "${GATEWAY_PORT}:8080" \ + -v "${CONFIG_DIR}:/config:ro" \ + -e ROS_DOMAIN_ID=67 \ + -e OPCUA_DISCOVERY_ENABLED=1 \ + -e OPCUA_DISCOVERY_SUBNETS="${NET_SUBNET}" \ + -e OPCUA_DISCOVERY_INTERVAL_S="${RESCAN_INTERVAL_S}" \ + -e OPCUA_NODE_MAP_PATH=/config/discovery_nodes.yaml \ + gateway-opcua:discovery-race \ + bash -c ' + set -e + mkdir -p /var/lib/ros2_medkit/rosbags + source /opt/ros/jazzy/setup.bash + source /root/ws/install/setup.bash + ros2 run ros2_medkit_fault_manager fault_manager_node \ + > /var/lib/ros2_medkit/fault_manager.log 2>&1 & + PLUGIN_PATH=$(find /root/ws/install -name "libros2_medkit_opcua_plugin.so" | head -1) + exec ros2 run ros2_medkit_gateway gateway_node \ + --ros-args --params-file /config/gateway_params.yaml \ + -p plugins.opcua.path:="${PLUGIN_PATH}" \ + -p discovery.mode:=hybrid \ + -p discovery.manifest_path:=/config/manifest.yaml \ + -p discovery.manifest_strict_validation:=false' >/dev/null + +echo "[3/9] Wait for the REST API" +wait_for_rest_api + +echo "[4/9] Assert the start-up scan found nothing and left the fallback endpoint" +# The scan runs during set_context(), so by the time the API answers it has +# already completed against an empty network. +endpoint="$(status_field endpoint_url)" +connected="$(status_field connected)" +[[ "${endpoint}" == "${FALLBACK_ENDPOINT}" ]] \ + || fail "expected the fallback endpoint before the server exists, got '${endpoint}'" +[[ "${connected}" == "False" ]] \ + || fail "expected no session before the server exists, got connected='${connected}'" +echo " OK no server found at start-up, endpoint left at ${FALLBACK_ENDPOINT}" + +echo "[5/9] Start the OPC-UA server (the PLC finishing its boot)" +docker run -d --name "${SERVER_NAME}" --network "${NET_NAME}" \ + ros2_medkit_alarm_test_server:dev --port "${SERVER_PORT}" >/dev/null +for _ in $(seq 1 30); do + if docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY '; then + break + fi + sleep 1 +done +docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY ' || fail "test server never became ready" +SERVER_IP="$(docker inspect -f "{{(index .NetworkSettings.Networks \"${NET_NAME}\").IPAddress}}" "${SERVER_NAME}")" +echo " server up at ${SERVER_IP}:${SERVER_PORT}" + +echo "[6/9] Assert the gateway adopts it within two re-scan intervals, unrestarted" +# Budget arithmetic. The re-scan is consulted once per reconnect attempt, and +# those are spaced by the reconnect backoff, so the adoption cadence is +# max(interval_s, backoff). The backoff ceiling is capped at interval_s while +# discovery is re-scanning, which is what keeps this budget in terms of +# interval_s alone: worst case is one full interval before the sweep is due plus +# one for the attempt that follows it, and the +40 s covers the sweep of a /24, +# the connect and the REST refresh. Generous enough not to flake, far short of +# "never", which is what the bug did. +DEADLINE=$((SECONDS + 2 * RESCAN_INTERVAL_S + 40)) +adopted="" +while [[ ${SECONDS} -lt ${DEADLINE} ]]; do + endpoint="$(status_field endpoint_url)" + connected="$(status_field connected)" + if [[ "${endpoint}" == "opc.tcp://${SERVER_IP}:${SERVER_PORT}" && "${connected}" == "True" ]]; then + adopted="${endpoint}" + break + fi + sleep 2 +done +[[ -n "${adopted}" ]] \ + || fail "endpoint still '${endpoint}' (connected='${connected}') after $((2 * RESCAN_INTERVAL_S + 40))s" +echo " OK re-scan adopted ${adopted} without a gateway restart" + +# The gateway must have adopted the server in the process that started before +# it, not in a fresh one: a restarted container would pass the check above +# while proving nothing. +restarts="$(docker inspect -f '{{.RestartCount}}' "${GATEWAY_NAME}")" +[[ "${restarts}" == "0" ]] || fail "gateway restarted ${restarts} time(s) during the run" +echo " OK gateway never restarted" + +# --------------------------------------------------------------------------- +# Config-less variant: the same race with NO node map. +# +# Without a node map the SOVD component is named from the device itself. A +# gateway that scanned before its PLC existed has no device to ask, so it can +# only name the component after the fallback endpoint. The defect this covers is +# that identity being pinned once and never revisited: the component kept +# serving opcua-localhost for the life of the process while the plugin was +# happily polling the adopted PLC. +# --------------------------------------------------------------------------- + +echo "[7/9] Config-less pass: stop everything, start the gateway with no node map" +docker rm -f "${SERVER_NAME}" "${GATEWAY_NAME}" >/dev/null 2>&1 || true + +docker run -d --name "${GATEWAY_NAME}" --network "${NET_NAME}" \ + -p "${GATEWAY_PORT}:8080" \ + -v "${CONFIG_DIR}:/config:ro" \ + -e ROS_DOMAIN_ID=67 \ + -e OPCUA_DISCOVERY_ENABLED=1 \ + -e OPCUA_DISCOVERY_SUBNETS="${NET_SUBNET}" \ + -e OPCUA_DISCOVERY_INTERVAL_S="${RESCAN_INTERVAL_S}" \ + gateway-opcua:discovery-race \ + bash -c ' + set -e + mkdir -p /var/lib/ros2_medkit/rosbags + source /opt/ros/jazzy/setup.bash + source /root/ws/install/setup.bash + ros2 run ros2_medkit_fault_manager fault_manager_node \ + > /var/lib/ros2_medkit/fault_manager.log 2>&1 & + PLUGIN_PATH=$(find /root/ws/install -name "libros2_medkit_opcua_plugin.so" | head -1) + exec ros2 run ros2_medkit_gateway gateway_node \ + --ros-args --params-file /config/gateway_params.yaml \ + -p plugins.opcua.path:="${PLUGIN_PATH}" \ + -p discovery.mode:=hybrid \ + -p discovery.manifest_path:=/config/manifest.yaml \ + -p discovery.manifest_strict_validation:=false' >/dev/null + +wait_for_rest_api + +echo "[8/9] Assert the component is named after the fallback while nothing answers" +ids="" +DEADLINE=$((SECONDS + 30)) +while [[ ${SECONDS} -lt ${DEADLINE} ]]; do + ids="$(component_ids)" + if [[ " ${ids} " == *" ${FALLBACK_COMPONENT_ID} "* ]]; then + break + fi + sleep 2 +done +[[ " ${ids} " == *" ${FALLBACK_COMPONENT_ID} "* ]] \ + || fail "expected the provisional component '${FALLBACK_COMPONENT_ID}' before any server, got '${ids}'" +echo " OK config-less component provisionally named ${FALLBACK_COMPONENT_ID}" + +echo "[9/9] Start the server and assert the component is renamed from the device" +docker run -d --name "${SERVER_NAME}" --network "${NET_NAME}" \ + ros2_medkit_alarm_test_server:dev --port "${SERVER_PORT}" >/dev/null +for _ in $(seq 1 30); do + if docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY '; then + break + fi + sleep 1 +done +docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY ' || fail "test server never became ready" +SERVER_IP="$(docker inspect -f "{{(index .NetworkSettings.Networks \"${NET_NAME}\").IPAddress}}" "${SERVER_NAME}")" +echo " server up at ${SERVER_IP}:${SERVER_PORT}" + +# Same budget as step 6, plus the discovery refresh that republishes entities. +DEADLINE=$((SECONDS + 2 * RESCAN_INTERVAL_S + 60)) +renamed="" +while [[ ${SECONDS} -lt ${DEADLINE} ]]; do + ids="$(component_ids)" + if [[ " ${ids} " == *" ${DEVICE_COMPONENT_ID} "* ]]; then + renamed="${DEVICE_COMPONENT_ID}" + break + fi + sleep 2 +done +[[ -n "${renamed}" ]] \ + || fail "component still '${ids}' after adoption - expected the device-derived '${DEVICE_COMPONENT_ID}'" + +# The renamed component is the one actually polling the adopted PLC, so the id +# the operator sees is not a second, stale entity next to a live opcua-localhost. +endpoint="$(status_field_for "${renamed}" endpoint_url)" +connected="$(status_field_for "${renamed}" connected)" +[[ "${endpoint}" == "opc.tcp://${SERVER_IP}:${SERVER_PORT}" ]] \ + || fail "renamed component reports endpoint '${endpoint}', expected the adopted server" +[[ "${connected}" == "True" ]] \ + || fail "renamed component reports connected='${connected}', expected a live session" +[[ "${renamed}" != "${FALLBACK_COMPONENT_ID}" ]] \ + || fail "component id never moved off ${FALLBACK_COMPONENT_ID}" +echo " OK component renamed to ${renamed}, connected at ${endpoint}" + +restarts="$(docker inspect -f '{{.RestartCount}}' "${GATEWAY_NAME}")" +[[ "${restarts}" == "0" ]] || fail "gateway restarted ${restarts} time(s) during the config-less run" +echo " OK gateway never restarted" + +echo "Discovery race scenario passed." diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/network_discovery.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/network_discovery.hpp index ed68d33e5..9f73d21d2 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/network_discovery.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/network_discovery.hpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -105,10 +106,17 @@ struct OpcuaDiscoveryConfig { int scan_concurrency{100}; ///< bounded, polite concurrent connect count int identify_timeout_ms{6000}; - /// Re-scan cadence. 0 = one-shot at startup (the only mode implemented in - /// this iteration); a positive value is accepted and validated but periodic - /// re-scan is a documented follow-up. - int interval_s{0}; + /// Re-scan cadence, in seconds, while no OPC-UA session is established. + /// Unset (the key absent) selects the built-in default, an explicit 0 turns + /// re-scanning off and keeps the startup scan one-shot - the two are + /// deliberately distinct, so a deployment can keep discovery on and still + /// stop the recurring sweep (see OpcuaPlugin::effective_rescan_interval_s). + /// The startup scan always runs once. The cadence only governs how often the + /// disconnected reconnect loop scans again, so a gateway that started before + /// its PLC finished booting adopts the PLC when it appears instead of retrying + /// the fallback endpoint forever. Never used once an endpoint is configured + /// explicitly, and never while a session is up. + std::optional interval_s; /// Only auto-register endpoints that expose a None + Anonymous endpoint (what /// the plugin connects with today). Secured-only servers are surfaced as @@ -154,7 +162,15 @@ class NetworkDiscovery { /// Run one full discovery pass (blocking). Read-only: TCP connect + /// GetEndpoints only. Deduplicated by ApplicationUri (fallback ip:port), /// sorted deterministically by ip:port. - std::vector run(); + /// + /// @param cancelled optional abort predicate, polled before every probe and + /// between the sweep and identify phases. A sweep of a legal /16 is + /// tens of thousands of probes and takes minutes, and the caller runs + /// it on the poll thread that a shutdown has to join, so without this + /// a ``docker stop`` grace period would expire mid-sweep. A pass still + /// cancelled at the next phase boundary returns an empty result rather + /// than a partial one, within one in-flight probe per worker. + std::vector run(const std::function & cancelled = {}); /// Resolve the subnets to scan: configured ``subnets`` if any, else the /// derived local /24. Exposed for logging / tests. diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp index e77a3a519..cfe7a5bed 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp @@ -15,11 +15,14 @@ #pragma once #include "ros2_medkit_opcua/address_space_browser.hpp" +#include "ros2_medkit_opcua/device_identity.hpp" #include "ros2_medkit_opcua/network_discovery.hpp" #include "ros2_medkit_opcua/node_map.hpp" #include "ros2_medkit_opcua/opcua_client.hpp" #include "ros2_medkit_opcua/opcua_poller.hpp" +#include + #include #include #include @@ -31,10 +34,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include @@ -141,6 +146,219 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, static void apply_auto_alarms_param(const nlohmann::json & value, AutoAlarmsConfig & cfg, const std::function & warn); + // Where one discovery pass reports to, plus the memory that keeps a repeated + // identical pass quiet. A rescan runs every ``interval_s`` for the life of a + // disconnected process, so re-emitting the same per-server lines, summary and + // "no auto-connectable server" WARN each time buries every other message in + // the log. ``previous_outcome`` is owned by the caller (the plugin keeps one + // across rescans): when it is non-null and the pass reaches the same outcome + // as the pass before it, the whole report goes to ``debug`` instead. The first + // pass, and every pass whose outcome changed, is always reported at info/warn. + // A null ``previous_outcome`` (tests that do not care) reports every pass. + // + // The "scanning [subnets]" announcement is NOT part of that report. It is sent + // before the sweep runs, because a wide subnet takes minutes and an operator + // watching start-up has to see the gateway working rather than hung. It says + // what the pass is about to do rather than what it found, so it carries the + // same first-pass / rescan levelling on its own. + struct DiscoveryReporter { + std::function info; + std::function warn; + std::function debug; + std::string * previous_outcome{nullptr}; + }; + + // Run one read-only discovery pass and return the endpoint URL to adopt. + // + // Returns nullopt - meaning "keep the endpoint you have" - when discovery is + // disabled, when an endpoint was configured explicitly, when no subnet could + // be resolved, or when the pass found no auto-connectable None/Anonymous data + // server. Both the startup scan and the reconnect rescan go through here, so + // the two cannot drift apart. Static with injected probes and log sinks so + // both are unit-testable without a network. + // + // @param config discovery configuration (subnets, ports, timeouts, ...) + // @param endpoint_configured true when the operator pinned endpoint_url, so + // discovery then selects nothing and can neither override the + // operator's target nor open a second session on an already polled PLC + // @param scan injected TCP port probe + // @param identify injected OPC-UA GetEndpoints identify + // @param reporter operator-visible log sinks + repeat-suppression memory + // @param cancelled abort predicate handed to NetworkDiscovery::run, so a + // shutdown does not have to wait out a full sweep + static std::optional discover_endpoint(const OpcuaDiscoveryConfig & config, bool endpoint_configured, + const PortScanFn & scan, const IdentifyFn & identify, + const DiscoveryReporter & reporter, + const std::function & cancelled = {}); + + // Seconds between reconnect rescans, or 0 when the reconnect loop must never + // rescan. That is the answer when discovery is disabled, when an endpoint was + // configured explicitly, and when the operator set ``interval_s: 0``, which + // means "keep discovery on but leave the startup scan one-shot". An UNSET + // interval is the config-less case - it cannot name a cadence and is the one + // that most needs its PLC adopted once it finishes booting - so it takes the + // built-in default instead of never rescanning. + static int effective_rescan_interval_s(const OpcuaDiscoveryConfig & config, bool endpoint_configured); + + // Default reconnect rescan cadence, in seconds, when discovery is enabled with + // no explicit ``interval_s``. Long enough that a bounded subnet sweep stays a + // background cost on the poll thread, short enough that a PLC finishing its + // boot is picked up in well under a minute. + static constexpr int kDefaultRescanIntervalS = 30; + + // One rate-limited rescan step: run ``sweep`` when the cadence is due, + // otherwise do nothing. + // + // The cadence is measured from the END of the previous sweep, which + // ``*last_scan_end`` stores. A sweep of a legal /16 takes minutes, so + // stamping its start would make the next one due the moment it returned: the + // poll thread would sweep back to back and the reconnect attempt would drop to + // one per sweep. ``now`` is injected so the spacing is unit-testable without + // sleeping. + // + // @return whatever ``sweep`` returned, or nullopt when it was not due yet. + static std::optional rescan_step(int interval_s, + const std::function & now, + std::chrono::steady_clock::time_point * last_scan_end, + const std::function()> & sweep); + + // Ceiling for the poller's exponential reconnect backoff. + // + // Without discovery this is ``default_ceiling`` (60 s). While the reconnect + // loop is rescanning, the rescan is only consulted once per reconnect attempt, + // so the real adoption cadence is max(interval_s, backoff) - a documented + // "every 30 s" would silently become every 60 s. Capping the backoff at the + // rescan interval makes the documented cadence the true one. Never shorter + // than ``base`` (the configured reconnect interval), so a tiny interval cannot + // turn the backoff into a hot retry loop. + static std::chrono::milliseconds effective_max_reconnect_wait(std::chrono::milliseconds base, + std::chrono::milliseconds default_ceiling, + int rescan_interval_s); + + // The component identity a config-less deployment should serve after a + // connect, or nullopt when the identity it already has still holds. + // + // A gateway that starts before its PLC connects to nothing, so the identity + // derived at start-up comes from an empty DeviceInfo and the fallback + // endpoint: the neutral ``opcua-`` placeholder. Once discovery adopts + // the real server, the device can finally name itself, and the SOVD component + // must stop serving the placeholder. Pure / static so the rule is testable + // without a server. + static std::optional rederived_component_identity(const std::string & current_id, + const OpcuaClient::DeviceInfo & info, + const std::string & endpoint_url); + + // Why a ClearFault is being sent. Two properties follow from it and nothing + // else does, so the origin travels instead of a pair of loose booleans: + // - whether the correlation cascade must be skipped + // (``clear_skips_correlation``), which goes on the wire, and + // - whether the clear is re-derivable (``clear_is_link_state``), which is + // what the pending buffer may give up first under pressure. + enum class ClearOrigin { + /// The device reported the condition inactive (an ``event_alarms`` / + /// ``auto_alarms`` condition, or a threshold rule going false). A one-shot + /// edge nothing will re-send, and a real resolution, so the cascade stands. + DeviceAlarm, + /// The OPC-UA session came back, so ``PLC_COMMS_LOST`` no longer holds. + /// Re-derived on the next reconnect if it is lost, and not an operator + /// resolving a root cause, so it must not cascade. + LinkState, + /// The SOVD per-entity ``DELETE /{entity}/faults/{code}`` route reached + /// FaultProvider::clear_fault. An operator scoped to one entity must not + /// cascade-clear symptoms reported by apps in other entities, which is the + /// same rule the gateway applies on its own (non-plugin) branch of that + /// route. One-shot: nothing re-derives an operator's decision. + ScopedOperator + }; + + // Whether this clear must leave the correlation engine's auto_clear_with_root + // cascade alone. True for everything except a device-reported clear. + static bool clear_skips_correlation(ClearOrigin origin) { + return origin != ClearOrigin::DeviceAlarm; + } + + // Whether this clear will be re-derived if it is dropped. Only the link-state + // clear will: the next reconnect sends it again. + static bool clear_is_link_state(ClearOrigin origin) { + return origin == ClearOrigin::LinkState; + } + + // Whether a discovery sweep must stop now, given the two independent stop + // signals. Static and pure so both inputs are testable: the member + // ``discovery_cancelled()`` only reads them off the process and hands them + // here, so this is the whole rule. + // + // - ``shutdown_requested`` is set by shutdown(), which the gateway calls + // after its executor returns. That ends a RESCAN sweep, which runs on the + // poll thread long after start-up. + // - ``rclcpp_ok`` is false once rclcpp's own SIGINT / SIGTERM handler has + // run. The START-UP sweep runs inside set_context(), during node + // construction and before the executor spins, so shutdown() cannot be + // reached while it is in progress and the signal is the only thing that + // can end it. + static bool discovery_cancelled_for(bool shutdown_requested, bool rclcpp_ok) { + return shutdown_requested || !rclcpp_ok; + } + + // Which kind of clear a fault-detection signal going inactive is. The poller + // emits the component-scoped ``PLC_COMMS_LOST`` clear through the same + // callback as every device alarm, and only that one is a link-state event. + static ClearOrigin clear_origin_for_signal(const std::string & fault_code) { + return fault_code == kCommsLostFaultCode ? ClearOrigin::LinkState : ClearOrigin::DeviceAlarm; + } + + // Build the ClearFault request for one fault code. + // ``skip_correlation_auto_clear`` goes on the wire verbatim (see ClearOrigin + // for who sets it and why). Static so the wire field is assertable without a + // fault manager. + static ros2_medkit_msgs::srv::ClearFault::Request make_clear_fault_request(const std::string & fault_code, + bool skip_correlation_auto_clear); + + // One entry in the bounded buffer of fault dispatches held while the + // fault_manager service is unmatched. + struct PendingFaultDispatch { + enum class Kind { Report, Clear }; + Kind kind{Kind::Report}; + std::string fault_code; ///< dedup key for a Clear, diagnostic for a Report + /// Clear only: this dispatch is re-derivable (ClearOrigin::LinkState), so + /// the buffer may drop it before anything that is not. + bool link_state{false}; + std::function dispatch; + }; + + // What ``enqueue_pending_dispatch`` did, so the caller can log it. + enum class PendingEnqueueOutcome { + Buffered, ///< appended, nothing lost + ReplacedClear, ///< superseded the pending clear for the same fault code + EvictedLinkStateClear, ///< buffer was full: dropped a re-derivable clear to make room + EvictedOldest, ///< buffer was full with nothing re-derivable in it: dropped the oldest entry + Refused ///< buffer was full with nothing re-derivable in it and the incoming + ///< dispatch was itself a re-derivable clear, so it was dropped instead + }; + + // Enqueue policy for the bounded pending-dispatch buffer. + // + // Only a link-state clear is re-derivable: the next reconnect sends it again. + // Everything else in the buffer is a one-shot edge nothing will re-send - a + // report, a device alarm going inactive, an operator's scoped clear - so those + // rank together and age out oldest-first, exactly as the buffer behaved before + // any of this. A link-state clear is what a full buffer gives up first, and an + // incoming one is refused rather than pushing a one-shot dispatch out. At most + // ONE clear per fault code is pending at a time (a newer one moves to the + // back, so an interleaved report-then-clear still flushes in that order). + // + // Without the link-state ranking a flapping link enqueued one connect-time + // clear per reconnect attempt and pushed real alarm reports out of the buffer. + // Without the "only link-state" part, a device alarm's inactive edge was + // evicted ahead of an older report and the flush replayed the raise with no + // clear behind it, leaving the fault standing while the device said inactive. + static PendingEnqueueOutcome enqueue_pending_dispatch(std::vector & buffer, size_t max_size, + PendingFaultDispatch entry); + + // Bound on the pending-dispatch buffer, so a deployment with no fault_manager + // cannot grow it without limit. + static constexpr size_t kMaxPendingDispatches = 256; + private: // Route handlers void handle_plc_data(const ros2_medkit_gateway::PluginRequest & req, ros2_medkit_gateway::PluginResponse & res); @@ -158,11 +376,21 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // Report/clear fault via ROS 2 service (private helpers, not the FaultProvider overrides) void send_report_fault(const std::string & entity_id, const std::string & fault_code, const std::string & severity_str, const std::string & message); - void send_clear_fault(const std::string & fault_code); + // ``origin`` says why the clear is being sent, which decides both the wire + // flag and how the pending buffer ranks it. See ClearOrigin. + void send_clear_fault(const std::string & fault_code, ClearOrigin origin = ClearOrigin::DeviceAlarm); + + // Clear PLC_COMMS_LOST after the initial connect in set_context() succeeded. + // Unconditional on purpose: the fault manager keys faults by fault_code and + // persists them, so a comms-lost fault raised before a gateway restart is + // still standing in the store while this process has no memory of raising it. + // The poller's own reconnect clear can never reach that case, because a + // successful first connect means the reconnect arm is never entered. + void clear_comms_lost_on_connect(); // Dispatch now if the fault_manager service is matched, else buffer the // dispatch (bounded, order-preserving) to be flushed once it appears. - void send_or_buffer(std::function dispatch); + void send_or_buffer(PendingFaultDispatch entry); // Flush buffered fault dispatches when the fault_manager service is ready. void flush_pending_reports(); @@ -185,6 +413,16 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // node_map_mutex_. No-op (never called) when auto_browse is disabled. void run_auto_browse(); + // Poll-thread hook (from publish_values): re-derive the SOVD component + // identity from the device once a NEW session is up, in config-less mode only + // (an explicit node map owns the name). This is what stops a gateway that + // started before its PLC from serving the ``opcua-`` + // placeholder for the life of the process after discovery adopted the real + // server. Logs the change at INFO and rebuilds every derived reference (the + // ``_alarms`` entity, entity_defs) under the node-map lock. + // No-op when the identity is unchanged. + void maybe_rederive_component_identity(); + // Poll-thread hook (from publish_values): re-run auto_browse when the client // has established a new session since the last walk. Covers the field case // where the gateway starts before the PLC is reachable (initial connect @@ -206,6 +444,25 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // an endpoint is already configured. void run_startup_discovery(); + // Log sinks for a discovery pass: the plugin's operator-visible info/warn + // plus the named ``opcua.plugin`` debug logger a repeated identical pass falls + // back to. ``previous_outcome`` is the caller's repeat-suppression memory + // (null to report every pass in full). + DiscoveryReporter discovery_reporter(std::string * previous_outcome) const; + + // Abort predicate handed to a discovery sweep: reads the two stop signals off + // the process and applies ``discovery_cancelled_for``, which holds the rule + // and the reasoning behind it. + bool discovery_cancelled() const; + + // Poll-thread hook bound into PollerConfig::rediscover_endpoint whenever + // discovery runs without a configured endpoint. Called from the poller's + // reconnect arm, so only while no session is up, and rate-limited to one scan + // per effective_rescan_interval_s(). Returns the newly selected endpoint when + // a rescan found a different server (and logs the swap at INFO), nullopt when + // the rescan is not due yet or changed nothing. + std::optional rescan_endpoint_for_reconnect(); + // Build JSON response for data endpoint nlohmann::json build_data_response(const std::string & entity_id) const; @@ -231,6 +488,15 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // a network. Set in configure(), consumed in run_startup_discovery(). PortScanFn discovery_scan_fn_; IdentifyFn discovery_identify_fn_; + // When the last discovery pass FINISHED, so the reconnect rescan honours the + // cadence instead of sweeping the subnet on every reconnect attempt. Stamped + // by the startup scan, then only ever read/written on the poll thread. See + // rescan_step for why the end of the sweep is the reference point. + std::chrono::steady_clock::time_point last_discovery_scan_end_{}; + // Outcome digest of the previous discovery pass, so an unchanged rescan + // reports at DEBUG instead of repeating the whole report every interval_s. + // Poll thread only (the startup scan runs before the poller exists). + std::string last_discovery_outcome_; std::unique_ptr client_; NodeMap node_map_; @@ -259,6 +525,13 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, AssetIdentity device_identity_; uint64_t device_identity_generation_{0}; + // OpcuaClient::connection_generation the config-less component identity was + // derived at (0 = derived with no session, i.e. from the fallback endpoint and + // an empty DeviceInfo). The poll thread re-derives when the live generation + // differs, mirroring device_identity_generation_. Written on the set_context + // thread (happens-before the poller starts) then only on the poll thread. + uint64_t component_identity_generation_{0}; + // ROS 2 service clients for fault reporting struct FaultClients; std::unique_ptr fault_clients_; @@ -278,7 +551,7 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // never held across the actual dispatch (async_send_request) to keep ROS I/O out // of the critical section. std::mutex pending_reports_mutex_; - std::vector> pending_reports_; + std::vector pending_reports_; // Tracks which non-numeric nodes have already been warned about (avoids log spam). // Instance member instead of static to survive plugin reload (dlclose/dlopen). 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 711c3ec47..5e3c183fd 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 @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -102,6 +103,13 @@ struct PollerConfig { double subscription_interval_ms{500.0}; std::chrono::milliseconds poll_interval{1000}; std::chrono::milliseconds reconnect_interval{5000}; + /// Ceiling for the exponential reconnect backoff (it doubles from + /// ``reconnect_interval`` up to this). A plugin whose reconnect arm also + /// rescans for a new endpoint lowers this to the rescan cadence: the rescan is + /// consulted once per reconnect attempt, so a backoff longer than the cadence + /// would silently stretch the documented "re-scan every interval_s" to the + /// backoff instead (see OpcuaPlugin::effective_max_reconnect_wait). + std::chrono::milliseconds max_reconnect_interval{60000}; /// Active-condition replay strategy on (re)subscribe (issue #389). /// Default Auto: ConditionRefresh with a read-based fallback so hardened /// servers that reject the method still recover their active alarms. @@ -133,8 +141,21 @@ struct PollerConfig { /// 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; + /// Optional endpoint rediscovery, bound by a plugin running config-less + /// network discovery with no endpoint configured. Called from the reconnect + /// arm - so only while no session is up - and expected to rate-limit itself. + /// Returns a new endpoint URL to reconnect against, nullopt to keep the + /// current one. Without it the reconnect loop retries the same endpoint + /// forever, which strands a gateway that scanned before its PLC had booted. + std::function()> rediscover_endpoint; }; +/// Fault code of the component-scoped OPC-UA connection fault the poller raises +/// on a sustained outage and clears on the next successful connect (issue #496). +/// Named here so the plugin can clear the same code from its own connect path +/// without keeping a second copy of the literal. +inline constexpr const char * kCommsLostFaultCode = "PLC_COMMS_LOST"; + /// Manages OPC-UA data collection via subscriptions (preferred) or polling class OpcuaPoller { public: @@ -237,6 +258,24 @@ class OpcuaPoller { std::chrono::steady_clock::time_point down_since, std::chrono::steady_clock::time_point now, std::chrono::milliseconds debounce); + /// Endpoint the next reconnect attempt should target. Asks + /// ``rediscover_endpoint`` (when bound) for a freshly discovered server and + /// returns it only when it names a DIFFERENT endpoint than ``current``. + /// nullopt means "keep the current one", which is also the answer when no + /// callback is bound, when the callback declines, or when it hands back an + /// empty string. Pure and static (the callback is injected) so the adoption + /// rule is unit-testable without a network. + static std::optional + adopt_rediscovered_endpoint(const std::string & current, + const std::function()> & rediscover); + + /// Wait before the next reconnect attempt: the current wait doubled, clamped + /// to ``max_wait`` (a wait already above the cap comes back down to it). Pure + /// and static so the backoff - and the cap that keeps a rescanning reconnect + /// loop on its documented cadence - is unit-testable. + static std::chrono::milliseconds next_reconnect_wait(std::chrono::milliseconds current, + std::chrono::milliseconds max_wait); + /// Zero-config native A&C (``auto_alarms``): the alarm sources that should /// actually be subscribed / replayed, i.e. every explicit ``event_alarms`` /// entry plus (when ``auto_cfg.enabled`` and no explicit entry already @@ -430,7 +469,10 @@ class OpcuaPoller { // 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. + // one-shot RAISE only. The clear is deliberately not guarded by it: it is sent + // on every successful reconnect, because the fault manager persists faults by + // fault_code and a comms-lost fault raised before a restart is standing in the + // store with nothing in this process's memory to remember it. std::optional comms_down_since_; bool comms_lost_raised_{false}; diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/network_discovery.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/network_discovery.cpp index 40f7dd2e5..f3eaceea7 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/network_discovery.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/network_discovery.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -71,8 +72,12 @@ bool ipv4_less(const std::string & a, const std::string & b) { // ``max_workers`` threads (never more than ``count``) that pull indices off a // shared atomic cursor. The single primitive backs both the connect sweep and // the GetEndpoints identify so they share the same concurrency bound. +// +// ``cancelled`` is polled by every worker before it takes the next index, so an +// abort stops the fan-out after at most one more probe per worker instead of +// running the remaining tens of thousands. An empty predicate never cancels. template -void parallel_for(size_t count, int max_workers, Body && body) { +void parallel_for(size_t count, int max_workers, const std::function & cancelled, Body && body) { if (count == 0) { return; } @@ -80,6 +85,9 @@ void parallel_for(size_t count, int max_workers, Body && body) { std::atomic next{0}; const auto worker = [&]() { for (;;) { + if (cancelled && cancelled()) { + return; + } const size_t i = next.fetch_add(1); if (i >= count) { return; @@ -306,9 +314,12 @@ OpcuaDiscoveryConfig parse_discovery_config(const nlohmann::json & j, if (j.contains("interval_s") && j["interval_s"].is_number_integer()) { const int v = j["interval_s"].get(); if (v >= 0) { + // An explicit 0 is kept as an explicit 0: it means "keep discovery on but + // never re-scan", which an unset interval (the built-in cadence) cannot + // express. cfg.interval_s = v; } else { - warn_fn("discovery: interval_s must be >= 0 - keeping default (0 = one-shot)"); + warn_fn("discovery: interval_s must be >= 0 (0 disables re-scanning) - keeping the default cadence"); } } if (j.contains("anonymous_none_only") && j["anonymous_none_only"].is_boolean()) { @@ -333,7 +344,7 @@ std::vector NetworkDiscovery::resolve_subnets() const { return {local}; } -std::vector NetworkDiscovery::run() { +std::vector NetworkDiscovery::run(const std::function & cancelled) { // "passive" has no active scan implementation yet (mDNS / LDS FindServers // are a documented follow-up) - a no-op stub rather than silently running // the active scan mode wasn't asked for. parse_discovery_config() already @@ -365,7 +376,7 @@ std::vector NetworkDiscovery::run() { // only the polite fan-out lives here. std::vector open_hits; std::mutex hits_mu; - parallel_for(targets.size(), cfg_.scan_concurrency, [&](size_t i) { + parallel_for(targets.size(), cfg_.scan_concurrency, cancelled, [&](size_t i) { const Target & t = targets[i]; if (scan_(t.ip, t.port, cfg_.connect_timeout_ms)) { std::lock_guard lk(hits_mu); @@ -373,6 +384,12 @@ std::vector NetworkDiscovery::run() { } }); + // Cancelled mid-sweep: the hit list is partial, so do not spend an identify + // round-trip per hit on a pass whose caller is shutting down. + if (cancelled && cancelled()) { + return {}; + } + // Deterministic identify order (numerically lowest ip:port first). std::sort(open_hits.begin(), open_hits.end(), [](const Target & a, const Target & b) { if (a.ip != b.ip) { @@ -388,7 +405,7 @@ std::vector NetworkDiscovery::run() { // sweep, writing each result by index so the deterministic ip:port order // (open_hits was sorted above) survives regardless of completion order. std::vector built(open_hits.size()); - parallel_for(open_hits.size(), cfg_.scan_concurrency, [&](size_t i) { + parallel_for(open_hits.size(), cfg_.scan_concurrency, cancelled, [&](size_t i) { const Target & t = open_hits[i]; DiscoveredEndpoint ep; ep.ip = t.ip; @@ -415,6 +432,12 @@ std::vector NetworkDiscovery::run() { built[i] = std::move(ep); }); + // Cancelled during the identify phase: ``built`` holds default-constructed + // (empty) entries for the hits no worker reached, which is not a result set. + if (cancelled && cancelled()) { + return {}; + } + // 4. Dedup by ApplicationUri (fallback ip:port), sequentially over the // deterministic order so the lowest ip:port always wins. std::vector ordered; 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 aebb164f9..bbdfc11d8 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 @@ -585,6 +585,7 @@ void OpcuaPlugin::set_context(PluginContext & context) { const bool connected = client_->connect(client_config_); if (connected) { log_info("Connected to OPC-UA server: " + client_config_.endpoint_url); + clear_comms_lost_on_connect(); } else { log_warn("Failed to connect to OPC-UA server: " + client_config_.endpoint_url); } @@ -603,6 +604,13 @@ void OpcuaPlugin::set_context(PluginContext & context) { node_map_.set_component_identity(ci.id, ci.name); log_info("Component identity derived from device: id='" + ci.id + "', name='" + ci.name + "'"); } + // Which session this identity speaks for. 0 when the connect failed: the id + // above then came from the fallback endpoint and an empty DeviceInfo, so it + // is provisional and the poll thread re-derives it on the first session + // (maybe_rederive_component_identity). Without that a gateway which started + // before its PLC would serve the placeholder for the life of the process, + // even after discovery adopted the real server. + component_identity_generation_ = connected ? client_->connection_generation() : 0; // Zero-config native A&C: with no node map and no explicit auto_alarms // block, subscribe the Server EventNotifier by default so discovered @@ -660,6 +668,22 @@ void OpcuaPlugin::set_context(PluginContext & context) { poller_config_.report_sink_ready = [this]() { return fault_clients_->report && fault_clients_->report->service_is_ready(); }; + // Config-less discovery with no configured endpoint: let the poller's + // reconnect arm ask for a fresh scan while it is down. Without this the + // startup scan is the only one that ever runs, so a gateway that scanned + // while its PLC was still booting retries the fallback endpoint forever and + // only a restart finds the PLC. + const int rescan_interval_s = effective_rescan_interval_s(discovery_config_, endpoint_configured_); + if (rescan_interval_s > 0) { + poller_config_.rediscover_endpoint = [this]() { + return rescan_endpoint_for_reconnect(); + }; + // The rescan is consulted once per reconnect attempt, so the real adoption + // cadence is max(interval_s, backoff). Cap the backoff at the cadence so + // the documented "re-scan every interval_s while down" is the true one. + poller_config_.max_reconnect_interval = effective_max_reconnect_wait( + poller_config_.reconnect_interval, poller_config_.max_reconnect_interval, rescan_interval_s); + } poller_->start(poller_config_); log_info("OPC-UA poller started (mode: " + std::string(poller_->using_subscriptions() ? "subscription" : "poll") + ")"); @@ -733,7 +757,11 @@ IntrospectionResult OpcuaPlugin::introspect(const IntrospectionInput & /*input*/ // Fault scope grants bare-id ownership only to external entities; the poller // reports PLC_COMMS_LOST under this component's own id. comp.external = true; - comp.description = "PLC runtime connected at " + client_config_.endpoint_url; + // Read the endpoint off the client, not off client_config_: a reconnect + // rescan can adopt a different server after startup, and the client is the + // one that holds the endpoint actually being connected to. + const std::string live_endpoint = client_ ? client_->endpoint_url() : client_config_.endpoint_url; + comp.description = "PLC runtime connected at " + live_endpoint; // INV2: fill the asset-identity nameplate from the live server's device-info // (ServerStatus/BuildInfo + optional OPC-UA DI nameplate). Read once per @@ -744,7 +772,7 @@ IntrospectionResult OpcuaPlugin::introspect(const IntrospectionInput & /*input*/ if (client_ && client_->is_connected()) { const uint64_t session_generation = client_->connection_generation(); if (session_generation != device_identity_generation_) { - device_identity_ = opcua_device_info_to_identity(client_->read_device_info(), client_config_.endpoint_url); + device_identity_ = opcua_device_info_to_identity(client_->read_device_info(), live_endpoint); device_identity_generation_ = session_generation; if (!device_identity_.empty()) { log_info("Populated asset identity from OPC-UA device-info (manufacturer='" + device_identity_.manufacturer + @@ -1020,7 +1048,11 @@ void OpcuaPlugin::on_alarm_change(const std::string & entity_id, send_report_fault(entity_id, signal.fault_code, signal.severity, signal.message); } else { log_info("Alarm cleared: " + signal.fault_code + " on " + entity_id); - send_clear_fault(signal.fault_code); + // The poller's own comms-lost clear on a successful reconnect is the same + // link-state event as the connect-time one. Every other code here is the + // device reporting its condition inactive, which is a real resolution and a + // one-shot edge, so it keeps the cascade and the buffer treats it as such. + send_clear_fault(signal.fault_code, clear_origin_for_signal(signal.fault_code)); } } @@ -1199,7 +1231,11 @@ void OpcuaPlugin::on_event_alarm(const AlarmEventDelivery & delivery) { break; case AlarmAction::ClearFault: log_info("AlarmCondition CLEARED: " + delivery.fault_code); - send_clear_fault(delivery.fault_code); + // The device itself reported the condition cleared (Part 9 lifecycle), so + // this IS a resolution at the source and the correlation engine may act on + // it. DeviceAlarm is also the default, spelled out here because this is + // the one call site where the cascade is deliberately kept. + send_clear_fault(delivery.fault_code, ClearOrigin::DeviceAlarm); break; case AlarmAction::NoOp: break; @@ -1234,42 +1270,111 @@ void OpcuaPlugin::send_report_fault(const std::string & entity_id, const std::st request->severity = ros2_medkit_msgs::msg::Fault::SEVERITY_INFO; } - send_or_buffer([this, request]() { - fault_clients_->report->async_send_request(request); - }); + send_or_buffer({PendingFaultDispatch::Kind::Report, fault_code, /*link_state=*/false, [this, request]() { + fault_clients_->report->async_send_request(request); + }}); } -void OpcuaPlugin::send_clear_fault(const std::string & fault_code) { +ros2_medkit_msgs::srv::ClearFault::Request OpcuaPlugin::make_clear_fault_request(const std::string & fault_code, + bool skip_correlation_auto_clear) { + ros2_medkit_msgs::srv::ClearFault::Request request; + request.fault_code = fault_code; + request.skip_correlation_auto_clear = skip_correlation_auto_clear; + return request; +} + +void OpcuaPlugin::send_clear_fault(const std::string & fault_code, ClearOrigin origin) { if (!fault_clients_->clear) { log_warn("ClearFault service client not available"); return; } - auto request = std::make_shared(); - request->fault_code = fault_code; + auto request = std::make_shared( + make_clear_fault_request(fault_code, clear_skips_correlation(origin))); - send_or_buffer([this, request]() { - fault_clients_->clear->async_send_request(request); - }); + send_or_buffer({PendingFaultDispatch::Kind::Clear, fault_code, clear_is_link_state(origin), [this, request]() { + fault_clients_->clear->async_send_request(request); + }}); } -void OpcuaPlugin::send_or_buffer(std::function dispatch) { +void OpcuaPlugin::clear_comms_lost_on_connect() { + if (!poller_config_.comms_lost_fault_enabled) { + return; + } + // ClearFault is idempotent from this side: send_clear_fault is + // fire-and-forget, so a "Fault not found" answer for a code that was never + // raised costs nothing here and is the normal case on a healthy start. + // + // LinkState: this says the session came back, not that an operator resolved + // anything, so a correlation rule naming PLC_COMMS_LOST as a root cause must + // not cascade-clear the symptoms the outage produced. It is also the one clear + // the next reconnect re-derives, so the pending buffer may drop it before + // anything one-shot. + log_info(std::string("OPC-UA connection established, clearing any standing ") + kCommsLostFaultCode); + send_clear_fault(kCommsLostFaultCode, ClearOrigin::LinkState); +} + +OpcuaPlugin::PendingEnqueueOutcome OpcuaPlugin::enqueue_pending_dispatch(std::vector & buffer, + size_t max_size, PendingFaultDispatch entry) { + const bool is_clear = entry.kind == PendingFaultDispatch::Kind::Clear; + + // At most one pending clear per fault code. A repeat moves to the BACK rather + // than overwriting in place, so an interleaved report-then-clear for the same + // code still flushes in the order the PLC produced it. + bool replaced = false; + if (is_clear) { + const auto same_code = std::find_if(buffer.begin(), buffer.end(), [&entry](const PendingFaultDispatch & pending) { + return pending.kind == PendingFaultDispatch::Kind::Clear && pending.fault_code == entry.fault_code; + }); + if (same_code != buffer.end()) { + buffer.erase(same_code); + replaced = true; + } + } + + PendingEnqueueOutcome outcome = replaced ? PendingEnqueueOutcome::ReplacedClear : PendingEnqueueOutcome::Buffered; + if (buffer.size() >= max_size) { + // Only a link-state clear is re-derivable: the next reconnect sends it + // again. A full buffer gives that up first, and refuses an incoming one + // rather than pushing out a dispatch nothing will re-send. Everything else - + // reports, a device alarm's inactive edge, an operator's scoped clear - is + // one-shot and ages out oldest-first. + const auto oldest_link_state = std::find_if(buffer.begin(), buffer.end(), [](const PendingFaultDispatch & pending) { + return pending.kind == PendingFaultDispatch::Kind::Clear && pending.link_state; + }); + if (oldest_link_state != buffer.end()) { + buffer.erase(oldest_link_state); + outcome = PendingEnqueueOutcome::EvictedLinkStateClear; + } else if (is_clear && entry.link_state) { + return PendingEnqueueOutcome::Refused; + } else { + buffer.erase(buffer.begin()); + outcome = PendingEnqueueOutcome::EvictedOldest; + } + } + + buffer.push_back(std::move(entry)); + return outcome; +} + +void OpcuaPlugin::send_or_buffer(PendingFaultDispatch entry) { // Bound the buffer so a deployment with no fault_manager cannot grow it - // without limit; drop the oldest (least relevant) pending dispatch. - // Runs on both the poll thread and the REST clear_fault thread, so the vector - // mutation is serialised by pending_reports_mutex_. - constexpr size_t kMaxPendingReports = 256; - bool dropped_oldest = false; + // without limit. Runs on both the poll thread and the REST clear_fault thread, + // so the vector mutation is serialised by pending_reports_mutex_. + PendingEnqueueOutcome outcome = PendingEnqueueOutcome::Buffered; { std::lock_guard lock(pending_reports_mutex_); - if (pending_reports_.size() >= kMaxPendingReports) { - pending_reports_.erase(pending_reports_.begin()); - dropped_oldest = true; - } - pending_reports_.push_back(std::move(dispatch)); - } - if (dropped_oldest) { - log_warn("pending fault report buffer full (" + std::to_string(kMaxPendingReports) + "), dropping oldest"); + outcome = enqueue_pending_dispatch(pending_reports_, kMaxPendingDispatches, std::move(entry)); + } + if (outcome == PendingEnqueueOutcome::EvictedOldest) { + log_warn("pending fault dispatch buffer full (" + std::to_string(kMaxPendingDispatches) + + "), dropping the oldest dispatch"); + } else if (outcome == PendingEnqueueOutcome::EvictedLinkStateClear) { + log_warn("pending fault dispatch buffer full (" + std::to_string(kMaxPendingDispatches) + + "), dropping a link-state clear the next reconnect re-derives"); + } else if (outcome == PendingEnqueueOutcome::Refused) { + log_warn("pending fault dispatch buffer full of one-shot dispatches (" + std::to_string(kMaxPendingDispatches) + + "), dropping this link-state clear instead"); } // Drains immediately (in order) if the sink is already matched. flush_pending_reports(); @@ -1285,7 +1390,7 @@ void OpcuaPlugin::flush_pending_reports() { // the vector is never being reallocated by a concurrent send_or_buffer while it // is iterated here (the use-after-free that corrupted the heap), and so the ROS // service call never runs under the mutex. - std::vector> batch; + std::vector batch; { std::lock_guard lock(pending_reports_mutex_); if (pending_reports_.empty()) { @@ -1293,8 +1398,8 @@ void OpcuaPlugin::flush_pending_reports() { } batch.swap(pending_reports_); } - for (auto & dispatch : batch) { - dispatch(); + for (auto & entry : batch) { + entry.dispatch(); } } @@ -1354,6 +1459,56 @@ void OpcuaPlugin::run_auto_browse() { (result.depth_cap_hit ? " [depth cap reached on at least one branch]" : "")); } +std::optional OpcuaPlugin::rederived_component_identity(const std::string & current_id, + const OpcuaClient::DeviceInfo & info, + const std::string & endpoint_url) { + const ComponentIdentity ci = derive_component_identity(info, endpoint_url); + if (ci.id.empty() || ci.id == current_id) { + return std::nullopt; + } + return ci; +} + +void OpcuaPlugin::maybe_rederive_component_identity() { + // An explicit node map owns the component name. Only the config-less path + // derives it from the device. + if (!node_map_path_.empty() || !client_ || !client_->is_connected()) { + return; + } + const uint64_t generation = client_->connection_generation(); + if (generation == component_identity_generation_) { + return; // identity already speaks for this session + } + + const std::string live_endpoint = client_->endpoint_url(); + const auto rederived = + rederived_component_identity(node_map_.component_id(), client_->read_device_info(), live_endpoint); + component_identity_generation_ = generation; + if (!rederived) { + return; + } + + const std::string previous_id = node_map_.component_id(); + { + // Serialize against the REST read paths, which hold references into + // node_map_ (entity_defs) while they answer. + std::unique_lock lock(node_map_mutex_); + // The auto_alarms fallback entity is derived from the component id, so a + // default-derived one has to follow the rename. An operator-chosen entity_id + // does not match the derived form and is left alone. + auto & auto_alarms = node_map_.mutable_auto_alarms(); + if (auto_alarms.entity_id == previous_id + "_alarms") { + auto_alarms.entity_id.clear(); + } + node_map_.set_component_identity(rederived->id, rederived->name); + // Re-derives the fallback entity id and rebuilds entity_defs, so every + // reference to the component id moves together. + node_map_.finalize_auto_alarms_overlay(); + } + log_info("Component identity re-derived from the adopted device at " + live_endpoint + ": id='" + rederived->id + + "', name='" + rederived->name + "' (was '" + previous_id + "')"); +} + void OpcuaPlugin::maybe_rebrowse_on_reconnect() { if (!node_map_.auto_browse_config().enabled || !client_ || !client_->is_connected()) { return; @@ -1372,6 +1527,11 @@ void OpcuaPlugin::publish_values(const PollSnapshot & snap) { // Poll-thread hook: drain any fault reports buffered before fault_manager was // discovered, so a late sink still receives them. flush_pending_reports(); + // Poll-thread hook: re-derive the config-less component identity from the + // device once a session is up, so an adopted PLC stops being served under the + // provisional endpoint-derived id. Runs BEFORE the re-walk below, which + // rebuilds entity_defs off the component id. + maybe_rederive_component_identity(); // Poll-thread hook: (re)run auto_browse after a fresh session so a PLC that // came up (or restarted) after the initial connect still gets walked. maybe_rebrowse_on_reconnect(); @@ -1466,37 +1626,124 @@ void OpcuaPlugin::log_security_profile() const { } } -void OpcuaPlugin::run_startup_discovery() { - if (!discovery_config_.enabled) { - return; +int OpcuaPlugin::effective_rescan_interval_s(const OpcuaDiscoveryConfig & config, bool endpoint_configured) { + if (!config.enabled || endpoint_configured) { + return 0; } - // Never override an explicitly configured endpoint: discovery must not open a - // second session on a PLC the operator already targets (and already polls). - if (endpoint_configured_) { - log_info("OPC-UA discovery enabled but endpoint_url is explicitly configured (" + client_config_.endpoint_url + - "); skipping auto-discovery to avoid a second session."); - return; + // Unset means "discovery is on but no cadence was stated" - the config-less + // deployment, which takes the built-in default. An explicit 0 is an operator + // saying "do not re-scan", and is honoured as written. + return config.interval_s.value_or(kDefaultRescanIntervalS); +} + +std::chrono::milliseconds OpcuaPlugin::effective_max_reconnect_wait(std::chrono::milliseconds base, + std::chrono::milliseconds default_ceiling, + int rescan_interval_s) { + if (rescan_interval_s <= 0) { + return default_ceiling; // no rescan: the plain backoff ceiling applies } - if (discovery_config_.interval_s > 0) { - log_warn("OPC-UA discovery interval_s=" + std::to_string(discovery_config_.interval_s) + - " set, but periodic re-scan is not implemented yet; running a one-shot scan at startup."); + const auto cadence = std::chrono::milliseconds(static_cast(rescan_interval_s) * 1000); + return std::max(base, std::min(default_ceiling, cadence)); +} + +std::optional OpcuaPlugin::rescan_step(int interval_s, + const std::function & now, + std::chrono::steady_clock::time_point * last_scan_end, + const std::function()> & sweep) { + if (interval_s <= 0 || !now || last_scan_end == nullptr || !sweep) { + return std::nullopt; + } + if (now() - *last_scan_end < std::chrono::seconds(interval_s)) { + return std::nullopt; + } + // Stamp the END of the sweep: a legal /16 runs for minutes, and stamping its + // start would make the next one due the moment this one returned - the poll + // thread would sweep back to back and only attempt a reconnect once a sweep. + // A sweep that threw still consumed that time, so the stamp is owed either + // way, or the next poll iteration would start another one immediately. + try { + const auto result = sweep(); + *last_scan_end = now(); + return result; + } catch (...) { + *last_scan_end = now(); + throw; } +} - NetworkDiscovery discovery(discovery_config_, discovery_scan_fn_, discovery_identify_fn_); +std::optional OpcuaPlugin::discover_endpoint(const OpcuaDiscoveryConfig & config, bool endpoint_configured, + const PortScanFn & scan, const IdentifyFn & identify, + const DiscoveryReporter & reporter, + const std::function & cancelled) { + if (!config.enabled) { + return std::nullopt; + } + // Never override an explicitly configured endpoint: discovery must not open a + // second session on a PLC the operator already targets (and already polls). + if (endpoint_configured) { + return std::nullopt; + } + + // The pass reports through a buffer rather than straight to the log: whether + // the report is operator-visible or a DEBUG trace depends on the outcome, + // which is only known once the sweep is done. A rescan runs for the life of a + // disconnected process, so an unchanged outcome must not repeat its whole + // report (a secured-only site would log the same WARN every interval_s). + struct ReportLine { + bool warning; + std::string text; + }; + std::vector report; + std::string outcome; + const auto info_line = [&report, &outcome](const std::string & text) { + report.push_back({false, text}); + outcome += text; + outcome += '\n'; + }; + const auto warn_line = [&report, &outcome](const std::string & text) { + report.push_back({true, text}); + outcome += text; + outcome += '\n'; + }; + const auto emit = [&report, &outcome, &reporter]() { + const bool repeat = reporter.previous_outcome != nullptr && *reporter.previous_outcome == outcome; + if (reporter.previous_outcome != nullptr) { + *reporter.previous_outcome = outcome; + } + for (const auto & line : report) { + const auto & sink = repeat ? reporter.debug : (line.warning ? reporter.warn : reporter.info); + if (sink) { + sink(line.text); + } + } + }; + + NetworkDiscovery discovery(config, scan, identify); const auto subnets = discovery.resolve_subnets(); if (subnets.empty()) { - log_warn("OPC-UA discovery: no subnet configured and could not derive a local /24; nothing to scan."); - return; + warn_line("OPC-UA discovery: no subnet configured and could not derive a local /24, nothing to scan."); + emit(); + return std::nullopt; } std::string subnet_list; for (const auto & s : subnets) { subnet_list += (subnet_list.empty() ? "" : ", ") + s; } - log_info("OPC-UA discovery: read-only active scan of [" + subnet_list + "] on " + - std::to_string(discovery_config_.ports.size()) + " port(s)..."); + // The announcement goes out NOW, not through the buffered report: a sweep of a + // wide subnet runs for minutes, and an operator watching start-up has to see + // that the gateway is scanning rather than hung. It says what the pass is + // about to do, not what it found, so it stays out of the outcome digest and is + // levelled on its own - the first pass announces at INFO, a rescan at DEBUG so + // the recurring sweep does not repeat it every interval. + const bool first_pass = reporter.previous_outcome == nullptr || reporter.previous_outcome->empty(); + const auto & announce_sink = first_pass ? reporter.info : reporter.debug; + if (announce_sink) { + announce_sink("OPC-UA discovery: read-only active scan of [" + subnet_list + "] on " + + std::to_string(config.ports.size()) + " port(s)..."); + } - const std::vector found = discovery.run(); + const std::vector found = discovery.run(cancelled); // Summarize what was found and what was skipped (leads, LDS, secured-only). size_t data_servers = 0; @@ -1520,26 +1767,131 @@ void OpcuaPlugin::run_startup_discovery() { if (!ep.anonymous_none_available) { ++secured_only; } - log_info("OPC-UA discovery: found data server " + ep.endpoint_url + " (uri='" + ep.application_uri + - "', product='" + ep.product_uri + "', None/Anonymous=" + (ep.anonymous_none_available ? "yes" : "no") + - ")"); + info_line("OPC-UA discovery: found data server " + ep.endpoint_url + " (uri='" + ep.application_uri + + "', product='" + ep.product_uri + "', None/Anonymous=" + (ep.anonymous_none_available ? "yes" : "no") + + ")"); } - log_info("OPC-UA discovery summary: " + std::to_string(data_servers) + " data server(s), " + - std::to_string(discovery_servers) + " discovery server(s)/LDS, " + std::to_string(secured_only) + - " secured-only (need credentials), " + std::to_string(leads) + " non-OPC-UA/unidentified lead(s)."); + info_line("OPC-UA discovery summary: " + std::to_string(data_servers) + " data server(s), " + + std::to_string(discovery_servers) + " discovery server(s)/LDS, " + std::to_string(secured_only) + + " secured-only (need credentials), " + std::to_string(leads) + " non-OPC-UA/unidentified lead(s)."); - const DiscoveredEndpoint * chosen = - NetworkDiscovery::select_auto_endpoint(found, discovery_config_.anonymous_none_only); + const DiscoveredEndpoint * chosen = NetworkDiscovery::select_auto_endpoint(found, config.anonymous_none_only); if (chosen == nullptr) { - log_warn( - "OPC-UA discovery: no auto-connectable None/Anonymous data server found; leaving endpoint at default. " + warn_line( + "OPC-UA discovery: no auto-connectable None/Anonymous data server found, leaving the endpoint unchanged. " "Secured-only servers require operator credentials."); + emit(); + return std::nullopt; + } + + info_line("OPC-UA discovery: selected endpoint " + chosen->endpoint_url + " (uri='" + chosen->application_uri + "')"); + emit(); + return chosen->endpoint_url; +} + +void OpcuaPlugin::run_startup_discovery() { + if (!discovery_config_.enabled) { return; } + if (endpoint_configured_) { + log_info("OPC-UA discovery enabled but endpoint_url is explicitly configured (" + client_config_.endpoint_url + + "). Skipping auto-discovery to avoid a second session."); + return; + } + + // The startup scan is the first pass, so its outcome digest is still empty and + // the report comes out in full. It is cancellable too, but not by shutdown(): + // this runs inside set_context(), i.e. during node construction and before the + // executor spins, so nothing can call shutdown() until this returns. What ends + // it is the SIGINT / SIGTERM that rclcpp's own handler turns into + // !rclcpp::ok() - see discovery_cancelled(). + const auto chosen = discover_endpoint(discovery_config_, endpoint_configured_, discovery_scan_fn_, + discovery_identify_fn_, discovery_reporter(&last_discovery_outcome_), [this]() { + return discovery_cancelled(); + }); + // Stamp when the sweep FINISHED: the rescan cadence is measured from the end + // of the previous sweep, so a long sweep is not immediately followed by + // another one. + last_discovery_scan_end_ = std::chrono::steady_clock::now(); + + if (!chosen) { + // The startup scan can legitimately find nothing - a gateway that boots + // alongside its PLC routinely scans while the PLC is still coming up. With a + // cadence the endpoint stays at its default and the reconnect arm rescans, + // so this is a delay rather than a dead end. With re-scanning switched off + // (an explicit interval_s: 0) it IS the end, and the operator has to be told + // which of the two they configured. + const int startup_interval_s = effective_rescan_interval_s(discovery_config_, endpoint_configured_); + if (startup_interval_s > 0) { + log_info("OPC-UA discovery: startup scan selected no endpoint. The reconnect loop rescans every " + + std::to_string(startup_interval_s) + "s while down."); + } else { + log_warn( + "OPC-UA discovery: startup scan selected no endpoint and re-scanning is off (interval_s: 0). The endpoint " + "stays at " + + client_config_.endpoint_url + " until the plugin is restarted."); + } + return; + } + + client_config_.endpoint_url = *chosen; + log_info("OPC-UA discovery: auto-selected endpoint " + *chosen + " - handing to the connect + introspect path."); +} + +bool OpcuaPlugin::discovery_cancelled() const { + // rclcpp::ok() only reads the default context's atomic shutdown flag, so it is + // safe to call from the set_context thread and the poll thread alike. The rule + // itself, and why both signals are needed, lives in discovery_cancelled_for. + return discovery_cancelled_for(shutdown_requested_.load(), rclcpp::ok()); +} + +OpcuaPlugin::DiscoveryReporter OpcuaPlugin::discovery_reporter(std::string * previous_outcome) const { + DiscoveryReporter reporter; + reporter.info = [this](const std::string & m) { + log_info(m); + }; + reporter.warn = [this](const std::string & m) { + log_warn(m); + }; + reporter.debug = [](const std::string & m) { + RCLCPP_DEBUG(opcua_plugin_logger(), "%s", m.c_str()); + }; + reporter.previous_outcome = previous_outcome; + return reporter; +} + +std::optional OpcuaPlugin::rescan_endpoint_for_reconnect() { + // A sweep is a bounded but multi-second blocking call on the poll thread, and + // stop() has to wait for whatever it is in the middle of. Do not start one the + // shutdown is going to throw away. + if (discovery_cancelled()) { + return std::nullopt; + } + const int interval_s = effective_rescan_interval_s(discovery_config_, endpoint_configured_); + + const auto chosen = rescan_step( + interval_s, + []() { + return std::chrono::steady_clock::now(); + }, + &last_discovery_scan_end_, + [this]() { + return discover_endpoint(discovery_config_, endpoint_configured_, discovery_scan_fn_, discovery_identify_fn_, + discovery_reporter(&last_discovery_outcome_), [this]() { + return discovery_cancelled(); + }); + }); + // The live client config, not client_config_: this runs on the poll thread + // and client_config_ is read by the refresh thread in introspect(). The + // client owns the endpoint once connect() has been called with it, and its + // accessors are mutex-guarded. + const std::string current = client_ ? client_->endpoint_url() : client_config_.endpoint_url; + if (!chosen || *chosen == current) { + return std::nullopt; + } - client_config_.endpoint_url = chosen->endpoint_url; - log_info("OPC-UA discovery: auto-selected endpoint " + chosen->endpoint_url + " (uri='" + chosen->application_uri + - "') - handing to the connect + introspect path."); + log_info("OPC-UA discovery: rescan while disconnected adopted endpoint " + *chosen + " (was " + current + ")"); + return chosen; } nlohmann::json OpcuaPlugin::build_data_response(const std::string & entity_id) const { @@ -2039,7 +2391,13 @@ tl::expected OpcuaPlugin::clear_f return tl::make_unexpected(FaultProviderErrorInfo{FaultProviderError::Internal, "plugin not initialized", 503}); } - send_clear_fault(fault_code); + // This is the per-entity SOVD route DELETE /{entity}/faults/{code}. The + // gateway sets skip_correlation_auto_clear on its own branch of that route so + // an operator scoped to one entity cannot cascade-clear symptoms reported by + // apps in other entities, and a plugin-owned entity must not be the hole in + // that rule: the request goes through this provider instead, so the same flag + // has to be set here. + send_clear_fault(fault_code, ClearOrigin::ScopedOperator); return dto::FaultClearResult{ nlohmann::json{{"status", "cleared"}, {"fault_code", fault_code}, {"entity_id", entity_id}}}; } 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 c24f9ca0b..019d0604c 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 @@ -1142,9 +1142,27 @@ bool OpcuaPoller::comms_lost_should_raise(bool enabled, bool already_raised, return (now - down_since) >= debounce; } +std::optional +OpcuaPoller::adopt_rediscovered_endpoint(const std::string & current, + const std::function()> & rediscover) { + if (!rediscover) { + return std::nullopt; + } + const std::optional found = rediscover(); + if (!found || found->empty() || *found == current) { + return std::nullopt; + } + return found; +} + +std::chrono::milliseconds OpcuaPoller::next_reconnect_wait(std::chrono::milliseconds current, + std::chrono::milliseconds max_wait) { + return std::min(current * 2, max_wait); +} + void OpcuaPoller::emit_comms_lost(bool active) { ros2_medkit::fault_detection::FaultSignal signal; - signal.fault_code = "PLC_COMMS_LOST"; + signal.fault_code = kCommsLostFaultCode; 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()); @@ -1157,7 +1175,6 @@ void OpcuaPoller::emit_comms_lost(bool active) { void OpcuaPoller::poll_loop() { auto reconnect_wait = config_.reconnect_interval; - constexpr auto max_reconnect_wait = std::chrono::milliseconds(60000); while (running_.load()) { // Handle reconnection @@ -1173,15 +1190,32 @@ void OpcuaPoller::poll_loop() { comms_down_since_ = std::chrono::steady_clock::now(); } - // Attempt reconnect with original config (preserves timeout, etc.) - if (client_.connect(client_.current_config())) { + // Reconnect with the original config (preserves timeout, security, ...). + // The endpoint is the one exception: when a rediscovery callback is bound + // and offers a different server, adopt it for this attempt. connect() + // stores the config it is given, so current_config() carries the adopted + // endpoint from here on and every later retry targets the new server. + OpcuaClientConfig reconnect_config = client_.current_config(); + if (auto adopted = adopt_rediscovered_endpoint(reconnect_config.endpoint_url, config_.rediscover_endpoint)) { + reconnect_config.endpoint_url = *adopted; + // A freshly discovered server deserves a prompt attempt: without this + // reset the backoff (up to 60 s) would keep the newly found PLC waiting + // for as long as the old dead endpoint had earned. + reconnect_wait = config_.reconnect_interval; + } + + if (client_.connect(reconnect_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_) { + // Issue #496: connection restored - clear the comms-lost fault. Sent on + // EVERY successful reconnect, not only when this process raised it: the + // fault manager keys faults by fault_code and persists them, so a fault + // raised before a restart is standing in the store with nothing in + // memory to remember it. The clear is fire-and-forget and the store + // answers "not found" harmlessly when there is nothing to clear. + if (config_.comms_lost_fault_enabled) { emit_comms_lost(/*active=*/false); - comms_lost_raised_ = false; } + comms_lost_raised_ = false; comms_down_since_.reset(); if (config_.prefer_subscriptions) { setup_subscriptions(); @@ -1214,14 +1248,16 @@ void OpcuaPoller::poll_loop() { comms_lost_raised_ = true; } } - // Exponential backoff capped at 60s. condition_variable so stop() wakes immediately. + // Exponential backoff, capped at config_.max_reconnect_interval (60 s by + // default, the rescan cadence while the reconnect arm also rescans). + // condition_variable so stop() wakes immediately. { std::unique_lock lock(stop_mutex_); stop_cv_.wait_for(lock, reconnect_wait, [this] { return !running_.load(); }); } - reconnect_wait = std::min(reconnect_wait * 2, max_reconnect_wait); + reconnect_wait = next_reconnect_wait(reconnect_wait, config_.max_reconnect_interval); continue; } } diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_network_discovery.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_network_discovery.cpp index 03643beb3..7ef7c0e4f 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_network_discovery.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_network_discovery.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -106,7 +107,32 @@ TEST(ParseDiscoveryConfig, DefaultsDisabled) { ASSERT_EQ(cfg.ports.size(), 1u); EXPECT_EQ(cfg.ports[0], 4840); EXPECT_TRUE(cfg.anonymous_none_only); - EXPECT_EQ(cfg.interval_s, 0); + // Unset, NOT 0: an absent key means "no cadence stated" (the caller takes its + // built-in default), while an explicit 0 means "never re-scan". + EXPECT_FALSE(cfg.interval_s.has_value()); +} + +TEST(ParseDiscoveryConfig, ExplicitZeroIntervalIsKeptAsAnExplicitZero) { + std::vector warnings; + const auto cfg = parse_discovery_config(nlohmann::json{{"interval_s", 0}}, [&](const std::string & m) { + warnings.push_back(m); + }); + ASSERT_TRUE(cfg.interval_s.has_value()); + EXPECT_EQ(*cfg.interval_s, 0); + EXPECT_TRUE(warnings.empty()); +} + +TEST(ParseDiscoveryConfig, NegativeIntervalWarnsAndLeavesTheCadenceUnset) { + std::vector warnings; + const auto cfg = parse_discovery_config(nlohmann::json{{"interval_s", -5}}, [&](const std::string & m) { + warnings.push_back(m); + }); + EXPECT_FALSE(cfg.interval_s.has_value()); + ASSERT_EQ(warnings.size(), 1u); + EXPECT_NE(warnings[0].find("interval_s"), std::string::npos); + // The warning must not tell the operator the kept default is one-shot: an + // unset interval re-scans on the built-in cadence, only an explicit 0 stops. + EXPECT_EQ(warnings[0].find("0 = one-shot"), std::string::npos) << warnings[0]; } TEST(ParseDiscoveryConfig, ReadsAllKnownKeys) { @@ -133,7 +159,8 @@ TEST(ParseDiscoveryConfig, ReadsAllKnownKeys) { EXPECT_EQ(cfg.connect_timeout_ms, 300); EXPECT_EQ(cfg.scan_concurrency, 64); EXPECT_EQ(cfg.identify_timeout_ms, 2000); - EXPECT_EQ(cfg.interval_s, 900); + ASSERT_TRUE(cfg.interval_s.has_value()); + EXPECT_EQ(*cfg.interval_s, 900); EXPECT_FALSE(cfg.anonymous_none_only); EXPECT_TRUE(warnings.empty()); } @@ -377,6 +404,91 @@ TEST(NetworkDiscoveryRun, IdentifyFailureRecordedAsLead) { EXPECT_EQ(NetworkDiscovery::select_auto_endpoint(eps, true), nullptr); } +// --------------------------------------------------------------------------- // +// run(cancelled): a shutdown must not wait out a whole sweep +// --------------------------------------------------------------------------- // +TEST(NetworkDiscoveryRun, CancelStopsTheSweepInsteadOfProbingEveryHost) { + // A /24 is 254 probes and a legal /16 is 65k, and the caller runs them on the + // poll thread a shutdown has to join. With scan_concurrency 1 the sweep is + // sequential, so the probe count is exactly what the cancel predicate allowed. + std::atomic probes{0}; + std::atomic stop{false}; + auto scan = [&probes, &stop](const std::string &, uint16_t, int) { + if (probes.fetch_add(1) + 1 >= 5) { + stop.store(true); // the shutdown flag flipping mid-sweep + } + return false; + }; + OpcuaDiscoveryConfig cfg; + cfg.enabled = true; + cfg.subnets = {"192.168.1.0/24"}; + cfg.ports = {4840}; + cfg.scan_concurrency = 1; + + NetworkDiscovery disc(cfg, scan, make_identify({})); + const auto eps = disc.run([&stop]() { + return stop.load(); + }); + + EXPECT_TRUE(eps.empty()); + // One in-flight probe per worker may still complete after the flag flips. + EXPECT_GE(probes.load(), 5); + EXPECT_LE(probes.load(), 6) << "the sweep kept probing after it was cancelled"; +} + +TEST(NetworkDiscoveryRun, WithoutCancellationEveryHostIsStillProbed) { + // Positive control for the test above on the same harness: the identical + // sweep with no cancel predicate visits all 254 hosts, so a low probe count + // there is the cancellation and not a broken fake. + std::atomic probes{0}; + auto scan = [&probes](const std::string &, uint16_t, int) { + probes.fetch_add(1); + return false; + }; + OpcuaDiscoveryConfig cfg; + cfg.enabled = true; + cfg.subnets = {"192.168.1.0/24"}; + cfg.ports = {4840}; + cfg.scan_concurrency = 1; + + NetworkDiscovery disc(cfg, scan, make_identify({})); + const auto eps = disc.run(); + EXPECT_TRUE(eps.empty()); + EXPECT_EQ(probes.load(), 254); +} + +TEST(NetworkDiscoveryRun, CancelBetweenSweepAndIdentifySkipsTheIdentifyRoundTrips) { + // The identify phase is a separate batch, and each GetEndpoints blocks for up + // to identify_timeout_ms. A cancel that arrives once the sweep is done must + // not still pay for one round-trip per hit. + std::atomic stop{false}; + auto scan = [&stop](const std::string & ip, uint16_t, int) { + const bool hit = ip == "192.168.1.10" || ip == "192.168.1.11"; + if (ip == "192.168.1.254") { + stop.store(true); // sweep finished, shutdown requested + } + return hit; + }; + std::atomic identifies{0}; + auto identify = [&identifies](const std::string &, int) { + identifies.fetch_add(1); + return IdentifyResult{}; + }; + + OpcuaDiscoveryConfig cfg; + cfg.enabled = true; + cfg.subnets = {"192.168.1.0/24"}; + cfg.ports = {4840}; + cfg.scan_concurrency = 1; + + NetworkDiscovery disc(cfg, scan, identify); + const auto eps = disc.run([&stop]() { + return stop.load(); + }); + EXPECT_TRUE(eps.empty()); + EXPECT_EQ(identifies.load(), 0); +} + // --------------------------------------------------------------------------- // // select_auto_endpoint // --------------------------------------------------------------------------- // diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_identity.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_identity.cpp index 48294503f..f96daf0e3 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_identity.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_identity.cpp @@ -12,15 +12,22 @@ // See the License for the specific language governing permissions and // limitations under the License. -// INV2 end-to-end (no HW): boot the test_alarm_server OPC-UA fixture, connect, -// and prove the asset-identity nameplate is filled from the server's device-info -// (ServerStatus/BuildInfo + the OPC-UA DI DeviceSet nameplate) with no manual -// entry. Exercises both the raw OpcuaClient::read_device_info read and the full -// OpcuaPlugin::introspect() path that lands identity on the SOVD Component. +// End-to-end against a live OPC-UA server (no HW): boot the test_alarm_server +// fixture and exercise the paths that only a real session can reach. +// +// INV2 identity: prove the asset-identity nameplate is filled from the server's +// device-info (ServerStatus/BuildInfo + the OPC-UA DI DeviceSet nameplate) with +// no manual entry, through both the raw OpcuaClient::read_device_info read and +// the full OpcuaPlugin::introspect() path that lands identity on the SOVD +// Component. +// +// Connection lifecycle: prove a successful connect clears the standing +// PLC_COMMS_LOST fault, which needs a connect that actually succeeds. #include "ros2_medkit_opcua/device_identity.hpp" #include "ros2_medkit_opcua/opcua_client.hpp" #include "ros2_medkit_opcua/opcua_plugin.hpp" +#include "ros2_medkit_opcua/opcua_poller.hpp" #include @@ -33,16 +40,23 @@ #include #include +#include #include #include #include #include +#include +#include +#include #include #include #include +#include #include #include +#include +#include #include "ros2_medkit_gateway/plugins/ros_plugin_context.hpp" @@ -183,17 +197,28 @@ class AlarmServer { if (pipe(pipefd) != 0) { return false; } + int stdin_pipe[2]; + if (pipe(stdin_pipe) != 0) { + close(pipefd[0]); + close(pipefd[1]); + return false; + } pid_ = fork(); if (pid_ < 0) { close(pipefd[0]); close(pipefd[1]); + close(stdin_pipe[0]); + close(stdin_pipe[1]); return false; } if (pid_ == 0) { dup2(pipefd[1], STDOUT_FILENO); dup2(pipefd[1], STDERR_FILENO); + dup2(stdin_pipe[0], STDIN_FILENO); close(pipefd[0]); close(pipefd[1]); + close(stdin_pipe[0]); + close(stdin_pipe[1]); std::string port_str = std::to_string(port); std::vector argv_vec{binary.c_str(), "--port", port_str.c_str()}; for (const auto & arg : extra_args) { @@ -204,11 +229,27 @@ class AlarmServer { _exit(127); } close(pipefd[1]); + close(stdin_pipe[0]); read_fd_ = pipefd[0]; + write_fd_ = stdin_pipe[1]; return wait_for_ready(15000); } + // One CLI command ("fire Overpressure 750", "clear Overpressure", ...). The + // fixture reads them line by line off stdin. + bool send(const std::string & command) { + if (write_fd_ < 0) { + return false; + } + const std::string line = command + "\n"; + return write(write_fd_, line.c_str(), line.size()) == static_cast(line.size()); + } + void stop() { + if (write_fd_ >= 0) { + close(write_fd_); + write_fd_ = -1; + } if (pid_ > 0) { kill(pid_, SIGTERM); int status = 0; @@ -248,6 +289,7 @@ class AlarmServer { pid_t pid_{-1}; int read_fd_{-1}; + int write_fd_{-1}; }; std::string fixture_binary() { @@ -559,4 +601,287 @@ TEST_F(OpcuaIdentityE2ETest, DiNameplateReadFollowsBrowseContinuationPoints) { client.disconnect(); } +// A gateway that restarts after a comms outage never raised PLC_COMMS_LOST in +// THIS process, yet the fault manager keys faults by fault_code alone and +// persists them, so the fault raised before the restart is still standing. +// The reconnect arm used to clear only when its own in-memory +// ``comms_lost_raised_`` flag was set, which no restart can satisfy, so the +// fault stayed CONFIRMED for good. The clear now goes out on every successful +// connect. Driven against the live fixture because the arm can only be reached +// by a connect that actually succeeds. +TEST_F(OpcuaIdentityE2ETest, SuccessfulConnectClearsCommsLostNeverRaisedHere) { + OpcuaClient client; + OpcuaClientConfig config; + config.endpoint_url = endpoint_; + config.connect_timeout = std::chrono::milliseconds(5000); + // Connect once to seed the client's stored config (what the poller reconnects + // with), then drop the session so the poll loop starts in its reconnect arm - + // the state a freshly started gateway is in while the PLC is already up. + ASSERT_TRUE(client.connect(config)); + client.disconnect(); + ASSERT_FALSE(client.is_connected()); + + NodeMap node_map; // config-less: no entries, nothing to poll + OpcuaPoller poller(client, node_map); + + std::mutex signals_mutex; + std::vector> signals; // (fault_code, active) + poller.set_alarm_callback( + [&signals_mutex, &signals](const std::string &, const ros2_medkit::fault_detection::FaultSignal & signal) { + std::lock_guard lock(signals_mutex); + signals.emplace_back(signal.fault_code, signal.active); + }); + + PollerConfig poller_config; + poller_config.poll_interval = std::chrono::milliseconds(100); + poller_config.reconnect_interval = std::chrono::milliseconds(100); + poller_config.comms_lost_fault_enabled = true; + poller.start(poller_config); + + bool cleared = false; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(15); + while (!cleared && std::chrono::steady_clock::now() < deadline) { + { + std::lock_guard lock(signals_mutex); + cleared = std::find(signals.begin(), signals.end(), std::make_pair(std::string(kCommsLostFaultCode), false)) != + signals.end(); + } + if (!cleared) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + } + poller.stop(); + + EXPECT_TRUE(cleared) << "a successful connect must clear PLC_COMMS_LOST even when this process never raised it"; + + // Absence control on the same harness: the connect succeeded, so nothing may + // have RAISED the fault. Without this a clear-everything-always regression + // would still pass the assertion above. + std::lock_guard lock(signals_mutex); + EXPECT_EQ(std::find(signals.begin(), signals.end(), std::make_pair(std::string(kCommsLostFaultCode), true)), + signals.end()) + << "comms-lost must not be raised while the connection is up"; +} + +namespace { + +// RAII rclcpp init/shutdown, tearing down only what it started. +struct ScopedRclcpp { + const bool owned_; + ScopedRclcpp() : owned_(!rclcpp::ok()) { + if (owned_) { + rclcpp::init(0, nullptr); + } + } + ~ScopedRclcpp() { + if (owned_ && rclcpp::ok()) { + rclcpp::shutdown(); + } + } + ScopedRclcpp(const ScopedRclcpp &) = delete; + ScopedRclcpp & operator=(const ScopedRclcpp &) = delete; +}; + +// The plugin only builds its fault-service clients when the context hands it a +// real node, which is what makes the ClearFault request observable on the wire. +class RealNodePluginContext : public FakePluginContext { + public: + explicit RealNodePluginContext(rclcpp::Node * node) : node_(node) { + } + rclcpp::Node * node() const override { + return node_; + } + + private: + rclcpp::Node * node_; +}; + +} // namespace + +// The connect-time clear, read off the wire. clear_comms_lost_on_connect() is +// only reachable through a connect that SUCCEEDS, so it needs the live fixture, +// and the flag it sets is only observable with a real fault-manager service on +// the other end. A correlation rule may name PLC_COMMS_LOST as the root cause of +// every symptom an outage produced, and the link coming back is not an operator +// resolving those, so this clear must not cascade. +TEST_F(OpcuaIdentityE2ETest, ConnectTimeCommsLostClearSkipsTheCorrelationCascade) { + ScopedRclcpp rclcpp_scope; + auto node = std::make_shared("opcua_identity_connect_clear"); + auto fault_manager = std::make_shared("opcua_identity_connect_clear_faultmgr"); + + std::mutex received_mutex; + std::vector cleared_requests; + auto report_srv = fault_manager->create_service( + "/fault_manager/report_fault", [](const std::shared_ptr, + std::shared_ptr res) { + res->accepted = true; + }); + auto clear_srv = fault_manager->create_service( + "/fault_manager/clear_fault", + [&cleared_requests, &received_mutex](const std::shared_ptr req, + std::shared_ptr res) { + { + std::lock_guard lock(received_mutex); + cleared_requests.push_back(*req); + } + res->success = true; + }); + + rclcpp::executors::MultiThreadedExecutor executor; + executor.add_node(node); + executor.add_node(fault_manager); + std::thread spin_thread([&executor]() { + executor.spin(); + }); + + const std::string yaml_path = write_minimal_node_map(); + OpcuaPlugin plugin; + nlohmann::json config; + config["endpoint_url"] = endpoint_; + config["node_map_path"] = yaml_path; + config["poll_interval_ms"] = 100; + plugin.configure(config); + + RealNodePluginContext ctx(node.get()); + ctx.entities["tank"] = {SovdEntityType::APP, "tank", "/test_plc", "/test_plc/test_runtime/tank"}; + // The connect inside set_context() succeeds against the fixture, which is the + // only way to reach the connect-time clear. + plugin.set_context(ctx); + + // The clear may be buffered until the stub service is DDS-matched. The poll + // thread drains the buffer on its next cycle. + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(20); + bool delivered = false; + while (!delivered && std::chrono::steady_clock::now() < deadline) { + { + std::lock_guard lock(received_mutex); + delivered = !cleared_requests.empty(); + } + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + + executor.cancel(); + if (spin_thread.joinable()) { + spin_thread.join(); + } + plugin.shutdown(); + std::remove(yaml_path.c_str()); + + std::lock_guard lock(received_mutex); + ASSERT_FALSE(cleared_requests.empty()) << "a successful connect sent no ClearFault at all"; + EXPECT_EQ(cleared_requests.front().fault_code, std::string(kCommsLostFaultCode)); + EXPECT_TRUE(cleared_requests.front().skip_correlation_auto_clear) + << "the connect-time clear cascade-cleared the symptoms of the outage it ended"; +} + +// The other side of the same rule, also on the wire: when the DEVICE reports its +// condition inactive, that IS a resolution at the source, so the correlation +// engine may act on it and the flag stays off. Only a live AlarmCondition +// lifecycle reaches on_event_alarm's ClearFault arm, so this drives the +// fixture's own CLI to fire and then clear a condition. +TEST_F(OpcuaIdentityE2ETest, DeviceReportedAlarmClearKeepsTheCorrelationCascade) { + ScopedRclcpp rclcpp_scope; + auto node = std::make_shared("opcua_identity_device_clear"); + auto fault_manager = std::make_shared("opcua_identity_device_clear_faultmgr"); + + std::mutex received_mutex; + std::vector reported; + std::vector cleared_requests; + auto report_srv = fault_manager->create_service( + "/fault_manager/report_fault", + [&reported, &received_mutex](const std::shared_ptr req, + std::shared_ptr res) { + { + std::lock_guard lock(received_mutex); + reported.push_back(req->fault_code); + } + res->accepted = true; + }); + auto clear_srv = fault_manager->create_service( + "/fault_manager/clear_fault", + [&cleared_requests, &received_mutex](const std::shared_ptr req, + std::shared_ptr res) { + { + std::lock_guard lock(received_mutex); + cleared_requests.push_back(*req); + } + res->success = true; + }); + + rclcpp::executors::MultiThreadedExecutor executor; + executor.add_node(node); + executor.add_node(fault_manager); + std::thread spin_thread([&executor]() { + executor.spin(); + }); + + OpcuaPlugin plugin; + nlohmann::json config; + config["endpoint_url"] = endpoint_; + config["poll_interval_ms"] = 100; + // Zero-config native A&C on the Server EventNotifier, with auto_clear so the + // condition going inactive clears the fault without an operator ack/confirm. + config["auto_alarms"] = nlohmann::json{{"enabled", true}, {"auto_clear", true}}; + plugin.configure(config); + + RealNodePluginContext ctx(node.get()); + plugin.set_context(ctx); + + const auto reported_count = [&received_mutex, &reported]() { + std::lock_guard lock(received_mutex); + return reported.size(); + }; + // The connect-time PLC_COMMS_LOST clear also lands here (this connect + // succeeded), so a clear is looked up by the code it names. + const auto clear_for = [&received_mutex, &cleared_requests](const std::string & code) -> std::optional { + std::lock_guard lock(received_mutex); + for (const auto & req : cleared_requests) { + if (req.fault_code == code) { + return req.skip_correlation_auto_clear; + } + } + return std::nullopt; + }; + + // Fire until the event subscription is up and a report lands. The retry is the + // subscription handshake, not flakiness in the assertion: an event fired + // before the subscribe simply is not delivered. + const auto fire_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(30); + while (reported_count() == 0 && std::chrono::steady_clock::now() < fire_deadline) { + ASSERT_TRUE(server_.send("fire Overpressure 750")); + std::this_thread::sleep_for(std::chrono::milliseconds(500)); + } + ASSERT_GT(reported_count(), 0u) << "the fixture's AlarmCondition never reached the fault manager"; + + std::string alarm_code; + { + std::lock_guard lock(received_mutex); + alarm_code = reported.front(); + } + ASSERT_TRUE(server_.send("clear Overpressure")); + const auto clear_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(30); + while (!clear_for(alarm_code).has_value() && std::chrono::steady_clock::now() < clear_deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + + executor.cancel(); + if (spin_thread.joinable()) { + spin_thread.join(); + } + plugin.shutdown(); + + const auto device_clear_skips = clear_for(alarm_code); + ASSERT_TRUE(device_clear_skips.has_value()) + << "the device reporting condition " << alarm_code << " inactive sent no ClearFault"; + EXPECT_FALSE(*device_clear_skips) << "a clear the device itself reported must keep the correlation cascade"; + + // The connect-time clear travelled the same wire in the same test, and it is + // the opposite case: not an operator resolving anything, so it does not + // cascade. Having both here is what makes the flag above a decision rather + // than a constant. + const auto link_state_clear_skips = clear_for(kCommsLostFaultCode); + ASSERT_TRUE(link_state_clear_skips.has_value()) << "the connect-time clear never arrived"; + EXPECT_TRUE(*link_state_clear_skips); +} + } // namespace ros2_medkit_gateway 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 4bfdf4bc6..0f53ddb05 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 @@ -27,7 +27,11 @@ #include #include #include +#include +#include #include +#include +#include #include #include #include @@ -638,6 +642,737 @@ TEST(CommsLostShouldRaise, IdempotentAndDisabled) { EXPECT_FALSE(OpcuaPoller::comms_lost_should_raise(/*enabled=*/false, false, t0, late, debounce)); } +// --------------------------------------------------------------------------- +// Endpoint rediscovery while disconnected (config-less discovery) +// --------------------------------------------------------------------------- + +namespace { + +// Fake port scanner backed by a set of open "ip:port" hosts. +PortScanFn fake_scan(std::set open) { + return [open = std::move(open)](const std::string & ip, uint16_t port, int) { + return open.count(ip + ":" + std::to_string(port)) > 0; + }; +} + +// Fake GetEndpoints identify keyed by connect URL. Anything else is unreachable. +IdentifyFn fake_identify(std::map table) { + return [table = std::move(table)](const std::string & url, int) -> IdentifyResult { + const auto it = table.find(url); + if (it != table.end()) { + return it->second; + } + IdentifyResult r; + r.error = "unreachable"; + return r; + }; +} + +IdentifyResult plc_identity() { + IdentifyResult r; + r.ok = true; + r.advertised_url = "opc.tcp://192.168.1.10:4840"; + r.application_uri = "urn:SIMATIC.S7-1500.OPC-UA.Application:Software PLC_1"; + r.product_uri = "https://www.siemens.com/s7-1500"; + r.application_name = "SIMATIC.S7-1500"; + r.application_type = 0; // Server + r.security_policies = {{"None", 1}}; + r.anonymous_none_available = true; + return r; +} + +OpcuaDiscoveryConfig rescan_cfg() { + OpcuaDiscoveryConfig cfg; + cfg.enabled = true; + cfg.subnets = {"192.168.1.0/24"}; // explicit, so no local-interface derivation + cfg.ports = {4840}; + return cfg; +} + +// Discards log output. The tests assert on the selected endpoint, not the text. +const std::function kSilent = [](const std::string &) {}; + +// Silent reporter: no repeat-suppression memory, so every pass reports in full +// (into the void). Tests that assert on the log build their own. +OpcuaPlugin::DiscoveryReporter silent_reporter() { + OpcuaPlugin::DiscoveryReporter reporter; + reporter.info = kSilent; + reporter.warn = kSilent; + return reporter; +} + +} // namespace + +TEST(DiscoverEndpoint, ScanBeforeThePlcIsUpSelectsNothingAndALaterRescanAdoptsIt) { + // The field race: the gateway scans 2 s after start while the PLC is still + // booting. Nothing answers, so nothing is selected and the caller keeps the + // default endpoint. + const auto empty_pass = OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({}), + fake_identify({}), silent_reporter()); + EXPECT_FALSE(empty_pass.has_value()); + + // The PLC finishes booting. The same call with the same config now finds it, + // which is what the reconnect arm applies to the next connect attempt. + const auto later_pass = OpcuaPlugin::discover_endpoint( + rescan_cfg(), /*endpoint_configured=*/false, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), silent_reporter()); + ASSERT_TRUE(later_pass.has_value()); + EXPECT_EQ(*later_pass, "opc.tcp://192.168.1.10:4840"); +} + +TEST(DiscoverEndpoint, AnExplicitEndpointIsNeverRescanned) { + // Positive control: the very scan that DOES find a server above finds the + // same server here, and is still refused because the operator pinned an + // endpoint. Discovery must not open a second session on a polled PLC. + const auto chosen = OpcuaPlugin::discover_endpoint( + rescan_cfg(), /*endpoint_configured=*/true, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), silent_reporter()); + EXPECT_FALSE(chosen.has_value()); +} + +TEST(DiscoverEndpoint, DisabledDiscoveryScansNothing) { + OpcuaDiscoveryConfig cfg = rescan_cfg(); + cfg.enabled = false; + bool scanned = false; + auto counting_scan = [&scanned](const std::string &, uint16_t, int) { + scanned = true; + return true; + }; + const auto chosen = OpcuaPlugin::discover_endpoint(cfg, /*endpoint_configured=*/false, counting_scan, + fake_identify({}), silent_reporter()); + EXPECT_FALSE(chosen.has_value()); + EXPECT_FALSE(scanned) << "a disabled discovery must not touch the network"; +} + +TEST(EffectiveRescanInterval, DefaultsWhenDiscoveryIsOnWithNoCadenceAndIsOffOtherwise) { + OpcuaDiscoveryConfig cfg = rescan_cfg(); + // Config-less: discovery on, no interval stated -> the built-in cadence, not + // "never rescan". This is the deployment that most needs the rescan. + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(cfg, /*endpoint_configured=*/false), + OpcuaPlugin::kDefaultRescanIntervalS); + // An operator-stated cadence wins. + cfg.interval_s = 120; + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(cfg, false), 120); + // An explicit endpoint, or discovery off, means no rescan at all. + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(cfg, /*endpoint_configured=*/true), 0); + cfg.enabled = false; + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(cfg, false), 0); +} + +TEST(EffectiveRescanInterval, ExplicitZeroKeepsDiscoveryOnAndStopsRescanning) { + // The three states an operator can be in, all with discovery enabled and no + // endpoint pinned. + OpcuaDiscoveryConfig unset = rescan_cfg(); // (1) unset -> the built-in cadence + EXPECT_FALSE(unset.interval_s.has_value()); + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(unset, false), OpcuaPlugin::kDefaultRescanIntervalS); + + OpcuaDiscoveryConfig explicit_zero = rescan_cfg(); // (2) explicit 0 -> one-shot + explicit_zero.interval_s = 0; + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(explicit_zero, false), 0) + << "an explicit interval_s: 0 must stop the rescan, not fall back to the default"; + + // (3) A negative value never reaches here: the parse warns and leaves the + // cadence unset, so what arrives is case (1). + std::vector warnings; + const auto parsed = + parse_discovery_config(nlohmann::json{{"enabled", true}, {"interval_s", -1}}, [&warnings](const std::string & m) { + warnings.push_back(m); + }); + EXPECT_EQ(warnings.size(), 1u); + EXPECT_EQ(OpcuaPlugin::effective_rescan_interval_s(parsed, false), OpcuaPlugin::kDefaultRescanIntervalS); +} + +// --------------------------------------------------------------------------- +// Rescan cadence: measured from the END of the previous sweep +// --------------------------------------------------------------------------- + +TEST(RescanStep, SpacesSweepsFromTheEndOfThePreviousOne) { + // A legal /16 sweep runs for minutes. With the cadence stamped at the START, + // the next sweep is due the instant the current one returns, so the poll + // thread sweeps back to back and the reconnect attempt drops to one a sweep. + const auto t0 = std::chrono::steady_clock::time_point{}; + const auto sweep_duration = std::chrono::seconds(390); // a /16 at the defaults + auto clock_now = t0; + const auto now = [&clock_now]() { + return clock_now; + }; + + int sweeps = 0; + const auto sweep = [&sweeps, &clock_now, sweep_duration]() -> std::optional { + ++sweeps; + clock_now += sweep_duration; // the sweep blocks for its whole duration + return std::nullopt; + }; + + auto last_end = t0; + clock_now = t0 + std::chrono::seconds(30); + OpcuaPlugin::rescan_step(30, now, &last_end, sweep); + ASSERT_EQ(sweeps, 1); + EXPECT_EQ(last_end, clock_now) << "the cadence must be stamped when the sweep finished"; + + // One second after the sweep returned: not due, even though it STARTED 391 s + // ago. + clock_now += std::chrono::seconds(1); + OpcuaPlugin::rescan_step(30, now, &last_end, sweep); + EXPECT_EQ(sweeps, 1) << "a rescan ran less than one interval after the previous sweep ended"; + + // A full interval after the end: due again. + clock_now += std::chrono::seconds(29); + OpcuaPlugin::rescan_step(30, now, &last_end, sweep); + EXPECT_EQ(sweeps, 2); +} + +TEST(RescanStep, AThrowingSweepStillStampsTheCadence) { + // A sweep that throws still consumed its minutes. If the stamp were owed only + // on the normal path, the next poll iteration would find the cadence due and + // start another sweep immediately, so a server that makes the identify throw + // would turn the poll thread into a continuous scanner. + const auto t0 = std::chrono::steady_clock::time_point{}; + auto clock_now = t0 + std::chrono::seconds(30); + const auto now = [&clock_now]() { + return clock_now; + }; + int sweeps = 0; + const auto throwing_sweep = [&sweeps, &clock_now]() -> std::optional { + ++sweeps; + clock_now += std::chrono::seconds(120); + throw std::runtime_error("identify blew up mid-sweep"); + }; + + auto last_end = t0; + EXPECT_THROW(OpcuaPlugin::rescan_step(30, now, &last_end, throwing_sweep), std::runtime_error); + EXPECT_EQ(sweeps, 1); + EXPECT_EQ(last_end, clock_now) << "a sweep that threw still has to stamp the cadence"; + + // Inside the interval after that failed sweep: not due, so no second sweep. + clock_now += std::chrono::seconds(29); + EXPECT_NO_THROW(OpcuaPlugin::rescan_step(30, now, &last_end, throwing_sweep)); + EXPECT_EQ(sweeps, 1) << "a failed sweep let the next one start inside the interval"; + + // Positive control: one full interval later it is due again (and throws again). + clock_now += std::chrono::seconds(1); + EXPECT_THROW(OpcuaPlugin::rescan_step(30, now, &last_end, throwing_sweep), std::runtime_error); + EXPECT_EQ(sweeps, 2); +} + +TEST(RescanStep, DoesNothingWithoutACadence) { + const auto t0 = std::chrono::steady_clock::time_point{}; + auto last_end = t0; + int sweeps = 0; + const auto now = [t0]() { + return t0 + std::chrono::hours(1); + }; + const auto sweep = [&sweeps]() -> std::optional { + ++sweeps; + return std::string("opc.tcp://192.168.1.10:4840"); + }; + // 0 is the operator's "do not re-scan" (and also discovery off / endpoint + // pinned, both of which effective_rescan_interval_s maps to 0). + EXPECT_FALSE(OpcuaPlugin::rescan_step(0, now, &last_end, sweep).has_value()); + EXPECT_EQ(sweeps, 0); + // Positive control on the same harness: with a cadence the very same call + // sweeps and hands the endpoint back. + const auto adopted = OpcuaPlugin::rescan_step(30, now, &last_end, sweep); + ASSERT_TRUE(adopted.has_value()); + EXPECT_EQ(*adopted, "opc.tcp://192.168.1.10:4840"); + EXPECT_EQ(sweeps, 1); +} + +// --------------------------------------------------------------------------- +// Reconnect backoff ceiling while the reconnect arm also rescans +// --------------------------------------------------------------------------- + +TEST(EffectiveMaxReconnectWait, CapsTheBackoffAtTheRescanCadence) { + using namespace std::chrono_literals; + // No rescan: the plain 60 s ceiling. + EXPECT_EQ(OpcuaPlugin::effective_max_reconnect_wait(5000ms, 60000ms, /*rescan_interval_s=*/0), 60000ms); + // Rescanning every 30 s: an uncapped backoff would make the real adoption + // cadence max(30 s, 60 s), not the documented 30 s. + EXPECT_EQ(OpcuaPlugin::effective_max_reconnect_wait(5000ms, 60000ms, 30), 30000ms); + // A cadence longer than the ceiling does not raise the ceiling. + EXPECT_EQ(OpcuaPlugin::effective_max_reconnect_wait(5000ms, 60000ms, 900), 60000ms); + // A cadence shorter than the configured reconnect interval does not turn the + // backoff into a hot retry loop. + EXPECT_EQ(OpcuaPlugin::effective_max_reconnect_wait(5000ms, 60000ms, 1), 5000ms); +} + +TEST(NextReconnectWait, DoublesUpToTheCeiling) { + using namespace std::chrono_literals; + EXPECT_EQ(OpcuaPoller::next_reconnect_wait(5000ms, 60000ms), 10000ms); + EXPECT_EQ(OpcuaPoller::next_reconnect_wait(40000ms, 60000ms), 60000ms); + EXPECT_EQ(OpcuaPoller::next_reconnect_wait(60000ms, 60000ms), 60000ms); + // Capped at a 30 s rescan cadence: the wait never exceeds it, so the rescan is + // consulted every cadence instead of every max(cadence, backoff). + EXPECT_EQ(OpcuaPoller::next_reconnect_wait(20000ms, 30000ms), 30000ms); + EXPECT_EQ(OpcuaPoller::next_reconnect_wait(30000ms, 30000ms), 30000ms); +} + +// --------------------------------------------------------------------------- +// Discovery report: quiet while the outcome does not change +// --------------------------------------------------------------------------- + +TEST(DiscoverEndpoint, AnUnchangedRescanReportsAtDebugInsteadOfRepeatingItself) { + // A secured-only site rescans for the life of the process and would otherwise + // log the whole report - scan line, per-server line, summary and the + // "no auto-connectable server" WARN - every interval_s. + IdentifyResult secured = plc_identity(); + secured.anonymous_none_available = false; + + std::vector info; + std::vector warn; + std::vector debug; + std::string outcome; + OpcuaPlugin::DiscoveryReporter reporter; + reporter.info = [&info](const std::string & m) { + info.push_back(m); + }; + reporter.warn = [&warn](const std::string & m) { + warn.push_back(m); + }; + reporter.debug = [&debug](const std::string & m) { + debug.push_back(m); + }; + reporter.previous_outcome = &outcome; + + const auto pass = [&]() { + return OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", secured}}), reporter); + }; + + EXPECT_FALSE(pass().has_value()); + const size_t first_info = info.size(); + const size_t first_warn = warn.size(); + EXPECT_GT(first_info, 0u); + EXPECT_EQ(first_warn, 1u) << "the first pass always reports the secured-only outcome"; + EXPECT_TRUE(debug.empty()); + + // Same network, same outcome: nothing new at INFO/WARN, the report goes to + // the debug logger instead. + EXPECT_FALSE(pass().has_value()); + EXPECT_EQ(info.size(), first_info) << "an unchanged rescan repeated its report at INFO"; + EXPECT_EQ(warn.size(), first_warn) << "an unchanged rescan repeated its WARN"; + EXPECT_EQ(debug.size(), first_info + first_warn) << "the repeated report must still be traceable at DEBUG"; + + // The server opens up an anonymous endpoint: the outcome changed, so the + // operator hears about it at INFO again. + const auto chosen = + OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), reporter); + ASSERT_TRUE(chosen.has_value()); + EXPECT_GT(info.size(), first_info) << "a changed outcome must be reported at INFO"; +} + +TEST(DiscoverEndpoint, APredicateThatFlipsMidSweepEndsThePass) { + // What a stop signal does to a sweep in progress. This predicate is the + // test's own, standing in for the one the plugin passes: it flips after a + // handful of probes, as either of the plugin's two stop signals would + // mid-sweep. The plugin's own predicate is pinned separately, by + // DiscoveryCancelledFor. + std::atomic probes{0}; + std::atomic stop{false}; + auto stopping_scan = [&probes, &stop](const std::string & ip, uint16_t port, int) { + if (probes.fetch_add(1) + 1 >= 5) { + stop.store(true); + } + return ip == "192.168.1.10" && port == 4840; // the PLC IS there to be found + }; + + OpcuaDiscoveryConfig cfg = rescan_cfg(); + cfg.scan_concurrency = 1; // sequential, so the probe count is the predicate's doing + const auto chosen = OpcuaPlugin::discover_endpoint(cfg, /*endpoint_configured=*/false, stopping_scan, + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), + silent_reporter(), [&stop]() { + return stop.load(); + }); + + EXPECT_FALSE(chosen.has_value()) << "a cancelled pass must not hand back a partial result"; + EXPECT_LE(probes.load(), 6) << "the sweep ran on after the stop signal"; + + // Positive control on the same fakes: without the predicate the very same + // sweep visits all 254 hosts and selects the PLC. + probes.store(0); + stop.store(false); + const auto uncancelled = OpcuaPlugin::discover_endpoint( + cfg, /*endpoint_configured=*/false, + [&probes](const std::string & ip, uint16_t port, int) { + probes.fetch_add(1); + return ip == "192.168.1.10" && port == 4840; + }, + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), silent_reporter()); + ASSERT_TRUE(uncancelled.has_value()); + EXPECT_EQ(*uncancelled, "opc.tcp://192.168.1.10:4840"); + EXPECT_EQ(probes.load(), 254); +} + +TEST(DiscoverEndpoint, TheScanIsAnnouncedBeforeTheSweepRuns) { + // A /16 sweep runs for minutes. If the announcement waited for the report at + // the end of the pass, start-up would log nothing while it swept and an + // operator would read that as a hung gateway. + std::vector info; + std::vector debug; + std::string announced_before_first_probe; + std::string outcome; + OpcuaPlugin::DiscoveryReporter reporter; + reporter.info = [&info](const std::string & m) { + info.push_back(m); + }; + reporter.warn = kSilent; + reporter.debug = [&debug](const std::string & m) { + debug.push_back(m); + }; + reporter.previous_outcome = &outcome; + + auto scan_recording_the_log = [&info, &announced_before_first_probe](const std::string &, uint16_t, int) { + if (announced_before_first_probe.empty() && !info.empty()) { + announced_before_first_probe = info.front(); + } + return false; + }; + OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, scan_recording_the_log, fake_identify({}), + reporter); + EXPECT_NE(announced_before_first_probe.find("read-only active scan of"), std::string::npos) + << "the sweep started before the operator was told anything (first INFO line: '" + << (info.empty() ? std::string("") : info.front()) << "')"; + + // On a rescan the announcement drops to DEBUG: the sweep repeats every + // interval_s for the life of the outage and must not narrate every pass. + const size_t info_after_first = info.size(); + OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({}), fake_identify({}), + reporter); + EXPECT_EQ(info.size(), info_after_first) << "the rescan announced itself at INFO again"; + EXPECT_FALSE(debug.empty()); +} + +TEST(DiscoverEndpoint, WithNoRepeatMemoryEveryPassIsReported) { + // Positive control for the test above: the same two identical passes with no + // previous_outcome (the startup scan's own reporter) report in full twice, so + // the silence above is the suppression and not a dead sink. + std::vector info; + OpcuaPlugin::DiscoveryReporter reporter; + reporter.info = [&info](const std::string & m) { + info.push_back(m); + }; + reporter.warn = kSilent; + + const auto pass = [&]() { + return OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), reporter); + }; + EXPECT_TRUE(pass().has_value()); + const size_t first = info.size(); + EXPECT_GT(first, 0u); + EXPECT_TRUE(pass().has_value()); + EXPECT_EQ(info.size(), 2 * first); +} + +// --------------------------------------------------------------------------- +// Config-less component identity across an adoption +// --------------------------------------------------------------------------- + +TEST(RederivedComponentIdentity, AdoptionReplacesTheProvisionalEndpointDerivedId) { + // The config-less race, end to end over the derivation path: the gateway + // starts before the PLC, its start-up scan finds nothing, and the identity is + // derived from the fallback endpoint plus an empty DeviceInfo. + const auto startup_pass = OpcuaPlugin::discover_endpoint(rescan_cfg(), /*endpoint_configured=*/false, fake_scan({}), + fake_identify({}), silent_reporter()); + ASSERT_FALSE(startup_pass.has_value()); + const std::string fallback_endpoint = "opc.tcp://localhost:4840"; // OpcuaClientConfig's default + const ComponentIdentity provisional = derive_component_identity(OpcuaClient::DeviceInfo{}, fallback_endpoint); + EXPECT_EQ(provisional.id, "opcua-localhost"); + + // The PLC finishes booting and the rescan adopts it. + const auto adopted = OpcuaPlugin::discover_endpoint( + rescan_cfg(), /*endpoint_configured=*/false, fake_scan({"192.168.1.10:4840"}), + fake_identify({{"opc.tcp://192.168.1.10:4840", plc_identity()}}), silent_reporter()); + ASSERT_TRUE(adopted.has_value()); + + // The session is up, so the device can finally name itself: the component + // must stop being served under the placeholder. + OpcuaClient::DeviceInfo info; + info.di_manufacturer = "Siemens AG"; + info.di_model = "CPU 1505SP F"; + const auto rederived = OpcuaPlugin::rederived_component_identity(provisional.id, info, *adopted); + ASSERT_TRUE(rederived.has_value()) << "an adopted device with a nameplate must replace opcua-localhost"; + EXPECT_EQ(rederived->id, "siemens_ag_cpu_1505sp_f"); + EXPECT_EQ(rederived->name, "Siemens AG CPU 1505SP F"); +} + +TEST(RederivedComponentIdentity, KeepsTheIdentityWhenNothingChanged) { + // Same device on a later reconnect: no rename, so no entity churn and no INFO + // line claiming an identity change that did not happen. + OpcuaClient::DeviceInfo info; + info.di_manufacturer = "Siemens AG"; + info.di_model = "CPU 1505SP F"; + EXPECT_FALSE(OpcuaPlugin::rederived_component_identity("siemens_ag_cpu_1505sp_f", info, "opc.tcp://192.168.1.10:4840") + .has_value()); + + // A nameplate-less server on an adopted endpoint still moves off the + // fallback host it was provisionally named after. + const auto host_derived = OpcuaPlugin::rederived_component_identity("opcua-localhost", OpcuaClient::DeviceInfo{}, + "opc.tcp://192.168.1.10:4840"); + ASSERT_TRUE(host_derived.has_value()); + EXPECT_EQ(host_derived->id, "opcua-192_168_1_10"); +} + +// --------------------------------------------------------------------------- +// The stop signals a discovery sweep watches +// --------------------------------------------------------------------------- + +TEST(DiscoveryCancelledFor, EitherStopSignalEndsASweep) { + // The plugin's own predicate is this rule applied to two values it reads off + // the process, so this is the whole of it. + EXPECT_FALSE(OpcuaPlugin::discovery_cancelled_for(/*shutdown_requested=*/false, /*rclcpp_ok=*/true)) + << "a running process must not cancel its own sweep"; + // shutdown() ends a rescan sweep on the poll thread. + EXPECT_TRUE(OpcuaPlugin::discovery_cancelled_for(/*shutdown_requested=*/true, /*rclcpp_ok=*/true)); + // SIGINT / SIGTERM ends the start-up sweep, which runs during node + // construction where shutdown() cannot be reached at all. + EXPECT_TRUE(OpcuaPlugin::discovery_cancelled_for(/*shutdown_requested=*/false, /*rclcpp_ok=*/false)) + << "a signal during the start-up sweep left it running"; + EXPECT_TRUE(OpcuaPlugin::discovery_cancelled_for(true, false)); +} + +TEST(DiscoveryCancelledFor, RclcppOkIsTheSignalTheStartUpSweepWatches) { + // The second input is not hypothetical: rclcpp's shutdown is what a SIGTERM + // turns into, and it is observable exactly this way. A private context keeps + // the process-wide default one (which other tests here initialise) untouched. + auto context = std::make_shared(); + context->init(0, nullptr); + ASSERT_TRUE(rclcpp::ok(context)); + EXPECT_FALSE(OpcuaPlugin::discovery_cancelled_for(/*shutdown_requested=*/false, rclcpp::ok(context))); + + context->shutdown("simulated SIGTERM"); + ASSERT_FALSE(rclcpp::ok(context)) << "rclcpp::ok did not follow the shutdown a signal performs"; + EXPECT_TRUE(OpcuaPlugin::discovery_cancelled_for(/*shutdown_requested=*/false, rclcpp::ok(context))); +} + +// --------------------------------------------------------------------------- +// ClearFault: only a clear the device itself reported may cascade +// --------------------------------------------------------------------------- + +TEST(ClearOriginForSignal, OnlyTheCommsLostCodeIsALinkStateClear) { + // The poller emits its component-scoped comms-lost clear through the same + // callback as every device alarm going inactive, so the fault code is the only + // thing that tells the two apart on that path. + EXPECT_EQ(OpcuaPlugin::clear_origin_for_signal(kCommsLostFaultCode), OpcuaPlugin::ClearOrigin::LinkState); + EXPECT_EQ(OpcuaPlugin::clear_origin_for_signal("PLC_TANK_HIGH"), OpcuaPlugin::ClearOrigin::DeviceAlarm); + EXPECT_EQ(OpcuaPlugin::clear_origin_for_signal(std::string(kCommsLostFaultCode) + "_UPSTREAM"), + OpcuaPlugin::ClearOrigin::DeviceAlarm) + << "the match must be the exact code, not a prefix"; +} + +TEST(ClearOrigin, OnlyADeviceReportedClearKeepsTheCorrelationCascade) { + using Origin = OpcuaPlugin::ClearOrigin; + // The link coming back is not an operator resolving a root cause, and neither + // is an operator scoped to ONE entity: a correlation rule naming + // PLC_COMMS_LOST as the root cause would otherwise clear symptom faults + // reported by apps in entities that operator cannot even see. The gateway + // applies exactly this rule on its own branch of the same DELETE route. + EXPECT_TRUE(OpcuaPlugin::clear_skips_correlation(Origin::LinkState)); + EXPECT_TRUE(OpcuaPlugin::clear_skips_correlation(Origin::ScopedOperator)); + // Positive control on the same predicate: the device reporting its own + // condition inactive IS a resolution at the source, so that clear cascades. + // Without this case the rule above would be indistinguishable from a + // hardcoded true. + EXPECT_FALSE(OpcuaPlugin::clear_skips_correlation(Origin::DeviceAlarm)); + + // The buffer's ranking is a different question from the wire flag: only the + // link-state clear is re-derivable, the operator's scoped clear is as + // one-shot as an alarm report. + EXPECT_TRUE(OpcuaPlugin::clear_is_link_state(Origin::LinkState)); + EXPECT_FALSE(OpcuaPlugin::clear_is_link_state(Origin::ScopedOperator)); + EXPECT_FALSE(OpcuaPlugin::clear_is_link_state(Origin::DeviceAlarm)); +} + +TEST(MakeClearFaultRequest, CarriesTheSkipFlagAndCodeVerbatim) { + const auto skipping = OpcuaPlugin::make_clear_fault_request(kCommsLostFaultCode, true); + EXPECT_EQ(skipping.fault_code, kCommsLostFaultCode); + EXPECT_TRUE(skipping.skip_correlation_auto_clear); + + const auto cascading = OpcuaPlugin::make_clear_fault_request("PLC_TANK_HIGH", false); + EXPECT_EQ(cascading.fault_code, "PLC_TANK_HIGH"); + EXPECT_FALSE(cascading.skip_correlation_auto_clear); +} + +// --------------------------------------------------------------------------- +// Pending fault dispatch buffer: only what is re-derivable may be dropped first +// --------------------------------------------------------------------------- + +namespace { + +OpcuaPlugin::PendingFaultDispatch report_entry(const std::string & code) { + return {OpcuaPlugin::PendingFaultDispatch::Kind::Report, code, /*link_state=*/false, []() {}}; +} + +// A clear the next reconnect will send again (PLC_COMMS_LOST). +OpcuaPlugin::PendingFaultDispatch link_state_clear_entry(const std::string & code) { + return {OpcuaPlugin::PendingFaultDispatch::Kind::Clear, code, /*link_state=*/true, []() {}}; +} + +// A clear nothing will re-send: the device reported its condition inactive, or +// an operator cleared through the scoped SOVD route. +OpcuaPlugin::PendingFaultDispatch device_clear_entry(const std::string & code) { + return {OpcuaPlugin::PendingFaultDispatch::Kind::Clear, code, /*link_state=*/false, []() {}}; +} + +size_t count_kind(const std::vector & buffer, + OpcuaPlugin::PendingFaultDispatch::Kind kind) { + return static_cast( + std::count_if(buffer.begin(), buffer.end(), [kind](const OpcuaPlugin::PendingFaultDispatch & entry) { + return entry.kind == kind; + })); +} + +} // namespace + +TEST(EnqueuePendingDispatch, ReconnectClearsNeverEvictABufferedAlarmReport) { + // A flapping link with no fault_manager: 300 reconnects, each enqueueing a + // connect-time clear, while ten real alarm reports wait to be flushed. The + // reports are one-shot edges from the PLC, the clears are re-derivable. + std::vector buffer; + for (int i = 0; i < 10; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + report_entry("PLC_ALARM_" + std::to_string(i))); + } + for (int i = 0; i < 300; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + link_state_clear_entry(kCommsLostFaultCode)); + } + + EXPECT_EQ(count_kind(buffer, OpcuaPlugin::PendingFaultDispatch::Kind::Report), 10u) + << "connect-time clears evicted buffered alarm reports"; + EXPECT_EQ(count_kind(buffer, OpcuaPlugin::PendingFaultDispatch::Kind::Clear), 1u) + << "at most one clear per fault code may be pending"; + for (int i = 0; i < 10; ++i) { + EXPECT_EQ(buffer[static_cast(i)].fault_code, "PLC_ALARM_" + std::to_string(i)); + } +} + +TEST(EnqueuePendingDispatch, AFullOneShotBufferRefusesALinkStateClearInsteadOfDroppingOne) { + std::vector buffer; + for (size_t i = 0; i < OpcuaPlugin::kMaxPendingDispatches; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + report_entry("PLC_ALARM_" + std::to_string(i))); + } + ASSERT_EQ(buffer.size(), OpcuaPlugin::kMaxPendingDispatches); + + EXPECT_EQ(OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + link_state_clear_entry(kCommsLostFaultCode)), + OpcuaPlugin::PendingEnqueueOutcome::Refused); + EXPECT_EQ(count_kind(buffer, OpcuaPlugin::PendingFaultDispatch::Kind::Report), OpcuaPlugin::kMaxPendingDispatches); + EXPECT_EQ(buffer.front().fault_code, "PLC_ALARM_0") << "the oldest report must survive an incoming link-state clear"; + + // A report arriving at the same full buffer still drops the oldest entry: + // one-shot dispatches do not outrank each other, so the bound still holds. + EXPECT_EQ( + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, report_entry("PLC_ALARM_NEW")), + OpcuaPlugin::PendingEnqueueOutcome::EvictedOldest); + EXPECT_EQ(buffer.size(), OpcuaPlugin::kMaxPendingDispatches); + EXPECT_EQ(buffer.front().fault_code, "PLC_ALARM_1"); + EXPECT_EQ(buffer.back().fault_code, "PLC_ALARM_NEW"); +} + +TEST(EnqueuePendingDispatch, AFullBufferGivesUpALinkStateClearBeforeAReport) { + std::vector buffer; + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + link_state_clear_entry(kCommsLostFaultCode)); + for (size_t i = 1; i < OpcuaPlugin::kMaxPendingDispatches; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + report_entry("PLC_ALARM_" + std::to_string(i))); + } + ASSERT_EQ(buffer.size(), OpcuaPlugin::kMaxPendingDispatches); + + EXPECT_EQ( + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, report_entry("PLC_ALARM_NEW")), + OpcuaPlugin::PendingEnqueueOutcome::EvictedLinkStateClear); + EXPECT_EQ(count_kind(buffer, OpcuaPlugin::PendingFaultDispatch::Kind::Clear), 0u); + EXPECT_EQ(buffer.front().fault_code, "PLC_ALARM_1") << "the re-derivable clear went, not the oldest report"; +} + +TEST(EnqueuePendingDispatch, ADeviceAlarmClearIsNotEvictedAheadOfAnOlderReport) { + // The device says an alarm went inactive while the fault_manager is + // unreachable. That edge is as one-shot as the raise: drop it and the flush + // replays the raise with nothing behind it, so the fault stands while the + // device reports it clear. Only the link-state clear is re-derivable. + std::vector buffer; + for (int i = 0; i < 100; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + report_entry("PLC_ALARM_" + std::to_string(i))); + } + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, report_entry("PLC_TANK_HIGH")); + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + device_clear_entry("PLC_TANK_HIGH")); + for (size_t i = buffer.size(); i < OpcuaPlugin::kMaxPendingDispatches; ++i) { + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + report_entry("PLC_FILLER_" + std::to_string(i))); + } + ASSERT_EQ(buffer.size(), OpcuaPlugin::kMaxPendingDispatches); + + EXPECT_EQ( + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, report_entry("PLC_ALARM_NEW")), + OpcuaPlugin::PendingEnqueueOutcome::EvictedOldest); + EXPECT_NE(buffer.front().fault_code, "PLC_ALARM_0") << "the oldest entry is what ages out"; + const auto device_clear = + std::find_if(buffer.begin(), buffer.end(), [](const OpcuaPlugin::PendingFaultDispatch & entry) { + return entry.kind == OpcuaPlugin::PendingFaultDispatch::Kind::Clear && entry.fault_code == "PLC_TANK_HIGH"; + }); + ASSERT_NE(device_clear, buffer.end()) << "a device alarm's inactive edge was evicted ahead of an older report"; + // ... and it still flushes after the raise it supersedes. + const auto raise = std::find_if(buffer.begin(), buffer.end(), [](const OpcuaPlugin::PendingFaultDispatch & entry) { + return entry.kind == OpcuaPlugin::PendingFaultDispatch::Kind::Report && entry.fault_code == "PLC_TANK_HIGH"; + }); + ASSERT_NE(raise, buffer.end()); + EXPECT_LT(raise - buffer.begin(), device_clear - buffer.begin()); +} + +TEST(EnqueuePendingDispatch, ARequeuedClearMovesToTheBackSoOrderStillHolds) { + // Report-then-clear for one code must still flush in that order after the + // clear is re-enqueued, or the flush would leave the fault standing. + std::vector buffer; + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, link_state_clear_entry("PLC_FLAP")); + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, report_entry("PLC_FLAP")); + EXPECT_EQ(OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, + link_state_clear_entry("PLC_FLAP")), + OpcuaPlugin::PendingEnqueueOutcome::ReplacedClear); + + ASSERT_EQ(buffer.size(), 2u); + EXPECT_EQ(buffer[0].kind, OpcuaPlugin::PendingFaultDispatch::Kind::Report); + EXPECT_EQ(buffer[1].kind, OpcuaPlugin::PendingFaultDispatch::Kind::Clear) + << "the newest clear must flush after the report it supersedes"; + // Clears for DIFFERENT codes are independent. + OpcuaPlugin::enqueue_pending_dispatch(buffer, OpcuaPlugin::kMaxPendingDispatches, device_clear_entry("PLC_OTHER")); + EXPECT_EQ(count_kind(buffer, OpcuaPlugin::PendingFaultDispatch::Kind::Clear), 2u); +} + +TEST(AdoptRediscoveredEndpoint, AdoptsOnlyADifferentNonEmptyUrl) { + const std::string current = "opc.tcp://localhost:4840"; + + // No callback bound (an explicit endpoint, or discovery off) -> keep current. + EXPECT_FALSE(OpcuaPoller::adopt_rediscovered_endpoint(current, nullptr).has_value()); + + // Rescan not due, or found nothing -> keep current. + EXPECT_FALSE(OpcuaPoller::adopt_rediscovered_endpoint(current, [] { + return std::optional{}; + }).has_value()); + + // Same server as before -> nothing to adopt, so no needless reconnect churn. + EXPECT_FALSE(OpcuaPoller::adopt_rediscovered_endpoint(current, [¤t] { + return std::optional{current}; + }).has_value()); + + // An empty URL is not an endpoint. + EXPECT_FALSE(OpcuaPoller::adopt_rediscovered_endpoint(current, [] { + return std::optional{""}; + }).has_value()); + + // A different server -> adopt it for the next connect attempt. + const auto adopted = OpcuaPoller::adopt_rediscovered_endpoint(current, [] { + return std::optional{"opc.tcp://192.168.1.10:4840"}; + }); + ASSERT_TRUE(adopted.has_value()); + EXPECT_EQ(*adopted, "opc.tcp://192.168.1.10:4840"); +} + // 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 @@ -1113,4 +1848,92 @@ component_id: race_runtime << "flush_pending_reports never dispatched - swap-vs-push path not covered"; } +// The SOVD per-entity route DELETE /{entity}/faults/{code} lands on +// FaultProvider::clear_fault for a plugin-owned entity, which is the branch the +// gateway takes INSTEAD of its own (where it sets skip_correlation_auto_clear +// itself). So the flag has to be set here or the documented guarantee - an +// operator scoped to one entity cannot cascade-clear symptoms reported by apps +// in other entities - has a hole exactly where a PLC is involved. This drives +// the real route entry point and reads the field off the wire. +TEST(OpcuaPluginScopedClear, SovdDeleteSkipsTheCorrelationCascade) { + ScopedRclcpp rclcpp_scope; + auto node = std::make_shared("opcua_scoped_clear_flag"); + auto fault_manager = std::make_shared("opcua_scoped_clear_faultmgr"); + + std::mutex received_mutex; + std::vector received; + auto report_srv = fault_manager->create_service( + "/fault_manager/report_fault", [](const std::shared_ptr, + std::shared_ptr res) { + res->accepted = true; + }); + auto clear_srv = fault_manager->create_service( + "/fault_manager/clear_fault", + [&received, &received_mutex](const std::shared_ptr req, + std::shared_ptr res) { + { + std::lock_guard lock(received_mutex); + received.push_back(*req); + } + res->success = true; + }); + + const std::string yaml_path = "/tmp/test_opcua_scoped_clear_nodemap.yaml"; + { + std::ofstream f(yaml_path); + f << R"( +area_id: scoped_plc +component_id: scoped_runtime +nodes: + - node_id: "ns=2;i=1" + entity_id: tank + data_name: level + data_type: float +)"; + } + + OpcuaPlugin plugin; + nlohmann::json config; + config["node_map_path"] = yaml_path; + config["endpoint_url"] = "opc.tcp://127.0.0.1:1"; // nothing listening, the fault sink is the subject + config["poll_interval_ms"] = 100; + plugin.configure(config); + + RealNodePluginContext ctx(node.get()); + ctx.entities["tank"] = {SovdEntityType::APP, "tank", "/scoped_plc", "/scoped_plc/scoped_runtime/tank"}; + plugin.set_context(ctx); + + ScopedExecutorSpin spinner({node, fault_manager}); + auto probe = node->create_client("/fault_manager/clear_fault"); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + while (!probe->service_is_ready() && std::chrono::steady_clock::now() < deadline) { + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + ASSERT_TRUE(probe->service_is_ready()) << "stub ClearFault server never became discoverable"; + + // The route's own entry point, not a helper it happens to call. + const auto result = plugin.clear_fault("tank", "PLC_TANK_HIGH"); + ASSERT_TRUE(result.has_value()); + + const auto flush_deadline = std::chrono::steady_clock::now() + std::chrono::seconds(10); + bool delivered = false; + while (!delivered && std::chrono::steady_clock::now() < flush_deadline) { + { + std::lock_guard lock(received_mutex); + delivered = !received.empty(); + } + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + + spinner.stop(); + plugin.shutdown(); + std::remove(yaml_path.c_str()); + + std::lock_guard lock(received_mutex); + ASSERT_FALSE(received.empty()) << "the scoped DELETE never reached the fault manager"; + EXPECT_EQ(received.front().fault_code, "PLC_TANK_HIGH"); + EXPECT_TRUE(received.front().skip_correlation_auto_clear) + << "a per-entity DELETE served by the plugin cascade-cleared correlated symptoms"; +} + } // namespace ros2_medkit_gateway