fix(gateway): freeze-frame the faults that were already raised at startup - #563
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a startup race in the gateway where faults that are already CONFIRMED at boot can miss their entity freeze-frame capture because the /fault_manager/events subscription is wired after plugins begin reporting faults.
Changes:
- Add a startup catch-up path (
capture_standing_fault_frames()) that lists currently confirmed faults after the capture is initialized and enqueues synthetic confirm events for frameless faults. - Add
EntityFreezeFrameCapture::capture_missed_fault()to enqueue a synthetic confirmed fault event through the existing bounded queue/worker. - Expose the new catch-up helper on
GatewayNodeand document the intended “missed confirm” behavior in headers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/ros2_medkit_gateway/src/gateway_node.cpp | Wires a detached startup catch-up thread to list confirmed faults and enqueue missed captures. |
| src/ros2_medkit_gateway/src/entity_freeze_frame_capture.cpp | Adds capture_missed_fault() to synthesize confirm events into the existing capture queue. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/gateway_node.hpp | Declares the new capture_standing_fault_frames() method on GatewayNode. |
| src/ros2_medkit_gateway/include/ros2_medkit_gateway/entity_freeze_frame_capture.hpp | Documents and declares capture_missed_fault() for missed-confirm catch-up. |
mfaferek93
force-pushed
the
fix/562-standing-fault-freeze-frames
branch
3 times, most recently
from
July 26, 2026 19:40
99ab9c0 to
695f691
Compare
…rtup Plugins start reporting while the capture is still being wired, so a device that is in fault when the box boots confirms before the subscription exists and its freeze frame is lost for as long as the fault stays raised. The capture now asks for the standing faults on its own worker thread - the one its destructor joins - so the wait for the fault services cannot outlive the node.
mfaferek93
force-pushed
the
fix/562-standing-fault-freeze-frames
branch
from
July 26, 2026 20:28
695f691 to
c146a1c
Compare
bburda
reviewed
Jul 27, 2026
Collaborator
There was a problem hiding this comment.
Additional findings outside the diff:
- Would you add a launch test for this?
create_test_launchalways starts the gateway first, sotest_entity_freeze_frame.test.pycannot reach the new path and the lister always sees an empty list. Starting the fault manager first, holding a confirmed fault, would prove the fix, and that test fails on main. - Could you update the docs?
docs/tutorials/snapshots.rstanddocs/config/server.rstdescribe freeze frames as captured "when the fault confirms", anddocs/tutorials/snapshots.rstanddocs/api/rest.rstdescribefreeze_frameas "captured at fault confirmation". None of them mention the startup catch-up, and the value-age caveat from your description is not written down anywhere a user would see it. - Could you extend
REQ_INTEROP_088indocs/requirements/specs/faults.rst? Its text ties capture to the confirm event, but both new tests verify the startup trigger.
bburda
reviewed
Jul 27, 2026
Make the lister abortable under one shared service deadline, order the snapshot after the events subscription matches, dedupe against queued confirms, cap stored frames at the retention bound, mark catch-up frames with a capture origin and warn on every silent failure path.
Launch the fault manager first, confirm a plugin-entity fault through a seeder process and start the gateway only on its exit, then assert via REST that the standing fault's snapshot carries a freeze frame marked with the startup capture origin.
Document that standing faults are framed at gateway start, what captured_at means for those frames and the x-medkit capture_origin marker; extend REQ_INTEROP_088 with the startup case.
Collaborator
Author
|
The outside-diff items:
|
bburda
approved these changes
Jul 27, 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.
Closes #562.
Plugins start reporting during
GatewayNodeconstruction, while the capture iswired later from
main.cpp(it needs the shared subscription executor, which isbuilt after the node). A device that is already in fault confirms in that window,
its
EVENT_CONFIRMEDis published to nobody, and since the capture only fires onthe confirm edge the fault stays frameless for as long as it is raised.
capture_standing_fault_frames()runs once the capture is live: it lists theconfirmed faults and enqueues a capture for each fault that has no frames yet.
Frames are process-local, so this restores what the missed edge would have
produced rather than duplicating anything, and
capture_missed_fault()is ano-op when the live event already captured the fault.
It runs detached, after
wait_for_serviceswith a bounded timeout, so startupnever blocks on the fault services being up. The synthetic event goes through the
same queue and worker as a real one, so the backlog bound and the plugin-call
threading rules are unchanged.
Worth naming: a frame captured this way holds the values at catch-up time, a
second or so after the confirm, not the values at the confirm itself. For a fault
that is standing at boot that is the only honest option - the earlier values no
longer exist anywhere.