Conversation
bburda
marked this pull request as draft
September 6, 2026 19:13
This was referenced Sep 9, 2026
bburda
force-pushed
the
fix/opcua-rescan-comms-lost-freeze-source
branch
from
September 13, 2026 18:34
0b47c79 to
a791243
Compare
bburda
marked this pull request as ready for review
September 13, 2026 19:34
mfaferek93
reviewed
Sep 14, 2026
The plugin decided two things once, at startup, and never revised them while it ran. Both now recover after a bad start. Rescan while no session is up. Config-less discovery ran one scan a few seconds after start. A gateway that boots together with its PLC scans while the PLC is still coming up. It finds nothing, falls back to opc.tcp://localhost:4840 and retries that endpoint until someone restarts it. The reconnect arm now asks the plugin for a fresh scan. It adopts a newly found server for its next connect attempt and resets the backoff, so the new endpoint is tried at once. The rules that make discovery safe are unchanged. An explicitly configured endpoint_url still wins and is never rescanned. The scan stays a bounded read-only TCP sweep plus GetEndpoints. Nothing is scanned while a session is up. Cadence. discovery.interval_s sets the rescan cadence, 30 s by default. The cadence is stamped when a sweep ends, so a sweep of a /16, which takes minutes at the defaults, does not make the next one due at once. While discovery rescans, the reconnect backoff is capped at the cadence, so the real cadence is interval_s. An unset interval_s takes the default. An explicit 0 keeps discovery on with a one-shot start-up scan. A negative value is refused with a warning. The warning and the start-up line describe the cadence that applies. NetworkDiscovery::run() takes a cancel predicate, bound to the shutdown flag. It is checked before each probe and between the sweep and the identify phase, so shutdown() does not wait out a sweep. Discovery report. A pass whose outcome matches the previous pass reports at DEBUG. The first pass and every changed outcome report at INFO or WARN. A site with a secured-only server otherwise logs the same WARN every 30 s. Component identity. With no node map, the SOVD component is named from the device. When the start-up connect fails there is no device to ask, so the name comes from the fallback endpoint and an empty DeviceInfo. The identity is now derived again on the first poll of a new session, in config-less mode only. An explicit node map still owns the name. The derived alarms entity follows the rename, and the rename is logged at INFO. Clear PLC_COMMS_LOST on every successful connect. The fault manager keys faults by fault code and persists them. A fault raised before a gateway restart stays in the store, and the new process has no memory of raising it. The initial connect and every reconnect now send the clear, whatever this process raised. The clear is fire and forget, so a clear for a fault that is not there costs nothing. The debounce that governs raising is unchanged. Both clears set skip_correlation_auto_clear. They report the link state, so they must not cascade-clear the symptom faults that a rule attributes to PLC_COMMS_LOST. Pending buffer. While the fault manager is unmatched, the buffer keeps at most one pending clear per fault code. When it is full, it evicts a clear before a report. A clear that meets a buffer full of reports is refused, and the reports stay. A flapping link otherwise pushes real alarm reports out of the bounded buffer. Tests. Unit tests drive the discovery pass and the endpoint adoption rule with injected probes. They include the positive control that a configured endpoint refuses the server an unconfigured one accepts. The comms-lost heal runs against the live test server, because only a connect that succeeds reaches that arm. A docker scenario starts the gateway first, with discovery on and no endpoint configured. It asserts that the gateway settled on the fallback endpoint with no session. It then starts an OPC UA server on the same subnet and asserts that the endpoint is adopted within two rescan intervals. It asserts that the container never restarted, because a restart would pass the endpoint check. A config-less pass asserts the rename against a real server. The network has an explicit /24, so the sweep covers 254 hosts and ends in seconds. The README documents the rescan, the cadence rules and the scenario.
A freeze-frame captured for a plugin-backed entity reaches a client with an empty topic and an empty message_type. Both are correct. The values are the plugin's live entity data, and no ROS message carries them. But no field names the origin of the snapshot. Two such frames from different bridges look the same. A frame also looks like a topic capture that lost its metadata. Frame now carries the capture path that read it, and the gateway serves it as x-medkit.source. The value is plugin_data_provider for a read through the owning plugin's DataProvider. It is plugin_x_plc_data_route for the in-process dispatch of the plugin's own x-plc-data route, which bridges without a DataProvider use. topic and message_type stay empty, because neither names a ROS topic here. When the capture names no path, the key is absent from the payload. A fault manager freeze-frame taken from a real topic is unchanged and carries its topic and message_type as before. The snapshots tutorial describes the field.
The gateway runs four nodes in its own process, all named after itself: the gateway node, "<node>_sub" for the subscription executor, "<node>_fault_clients" for the fault-service transport, and "<node>_lifecycle_state_reader" for the lifecycle reader. None of them starts with an underscore, so the ROS 2 hidden-node convention does not cover them. Runtime introspection returned all four as ordinary Apps. An operator who browsed /api/v1/apps saw four entries that answer nothing useful. count_peer_nodes knew the gateway's own FQN plus "_sub" and "_fault_clients", but not the lifecycle reader. The app filter knew only the underscore rule, so it dropped none of the four. Both now use one predicate, is_own_gateway_node, so a fifth helper is declared in one place. The match is exact for each suffix. A real peer named "<node>_monitor" or "<node>2" stays visible, because hiding a real node is the worse error. A fault_manager in the same process is not ours and stays listed. Remote entities are left alone. A peer's helper nodes carry the same fully qualified names, and the peer's own filter handles them.
The route and DataProvider loss-of-comms tests now each assert the Frame::source constant that their own capture path must produce. They also assert the literal wire value. A comparison of symbol against symbol stays equal when the two constants swap values, and the string is what every consumer of x-medkit.source reads. The merge test that omits the key stays. It covers the helper contract for a frame that a caller built without naming a path. The capture paths always name one, and the test comment says so. The peer-node count test lists all three helper nodes that the gateway creates in its own process, the lifecycle reader included. A lone gateway with a lifecycle reader must count zero peers and give the empty-graph warning. The REST fault snapshot reference documents the source field of an entity frame. An entity frame carries no topic or message type, so source is the only provenance a consumer gets.
…uffer
Scoped clear. The per-entity route DELETE /{entity}/faults/{code} calls
FaultProvider::clear_fault when a plugin owns the entity. On its own
clear path the gateway sets skip_correlation_auto_clear. An operator
scoped to one entity must not cascade-clear correlated symptoms that
apps in other entities reported, and the ClearFault contract documents
that guarantee. This plugin sent the flag as false, so the guarantee
failed wherever a PLC was involved. The plugin now sets the flag. A
ClearOrigin travels with every clear and says why:
- the device reported the condition inactive. This is a real
resolution, so the cascade stays.
- the link came back.
- an operator cleared the fault through the scoped route.
The poller's own clear and a device alarm use the same callback.
clear_origin_for_signal tells them apart by an exact fault-code match.
Pending buffer. The buffer gave up any clear before any report. Only the
link-state clear can be derived again, because the next reconnect sends
it again. A device alarm's inactive edge is as one-shot as its raise.
When the buffer evicted it, the flush replayed the raise alone, and the
fault stood while the device said inactive. A full buffer now gives up
the link-state clear first. Everything else ages out oldest-first.
Sweep cancellation. The start-up sweep runs inside set_context(),
during node construction, before the gateway's executor spins. Nothing
can set the shutdown flag then, so a SIGTERM during a wide sweep waited
for the sweep to finish. Both sweeps now ask discovery_cancelled(). It
reads rclcpp::ok() next to the shutdown flag and applies the rule in
discovery_cancelled_for. rclcpp's own signal handler turns rclcpp::ok()
false, so the start-up sweep ends on the signal. The rescan still ends
on shutdown(). The README says what ends each sweep.
A rescan sweep that throws now stamps the cadence on its way out, so
the next poll iteration does not start another sweep at once. The
"scanning [subnets]" line is logged before the sweep, at INFO on the
first pass and at DEBUG on a rescan. A long sweep with no output looks
like a hung process. The comment on the start-up scan gives the right
reason why its report comes out in full.
Tests. The two clear-origin sites that need a live session run against
the test_alarm_server fixture, with stub fault-manager services on the
ROS graph, so the tests read the flag off the wire. One test requires a
successful connect to clear PLC_COMMS_LOST without a cascade. A second
fires and clears a condition through the fixture's CLI and requires
that clear to keep the cascade, next to the connect-time clear of the
same run. The fixture harness gained a stdin pipe for these commands.
clear_origin_for_signal and discovery_cancelled_for are tested on both
branches. A test shuts a private rclcpp context down and reads
rclcpp::ok() back. The comment on the cancellation test states what it
exercises.
Prose semicolons become periods and commas in plugin comments,
operator-visible log strings, the README, the docker scenario script,
rest.rst, gateway_node.hpp and test comments. The doc comment of the
Refused outcome says what it means.
The app filter now removes only the helper nodes that the gateway runs in its own process: "<gateway>_sub", "<gateway>_fault_clients" and "<gateway>_lifecycle_state_reader". The gateway's own node stays an App. SOVD serves its ROS parameters as that App's configurations, and two gateways that watch one graph have to agree on what is on it. The predicate becomes is_own_gateway_helper_node, and the peer count still skips the gateway's own node. A feature test launches a gateway with its real process node names. The suite's usual launch remaps all four nodes to one name. The test pins both halves: the gateway is listed, addressable and configurable, and each helper is on the graph and absent from the app list.
…ning goal An accepted stop leaves the goal CANCELING or CANCELED. Which one a caller sees depends on how fast the server winds down. The body renders the status that the handler read. That status is at or before the one the test reads afterwards. A goal still CANCELING therefore pins the body exactly, and a goal already CANCELED allows either rendering. The test still rules out a goal that keeps running or completes. The fixture server succeeds at the requested sequence length, one element per 100 ms tick, so the order sets how long the goal stays cancellable. At 20 the goal finishes on its own in under two seconds. A loaded or instrumented runner can spend that time on the create and the cancel round trip, and the assertions then describe a completed goal. The test uses 50, the largest order the server accepts. The tracked-goal helper returns its optional, and the caller asserts, so a missing goal gives a readable failure. Dereferencing it after a non-fatal expectation would be undefined behaviour.
The e2e tests that drive the plugin against a live fault-manager stub spin a MultiThreadedExecutor on a thread of their own. A gtest ASSERT_* returns from the middle of the test body. A std::thread destroyed while it is still joinable calls std::terminate. The run then ends in SIGABRT, the assertion message never reaches the report, and the alarm-server child outlives the test and holds its port. An RAII guard now owns the thread. stop() cancels the executor and then joins the thread. A cancel issued before spin() has begun is refused and lost, so the guard issues the cancel again until the spin function reports that it returned, and gives up with a message after 10 s. A cancel that throws is contained and issued again, and the join runs whatever the cancel did. stop() is idempotent. It sets its flag after the join, so a failed attempt can run again. The destructor catches what cancel() or join() throw and prints it, because an exception that escapes a destructor also ends in std::terminate. The test has recorded its verdict by then. The guard takes the cancel as a callable that defaults to the executor's own. rclcpp::Executor::cancel() is virtual on Jazzy and later but not on Humble. There, a derived executor's cancel() does not compile with override, and a call through a base reference would not reach it. A test passes a cancel that throws after it stops the spin, and the test reaches the end of its body.
The fixture's open62541 is built with UA_MULTITHREADING 0, so only one thread may use the server. The command handler ran on a second thread. It wrote condition fields and triggered condition events there while UA_Server_run iterated on the main thread. The mutex it held was taken nowhere else, so it protected nothing against the server. The reader thread now only queues command lines. The main thread drives the server with UA_Server_run_startup, run_iterate and run_shutdown, and it runs each queued command between iterations. The address space and the Alarms & Conditions subsystem have a single user. The reader ends on EOF. The harness produces it by closing its write end of the pipe before it signals the server. The quit branch does not close the process's own stdin, because close(2) does not unblock a reader that already waits in read(2). Startup and shutdown each report their own bad status on exit. The return value of run_iterate is how long the server may idle, so it bounds the pause between iterations.
The three helper FQNs are not built the same way. The subscription executor passes the gateway's own namespace, so its node follows the gateway. The fault-client and lifecycle-reader nodes are built from the gateway's node name alone and take the process default namespace. A remap that names only the gateway, "-r <gateway>:__ns:=/x", moves the gateway and the subscription node and leaves the other two behind. The gateway then served both of them as diagnosable apps and counted them as peers. The predicate derives each helper's FQN from its own creation site. It matches both spellings for the two nodes that do not follow the namespace. A declared app that is bound to a helper node is dropped with a warning that names the app and the node. An app is declared when its source is the manifest, the inventory or a plugin.
…ared The undeclared-node warning on GET /health is an instruction: "Declare them in the manifest". It named the three nodes that the gateway runs in its own process. A manifest app declared for one of them is then removed by the app filter, so the two halves contradicted each other. The orphan scan in RuntimeLinker skips the helper nodes with the same predicate that the app filter uses. A linker built without a node has an empty self FQN, which matches nothing. A feature test drives a hybrid gateway with "unmanifested_nodes: error" and pins both halves: the report names an undeclared node of the test's own and none of the helpers.
The gateway creates its helper nodes at three points of start-up. Two of them come after the first refresh and after /health starts to answer. A settle gate that waits for the gateway's own node can therefore read a list built from a graph with no helper at all. The absence assertions then pass whatever the filter does. The settle sequence now proves its own order. First all three helpers are on the graph. Then the test creates a witness node. Then it reads an /apps snapshot that lists the witness, so that snapshot came from a graph that held the helpers too. The three stage budgets add up to well inside the file's ctest timeout, so a failure reports its assertion before ctest kills the test. The config reference documents the rule. It lists the filtered nodes and says why the gateway's own node stays an App. It explains the namespaces of the helper nodes and what the switch exposes again when it is off. It says that the helpers stay out of the unmanifested_nodes report, and that a declared App bound to one is dropped with a warning.
An app declared against one of the gateway's helper nodes is a static misconfiguration. refresh_cache() runs on every graph event and on the backstop cadence, which the integration fixtures set to one second, so the warning repeated for the life of the process. It now fires only when the set of offending apps changes, through remember_dropped_declared_apps. The entity-cache warning in the same function fires once per run. This one also fires again when the set changes. The predicate's doc comment and the config reference describe what the root-namespace spelling costs. A helper-named node at the root counts as plumbing whichever gateway created it. Two gateways that keep the default node name and differ only in namespace build the same fully qualified name for it. The comment in the /health handler states that the orphan list excludes the helper nodes. Each app-filter declaration sits under its own doc block, so doxygen attaches every block to the right function.
…ateway The three helper node names take two shapes. A remap that names only the gateway moves the gateway and the subscription node into a namespace. The fault-client and lifecycle-reader nodes stay at the root. The predicate builds that second shape from the gateway's name. A new feature test launches exactly that invocation. It asserts that the split is on the graph. It pins the app list to the gateway plus the test's own two nodes.
…p filter is on RuntimeLinker takes discovery.runtime.filter_internal_nodes. While it is true, the gateway's own helper nodes stay out of orphan_nodes, which feeds linking.orphan_count and the unmanifested_nodes warning on /health. While it is false, the helpers are served as Apps and counted like any other undeclared node. discovery-options.rst and the comment in the health handler describe this coupling. A unit test links the same runtime apps with the setting on and off and checks a helper node against a real unmanifested node. Comments in fault_handlers.cpp and in six gateway and integration-test files are reworded to state what the code does.
…ep the PLC's order Comms-lost clear. The plugin decides the PLC_COMMS_LOST clear on the poll thread, and only while the link is up. It asks the fault manager who reported the standing row. It clears the row only when every source is an id that this bridge reports under: the current component id, the stand-ins it assigned and the NodeMap default. A row that two bridges hold stays for an operator. - One fault-store read is outstanding at a time. It is stamped with the probe and the connection generation. - A read that the store does not answer within fault_service_timeout_ms is dropped and taken again. - An answer from before a link drop is discarded, and the decision is owed again. - A link-state clear that the bounded buffer could not keep is owed. It is decided once the sink is ready. Order. The pending buffer merges a clear into an earlier pending clear for the same code only when nothing was reported for that code in between. So report, clear, report, clear flushes in that order. Identity in config-less mode: - The connect hook derives the component name from the device again before the session subscribes, so replayed conditions land on the new entity. - A nameplate replaces a stand-in. Nothing replaces a nameplate. - An empty read renames a stand-in only when the endpoint changed. - Reads are budgeted per session. - Conditions that the poller already pinned move to the new entity. - The event path reads a copy of the alarm routing that the poller owns. Binding. The plugin remembers the ApplicationUri of the server a session reached. A rescan while disconnected looks for that server at any address and refuses every other server. A different server at the bound address is dropped at connect. The first adoption, a configured endpoint_url and a server without an ApplicationUri have no binding constraint. The README says so. A rescan that throws is caught on the poll thread. The cancel predicate reads the node's own context. Tests. End-to-end tests against the fixture cover: - a row that two bridges hold, left standing - an owed clear, sent after the buffer drains - no decision while the link is down - an unanswered probe, dropped and taken again - a stale answer after a probe timeout and after an outage - a fault raised under a stand-in, healed once the device names itself - the binding across an address change and a swap - the connect hook, run before the event routing is copied Unit tests cover the clear gate, the probe conditions, the identity rules and read budget, the buffer order, bound selection, the guarded rescan, the context-aware cancel, and the poller's routing copy and repin. The fixture takes --app-uri, so two fixtures can stand for two PLCs.
…e restart checks The config-less pass asserts that the stand-in component left /components in the same sample that shows the renamed one. The containers run with no restart policy, so a gateway that dies is reported by the endpoint and component assertions; the RestartCount checks are gone. The deadline follows RESCAN_INTERVAL_S like its siblings.
…, and own every reconnect Binding file. When the binding is established, the plugin writes the bound ApplicationUri into discovery.binding_file. The default path is /var/lib/ros2_medkit/opcua/binding. OPCUA_DISCOVERY_BINDING_FILE sets it, and an empty value turns it off. configure() reads the file, so a restart keeps the identity. The start-up sweep and every rescan then select only the bound server, at any address. A different server at the bound address is dropped at connect, and the outage stays standing. The refusal names the file as the way to recommission the box. A configured endpoint_url neither reads nor writes the file. A server that publishes no ApplicationUri is never persisted. The file is written to a temporary file, fsynced and renamed over the resolved target. It holds one line. On read, a UTF-8 BOM and the blanks around the URI are stripped, and anything after the first line is ignored. A control character in the first line is refused with a warning and reads as no binding. A URI with a line break is refused at the write. Reconnects. The reconnect arm owns every reconnect. After each client iterate the client reads its session state. A session that the library opened again on its own is dropped and handed to the arm. The arm connects through its own path, runs the binding check and creates the alarm subscription again. Every path that marks the client disconnected drops the subscription bookkeeping of the dead session. The new monitored item therefore delivers events again after a PLC reboot. Tests. Unit tests cover the file format and the write contract. End-to-end tests against the fixture cover: - a restarted plugin that refuses a foreign server at the bound address - a configured endpoint_url that ignores the file - the start-up sweep that picks the bound server over a lower address - a swap and an outage under a config-less session - a device alarm delivered after the bound server reboots The ctest timeout of the identity E2E is twice the idle pace plus one stuck test's wait budget, net of its passing time. The registration shows the arithmetic.
…check each built item test_a_scoped_item_says_what_its_templated_sibling_says compares every entity in both listings, counts built items per entity type, collection and scope, requires a built item for every listed resource, and checks that the item at each key names that resource. test_every_advertised_collection_is_served follows every app's hrefs and requires a cache-built href per entity type. The class waits for calibration's operation through REQUIRED_OPERATIONS and polls temp_sensor's data with the discovery budget before comparing, so the counters do not depend on how fast the runner propagates a service.
…dentity check covers The binding section of the README covers: - the path and the format of the file - the rule that only discovery uses it - how to clear it - the start-up sweep and the rescan behaviour - who owns a reconnect, and the cases the end-to-end tests cover - the probing cadence at a refused address - the last eight refused URIs - one file per plugin instance - the mount that a re-created container needs - the configuration and environment rows
… the plugin's own threads The ReportFault, ClearFault and GetFault clients live in a callback group that is not added to an executor together with the node. A single-threaded executor owned by the plugin serves that group from a thread the plugin starts in set_context. shutdown stops the poller, stops and joins that thread, and then drops all three clients on the calling thread, so a client destructor never runs on a gateway executor thread while another plugin or the gateway creates an entity on the same node. The GetFault answer is parked from the plugin's thread under the probe state's mutex, as before. The client thread also ends once the node's context has shut down. set_context is documented as called once per plugin instance. A new end-to-end case builds two plugin instances in sequence on one node while the gateway's executor spins, and checks that the first shutdown joins the client thread and that the second instance's clients reach the fault services.
EntityDetailStatusLinkTest and LifecycleHandlersWithProviderTest initialise rclcpp when it is not running, and their TearDownTestSuite now shuts it down the way LifecycleHandlersTest does. The test process exits with no rclcpp context left initialised, so static destruction does not run alongside live middleware threads.
…and name the test stubs' parameters The GetFault answer and the fault_service_timeout_ms config value are read through const references, and the persisted-binding and rescan results are returned from non-const locals so they are moved out. The severity band order check is written as the direct comparison of the integer bands. The GetFault response callback keeps its future by value, because rclcpp offers the response-callback overload of async_send_request only to a callable taking exactly SharedFuture; the check is suppressed at that line with the reason next to it. In the identity tests, the stub definitions name every parameter, the child process fixture and the rclcpp scope guard are neither copyable nor movable, and the fault store stub's service callbacks take their arguments by const reference.
…ning The DDS RMWs take a participant's nodes from the ros_discovery_info topic on a listener thread, and its enclave from participant discovery. When the listener thread takes a discovery sample only after its participant was removed, the sample re-creates the participant entry and nothing removes it again: the graph keeps listing the node, with an empty enclave and none of its endpoints, for as long as the gateway runs. An empty enclave alone does not identify such a leftover, because nodes behind a DDS router and nodes whose participant was created outside rcl read the same way. The node-list reader leaves an entry out only for a name it saw running and that has departed, when no other entry of that name has an enclave and the graph resolves no publisher or subscriber for the node. Names it never saw running are listed. The names running on the previous read are that read's own list and cost nothing. A name that ran on the previous read and does not run now has departed. The reader forgets a departed name when it runs again, or on the first read that finds no entry of it more than 10 s after the first read that found none; while a leftover of it is listed, that time does not start. A late sample that arrives before the read that forgets the name stays hidden, however late it comes. The 10 s cover the listener thread's delay in taking a sample it already holds. The reader remembers at most 1024 departed names; past that it forgets the names no entry has listed for longest first, then the names that departed longest ago. Endpoints are read with the publisher and subscriber queries without demangling, which on these RMWs also return service and client endpoints. Only rclcpp's error for a node the graph no longer lists counts as no endpoints; any other error, such as the one a shut-down context raises, reaches the caller. Discovery owns one reader and the startup peer count reads through it, so both count the same nodes. Discovery logs a warning when it starts leaving a node out. The check for the gateway's in-process helper nodes moves into the same header-only file, so the discovery plugins can use it. parameter_beacon reads the graph through a reader of its own, and skips the gateway's own node and its helper nodes, which carry no beacon. It keeps one parameter client per poll target across cycles: an rclcpp client for list_parameters and one for get_parameters, spun on the parameter client's own executor for the length of a call. Every call gives up after param_timeout_sec and removes the unanswered request from its client, so a node whose parameter services never answer neither stalls polling nor leaves requests pending. The waits for the list and get services share one deadline, and each wait gets the time left, at least zero: rclcpp waits forever when it is given a negative time. A timeout puts the node on backoff and any answer clears it, including a get answer without values: rclpy answers so when a typed parameter has no value, and the plugin then stores no hint. A node that is no longer a target, also when no node is, loses its client and its backoff. Only the poll thread spins the plugin's node. The poll thread destroys the clients, and so does shutdown() after it has joined that thread. The plugin's node joins rclcpp's graph listener when it is created: a first join after rclcpp shuts down fails half-way, and the node's destructor then terminates the process. A try/catch around reset() cannot stop that, because reset() and the destructors it runs are noexcept, so both beacons drop the try/catch they had there in shutdown(). parameter_beacon's destructor still catches what shutdown() throws, because locking clients_mutex_ can throw std::system_error. topic_beacon's shutdown() throws nothing, and its destructor calls it directly. Every duration parameter_beacon reads takes its minimum up to 2147483647 s. Fast DDS keeps the seconds of a wait in a signed 32-bit field, and a longer param_timeout_sec makes the poll thread spin. A larger value, +inf included, becomes the maximum; NaN, -inf and a smaller value become the minimum. Each replacement is logged with the value that replaced it. topic_beacon applies the same rules to beacon_ttl_sec and beacon_expiry_sec (up to 2147483647 s) and to max_messages_per_second (1 to 10000), so NaN no longer drops every beacon or keeps hints forever, and +inf no longer removes the rate limit. Both beacons read max_hints as a 64-bit integer before they narrow it. A 64-bit integer below 1 becomes 1, and one above 2147483647 becomes 2147483647, each with a warning, so a value such as 4294967297 no longer wraps to a capacity of 1. The parameter parser reads an integer that does not fit in 64 bits as a double. A double, such as 1e12 or NaN, is refused with a warning and the default of 10000 stays.
bburda
force-pushed
the
fix/opcua-rescan-comms-lost-freeze-source
branch
from
September 18, 2026 18:09
966de76 to
7bb8958
Compare
…der exposes ghost_node_injector gains --leftover: it runs a node in an rclcpp context of its own, captures the discovery message that node's participant publishes, removes the participant on request and, after a given delay or on a publish command, publishes the captured message again, the way a late sample re-creates the node. The participant GID is derived from the node's publisher GID and has to match a received discovery message before the node is reported ready. --announce adds more nodes to that participant's message. --ghost and --backed GIDs now sort before those of real participants, and their messages go out one per node in command-line order. test_graph_leftover_nodes checks discovery on one gateway: the leftover of a listed node is hidden when its late sample arrives inside the hold; nodes never seen running without an enclave, next to a ghost of their name, and a running node without endpoints are listed; a restarted node is listed once; a leftover does not rename a live node sharing its bare name; a node seen running that stays behind an endpoint is listed; 50 leftovers leave live nodes and a node started afterwards listed. The endpoint case stops the node only once GET /apps lists a ghost sent after the backed entry of its name, so the gateway's graph holds that entry when the node exits. The last case checks that leftovers the earlier cases hid are still hidden once they have been listed for longer than the hold, and that a late sample for a node that departed with no entry of it left is listed once a refresh ran past the hold. Every case first checks that the test's own graph lists the leftover, and cases that rely on the gateway's graph holding it check the gateway's warning. test_graph_leftover_nodes_scale announces more running nodes than discovery remembers and checks that a node departing next to them is still hidden. It then leaves all of them over and checks how many stay hidden, that a node departing with no entry of it left is the first name forgotten past the capacity, and that a new node is listed. test_graph_leftover_nodes_startup checks the startup peer count, and that parameter_beacon polls a node it never saw running and does not poll the leftover of a node it polled, after checking that the plugin's node shares the gateway's participant and that discovery warned about the leftover. test_param_beacon_out_of_range_config loads parameter_beacon once per sweep point: NaN, -inf, below the minimum, the minimum, the maximum, just above it, +inf and 3e9, plus one instance with a working configuration. It checks every logged clamp and that in-range values log none. Instances at the low points keep polling past nodes that never answer; instances whose timeout sits at the high points send one request to such a node and wait there, and no gateway thread uses more than half a core meanwhile; instances whose interval sits at the high points run at most one cycle. The working instance polls an rclpy node whose typed, unset parameter makes its get answers carry no values on every cycle, backs off a node whose get requests go unanswered, and keeps the request writer of its client for a node that never answers across cycles. The test counts a list answer when it is sent and a get request when a polling thread takes it, so it compares the two only on samples where every list answer has its get counted, and fails when the two counts do not settle to the same number. test_topic_beacon_config_bounds sweeps the same points over beacon_ttl_sec, beacon_expiry_sec and max_messages_per_second, checks every logged clamp, that each instance stores a beacon, and that a burst of ten is cut to one or two at 1 per second and passes whole at 10000 per second. Both tests load one more instance per max_hints point: 0, -5, 1, 2147483647, 2147483648, 2^32 + 1, 1e12 and NaN. They check each warning, that the instances whose max_hints ends at 1 keep one of two hints and are the only ones that reach capacity, and that the others keep both. The instance each test loads first answers the beacon endpoint and has NaN TTL and expiry. The tests give it one beacon for a node the gateway lists, stop refreshing it and check that the endpoint reports it stale and then removes it. topic_beacon's test checks that the removal comes no earlier than 1 s and within a few seconds. parameter_beacon's test stops refreshing while the beacon is active and times each change from the node's last get answer. The beacon must go stale between 0.3 s, its TTL, and 0.3 s later, and be removed between 1 s, its expiry, and 0.3 s later. The 0.3 s margin grows with the test time scale. The unit tests cover the reader's decisions, the hold, the eviction order and that running names do not count against the capacity, and graph_node_has_endpoints against a real rcl node before and after its context is shut down. The parameter_beacon unit tests run the real parameter client against parameter services that do not answer, answer without values or answer normally: a timed-out request leaves nothing pending, a node keeps one client across cycles, a graph read without targets drops every client, the gateway's helper nodes are not polled, a wait with no time left returns within 1 s for a service that does not exist, and the plugin's node can be destroyed after rclcpp shuts down. The leftover tests scale their default wait for the gateway's warning with the test time scale. The graph_watchdog ghost_departure scenario runs the watched node inside the fixture and expects GRAPH_NODE_DISAPPEARED once it leaves a leftover the gateway warned about. test_node_death_integration's X2 case attributes a fault report to a tick only after the fake fault service has recorded it.
bburda
force-pushed
the
fix/opcua-rescan-comms-lost-freeze-source
branch
from
September 19, 2026 09:49
7bb8958 to
f6dc898
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In config-less discovery mode (
OPCUA_DISCOVERY_ENABLED=1, subnets set, noOPCUA_ENDPOINT_URL, no node map), a gateway that boots before its controller neverfinds it. The start-up scan runs while the controller is still coming up, finds nothing,
falls back to
opc.tcp://localhost:4840and retries that endpoint until someonerestarts it. Three more things read wrong beside it: the gateway lists its three helper nodes
as apps, a plugin entity's freeze-frame names no origin, and a
PLC_COMMS_LOSTraised before a restart stays CONFIRMED, because only the process thatraised it would clear it.
CI also showed the gateway listing a node after its process exited. The DDS RMWs can
re-create a removed participant from a late discovery message, and the graph then lists
the node, with no enclave and no endpoints, until the gateway stops. graph_watchdog reads
presence from the same list, so it can miss that node's death. The parameter_beacon and
topic_beacon configuration checks let NaN, infinite and out-of-range values through; they
are fixed here too.
What changes
Discovery and reconnect
endpoint_urlconfigured, the reconnect arm rescans ondiscovery.interval_s, adopts a server that appeared after start-up and resets thebackoff. The cadence is measured from the end of a sweep, the backoff is capped at it
so the documented interval is the real one, and an unchanged outcome reports at DEBUG.
The plugin is bound to the ApplicationUri of the server a session reached: the start-up
sweep and every rescan look for that server at any address and refuse every other, and a
different server answering at the bound address is dropped at connect with the outage
left standing. The binding is written to
discovery.binding_file(default/var/lib/ros2_medkit/opcua/binding, envOPCUA_DISCOVERY_BINDING_FILE, emptydisables) when it is established and read at start, so a restart keeps the identity;
removing the file and restarting is the recommissioning gesture. The first adoption of a
process with no file, a configured
endpoint_urland a server that publishes noApplicationUri are outside the binding. One file belongs to one plugin instance.
The reconnect arm owns every reconnect: after each client iterate the session state is
read, a session the library re-opened underneath is dropped and handed to the arm, which
connects through its own path, runs the binding check and re-creates the alarm
subscription. Every path that marks the client disconnected drops the session's
subscription bookkeeping, so the re-created monitored item delivers again.
throws is caught on the poll thread, logged at WARN, and still stamps the cadence.
is up, before the session subscribes to events, so the provisional
opcua-<host>namegoes and the conditions the server replays land on the new entity. A name the device
gave itself is never replaced by the fallback; an empty read is retried a bounded
number of times per session (one read at connect, up to three more in the connect
hook, then at most five on later polls). Conditions the poller already pinned under
the old alarms entity are moved to the new one; the event path reads a copy of the
alarm routing owned by the poller, refreshed from the poll thread.
Comms-lost clears and the pending buffer
PLC_COMMS_LOSTand clears it only when every reporting source is an id this bridgereports under: the current component id and the
opcua-<host>stand-ins it assignedin this process. A row another bridge holds is left standing. The decision runs on the
poll thread, only while the link is up, with one fault-store read outstanding at a
time; a read the store does not answer within
fault_service_timeout_ms(new key,default 5000) is dropped and taken again, and an answer that arrives after the link
dropped is held until the next connect. A link-state clear the bounded buffer could
not keep is owed and decided again once the sink is ready.
skip_correlation_auto_clear. Aclear the device itself reports still cascades.
fault_manageris unreachable keeps the PLC's order: a clear iscoalesced onto a pending clear for the same code only when nothing was reported for
that code in between, so report, clear, report, clear flushes in that order. A device
alarm's inactive edge ages out like a report.
Gateway entity listing
<node>_sub,<node>_fault_clients,<node>_lifecycle_state_reader) for the peer count and the appfilter, matched per exact suffix so a peer named
<node>_monitoror<node>2staysvisible. The gateway's own node stays an App: its parameters are that App's
configurations.
_subfollows the gateway's namespace; the other two are created in theroot namespace, so a helper-named node there counts as plumbing whichever gateway created
it. While
discovery.runtime.filter_internal_nodesis true the same predicate keeps thehelpers out of
/health'slinking.orphan_countand out of theunmanifested_nodeswarning; with the flag false they are served as Apps and counted there like any other
undeclared node. A manifest App bound to a helper is dropped with one warning per
distinct set. The peer count excludes
_lifecycle_state_reader. Remote entities areuntouched.
Freeze-frame source
x-medkit.source,plugin_data_providerorplugin_x_plc_data_route.topicandmessage_typestay empty, and the field isomitted when the capture named no path.
Docs
docs/api/rest.rst,docs/tutorials/snapshots.rst,docs/config/discovery-options.rst.Graph leftovers
ros2_common/graph_node_list.hpp, header-only) leaves out anentry without an enclave for a node it saw running and that has departed, when the
graph resolves no publisher or subscriber for it. A node it never saw running is
listed, so nodes behind a DDS router and micro-ROS nodes read as they do on main. A
departed name is remembered until it runs again, or until a read finds no entry of it
more than 10 s after the first read that found none. At most 1024 departed names are
remembered. Discovery logs one warning when it starts leaving a node out. Discovery and
the startup peer count share one reader; parameter_beacon has its own.
bare-name collision check, so it cannot rename a live node that shares its name.
Discovery plugins
executor. A request that times out is removed from its client, so a node that never
answers leaves nothing pending. The list and get waits share one deadline,
param_timeout_sec. A get answer with no values (rclpy does this for a typed parameterwithout a value) stores no hint and causes no backoff. The gateway's own node and its
helper nodes are not polled. A graph read with no targets drops every client.
minimum; values above 2147483647 s, +inf included, become 2147483647 s, because Fast
DDS keeps the seconds of a wait in a signed 32-bit field. Each replacement is logged.
max_messages_per_secondis limited to 1..10000.max_hintsis read as a 64-bit integer before it is narrowed: below 1 becomes 1,above 2147483647 becomes 2147483647, with a warning. The parameter parser reads an
integer that does not fit in 64 bits as a double. A double, such as 1e12 or NaN, keeps
the default of 10000.
docs/config/server.rst(departure section),docs/config/discovery-options.rst,the READMEs of the gateway, parameter_beacon, topic_beacon and graph_watchdog, and the
two beacon design pages.
Behaviour changes to know about
interval_sunset means the 30 s default, an explicit0means discovery on with norescan. The old README line was
interval_s: 0, so a config copied from it gains norescanning on upgrade. Drop the key for the default. A negative value is refused.
This one is breaking.
DELETE /{entity}/faults/{code}served by this plugin now setsskip_correlation_auto_clear, as theClearFaultcontract already promised./api/v1/appsentries disappear with the defaultfilter_internal_nodes: true:the gateway's helper nodes. The gateway's own node stays.
PLC_COMMS_LOSTthat two bridges raised into one fault manager is not cleared byeither link coming back; an operator clears it. The fault manager has no per-source
de-assert today.
fault_service_timeout_ms(default 5000, out of range refusedwith a warning).
comms_lost_debounce_mslogs a warning when it clamps.discovery.binding_file. The identity survives a restart; areplaced PLC at the bound address is refused until the file is removed. In the shipped
image the directory exists but persists across a re-created container only when mounted.
noticed: the outage is reported and the alarm subscription is re-created on the bound
server when it returns. Before, the client library re-opened the session silently and
device alarms stayed dead until a restart.
GET /appsstops listing a node the gateway saw running once only a leftover of itremains, and the gateway logs one warning for it.
max_messages_per_secondabove 10000 is lowered to 10000 with a warning.A config with a higher rate, or with
.infto remove the limit, now gets 10000 persecond.
with a warning. On main a
NaN passed the topic_beacon checks: it then dropped every beacon or kept hints forever.
Testing
contract tests that failed on every distro (
test_configuration_api,test_locking_disabled_contract,test_relay_peer_credential,test_unmanifested_policy) pass, with three new feature tests beside them(
test_own_node_apps,test_own_node_apps_namespaced,test_own_node_undeclared)that fail when the helper filter or the own-node rule is broken on purpose.
test_operation_handlerspins the stop response on the contract (goal CANCELING orCANCELED), 10 of 10. The alarm-server test fixture runs every
UA_Server_*call on the server's own thread, asUA_MULTITHREADING 0requires, andthe identity tests tear their executor down on every exit path.
the stand-in heals after the device names itself; a row two bridges hold is left
standing; no decision is taken while the link is down; a probe the store never answers
is dropped and taken again; an answer to a timed-out probe is not read as the next
one's; a link-state clear the buffer refused is sent last once the sink is ready; the
connect hook runs before the event routing is copied; a rename moves the conditions
already pinned. Unit tests pin the gate predicate, the decision predicates, the buffer
order (a clear never merges past a report for the same code), the identity rule, the read
budget and the burst cadence, the guarded rescan, the node-context cancel, and the
linker skip in both flag positions. Each new test was made to fail first. The binding
file has unit tests for its format (one line, BOM, control characters, CRLF, a second
line, a symlinked path, an unwritable parent) and e2e tests against the fixture: a
restarted plugin keeps the binding and refuses a foreign server at the bound address, a
configured
endpoint_urlignores the file, the start-up sweep selects the bound serverand not the lowest address, a swap and an outage under a config-less session are caught,
and a device alarm is delivered after the bound server reboots.
test_openapi_contractsweeps every entity, requires a built item for every listedresource, checks the item each one names, and waits for calibration's operation through
the shared class wait.
run_integration_tests.shTotal: 18 / Passed: 18,run_alarm_tests.shpassed,
run_discovery_race_test.shpassed. That race scenario fails on the mergebase at step 6 of 9, the adoption assertion (
endpoint still 'opc.tcp://localhost:4840' (connected='False') after 50s), and passes on the branchthrough step 9 of 9, the config-less rename, which now also asserts the stand-in
component left
/components.PLC_TANK_HIGHstill CONFIRMED afterPLC_COMMS_LOSTwas clearedthrough the per-entity route. The control with the flag off cascades.
uncancelled.
uninstrumented rclcpp and is not usable on the development machine.
its own; its participant is removed, and its captured discovery message is published
again afterwards, as the late message does. Every case first checks that the test's own
graph lists the leftover.
test_graph_leftover_nodes,test_graph_leftover_nodes_scale(past the 1024 cap) and
test_graph_leftover_nodes_startupcheck discovery, the startupcount and parameter_beacon. graph_watchdog's
ghost_departuree2e checks that thenode's death is still raised. Each rule element was mutated and turned a test red.
test_param_beacon_out_of_range_configandtest_topic_beacon_config_boundssweepNaN, +inf, -inf, below the minimum, the minimum, the maximum, above it and 3e9 on a real
gateway, plus
max_hints0, -5, 1, 2147483647, 2147483648, 2^32 + 1, 1e12 and NaN. Abeacon with NaN TTL and expiry is observed going stale and then removed.
parameter_beacon 44, topic_beacon 30, all with 0 failures. The changed tests also pass
on Cyclone DDS.
Not in this PR
PLC_COMMS_LOST. Thatcase has plugin-level e2e coverage with stub fault services.
source_idonPLC_COMMS_LOSTin the/api/v1/faultslist view while thefault manager logged a source. Unverified, filed on its own if it holds.
gateway_nodeexiting 139 on shutdown with the plugin loaded. It reproduces onmain,see [BUG] gateway_node segfaults on shutdown when the OPC UA plugin is loaded #659.
one shared
PLC_COMMS_LOSTrow.services, and so no operations, because the graph answers per-node queries from
whichever of the two participants sorts first.
docs/config/server.rststates it.Issue
Type
Checklist