feat(opcua): raise PLC_COMMS_LOST fault on connection loss#507
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds debounced “PLC communications lost” fault emission to the OPC-UA plugin so prolonged connection loss becomes visible via /faults, aligning behavior with other PLC bridges and issue #496’s expectations.
Changes:
- Introduces a debounced comms-lost raise/clear path in the OPC-UA poller (
PLC_COMMS_LOST, component-scoped). - Adds configuration knobs (YAML + env overrides) for enabling and tuning the comms-lost fault behavior and severity.
- Adds unit tests for the pure debounce gate helper and updates plugin README parameter documentation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp | Adds unit tests for the comms-lost debounce decision helper. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_poller.cpp | Raises/clears PLC_COMMS_LOST based on debounced connection-down duration. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp | Parses new comms-lost config knobs (JSON + env overrides). |
| src/ros2_medkit_plugins/ros2_medkit_opcua/README.md | Documents new comms-lost YAML parameters in the gateway parameter table. |
| src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_poller.hpp | Adds comms-lost config fields and poller state for debouncing/idempotence. |
bburda
reviewed
Jul 6, 2026
bburda
left a comment
Collaborator
There was a problem hiding this comment.
Additional notes outside the diff:
ros2_medkit_opcua/design/index.rstdocuments the PLC condition/event to fault flow but notPLC_COMMS_LOST, which is different in kind (raised by the poll thread on connection loss, not sourced from a PLC condition node). A short note there would keep the design doc in step with the README.- Only the pure
comms_lost_should_raisegate is unit-tested. The clear path (emit_comms_lost(false)plus resettingcomms_lost_raised_andcomms_down_since_on reconnect) has no coverage and is the more state-heavy half. Extracting a pure reconnect-transition helper would let it be tested the same way as the raise gate.
bburda
previously approved these changes
Jul 7, 2026
The poller raises a single component-scoped PLC_COMMS_LOST fault (severity ERROR) once the OPC UA connection has been continuously down for comms_lost_debounce_ms (default 5000, debouncing reconnect blips), and clears it on the next successful reconnect. Knobs: comms_lost_fault_enabled / comms_lost_debounce_ms / comms_lost_severity.
Clamp comms_lost_debounce_ms to >= 0 and keep the existing value on a non-numeric/negative OPCUA_COMMS_LOST_DEBOUNCE_MS (atoi parsed it as 0, silently disabling the debounce). Validate comms_lost_severity against the SOVD buckets and default to ERROR on a typo so the fault is not downgraded to INFO. Document the comms-lost env overrides.
Reset errno and reject ERANGE so an overflowing OPCUA_COMMS_LOST_DEBOUNCE_MS no longer saturates to LONG_MAX and silently disables the fault, and clamp both the YAML and env paths to [0, 3600000] ms.
Gate the comms_lost_raised_ latch on the ReportFault service being discovered, so a fire-and-forget report raised before DDS match is not dropped and then permanently suppressed by the raised flag. The poll arm retries on each cycle until the sink appears.
18bc2b7 to
897785e
Compare
bburda
approved these changes
Jul 7, 2026
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
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.
Raises a single component-scoped
PLC_COMMS_LOSTfault (severityERROR, configurable) when the OPC UA connection stays down, so a controller going dark surfaces on/faultsinstead of only thex-plc-statusconnectedboolean. Brings the opcua plugin to parity with the modbus/s7/ads bridges.comms_lost_debounce_ms(default 5000), so a brief blip during a normal reconnect does not flap a fault. Idempotent (raised once), cleared on the next successful reconnect.comms_lost_fault_enabled(default true),comms_lost_debounce_ms(default 5000),comms_lost_severity(default ERROR); envOPCUA_COMMS_LOST_ENABLED/OPCUA_COMMS_LOST_DEBOUNCE_MS.CommsLostShouldRaise).Closes #496