build(docker): Add remote radio support in docker

[Lakshmi97-velampati]

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

[Copilot]

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-radio Docker image + entrypoint that brings up D-Bus, Avahi, cpcd, then otbr-agent over CPC (spinel+cpc://).
• Add a Compose overlay to run otbr-radio with host networking/privileges and share /var/run/dbus with barton.
• Extend tooling/docs: dockerw -T flag, .env variables (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]

I have a couple open questions, but requesting changes for the title. This effort should fall into the build commit type, not feat

[Lakshmi97-velampati]

I have a couple open questions, but requesting changes for the title. This effort should fall into the build commit type, not feat

Updated

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

[tleacmcsa]

Be sure to take this PR out of draft when you are ready for re-review.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

[Copilot]

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_id file, 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_id currently accepts partial parses (e.g. "1abc" becomes 1) 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.sh invokes python3 for the HCI PTY proxy, but the otbr-radio image's apt dependencies don't explicitly install Python. The base ubuntu:24.04 image doesn't guarantee python3 is present, so this can fail at runtime. Add python3 (or python3-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 a SyntaxError before 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() computes port = 20000 + uid without 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.py currently requires Python 3.10+ (PEP 604 union types). Update the prerequisite version so users don't hit a SyntaxError immediately.

#### 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.

2. If using Matter, configure and build your Matter SDK as described in MATTER_SUPPORT.md.
3. If using Zigbee, see ZIGBEE_SUPPORT.md.
4. 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.

1. Auto-detects the Silicon Labs radio serial port.
2. 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.
3. Starts a local TCP server that relays bytes between the serial port and

</details>

[Copilot]

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_ARGS is 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.bleAdapterId is present but invalid, this function returns the default (hci0) immediately and never falls back to the /var/run/otbr-dbus/ble_adapter_id file. 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 containing 1abc would 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 (in deviceServiceProps.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 remaining timer instead.

                    // 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;

[Copilot]

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_ARGS but never uses it; instead it interpolates ${*} into a bash -c string 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 the Cmd array with proper argv elements (e.g., via python3 + 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.py currently 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.bleAdapterId is 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_ID in deviceServiceProps.h, using the macro would prevent future drift between the reference app and Matter::ResolveBleAdapterId().

            b_core_property_provider_set_property_string(
                propProvider, "device.matter.bleAdapterId", bleAdapterEnv);
        }

[Copilot]

Pull request overview

Copilot reviewed 24 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (7)

core/src/subsystems/matter/Matter.cpp:421

• The adapter index read from /var/run/otbr-dbus/ble_adapter_id is also cast to uint32_t without a range check, so malformed/overflowed content can silently truncate to an unintended adapter index.

        if (fgets(buf, sizeof(buf), f) != nullptr)
        {
            char *endPtr = nullptr;
            unsigned long val = strtoul(buf, &endPtr, 10);

            // Accept only if at least one digit was parsed and the remainder
            // is purely whitespace/newline (reject partial parses like "1abc").
            if (endPtr != buf)
            {
                while (*endPtr == ' ' || *endPtr == '\t' || *endPtr == '\n' || *endPtr == '\r')
                {
                    endPtr++;
                }

                if (*endPtr == '\0')
                {
                    adapterId = static_cast<uint32_t>(val);
                    icInfo("Using BLE adapter hci%u from %s", adapterId, BLE_CONTROLLER_ADAPTER_ID_FILE);
                }
                else
                {
                    icWarn("Invalid content in %s, using default hci%u", BLE_CONTROLLER_ADAPTER_ID_FILE, adapterId);
                }
            }

docker/otbr-agent.conf:12

• The private D-Bus config enables ANONYMOUS auth and allows any client to own any bus name (<allow own="*"/>). Since this socket is shared between containers, this makes it easy for any process in either container to spoof services (e.g., org.bluez or OTBR) and intercept calls. Prefer EXTERNAL auth and a least-privilege allowlist of owned names/destinations.

  <listen>unix:path=/var/run/otbr-dbus/system_bus_socket</listen>
  <auth>ANONYMOUS</auth>
  <allow_anonymous/>
  <policy context="default">
    <allow own="*"/>
    <allow send_type="*"/>
    <allow send_destination="*"/>
    <allow receive_type="*"/>
    <allow receive_sender="*"/>

scripts/remote-radio/validate.sh:197

• --fix is parsed into FIX_MODE, but FIX_MODE is never referenced anywhere else in the script. Either implement fix-mode actions, or remove the flag parsing to avoid implying the script can auto-remediate.

FIX_MODE=false
JSON_MODE=false
for arg in "$@"; do
    case "$arg" in
        --fix)  FIX_MODE=true ;;
        --json) JSON_MODE=true ;;
    esac
done

reference/src/barton-core-reference-io.c:118

• The pipe write end is switched to non-blocking, but the current write path doesn't handle short writes/EAGAIN; with O_NONBLOCK this can truncate or drop log output under load. To keep the non-blocking behavior (to avoid deadlocks) without losing output unexpectedly, the write side should handle partial writes and treat EAGAIN/EWOULDBLOCK as a non-fatal drop/queue condition.

            // 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);
            }

