Skip to content

Bundle the fault bridges in the image; docker-first quick start#470

Merged
mfaferek93 merged 3 commits into
mainfrom
fix/docker-bundle-bridges
Jun 23, 2026
Merged

Bundle the fault bridges in the image; docker-first quick start#470
mfaferek93 merged 3 commits into
mainfrom
fix/docker-bundle-bridges

Conversation

@mfaferek93

Copy link
Copy Markdown
Collaborator

What

The documented install paths did not deliver the headline value:

  • log_bridge and action_status_bridge were never COPYed into the Docker image (only diagnostic_bridge was), so ros2 launch ros2_medkit_gateway bringup.launch.py errored on the missing bridge packages.
  • The README quick start launched gateway.launch.py (gateway only, no fault_manager, no bridges), so a first-time user saw the entity tree but zero faults.

Change

  • Dockerfile: COPY ros2_medkit_log_bridge and ros2_medkit_action_status_bridge into the builder so the runtime image ships all the generic bridges. Verified end to end: with the bridges present, bringup.launch.py starts gateway + fault_manager + log_bridge + action_status_bridge cleanly, and a /rosout ERROR becomes a structured fault (GET /faults).
  • README: lead the quick start with a one-command docker run that attaches to a running graph (forwarding ROS_DOMAIN_ID/RMW_IMPLEMENTATION so it matches the user's DDS; the image already ships both Fast DDS and CycloneDDS). Frame apt install as rolling out to the ROS index, and fix the source-build snippet to use bringup.launch.py.

Pushing to main rebuilds the jazzy/humble/lyrical images via the docker-publish workflow.

Copilot AI review requested due to automatic review settings June 22, 2026 21:33
log_bridge and action_status_bridge were never COPYed into the image, so
bringup.launch.py errored on the missing packages. Bundle both. The published
apt/ghcr artifacts do not carry the bridges yet, so lead Run it with the docker
one-liner that works today and frame apt as rolling out to the ROS index.
@mfaferek93 mfaferek93 force-pushed the fix/docker-bundle-bridges branch from 03bb90f to 93d0888 Compare June 22, 2026 21:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Docker build context and top-level documentation so the “bringup” stack (gateway + fault manager + generic fault bridges) is usable out-of-the-box, especially for Docker-first users attaching to an existing ROS 2 graph.

Changes:

  • Dockerfile: include ros2_medkit_log_bridge and ros2_medkit_action_status_bridge in the image build context so they’re installed into the runtime image.
  • README: change the Quick Start to run bringup.launch.py and describe attaching the container to the user’s DDS domain/RMW implementation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
README.md Reworks Quick Start to emphasize Docker attach flow and to use bringup.launch.py instead of gateway.launch.py.
Dockerfile Copies additional bridge packages into the builder workspace so they’re present in the built/install tree.
Comments suppressed due to low confidence (3)

README.md:34

  • The quick-start text says the bringup stack will turn /diagnostics into faults, but bringup.launch.py defaults enable_diagnostic_bridge to false, so /diagnostics will not be bridged unless the user opts in. This is likely to confuse first-time users trying diagnostic_updater-based stacks.
**README.md:41**
* The quick-start `docker run ... ros2 launch ...` command won’t actually run `ros2 launch` with the current image because the Dockerfile sets `ENTRYPOINT ["/entrypoint.sh"]`, and `entrypoint.sh` always execs `ros2 run ros2_medkit_gateway gateway_node "$@"`. As written, `ros2 launch ...` becomes extra args to `gateway_node` rather than executing the launch file.

ros2_medkit's action bridge turns that into a structured fault on the bt_navigator entity -
no instrumentation, no callbacks added to Nav2.

[!TIP]
Nothing was added to Nav2. The bridge reads the action status your stack already publishes,

**README.md:46**
* The phrase “Rolling out to the ROS index now” is time-sensitive and will become stale quickly (either after the release is done, or if the release gets delayed). Consider phrasing this as a stable install path without “now”.
</details>

Comment thread Dockerfile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread README.md
The entrypoint only ever exec'd gateway_node, so `ros2 launch ... bringup`
was swallowed as args and the launch never ran; dispatch a full command
through instead. The fault_manager also could not create its default
/var/lib/ros2_medkit DB dir as the non-root user, so bringup crashed on
start; create and chown it. Default ROS_DOMAIN_ID/RMW in the quick-start.
Comment thread README.md
The bringup launch starts the log and action-status bridges by default
but leaves enable_diagnostic_bridge false; call that out so diagnostic_updater
stacks know to pass enable_diagnostic_bridge:=true.
@mfaferek93 mfaferek93 merged commit a250eac into main Jun 23, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants