build(docker): Add remote radio support in docker - #201
build(docker): Add remote radio support in docker#201Lakshmi97-velampati wants to merge 33 commits into
Conversation
Add an optional otbr-radio Docker container that connects a Silicon Labs BRD2703 xG24 Explorer Kit to the Barton devcontainer via D-Bus, enabling Thread integration testing with real hardware. - Dockerfile.otbr-radio: builds image with cpcd and otbr-agent compiled for CPC transport; uses the URL spinel+cpc://cpcd_0?iid=1&iid-list=0 to handle both unicast and broadcast Spinel frames - compose.otbr-radio.yaml: compose overlay that shares a D-Bus socket volume with the barton service and maps RADIO_DEVICE at the same path on both the host and container sides - otbr-radio-entrypoint.sh: writes a fresh cpcd config on every start (uart_device_file, uart_hardflow: true), waits for "Daemon startup was successful" before launching otbr-agent, and auto-detects BACKBONE_IF from the host default route at runtime - setupDockerEnv.sh: auto-detects the backbone interface via ip route and writes it to docker/.env; Refs: BARTON-359 Signed-off-by: mvelam850 <munilakshmi_velampati@comcast.com>
There was a problem hiding this comment.
Pull request overview
Adds an optional “real USB radio” Thread Border Router path for Barton’s Docker-based dev environment by introducing an otbr-radio sidecar container (cpcd + otbr-agent) that shares a D-Bus socket volume with the main barton container, enabling Thread integration testing against real BRD2703 hardware.
Changes:
- Add an
otbr-radioDocker image + entrypoint that brings up D-Bus, Avahi, cpcd, then otbr-agent over CPC (spinel+cpc://). - Add a Compose overlay to run
otbr-radiowith host networking/privileges and share/var/run/dbuswithbarton. - Extend tooling/docs:
dockerw -Tflag,.envvariables (RADIO_DEVICE,BACKBONE_IF), and setup instructions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/THREAD_BORDER_ROUTER_SUPPORT.md | Adds end-to-end documentation for simulated vs real-radio Thread setup, including USB-IP guidance. |
| dockerw | Adds -T flag to include the OTBR radio compose overlay and start the otbr-radio service. |
| docker/setupDockerEnv.sh | Writes optional RADIO_DEVICE and BACKBONE_IF into docker/.env, with backbone IF auto-detection. |
| docker/README.md | Documents the new compose overlay and the -T flag. |
| docker/otbr-radio-entrypoint.sh | New entrypoint that orchestrates D-Bus, Avahi, cpcd, and otbr-agent startup for CPC-based RCP. |
| docker/Dockerfile.otbr-radio | New build for cpcd + otbr-agent (Silabs GSDK transport/CPC) and D-Bus policy. |
| docker/compose.otbr-radio.yaml | New compose overlay that runs otbr-radio privileged with host networking and shares D-Bus socket volume with barton. |
kfundecmcsa
left a comment
There was a problem hiding this comment.
I have a couple open questions, but requesting changes for the title. This effort should fall into the build commit type, not feat
Updated |
|
Be sure to take this PR out of draft when you are ready for re-review. |
…tunnels When the remote-radio SSH tunnel has high RTT (e.g., 300ms across continents), cpcd's default 100ms retry timeout causes commands to be retransmitted before the radio can respond. For the reboot command, this results in the radio receiving multiple reset requests, triggering an infinite "Secondary has reset, reset the daemon" loop. Changes: - Patch cpcd source at build time to increase retry timeout from 100ms to 1000ms (server_core.c) — no impact on low-latency setups since the timer is cancelled as soon as the response arrives - Add TCP_NODELAY to socat's TCP connection to eliminate Nagle buffering - Add TCP_NODELAY to remote-serial.py relay client sockets Root cause: cpcd retransmits reboot commands every 100ms. With 300ms RTT, 3 duplicate reboots reach the radio before the first response returns. The radio resets repeatedly, sending unsolicited reset notifications that cpcd interprets as unexpected secondary resets.
… serial tunnels" This reverts commit aae12a8.
…tunnels The CPC daemon retries system commands every 100ms. When the serial link RTT exceeds 100ms (e.g. remote radio over SSH tunnel), responses arrive after the retry fires, causing duplicate commands. For the reboot command this triggers an infinite reset loop. Patch server_core.c at build time to use a 1s retry interval. This is safe for low-latency setups — the timer is cancelled immediately when the response arrives. Only the error-path delay (radio genuinely unresponsive) is affected.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Suppressed comments (11)
docker/otbr-agent.conf:13
- The private D-Bus config enables anonymous auth and allows any user to own/send to any bus name (
own="*",send_destination="*"). Even on a private socket this makes it easy for an accidental/buggy process to hijack well-known names (e.g.org.bluez,io.openthread.*) and break the stack. Prefer a least-privilege policy that only allows the names/interfaces needed by otbr-agent, bluetoothd, and basic introspection.
<listen>unix:path=/var/run/otbr-dbus/system_bus_socket</listen>
<auth>ANONYMOUS</auth>
<allow_anonymous/>
<policy context="default">
<allow user="*"/>
<allow own="*"/>
<allow send_type="*"/>
<allow send_destination="*"/>
<allow receive_type="*"/>
<allow receive_sender="*"/>
core/src/subsystems/matter/Matter.cpp:388
ResolveBleAdapterId()returns immediately when the runtime property exists, even if the value is invalid. That prevents falling back to the/var/run/otbr-dbus/ble_adapter_idfile, which is likely the correct source in real-radio mode. Consider only returning early when the property parses cleanly; otherwise log a warning and continue to file fallback.
if (propVal != nullptr)
{
char *endPtr = nullptr;
unsigned long val = strtoul(propVal, &endPtr, 10);
if (endPtr != propVal && *endPtr == '\0')
{
adapterId = static_cast<uint32_t>(val);
icInfo("Using BLE adapter hci%u from property %s", adapterId, DEVICE_PROP_MATTER_BLE_ADAPTER_ID);
}
else
{
icWarn("Invalid %s value '%s', using default hci%u", DEVICE_PROP_MATTER_BLE_ADAPTER_ID, propVal, adapterId);
}
return adapterId;
}
core/src/subsystems/matter/Matter.cpp:407
- Parsing the adapter id from
/var/run/otbr-dbus/ble_adapter_idcurrently accepts partial parses (e.g."1abc"becomes1) and doesn't verify range. This can silently select the wrong adapter. It should require the rest of the line to be just newline/whitespace and clamp to uint32_t.
if (fgets(buf, sizeof(buf), f) != nullptr)
{
char *endPtr = nullptr;
unsigned long val = strtoul(buf, &endPtr, 10);
if (endPtr != buf)
{
adapterId = static_cast<uint32_t>(val);
icInfo("Using BLE adapter hci%u from %s", adapterId, BLE_CONTROLLER_ADAPTER_ID_FILE);
}
docker/Dockerfile.otbr-radio:79
otbr-radio-entrypoint.shinvokespython3for the HCI PTY proxy, but the otbr-radio image's apt dependencies don't explicitly install Python. The baseubuntu:24.04image doesn't guaranteepython3is present, so this can fail at runtime. Addpython3(orpython3-minimal) to the package list.
socat \
iproute2 \
iptables \
ipset \
udev \
libglib2.0-bin \
bluez \
&& rm -rf /var/lib/apt/lists/*
scripts/remote-radio/remote-serial.py:46
- The script advertises Python 3.8+, but it uses PEP 604 union types (
str | None,socket.socket | None) which require Python 3.10+. As written, running this with Python 3.8/3.9 will raise aSyntaxErrorbefore any helpful message is printed.
Requirements (workstation):
- Python 3.8+
- pyserial (pip install pyserial)
- ssh client on PATH
scripts/remote-radio/remote-serial.py:229
compute_tunnel_addr()computesport = 20000 + uidwithout validating the result. On systems with large UIDs (e.g. >= 65536), this will exceed the valid TCP port range and fail later with a confusing bind/tunnel error. Validate the computed port and fail fast with a clear message.
def compute_tunnel_addr(uid: int) -> tuple[str, int]:
"""Derive per-user tunnel port from UID.
Port: 20000 + UID (unique per user on shared servers)
The SSH reverse tunnel binds to 0.0.0.0 on the remote so it is
reachable from Docker containers (which cannot access the host's
loopback directly). Per-user port isolation prevents conflicts.
"""
port = BASE_PORT + uid
return "0.0.0.0", port
docs/REMOTE_RADIO_FOR_DEVELOPMENT.md:151
- The documentation says Python 3.7+ is sufficient, but
remote-serial.pycurrently requires Python 3.10+ (PEP 604 union types). Update the prerequisite version so users don't hit aSyntaxErrorimmediately.
#### Prerequisites
1. **Python 3.7+** and **pyserial** on the workstation:
```bash
pip install pyserial
**docs/USING_BARTON_GUIDE.md:18**
* The new reference to REMOTE_RADIO_FOR_DEVELOPMENT.md is still an empty Markdown link target (`[... ]()`). That renders as plain text rather than a clickable link. Use a relative link target so the doc is navigable.
- If using Matter, configure and build your Matter SDK as described in MATTER_SUPPORT.md.
- If using Zigbee, see ZIGBEE_SUPPORT.md.
- If using OpenThread's Border Router, see REMOTE_RADIO_FOR_DEVELOPMENT.md.
**reference/src/barton-core-reference-io.c:108**
* The pipe write end is set to non-blocking, but `emitToPipe()` doesn't handle `EAGAIN` / partial writes. With `O_NONBLOCK`, `write()` can fail or short-write under load, potentially dropping or truncating log/output in hard-to-debug ways. Either (a) add `EINTR` retry + partial-write handling and intentionally drop only on `EAGAIN`, or (b) keep the fd blocking and avoid deadlock some other way.
// Set write end to non-blocking so log writes never block when the
// pipe buffer is full. This prevents a deadlock when subsystem
// initialization (e.g. Zigbee) blocks the main thread before the
// GLib main loop starts draining the pipe.
int flags = fcntl(outputSendPipe, F_GETFL);
if (flags != -1)
{
fcntl(outputSendPipe, F_SETFL, flags | O_NONBLOCK);
}
**reference/src/barton-core-reference-app.c:265**
* Setting the BLE adapter property using a raw string literal duplicates the canonical name already defined in `DEVICE_PROP_MATTER_BLE_ADAPTER_ID`. This makes it easy for the reference app and `Matter::ResolveBleAdapterId()` to drift out of sync (typos, namespace changes). Prefer including `deviceServiceProps.h` and using the shared macro.
// Translate BARTON_BLE_ADAPTER_ID env var to a runtime property
const char *bleAdapterEnv = getenv("BARTON_BLE_ADAPTER_ID");
if (bleAdapterEnv != NULL)
{
b_core_property_provider_set_property_string(
propProvider, "device.matter.bleAdapterId", bleAdapterEnv);
}
**scripts/remote-radio/remote-serial.py:37**
* The module docstring says a per-user loopback address (127.0.<hi>.<lo>) is computed, but the implementation always uses `127.0.0.1` locally and binds the reverse tunnel on `0.0.0.0` remotely. This mismatch can confuse users reading the output; update the docstring to match what the code actually does.
- Auto-detects the Silicon Labs radio serial port.
- Computes a per-user TCP port from the remote UID (base 20000 + UID) and
a per-user loopback address (127.0..) to avoid conflicts on
shared servers. - Starts a local TCP server that relays bytes between the serial port and
</details>
| # Use the Docker Engine API to exec into the container. | ||
| # Read the script content, base64-encode it, and pass as a | ||
| # command argument to avoid Docker exec stdin piping issues. | ||
| _ESCAPED_ARGS="" | ||
| for arg in "$@"; do | ||
| _ESCAPED_ARGS="${_ESCAPED_ARGS}, \"${arg}\"" | ||
| done | ||
| _SCRIPT_B64=$(base64 -w0 "$0") | ||
| _EXEC_ID=$($_CURL -X POST "$_DOCKER_API/containers/${OTBR_CONTAINER}/exec" \ | ||
| -H "Content-Type: application/json" \ | ||
| -d "{\"Cmd\":[\"bash\", \"-c\", \"echo ${_SCRIPT_B64} | base64 -d | bash -s -- ${*}\"],\"AttachStdout\":true,\"AttachStderr\":true}" \ | ||
| | python3 -c "import json,sys; print(json.load(sys.stdin)['Id'])") |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
scripts/remote-radio/validate.sh:114
- This Docker API exec path builds the command string using
${*}and injects it into JSON unescaped. That breaks when args contain spaces/shell metacharacters and also creates a shell-injection risk._ESCAPED_ARGSis computed but never used—use a shell-escaped argument string instead.
# Use the Docker Engine API to exec into the container.
# Read the script content, base64-encode it, and pass as a
# command argument to avoid Docker exec stdin piping issues.
_ESCAPED_ARGS=""
for arg in "$@"; do
_ESCAPED_ARGS="${_ESCAPED_ARGS}, \"${arg}\""
done
_SCRIPT_B64=$(base64 -w0 "$0")
_EXEC_ID=$($_CURL -X POST "$_DOCKER_API/containers/${OTBR_CONTAINER}/exec" \
-H "Content-Type: application/json" \
-d "{\"Cmd\":[\"bash\", \"-c\", \"echo ${_SCRIPT_B64} | base64 -d | bash -s -- ${*}\"],\"AttachStdout\":true,\"AttachStderr\":true}" \
| python3 -c "import json,sys; print(json.load(sys.stdin)['Id'])")
scripts/remote-radio/remote-serial.py:45
- The docstring claims Python 3.8+, but this file uses PEP 604 union types (e.g.
str | None), which require Python 3.10+. Either adjust the code to be 3.8-compatible (Optional[str]) or update the documented requirement so users don’t hit a SyntaxError.
Requirements (workstation):
- Python 3.8+
- pyserial (pip install pyserial)
core/src/subsystems/matter/Matter.cpp:390
- If
device.matter.bleAdapterIdis present but invalid, this function returns the default (hci0) immediately and never falls back to the/var/run/otbr-dbus/ble_adapter_idfile. That makes an accidental/empty env var override the real-radio adapter selection. Prefer falling through to the file fallback when the property value fails validation.
icWarn("Invalid %s value '%s', using default hci%u", DEVICE_PROP_MATTER_BLE_ADAPTER_ID, propVal, adapterId);
}
return adapterId;
}
core/src/subsystems/matter/Matter.cpp:409
- The file fallback accepts partial parses because it only checks
endPtr != buf. For example, a file containing1abcwould be accepted as adapter 1. Validate that the entire line is a number (allowing trailing whitespace/newline) before using it.
char *endPtr = nullptr;
unsigned long val = strtoul(buf, &endPtr, 10);
if (endPtr != buf)
{
reference/src/barton-core-reference-app.c:271
- This hard-codes the Matter BLE adapter property name string. Since the canonical name is now defined as
DEVICE_PROP_MATTER_BLE_ADAPTER_ID(indeviceServiceProps.h), using the macro avoids drift/typos and makes refactors safer.
{
b_core_property_provider_set_property_string(
propProvider, "device.matter.bleAdapterId", bleAdapterEnv);
}
core/src/subsystems/thread/OpenThreadClient.cpp:183
- The retry loop always dispatches D-Bus messages for 2000ms even when less time remains in
timer. That can overshoot the intended overall timeout (ATTACH_WAIT_SECONDS). Use a dispatch timeout capped to the remainingtimerinstead.
// Dispatch D-Bus messages for up to 2 seconds before retrying
dbus_connection_read_write_dispatch(dbusConnection.get(), 2000);
next = steady_clock::now();
timer = timer - duration_cast<milliseconds>(next - current);
current = next;
| <policy context="default"> | ||
| <allow own="io.openthread.BorderRouter.wpan0"/> | ||
| <allow send_destination="io.openthread.BorderRouter.wpan0"/> | ||
| <allow send_interface="*"/> | ||
| <allow user="*"/> | ||
| <allow own="*"/> |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Suppressed comments (6)
scripts/remote-radio/remote-serial.py:46
- The script advertises Python 3.8+, but it uses PEP 604 union types (e.g.
str | None) which require Python 3.10+. Either adjust the code to be compatible with 3.8 (typing.Optional/Union + typing.Tuple/etc.), or update the stated requirement so users don't hit a SyntaxError on older interpreters.
Requirements (workstation):
- Python 3.8+
- pyserial (pip install pyserial)
- ssh client on PATH
scripts/remote-radio/validate.sh:110
- The Docker-API re-exec path builds
_ESCAPED_ARGSbut never uses it; instead it interpolates${*}into abash -cstring inside JSON. This loses argument boundaries (breaks when args contain spaces) and also makes the exec payload vulnerable to shell injection via crafted arguments. Build theCmdarray with proper argv elements (e.g., viapython3+json.dumps) and pass args as separate array entries.
_ESCAPED_ARGS=""
for arg in "$@"; do
_ESCAPED_ARGS="${_ESCAPED_ARGS}, \"${arg}\""
done
_SCRIPT_B64=$(base64 -w0 "$0")
scripts/remote-radio/remote-serial.py:35
- The module docstring claims a per-user loopback address ("127.0..") is used for isolation, but the implementation always binds the local TCP server to 127.0.0.1 and uses only a per-user port. This is misleading for users troubleshooting shared-host conflicts; either implement the per-user loopback address or update the description.
This issue also appears on line 43 of the same file.
2. Computes a per-user TCP port from the remote UID (base 20000 + UID) and
a per-user loopback address (127.0.<hi>.<lo>) to avoid conflicts on
shared servers.
docs/REMOTE_RADIO_FOR_DEVELOPMENT.md:147
- The docs say the workstation only needs Python 3.7+, but
remote-serial.pycurrently uses Python 3.10-only type-hint syntax (str | None). Update the prerequisite version (or adjust the script) so the documentation matches what will actually run.
1. **Python 3.7+** and **pyserial** on the workstation:
core/src/subsystems/matter/Matter.cpp:390
- If
device.matter.bleAdapterIdis set but invalid (non-numeric), the function returns the default adapter ID immediately and never attempts the file fallback (/var/run/otbr-dbus/ble_adapter_id). That makes a single bad env/property value permanently override the auto-detected adapter. Consider only returning early when the property value parses cleanly; otherwise warn and continue to the file fallback.
icWarn("Invalid %s value '%s', using default hci%u", DEVICE_PROP_MATTER_BLE_ADAPTER_ID, propVal, adapterId);
}
return adapterId;
}
reference/src/barton-core-reference-app.c:271
- The reference app sets the new BLE adapter property using a hard-coded string literal ("device.matter.bleAdapterId"). Since the canonical name is now defined as
DEVICE_PROP_MATTER_BLE_ADAPTER_IDindeviceServiceProps.h, using the macro would prevent future drift between the reference app andMatter::ResolveBleAdapterId().
b_core_property_provider_set_property_string(
propProvider, "device.matter.bleAdapterId", bleAdapterEnv);
}
Add an optional otbr-radio Docker container that connects a Silicon Labs BRD2703 xG24 Explorer Kit to the Barton devcontainer via D-Bus, enabling Thread integration testing with real hardware.
Refs: BARTON-359