core/src/subsystems/matter/Matter.cpp:389

• ResolveBleAdapterId() accepts any strtoul result and casts it to uint32_t without validating the range. Large values (or ULONG_MAX on overflow) will wrap/truncate and can select an unintended HCI adapter index.

This issue also appears on line 398 of the same file.

    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, B_CORE_BARTON_MATTER_BLE_HCI_INDEX);

            return adapterId;
        }

        icWarn("Invalid %s value '%s', falling back to file detection", B_CORE_BARTON_MATTER_BLE_HCI_INDEX, propVal);
    }

core/src/subsystems/thread/OpenThreadClient.cpp:185

• In the dataset retry loop, dbus_connection_read_write_dispatch() return value is ignored. If the D-Bus connection drops, the loop can spin (GetActiveDatasetTlvs errors + dispatch failing) until timeout without surfacing the underlying connection failure.

                    // Dispatch D-Bus messages for up to 2 seconds before retrying,
                    // but never exceed the remaining timeout.
                    auto dispatchMs = std::min(milliseconds(2000), timer);
                    dbus_connection_read_write_dispatch(dbusConnection.get(), dispatchMs.count());
                    next = steady_clock::now();
                    timer = timer - duration_cast<milliseconds>(next - current);
                    current = next;

scripts/remote-radio/validate.sh:36

• The usage header advertises --fix, but the script doesn't implement any remediation behavior (the flag is parsed but never used). This is misleading for users and automation.

This issue also appears on line 190 of the same file.

# Usage:
#   ./validate.sh              # Run all checks
#   ./validate.sh --fix        # Attempt to fix common issues
#   ./validate.sh --json       # Output results as JSON (for automation)

[Copilot]

Pull request overview

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

Suppressed comments (3)

docker/otbr-agent.conf:8

• This private D-Bus daemon is configured to allow ANONYMOUS connections and lets any client own any bus name (<allow own="*"/>). Even on a "private" bus shared via a volume, this makes it trivial for a compromised process in either container to spoof critical services (e.g., org.bluez or io.openthread.BorderRouter.wpan0) and intercept or manipulate traffic. It would be safer to use EXTERNAL auth and restrict own/send_destination to only the required well-known names.

  <listen>unix:path=/var/run/otbr-dbus/system_bus_socket</listen>
  <auth>ANONYMOUS</auth>
  <allow_anonymous/>
  <policy context="default">
    <allow own="*"/>

scripts/remote-radio/validate.sh:876

• --json output is not reliably valid JSON because detail is only escaping double quotes. Any backslashes, newlines, tabs, or other control characters in check details can produce malformed JSON and break automation consumers. Consider generating each result object via a real JSON encoder (python/json or jq) instead of manual escaping.

            printf '    {"status":"%s","check":"%s","detail":"%s"}' \
                "$status" "$name" "$(echo "$detail" | sed 's/"/\\"/g')"

scripts/remote-radio/remote-serial.py:227

• The tunnel port is computed as BASE_PORT + uid without validating the TCP port range. On systems with high UIDs (e.g., LDAP where UID can be > 45535), this can exceed 65535 and will fail at bind/SSH forwarding time with a confusing error. Add a range check with a clear message (or provide an override).

    port = BASE_PORT + uid

    return "0.0.0.0", port