Add TCPLoRaRadio and USBLoRaRadio for pymc_usb firmware#68
Merged
Conversation
Collaborator
|
Overall, this PR is a great addition. Please remove the legacy alias, as I’d like to treat this as the first stable version going forward. Any backwards-compatibility code should be removed and considered out of scope for v1. Could you also create a Python protocol_constants file to reduce the amount of duplicated code? If possible, please keep usb_radio.py and tcp_radio.py as DRY as possible. I’m happy for both usb and tcp to be submitted in the same PR, and I’m fine with your proposed naming convention. Finally, could you please use 0x12 as the default sync value. |
Author
|
Done. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a shared wire-protocol module plus two new LoRaRadio implementations (TCP + USB-CDC) that talk to a remote pymc_usb firmware modem, enabling MeshCore host deployments where the SX1262 is not physically attached to the host running pymc_core.
Changes:
- Added
protocol_constants.pywith shared framing + CRC helpers and protocol constants. - Added
TCPLoRaRadioandUSBLoRaRadioimplementations using that shared protocol. - Updated hardware exports and example factory code to support new
radio_typevalues; added offline protocol tests.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/pymc_core/hardware/protocol_constants.py |
Defines framing/CRC + command constants shared by both transports. |
src/pymc_core/hardware/tcp_radio.py |
New TCP-based LoRaRadio implementation with background RX and reconnect logic. |
src/pymc_core/hardware/usb_radio.py |
New USB-CDC (pyserial) LoRaRadio implementation with background RX. |
src/pymc_core/hardware/__init__.py |
Conditional exports for the new radio implementations. |
examples/common.py |
Adds pymc_tcp / pymc_usb branches to create_radio() and updates docs/errors. |
tests/hardware/test_protocol_constants.py |
Adds offline test vectors for CRC + frame layout. |
Comments suppressed due to low confidence (5)
src/pymc_core/hardware/tcp_radio.py:648
- TCPLoRaRadio._reopen_socket() references self._custom_cad_peak/_custom_cad_min, but these attributes are never initialized in init. If a reconnect happens before set_custom_cad_thresholds()/perform_cad runs, this will raise AttributeError and prevent reconnection. Initialize both to None in init (mirrors SX1262Radio) or use getattr() here.
# Re-apply custom CAD if host had programmed any.
if self._custom_cad_peak is not None and self._custom_cad_min is not None:
try:
self.set_custom_cad_thresholds(peak=self._custom_cad_peak,
min_val=self._custom_cad_min)
src/pymc_core/hardware/tcp_radio.py:643
- Black/PEP8 formatting: multiple statements are on one line (e.g.
self._close_sock(); return False). This will be reformatted by the repo’s Black pre-commit hook and makes diffs noisier. Split these into separate lines.
if self.token and not self._auth_sync(timeout=3.0):
logger.error("Reconnect: AUTH rejected")
self._close_sock(); return False
if not self._ping_sync(timeout=3.0):
logger.error("Reconnect: PING failed")
self._close_sock(); return False
if not self._apply_config_sync():
logger.error("Reconnect: SET_CONFIG failed")
self._close_sock(); return False
src/pymc_core/hardware/tcp_radio.py:736
- RX worker parses frames using the LEN field without any sanity limit. If the stream is desynced or a malicious peer sends a very large LEN,
frame_sizecan become huge and the buffer will grow unbounded waiting for the rest of the frame. Add a max-length guard (e.g., drop/skip frames where LEN exceeds a reasonable upper bound such as MAX_LORA_PAYLOAD + protocol overhead).
cmd = buf[1]
length = buf[2] | (buf[3] << 8)
frame_size = 1 + 1 + 2 + length + 2
if len(buf) < frame_size:
break
src/pymc_core/hardware/usb_radio.py:174
- The comment says
dsrdtr=False(to avoid reboot) and that macOS needsrtscts=True, but the code setsdsrdtr=Trueandrtscts=False. This inconsistency is likely to cause the exact reset/CDC issues the comment warns about. Either fix the serial settings to match the intended behavior or update the comment to reflect the correct, tested configuration.
try:
# dsrdtr=False avoids rebooting the ESP32 on every port open
# (CP2102 on Heltec V3 pulses EN on DTR transitions). macOS CDC
# drivers in this mode only reliably deliver TX/RX if we force
# rtscts=True and pre-assert RTS — otherwise read returns
# trickle-dribbles after the first response.
self._serial = serial.Serial()
self._serial.port = self.port
self._serial.baudrate = self.baudrate
self._serial.timeout = 0.1
self._serial.write_timeout = 2.0
self._serial.dsrdtr = True
self._serial.rtscts = False
self._serial.open()
src/pymc_core/hardware/usb_radio.py:790
- RX worker parses frames using the LEN field without any sanity limit. If the stream is desynced or bytes are corrupted,
frame_sizecan become very large and the buffer will grow unbounded waiting for completion. Add a max-length guard similar to _read_frame_sync() (e.g., drop/skip frames where LEN exceeds a reasonable upper bound).
cmd = buf[1]
length = buf[2] | (buf[3] << 8)
frame_size = 1 + 1 + 2 + length + 2
if len(buf) < frame_size:
break # Incomplete frame, wait for more
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+44
to
+56
| CMD_TX_REQUEST, CMD_SET_CONFIG, CMD_GET_CONFIG, | ||
| CMD_STATUS_REQ, CMD_NOISE_REQ, | ||
| CMD_CAD_REQUEST, CMD_RX_START, CMD_SET_CAD_PARAMS, | ||
| CMD_SET_WIFI, CMD_AUTH, CMD_WIFI_RESET, | ||
| CMD_GET_WIFI, CMD_GET_VERSION, CMD_PING, | ||
| CMD_TX_DONE, CMD_TX_FAIL, CMD_RX_PACKET, | ||
| CMD_CONFIG_RESP, CMD_STATUS_RESP, CMD_NOISE_RESP, | ||
| CMD_CAD_RESP, CMD_RX_STARTED, CMD_CAD_PARAMS_RESP, | ||
| CMD_AUTH_OK, CMD_WIFI_STATUS, CMD_VERSION_RESP, | ||
| CMD_ERROR, CMD_PONG, | ||
| WIFI_MODE_OFFLINE, WIFI_MODE_STA_CONNECTING, | ||
| WIFI_MODE_STA_CONNECTED, WIFI_MODE_AP_CONFIG, | ||
| RADIO_CONFIG_FMT, RADIO_CONFIG_SIZE, |
Comment on lines
+1
to
+13
| """ | ||
| Offline tests for the shared pymc_usb wire-protocol primitives. | ||
|
|
||
| Verifies CRC-16/CCITT-FALSE (poly 0x1021, init 0xFFFF, no reflect, | ||
| no xor-out) and frame layout produced by `crc16_ccitt` / `build_frame` | ||
| against fixed reference vectors so the firmware and both Python drivers | ||
| (USB + TCP) cannot drift apart silently. | ||
|
|
||
| No network, no hardware — these run in CI on a bare interpreter. | ||
|
|
||
| Place this file at: | ||
| tests/hardware/test_protocol_constants.py | ||
| """ |
| self.last_rssi: int = -99 | ||
| self.last_snr: float = 0.0 | ||
| self.last_signal_rssi: int = -99 | ||
| self._noise_floor: float = -99.0 |
Comment on lines
+636
to
+642
| self._close_sock(); return False | ||
| if not self._ping_sync(timeout=3.0): | ||
| logger.error("Reconnect: PING failed") | ||
| self._close_sock(); return False | ||
| if not self._apply_config_sync(): | ||
| logger.error("Reconnect: SET_CONFIG failed") | ||
| self._close_sock(); return False |
Comment on lines
+168
to
+173
| self._serial.port = self.port | ||
| self._serial.baudrate = self.baudrate | ||
| self._serial.timeout = 0.1 | ||
| self._serial.write_timeout = 2.0 | ||
| self._serial.dsrdtr = True | ||
| self._serial.rtscts = False |
Comment on lines
+44
to
+58
| CMD_TX_REQUEST, CMD_SET_CONFIG, CMD_GET_CONFIG, | ||
| CMD_STATUS_REQ, CMD_NOISE_REQ, | ||
| CMD_CAD_REQUEST, CMD_RX_START, CMD_SET_CAD_PARAMS, | ||
| CMD_SET_WIFI, CMD_AUTH, CMD_WIFI_RESET, | ||
| CMD_GET_WIFI, CMD_GET_VERSION, CMD_PING, | ||
| CMD_TX_DONE, CMD_TX_FAIL, CMD_RX_PACKET, | ||
| CMD_CONFIG_RESP, CMD_STATUS_RESP, CMD_NOISE_RESP, | ||
| CMD_CAD_RESP, CMD_RX_STARTED, CMD_CAD_PARAMS_RESP, | ||
| CMD_AUTH_OK, CMD_WIFI_STATUS, CMD_VERSION_RESP, | ||
| CMD_ERROR, CMD_PONG, | ||
| WIFI_MODE_OFFLINE, WIFI_MODE_STA_CONNECTING, | ||
| WIFI_MODE_STA_CONNECTED, WIFI_MODE_AP_CONFIG, | ||
| RADIO_CONFIG_FMT, RADIO_CONFIG_SIZE, | ||
| STATUS_RESP_FMT, STATUS_RESP_SIZE, | ||
| crc16_ccitt, build_frame, |
Comment on lines
+432
to
+439
| if det_peak is not None and det_min is not None: | ||
| new_peak = int(det_peak) | ||
| new_min = int(det_min) | ||
| # Skip the firmware roundtrip when thresholds haven't changed | ||
| # since the previous call. Saves ~50-80 ms per CAD during the | ||
| # repeated-sample phase of the calibration sweep. | ||
| cached_peak = getattr(self, "_custom_cad_peak", None) | ||
| cached_min = getattr(self, "_custom_cad_min", None) |
Comment on lines
+604
to
+616
| WIFI_MODE_OFFLINE: "offline", | ||
| WIFI_MODE_STA_CONNECTING: "connecting", | ||
| WIFI_MODE_STA_CONNECTED: "sta", | ||
| WIFI_MODE_AP_CONFIG: "ap", | ||
| } | ||
| return { | ||
| "mode": mode, | ||
| "mode_name": mode_names.get(mode, "unknown"), | ||
| "ip": ip, | ||
| "port": port, | ||
| "ssid": ssid, | ||
| "hostname": host, | ||
| "mdns": f"{host}.local" if host else "", |
|
|
||
| # Response synchronization (command → response matching) | ||
| self._response_events: dict[int, asyncio.Event] = {} | ||
| self._response_data: dict[int, bytes] = {} |
Comment on lines
+417
to
+423
| async def perform_cad( | ||
| self, | ||
| det_peak: Optional[int] = None, | ||
| det_min: Optional[int] = None, | ||
| timeout: float = 1.0, | ||
| calibration: bool = False, | ||
| ) -> bool: |
Comment on lines
+452
to
+458
| async def perform_cad( | ||
| self, | ||
| det_peak: Optional[int] = None, | ||
| det_min: Optional[int] = None, | ||
| timeout: float = 1.0, | ||
| calibration: bool = False, | ||
| ) -> bool: |
Two stdlib-friendly LoRa radio drivers that talk to the pymc_usb firmware (https://github.com/itk80/pymc_usb) over a TCP socket or USB-CDC respectively. Drop-in replacement for SX1262Radio when the SX1262 module isn't attached to the same host as pymc_core (sector arrays, distant antennas, multi-modem deployments). Wire protocol is bit-identical between the two; only the transport differs. CRC-16/CCITT-FALSE framing verified against fixed reference vectors in tests/hardware/test_tcp_radio_protocol.py (20 tests, no hardware required). examples/common.py: support radio_type='pymc_tcp' and 'pymc_usb', configurable via PYMC_TCP_HOST / PYMC_TCP_PORT / PYMC_TCP_TOKEN env vars (with legacy HELTEC_* aliases retained for backward-compat).
Author
|
Pushed an amend addressing the Copilot reviewer pass on the previous head ( Real bugs
Defensive
Lint
Tests: 21/21 pass ( Not addressed in this revision (flagging for your call):
|
rightup
pushed a commit
to pyMC-dev/pyMC_Repeater
that referenced
this pull request
May 13, 2026
Wires the TCPLoRaRadio and USBLoRaRadio drivers that landed in pyMC_core on 2026-05-13 (PR pyMC-dev/pyMC_core#68) into get_radio_for_board() so they can be selected from a repeater config file without any code change in main.py / api_endpoints. Both branches follow the existing pattern: read host/port (TCP) or serial port (USB) plus auth/LBT options from their own config section, share the LoRa parameters from the common `radio` section, fall back to the firmware-default sync word 0x12, and surface ImportError as a clear RuntimeError if the installed pymc_core is too old to ship the drivers. config.yaml.example documents both sections and updates the radio_type header comment with the full supported list. Five new tests in tests/test_radio_config.py monkeypatch the radio classes and verify the section/parameter wiring + missing-required-field errors. No web UI / endpoint changes — the deployment this targets edits the config file directly. A GUI wizard for these radio types can land separately if there's appetite.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new
LoRaRadioimplementations undersrc/pymc_core/hardware/:TCPLoRaRadio— talks to a pymc_usb firmware modem over a TCP socket (Wi-Fi or Ethernet). Stdlib-only (socket / threading / asyncio); no new pymc_core dependencies.USBLoRaRadio— same firmware, but over USB-CDC. Usespyserial(already a transitive dep of the KISS wrappers).Both are drop-in replacements for
SX1262Radiowhen the SX1262 module isn't attached to the host running pymc_core — sector arrays, distant antennas, multi-modem deployments, or simply moving the radio away from RPi RFI.The firmware itself supports 7 boards from one code tree (Heltec V3, Heltec T114, XIAO Wio-SX1262, RAK3112 WisMesh, ESP32-P4-Nano, Lilygo T3-S3, Ikoka Stick). All speak the same wire protocol; the host driver doesn't need to care which board it's talking to.
What's included
src/pymc_core/hardware/tcp_radio.pyLoRaRadioimpl, async reader thread, auto-reconnect with exponential backoff, optional shared-secret AUTHsrc/pymc_core/hardware/usb_radio.pypyserial; same wire protocolsrc/pymc_core/hardware/__init__.pyWsRadio/SX1262Radio/Kiss*patternexamples/common.pyradio_type='pymc_tcp'and'pymc_usb'branches increate_radio(). Env vars:PYMC_TCP_HOST/PYMC_TCP_PORT/PYMC_TCP_TOKEN, plus legacyHELTEC_*aliases for users migrating from the standalone drivertests/hardware/test_tcp_radio_protocol.pyWire protocol
Bit-identical between both transports. Documented in pymc_usb/firmware/include/protocol.h.
CRC is computed over
CMD + LEN + PAYLOAD(not over SYNC), poly0x1021, init0xFFFF, no reflect, no xor-out. Same as the canonical XMODEM/CCITT-FALSE; the test file pins the123456789 → 0x29B1reference vector so firmware and driver can't drift silently.Tested against firmware
v0.7.0(release with prebuilt binaries for all 7 boards).Test plan
python3 -m pytest tests/hardware/test_tcp_radio_protocol.py -v→ 20 passedfirmware.binfrom the v0.7.0 release)Open questions
Splitting: would you prefer this as two PRs (TCP first, USB follow-up)? They're independent and the TCP path is what the user-facing sector-array deployments actually use; USB is mostly for bench testing. Happy to split if it helps review.
Naming: I went with
pymc_tcp/pymc_usbfor theradio_typestrings (matching the firmware repo name post-2025 rename). Kepttcp_heltec/usb_heltecas aliases so anyone with an existing config keeps working. OK or would you prefer different names?Sync word default: firmware ships with
0x12(RadioLibSYNC_WORD_PRIVATE, MeshCore default). Historic pymc_core default in some examples was0x3444. Thepymc_tcp/pymc_usbbranches default to0x12to match what the firmware does on first boot, butSET_CONFIGfrom the host overrides it atbegin()time anyway. Flag if you want this surfaced more loudly in the docstring.Out of scope
pymc_core>=X.Ydep bump).Backward-compat
radio_typevalues (waveshare,uconsole,meshadv-mini,kiss-tnc,kiss-modem,ch341) untouched.pyserial(USBLoRaRadio just won't be in__all__).HELTEC_*env-var names accepted on top ofPYMC_TCP_*preferred names; legacytcp_heltec/usb_heltecradio_typealiases also work.References