Improve serial bridge reliability

[Yeraze]

Summary

• Add clocal and cs8 to socat serial options — prevents false carrier detect drops on USB serial adapters and ensures proper 8-bit framing for protobuf binary data
• Add set -e for strict error handling during setup phase, with appropriate guards (|| true) on functions that can legitimately fail
• Add configurable DEVICE_TIMEOUT env var (default 60s) — wait_for_device() no longer hangs indefinitely; Docker's restart: unless-stopped handles retries
• Quote variables in socat command to prevent word splitting

Test plan

• Rebuild Docker image: docker build -t meshtastic-serial-bridge -f src/Dockerfile src/
• Start bridge: docker compose up -d
• Verify startup logs show HUPCL disabled and bridge listening
• Test connectivity: meshtastic --host localhost --info
• Test device-missing scenario: start without USB device, verify timeout after 60s and clean exit
• Verify bidirectional data flow over extended period

🤖 Generated with Claude Code

[claude]

Code Review

Overall this is a solid set of improvements. The changes are well-scoped, fix real problems, and align with the project's simplicity goals. A few observations below:

---

✅ What's good

• clocal + cs8 socat options — Good fix. clocal prevents false carrier-detect drops (common on USB-serial adapters), and cs8 correctly enforces 8-bit framing for protobuf binary data. Both are well-justified.

• EXIT_CODE=0 + || EXIT_CODE=$? pattern — Correct way to capture exit codes under set -e. The original EXIT_CODE=$? on its own line would have been killed by set -e if socat exited non-zero. Good catch.

• disable_hupcl || true — Right call. The function already handles the error case internally (with a print to stderr), but the return 1 on missing device would have killed the script under set -e. The || true guard is appropriate here since a missing device at reconnect time isn't fatal.

• Variable quoting in socat command — Correctly guards against word splitting on $TCP_PORT, $DEVICE, $BAUD.

---

⚠️ Issues / suggestions

1. DEVICE_TIMEOUT has no input validation (gap parity with RECONNECT_DELAY)

RECONNECT_DELAY has a case guard against non-integers and zero, but DEVICE_TIMEOUT does not. If a user sets DEVICE_TIMEOUT=abc, the arithmetic test [ "$WAITED" -ge "$DEVICE_TIMEOUT" ] will fail with a shell error. Suggest adding the same-style validation:

case "$DEVICE_TIMEOUT" in
    ''|*[!0-9]*|0)
        echo "Warning: Invalid DEVICE_TIMEOUT '$DEVICE_TIMEOUT', using default of 60"
        DEVICE_TIMEOUT=60
        ;;
esac

2. WAITED variable leaks into outer scope

POSIX sh has no local keyword, so WAITED set inside wait_for_device() persists in the environment. It's not harmful here (it gets reset each call), but it's worth being aware of if the script grows.

3. Behavior change worth documenting: device timeout now exits the container

Previously wait_for_device looped indefinitely, so the container would wait forever for the device. Now it exits after DEVICE_TIMEOUT seconds, relying on restart: unless-stopped to retry. This is a better design, but it's a behavioral change that operators upgrading should know about — worth a note in the README (e.g., under the troubleshooting section or the DEVICE_TIMEOUT env var docs).

4. set -e placement (minor style)

set -e after the shebang but before the file comment is fine functionally, but convention is usually shebang → comments → set options. Purely cosmetic.

---

Summary

The reliability improvements are sound and the set -e adoption is handled correctly. The main gap is the missing validation for DEVICE_TIMEOUT. Everything else is either minor style or documentation. Happy to approve once the validation is added.

🤖 Generated with Claude Code