Skip to content

fix: one node per gateway, container bind docs, and two test races - #704

Draft
bburda wants to merge 5 commits into
mainfrom
fix/gateway-in-container
Draft

fix: one node per gateway, container bind docs, and two test races#704
bburda wants to merge 5 commits into
mainfrom
fix/gateway-in-container

Conversation

@bburda

@bburda bburda commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

Summary

Fixes found while running the gateway in a container.

  • One gateway now shows up as one node. launch_ros adds -r __node:=<name> to the whole process, so the three helper nodes (fault-service clients, lifecycle state reader, subscription executor) took the gateway's name. They are now created by make_helper_node(): a hidden name _<gateway name><suffix>, a node-local __node remap, the gateway's namespace and use_sim_time, and no parameter services. The other process arguments (namespace, remaps, parameter files) still reach them.
  • The docs now say when server_host:=0.0.0.0 is needed. bringup.launch.py and gateway.launch.py bind 127.0.0.1, and in a bridge-network container a published port then answers with Connection reset by peer. README, troubleshooting and the bringup, Docker and web UI tutorials show the startup line to check and the fix. They also say that a CORS origin must match exactly and that cors_allowed_origins:= replaces the defaults. The web UI compose example used an image without the gateway and served the UI on an origin the defaults reject.
  • Two test races are fixed. A test that runs spin() on its own thread now waits for is_spinning() before it can cancel the executor: a cancel() that comes before spin() starts is lost, and join() then blocks until the ctest timeout. The node_death integration cases wait for fault reports with a service round trip on the same client.
  • The test files touched here also get their clang-tidy findings fixed.

Changes users can see: the helper loggers are named _ros2_medkit_gateway_*, the helpers no longer serve parameters, and with ros2 run (the Docker image's default command) /apps no longer lists the helpers as Apps.

Issue

Type

  • Bug fix
  • New feature or tests
  • Breaking change
  • Documentation only

Testing

  • New integration tests test_gateway_helper_nodes and test_gateway_helper_nodes_namespaced launch the gateway through launch_ros, once plainly and once in a namespace with use_sim_time and remap rules. On main they fail with /ros2_medkit_gateway appears 4 times.
  • Container bind, with the Jazzy apt package behind -p 8080:8080 on Docker Engine: the default bringup gives curl: (56) Recv failure: Connection reset by peer, server_host:=0.0.0.0 gives HTTP 200.
  • Test races: in 20 runs each of the gateway unit suite with ctest -j16, main timed out in 6 runs and this branch in none.
    A 300 ms delay before spin() makes test_discovery_manager hang every time without the fix and pass with it. A 20 ms delay in the node_death fake fault manager fails 9 of 49 cases without the round trip and none with it.
  • colcon test on gateway, fault_manager, graph_watchdog, opcua and integration_tests: 0 failures. clang-tidy on the changed files: no findings. Run on Jazzy only.

Checklist

  • Breaking changes are clearly described (and announced in docs / changelog if needed)
  • Tests were added or updated if needed
  • Docs were updated if behavior or public API changed

@bburda
fix: keep the gateway's helper nodes off the gateway's own name
72e91a3 
launch_ros Node(name=...) passes -r __node:=<name> to the whole process.
The fault-service clients, the lifecycle state reader and the subscription
executor each create an rclcpp::Node that honoured that rule, so one
gateway showed up as four nodes named /ros2_medkit_gateway: ros2 node list
warned about duplicate names, rosout logged "Publisher already registered"
three times, and ros2 param list printed every parameter four times.

The helpers are now built by make_helper_node():

- named _<gateway name><suffix>, so they are hidden from ros2 node list and
  from discovery (no new Apps; count_peer_nodes no longer needs the helper
  exception list);
- a node-local __node remap overrides the process-wide one. The other
  process arguments (namespace, remaps, parameter files) still apply;
- use_sim_time is copied from the gateway, because a parameters file keyed
  by the gateway's name does not match the helper;
- no parameter services or parameter event publisher: the fault-client
  node is spun only during its own requests, and a get_parameters call to it
  timed out; the lifecycle reader has the same shape.

New integration tests launch the gateway through launch_ros, once plainly
and once in a namespace with use_sim_time and with remap rules that are the
only route to the fault manager. They check a single gateway node in the
graph, ros2 param list without duplicates, unchanged /apps, hidden helpers
in the gateway namespace, /clock subscriptions on the helpers under sim
time, and that faults, topic samples and lifecycle states still work.

The graph watchdog compiles the lifecycle state reader in, so it now
compiles helper_node.cpp too. test_gateway_node.cpp drops redundant c_str()
calls flagged by clang-tidy.
@bburda
docs: explain server_host in containers and exact CORS origins
0c43f9b 
bringup.launch.py and gateway.launch.py bind 127.0.0.1. Inside a container
on a bridge network that is the container's loopback, so a published port
answers with "Connection reset by peer" or an empty reply. The image's
default command binds 0.0.0.0. The README, the bringup and Docker tutorials
and the troubleshooting page now say so, show the startup line that names
the bind address, and give the fix with -p 127.0.0.1:8080:8080 to keep the
unauthenticated API off the LAN.

The README no longer tells native users to set cors.allowed_origins: the
launch files already allow http://localhost:3000 and http://localhost:5173.
The pages now state that the origin must match exactly
(http://127.0.0.1:3000 and http://localhost:3000 are different origins)
and that cors_allowed_origins:= replaces the defaults. The bringup
tutorial and the gateway README list that launch argument.

The web UI compose example used ros:jazzy, which has no gateway, and served
the UI on port 80, an origin the defaults reject; it now uses the gateway
image on the host network and the UI on port 3000. The getting started page
quotes the gateway's real startup lines.
@bburda
test: wait for the executor to spin before a test can cancel it
affedbf 
Executor::cancel() only clears the spinning flag, and spin() sets it
again when it starts. A test that starts spin() on its own thread and
cancels it later loses the cancel when the thread has not reached spin()
yet. spin() then never returns and join() blocks until the ctest timeout.
Short cases under parallel load hit this in test_discovery_manager,
test_ros2_topic_data_provider and test_ros2_subscription_executor.

Wait for is_spinning() after each spin thread starts, in every test that
cancels an executor spun on its own thread.

Gateway unit suite with ctest -j16: 10 of 40 runs timed out before,
0 of 20 after.
@bburda
test: wait for node_death reports with a service round trip
f4b3af5 
The node_death integration cases slept 5-100 ms after a detector tick
and then read what the fake fault manager had received. Under parallel
load a report can take longer, so a case sees it one tick late (X2
expected the raise at tick 16 and saw 17), or a late report from the
previous case lands in the next one.

flush_reports() sends a marker request on the same client and waits for
the response. The sink serves one request at a time in arrival order,
so every earlier report is recorded by then. SetUp flushes before it
clears the record.

With a 20 ms delay injected into the sink, 9 of 49 cases failed before
and 49 of 49 pass after.
@bburda
test: clear clang-tidy findings in test files this branch touches
3b14699 
container-size-empty, unnecessary-value-param, named-parameter,
inefficient-vector-operation and special-member-functions in
test_fault_manager (fault manager), test_ros2_subscription_slot,
test_graph_watchdog_plugin and test_opcua_plugin. No behaviour change.
@bburda bburda self-assigned this Sep 26, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@bburda bburda

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Gateway in a bridge-network container is unreachable from the host, and one gateway shows up as four nodes

1 participant

@bburda