From 9d6b21a2649022998849460589cd49665ba324a9 Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Tue, 6 Oct 2026 21:43:35 +0200 Subject: [PATCH] comment diet vol.2 --- docs/ARCHITECTURE.md | 28 ++- zephcore/adapters/ble/ZephyrBLE.cpp | 75 ++----- zephcore/adapters/ble/ble_dfu.cpp | 32 +-- zephcore/adapters/board/ZephyrBoard.cpp | 82 +------ zephcore/adapters/board/zephyr_poweroff.c | 15 +- zephcore/adapters/clock/ZephyrRTCDiscover.c | 25 +-- zephcore/adapters/clock/ZephyrRTCDiscover.h | 46 +--- .../adapters/datastore/ZephyrDataStore.cpp | 53 +---- zephcore/adapters/datastore/ZephyrFsFormat.c | 35 +-- zephcore/adapters/datastore/ZephyrFsFormat.h | 13 +- zephcore/adapters/gps/ZephyrGPSManager.cpp | 107 ++------- zephcore/adapters/gps/ZephyrGPSManager.h | 24 +- zephcore/adapters/gps/gps_internal.h | 12 +- zephcore/adapters/gps/gps_module_cfg.cpp | 207 +++--------------- zephcore/adapters/gps/gps_power.cpp | 206 +++-------------- zephcore/adapters/radio/LoRaRadio.cpp | 14 +- zephcore/adapters/radio/radio_common.h | 79 ++----- zephcore/adapters/rng/ZephyrRNG.cpp | 143 ++---------- zephcore/adapters/rng/ZephyrRNG.h | 47 +--- .../adapters/sensors/ZephyrEnvSensors.cpp | 17 +- zephcore/adapters/sensors/t1000e_analog.c | 28 +-- zephcore/adapters/usb/ZephyrCompanionUSB.cpp | 37 +--- zephcore/adapters/usb/ZephyrUSBCDC.cpp | 33 +-- zephcore/app/CompanionMesh.cpp | 92 ++------ zephcore/app/CompanionMesh.h | 13 +- zephcore/app/ObserverMesh.cpp | 13 +- zephcore/app/RepeaterDataStore.cpp | 17 +- zephcore/app/RepeaterMesh.cpp | 68 +----- zephcore/app/RoomServerMesh.cpp | 27 +-- zephcore/app/VContact.cpp | 9 +- zephcore/app/main_observer.cpp | 28 +-- zephcore/helpers/CommonCLI.cpp | 158 +++---------- zephcore/helpers/NodePrefs.h | 146 +++--------- zephcore/helpers/battery_curve.c | 10 +- zephcore/helpers/boot_prefs.h | 15 +- zephcore/helpers/input/stc8h_keypad.c | 12 +- zephcore/helpers/led_gate.h | 36 +-- zephcore/helpers/ui-button/ui_pages.c | 31 +-- zephcore/helpers/ui-button/ui_task.c | 75 ++----- zephcore/helpers/ui-joystick/joystick_defs.h | 23 +- zephcore/helpers/ui/display.c | 39 +--- zephcore/helpers/ui/display.h | 57 +---- zephcore/helpers/ui/doom_game.c | 34 +-- zephcore/helpers/ui/haptic.c | 12 +- zephcore/helpers/ui/ui_common.c | 29 +-- zephcore/helpers/ui/ui_task.h | 16 +- zephcore/src/ContentionTracker.cpp | 11 +- zephcore/src/main_companion.cpp | 17 +- zephcore/src/server_main_common.cpp | 97 ++------ zephcore/tools/crypto_bench/src/main.cpp | 24 +- .../lr1110_updater/src/lr11xx_hal_updater.c | 84 ++----- zephcore/tools/lr1110_updater/src/main.c | 144 +++--------- zephcore/tools/rng_selftest/src/main.cpp | 10 +- 53 files changed, 515 insertions(+), 2190 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c6aa673..a0ea9b0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -598,7 +598,8 @@ mesh::Mesh Handles the binary BLE protocol with ~50 command opcodes. Key features: - **Offline queue**: circular buffer with peek/confirm pattern (survives BLE drops); `CONFIG_ZEPHCORE_OFFLINE_QUEUE_SIZE`, default 256 frames (lowered on RAM-bound boards) - **ACK tracking**: 8-slot table, computes expected ACK = SHA256(secret + hash)[0:4] -- **Contact iteration**: Streaming protocol with `lastmod` filtering for incremental sync +- **Contact iteration**: Streaming protocol with `lastmod` filtering for incremental sync. The dump advances only + while a client is connected and every connected transport has room; it is cancelled when the last client leaves - **Lazy write batching**: Dirty contacts/channels flush after 5-second delay - **Protocol versioning**: V2/V3 frame format negotiation with phone app - **Ed25519 signing**: 3-phase flow (start→data→finish) for signing up to 8KB @@ -763,7 +764,7 @@ Full command reference with constraints and remote-admin restrictions: `CLI_comm - TX congestion control: queue (12 frames) + overflow buffer + retry + timeout watchdog - Fast/slow advertising switching with post-disconnect flap prevention - DLE (Data Length Extension) to 251 bytes -- Interface coexistence: BLE vs USB, one active at a time +- One of the companion transports served at the same time (BLE, USB/UART, TCP; [7.6](#76-wifi--mqtt--tcp-transports)) - Debug: build with `debug.conf` plus `-DCONFIG_ZEPHCORE_BLE_LOG_LEVEL_DBG=y` for adapter-level DBG logging ### 7.2 DataStore (`adapters/datastore/`) @@ -771,7 +772,9 @@ Full command reference with constraints and remote-admin restrictions: `CLI_comm - **Internal**: LittleFS on flash (`/lfs`), 256-byte cache for reduced flash I/O - **External**: Optional LittleFS on QSPI (`/ext`) with auto-migration. The flash is `zephyr,deferred-init` and `/ext` is not automounted (`boards/common/qspi-ext.dtsi`): `zephcore_fs_mount_ext()` brings it up on first use, because a - boot-time probe could beat a cold power rail (first boot after a UF2 update) and silently drop to internal flash + boot-time probe could beat a cold power rail (first boot after a UF2 update) and silently drop to internal flash. + Roles that keep nothing on `/ext` (repeater, room server, observer) put the flash into deep power-down at start-up + (`zephcore_fs_ext_power_down()`), with chip-select held high; a factory `erase` resumes it and puts it back - **BLE bonds**: NVS (`storage_partition`, 0xD0000 on nRF52) via Zephyr settings backend (≥1.16.2) - **Prefs**: `prefs.json` through upstream's `ConfigSerializer`, with upstream's key names for shared fields and ZephCore's own under `zc` (see §13); the older binary file is read once to migrate and kept - **Contacts**: 152-byte records, stored on external flash if available @@ -829,9 +832,20 @@ Repeaters and room servers default to `CONFIG_ZEPHCORE_REPEATER_GPS_INTERVAL_SEC ### 7.4 USB (`adapters/usb/`) -- **CompanionUSB**: V3-framed CDC (little-endian 16-bit length prefix + payload) -- **RepeaterUSB**: Minimal CDC with 1200-baud DFU touch detection -- Both share message queues with BLE adapter (transport-agnostic mesh layer) +- **`ZephyrUSBCDC`** (every role): the USB device and its CDC ACM class, the 1200-baud touch into the bootloader, DTR + tracking, and `zephcore_usbd_detach()` before a reset (a soft reset leaves the ESP32-S3's PHY attached). A host that + went away is reported as a DTR drop in three cases: DTR low, VBUS removed, and a bus reset. The reset case exists + because the CDC class keeps the old line state across a reset, so nothing else says that the port the host had open + is gone (a hub resetting upstream leaves VBUS up). Bus suspend, resume and reset also reach a second callback. +- **`ZephyrCompanionUSB`** (companion): the wired companion transport, over the CDC port or a plain UART + (`zephcore,companion-uart`). Framing: `<` (to the node) or `>` (from it), a little-endian 16-bit length, the payload; + a first byte that is printable and not `<` starts the text CLI instead. It has its own receive queue and an + interrupt-driven 2 KB TX ring whose "drained" callback paces the contact dump. +- **Session rule**: a session starts on the first inbound frame or CLI line and ends on a DTR drop as defined above. + While the bus is suspended (the computer asleep with the port open) the session is kept but is not a connected + client: nothing is queued for it, and it does not hold up the clean-up that runs when the last client leaves. Each + change reaches the main loop as a connect or a disconnect. +- Servers (repeater, room server, observer) use the CDC port as their serial console. ### 7.5 Board (`adapters/board/`) @@ -845,7 +859,7 @@ Repeaters and room servers default to `CONFIG_ZEPHCORE_REPEATER_GPS_INTERVAL_SEC **Telemetry** (`REQ_TYPE_GET_TELEMETRY_DATA`, every role, upstream's shape): battery on channel 1, then `sensors.querySensors()` (`adapters/sensors/ZephyrSensorManager.cpp`): GPS position on channel 1 while the GPS is on and has a fix, each environment sensor and power-monitor channel found at boot on its own channel from 2 up (probe order), board-local analog sensors (T1000-E) on channel 1; then the MCU temperature on channel 1. Servers give guests battery + MCU temperature only and honour the requester's inverse permission mask, as upstream. Encoded by `helpers/compat/CayenneLPP.h`: upstream's library API and wire format, but values round to the nearest step where the library truncates. -**Clock** (`adapters/clock/ZephyrRTCClock`): `setCurrentTime()` also writes the hardware RTC when the board has one (coalesced on the system work queue), as upstream's `AutoDiscoverRTCClock` writes its chip; `seedCurrentTime()` is the boot restore, not written back. Without a hardware RTC, the last time read is kept in no-init memory (RTC slow memory on ESP32) and restored after a reset that keeps RAM (reboot, crash, watchdog), as upstream's `ESP32RTCClock`; a power cycle starts from 1970 again. +**Clock** (`adapters/clock/ZephyrRTCClock`): `setCurrentTime()` also writes the hardware RTC when the board has one (coalesced on the system work queue), as upstream's `AutoDiscoverRTCClock` writes its chip; `seedCurrentTime()` is the boot restore, not written back. A chip is adopted at boot only once it is identified (`adapters/clock/ZephyrRTCDiscover.c`), because other parts can answer at the same address: a device is passed over when two reads each show a bit set that the chip's data sheet fixes at zero, or a time field out of range, and a time is restored only from a read in which every field is valid. Without a hardware RTC, the last time read is kept in no-init memory (RTC slow memory on ESP32) and restored after a reset that keeps RAM (reboot, crash, watchdog), as upstream's `ESP32RTCClock`; a power cycle starts from 1970 again. **LED master switch** (`helpers/led_gate.{c,h}`, `set leds on|off`, all roles): one process-wide flag every LED driver consults — heartbeat and unread-message LEDs in `helpers/ui/ui_common.c`, the diff --git a/zephcore/adapters/ble/ZephyrBLE.cpp b/zephcore/adapters/ble/ZephyrBLE.cpp index 5324cba..49fc0c9 100644 --- a/zephcore/adapters/ble/ZephyrBLE.cpp +++ b/zephcore/adapters/ble/ZephyrBLE.cpp @@ -117,17 +117,9 @@ static bool fast_adv_active; * Cleared by recycled() itself (both run on the cooperative system work queue). */ static bool adv_stop_for_interval_change; -/* Ground truth for "controller is currently broadcasting adv PDUs": - * set TRUE : bt_le_adv_start() returned success - * set FALSE : bt_le_adv_stop() called explicitly (set_enabled(false), - * adv_slow_work interval change, update_name restart) - * set FALSE : a phone connected — Zephyr stops adv internally to consume - * the BT_MAX_CONN=1 slot (no slot left to advertise from). - * Re-set TRUE later when recycled() → start_adv() runs. - * Exposed via zephcore_ble_is_advertising() for the companion advertising - * watchdog (main_companion.cpp housekeeping) that catches transient - * bt_le_adv_start failures. Arduino nrf52 has an equivalent 10s watchdog - * (SerialBLEInterface.cpp:343). */ +/* True while the controller is advertising: set when bt_le_adv_start() + * succeeds, cleared on an explicit stop and when a central connects. Read by + * the companion's advertising watchdog. */ static bool adv_running; /* Administrative BLE state */ @@ -159,13 +151,8 @@ static void kick_tx_drain(void); /* ========== GATT Service ========== */ /* - * NUS service — secured with AUTHEN permissions. - * Matches Arduino's SECMODE_ENC_WITH_MITM on bleuart. - * - * When the phone tries to subscribe (CCC write) or send data (RX write), - * Zephyr returns ATT_ERR_AUTHENTICATION. The phone's BLE stack then - * initiates pairing (PIN dialog). After pairing succeeds, - * security_changed() fires at L3+ and the phone retries the operation. + * NUS service; every attribute needs an authenticated link. An unpaired + * phone gets ATT_ERR_AUTHENTICATION, pairs, and retries. */ BT_GATT_SERVICE_DEFINE(secure_nus_svc, BT_GATT_PRIMARY_SERVICE(BT_UUID_NUS_SERVICE), @@ -191,23 +178,9 @@ K_WORK_DELAYABLE_DEFINE(tx_drain_work, tx_drain_work_fn); K_WORK_DELAYABLE_DEFINE(adv_slow_work, adv_slow_work_fn); /* ========== Unpaired-connection timeout ========== - * - * A connection that never reaches L2 holds the node's only peripheral slot. - * With CONFIG_BT_MAX_CONN=1 Zephyr stops advertising while that slot is taken, - * and the companion's advertising watchdog (main_companion.cpp) deliberately - * skips any state where a connection exists — so a client that connects and - * never pairs makes the node invisible to everyone else until it is power - * cycled. A BLE scanner app left connected does it by accident; iOS does it - * routinely. Nothing else times the connection out: pairing here is reactive - * by design (Apple §55 — we never send a Security Request, we wait for the - * phone to hit ATT insufficient-authentication and start pairing itself), so - * "connected but idle forever" is a state the node otherwise accepts happily. - * - * Dropping it costs a legitimate client nothing: every characteristic on both - * services is *_AUTHEN, so an unsecured connection cannot read, write or - * subscribe to anything. The window has to cover discovery plus the phone's - * own pairing dialog — 15 s matches upstream MeshCore PR #3263, which measured - * a real unpaired connection being dropped at ~13 s. */ + * A connection that never reaches security L2 holds the only peripheral slot + * and stops advertising, so it is dropped after this long. An unsecured + * connection can do nothing here, so the drop costs a real client nothing. */ #define BLE_SECURITY_TIMEOUT_MS 15000 static void sec_timeout_conn_cb(struct bt_conn *conn, void *user_data) @@ -278,15 +251,9 @@ static void build_device_name_and_adv(const char *name_from_prefs) /* Prepend "MeshCore-" prefix so apps that filter on it can find us */ snprintf(device_name, sizeof(device_name), "MeshCore-%s", name_from_prefs); - /* Sanitize the BLE-advertised name to printable ASCII, compacting - * in place (write index w never outpaces read index r): - * - ':' / ';' -> '-' (Apple Accessory Design Guidelines) - * - non-ASCII bytes -> dropped (e.g. emoji in the node name). - * iOS's BLE scanner blanks the WHOLE advertised name if it - * contains any non-ASCII byte, showing the device nameless. - * This only sanitizes the BLE/GAP copy; the mesh node name (emoji - * and all) is untouched and still shown by the companion app. - */ + /* Sanitize the advertised name to printable ASCII, in place: ':' and ';' + * become '-', non-ASCII bytes are dropped (iOS blanks a name containing + * any). The mesh node name is untouched. */ size_t w = 0; for (size_t r = 0; device_name[r]; r++) { unsigned char c = (unsigned char)device_name[r]; @@ -357,15 +324,8 @@ static void connected(struct bt_conn *conn, uint8_t err) * with a fallback in security_changed() if PHY update never fires. */ dle_requested = false; - /* Do NOT proactively request security here. - * - * Apple Accessory Design Guidelines §55 (Pairing): the accessory should - * not request pairing until an ATT request is rejected with "Insufficient - * Authentication." Pairing is triggered reactively when the phone tries - * to access our AUTHEN-secured GATT attributes (CCC write / RX write). - * - * For bonded reconnects, Zephyr auto-encrypts with stored keys when - * CONFIG_BT_SMP and CONFIG_BT_BONDABLE are enabled. */ + /* Do NOT request security here (ADR 0004): pairing starts when the phone + * hits an AUTHEN attribute. Bonded reconnects are encrypted by the stack. */ /* Notify main of BLE connection */ if (ble_cbs && ble_cbs->link.on_connected) { @@ -636,13 +596,8 @@ static void tx_drain_work_fn(struct k_work *work) int err; /* - * BLE TX path - Event-driven (like Arduino's HVN_TX_COMPLETE) - * Uses bt_gatt_notify_cb() callback to chain TX without polling. - * Re-entrancy guard prevents concurrent notify calls. - * - * IMPORTANT: Must wait for ble_tx_ready before sending. This is set in - * security_changed() after encryption is established. Sending before the - * connection is fully secured causes "No ATT channel for MTU" errors. + * Event-driven TX: the notify-complete callback chains the next frame. + * Nothing is sent before ble_tx_ready (set once the link is encrypted). */ if (!current_conn || !nus_notif_enabled || !ble_tx_ready) { LOG_DBG("tx_drain[BLE]: not ready (conn=%p notif=%d ready=%d)", diff --git a/zephcore/adapters/ble/ble_dfu.cpp b/zephcore/adapters/ble/ble_dfu.cpp index ddc2c41..47f7c22 100644 --- a/zephcore/adapters/ble/ble_dfu.cpp +++ b/zephcore/adapters/ble/ble_dfu.cpp @@ -22,34 +22,12 @@ LOG_MODULE_DECLARE(zephcore_ble, CONFIG_ZEPHCORE_BLE_LOG_LEVEL); #include "ble_internal.h" /* ========== Legacy Nordic/Adafruit buttonless DFU service ========== + * As Adafruit BLEDfu: a paired phone writes 0x01 to the control point and the + * node resets into the bootloader's BLE OTA mode (GPREGRET 0xA8). All three + * characteristics are present because iOS requires them; Packet is a no-op. * - * Mirrors Adafruit BLEDfu (Arduino MeshCore >=1.15.0) so the same DFU tools - * interoperate: a paired phone writes 0x01 to the control point and the device - * resets into the bootloader's BLE OTA mode. We use the *unbonded* OTA reset - * (GPREGRET 0xA8, same as `start ota`): the bootloader comes up as a fresh DFU - * target and the tool re-scans for it (the legacy buttonless flow). Adafruit's - * 0xB1 bonded-resume path needs SoftDevice peer-data enrollment we can't - * replicate on Zephyr, so the phone makes a fresh connection to the bootloader. - * - * The service exposes the FULL legacy DFU shape Adafruit ships (not just the - * control point): iOS's LegacyDFUService treats the DFU Packet (1532) char as - * a *required* characteristic — discovery fails (DFU "instantly fails") without - * it — and reads the DFU Revision (1534 = 0x0001 "app mode") to classify the - * device as an application that supports the buttonless jump (missing → the app - * can't identify the device → shows it nameless). Packet is a no-op here: the - * real image upload happens in the bootloader, not the running app. All three - * chars sit behind AUTHEN, matching the Arduino companion's service-wide MITM - * floor (`bledfu.setPermission(SECMODE_ENC_WITH_MITM, ...)`); a bonded phone's - * DFU app reuses the OS-level bond, an unpaired stranger is rejected. - * - * NOTE on the service symbol name: Zephyr registers static GATT services in - * the order ld's SORT_BY_NAME emits them — i.e. alphabetically by the symbol - * passed to BT_GATT_SERVICE_DEFINE. This service MUST sort *after* - * `secure_nus_svc` so the NUS attribute handles stay fixed; otherwise every - * NUS handle shifts and bonded phones with cached handles get ATT 0x03 - * (Write Not Permitted) on the NUS RX write. Hence the `secure_nus_svc_dfu` - * name (a string sorts before its own extensions, so NUS keeps the low range). - */ + * The service symbol must sort after secure_nus_svc (static services are + * registered in symbol order), or the NUS handles shift under bonded phones. */ static struct bt_uuid_128 dfu_svc_uuid = BT_UUID_INIT_128( BT_UUID_128_ENCODE(0x00001530, 0x1212, 0xefde, 0x1523, 0x785feabcd123)); static struct bt_uuid_128 dfu_ctrl_uuid = BT_UUID_INIT_128( diff --git a/zephcore/adapters/board/ZephyrBoard.cpp b/zephcore/adapters/board/ZephyrBoard.cpp index 851398f..7310826 100644 --- a/zephcore/adapters/board/ZephyrBoard.cpp +++ b/zephcore/adapters/board/ZephyrBoard.cpp @@ -37,59 +37,17 @@ #endif /* ESP32-S3: reboot into the ROM download mode rather than back into the app. - * - * The USB CDC companion transport (boards/common/esp32s3_usb.conf) hands the - * shared D+/D- pads to USB OTG and disables USB-Serial-JTAG. USJ is exactly - * what esptool drives to put the chip into download mode, so once our CDC-ACM - * device owns the port esptool's auto-reset is inert: DTR/RTS land on a Zephyr - * CDC endpoint that is not wired to EN/BOOT, and the only way back into the - * bootloader is the physical BOOT button. - * - * FORCE_DOWNLOAD_BOOT lives in the RTC domain, which a software system reset - * does not clear (only a power-on reset does), so setting it and rebooting - * brings the ROM up in download mode. That gives both `start dfu` and the - * Arduino-style 1200-baud touch a way to hand the port back to esptool / - * esptool-js / the web configurator. - * - * Setting it is not enough on its own, though. WHICH USB controller the ROM - * appears on is a separate mux, and it lives in the same RTC domain: Zephyr's - * DWC2 quirk layer claims the internal PHY for USB OTG at init - * (usb_wrap_ll_phy_enable_external(hw, false) -> RTC_CNTL_USB_CONF_REG - * SW_HW_USB_PHY_SEL=1, SW_USB_PHY_SEL=1), and those bits survive the reboot - * exactly like FORCE_DOWNLOAD_BOOT does. Left alone the ROM therefore comes up - * on USB OTG (303a:0009), not USB-Serial-JTAG (303a:1001). That state is still - * flashable -- esptool reports "USB mode: USB-OTG" and even loads the stub -- - * but it breaks everything that expects USJ: esptool's auto-reset does nothing - * over ROM OTG CDC so it needs --before no-reset, and esptool-js special-cases - * only PID 0x1001, so browser flashers fall into a classic DTR/RTS reset path - * that cannot work. Clearing SW_HW_USB_PHY_SEL hands the mux back to - * hardware/eFuse control, whose default routes the internal PHY to USJ -- i.e. - * it reproduces what a power-on reset would have left behind. - * - * Same register and sequence Arduino-ESP32 uses in usb_persist_restart() - * (RESTART_BOOTLOADER). OPTION1 carries no other bits on this SoC, but set the - * bit rather than writing the word so it stays correct if that changes. Scoped - * to the S3 deliberately: it is the only part we ship whose USB port can be - * taken away from the ROM this way (C3/C6 use a different LP_AON register, and - * classic ESP32 has no native USB — it always flashes through its UART bridge). - */ + * FORCE_DOWNLOAD_BOOT and the USB PHY mux both live in the RTC domain and + * survive a software reset. */ #if defined(CONFIG_SOC_SERIES_ESP32S3) #include #include #define ESP32_FORCE_DOWNLOAD_BOOT 1 #endif -/* Detach the USB device stack before a reset so the host sees a real unplug — - * see zephcore_usbd_detach(). Matters most on the ESP32-S3 USB companion, - * whose OTG PHY survives a soft reset with D+ still pulled up. - * - * The condition below must be EXACTLY the one CMakeLists.txt uses to compile - * adapters/usb/ZephyrUSBCDC.cpp, or this call site compiles against an - * implementation that is never linked (findings #22 — that failure mode has - * already cost this repo four separate undefined-reference bugs). The - * repeater/observer/room-server branches use the inner half alone; the - * companion branch wraps it in (LOG || COMPANION_USB || COMPANION_SERIAL), and - * ZEPHCORE_COMPANION is the compile definition that identifies that branch. */ +/* Detach the USB device stack before a reset so the host sees a real unplug. + * The condition must be exactly the one CMakeLists.txt uses to compile + * adapters/usb/ZephyrUSBCDC.cpp, or the call is never linked. */ #if !defined(CONFIG_CDC_ACM_SERIAL_INITIALIZE_AT_BOOT) && \ (defined(CONFIG_USB_CDC_ACM) || defined(CONFIG_USBD_CDC_ACM_CLASS)) #if !defined(ZEPHCORE_COMPANION) || defined(CONFIG_LOG) || \ @@ -146,13 +104,8 @@ static const struct device *vbat_enable_dev = NULL; #endif /* - * Battery voltage multiplier - prefer devicetree, fallback to Kconfig - * Formula: Battery_mV = (raw * VBAT_MV_MULTIPLIER) / 4096 - * - * To define in devicetree, add to board's DTS/overlay: - * zephyr,user { - * vbat-mv-multiplier = <7200>; - * }; + * Battery_mV = raw * VBAT_MV_MULTIPLIER / 4096. Devicetree + * (zephyr,user vbat-mv-multiplier) wins over the Kconfig fallback. */ #define ZEPHYR_USER_NODE DT_PATH(zephyr_user) #if DT_NODE_HAS_PROP(ZEPHYR_USER_NODE, vbat_mv_multiplier) @@ -521,26 +474,15 @@ void ZephyrBoard::rebootToBootloader() usb_serial_jtag_ll_phy_enable_pad(false); k_msleep(100); #ifdef ESP32_FORCE_DOWNLOAD_BOOT - /* ...but put the pad back before resetting into the ROM. The pad-enable - * bit survives a software reset, and unlike the app the ROM download mode - * never re-enables it, so leaving it off keeps USB-Serial-JTAG off the bus: - * the host sees the port die and only a power cycle brings it back. That - * was `start dfu` on every S3 build whose console is USJ (repeater, - * observer, room server, debug). The 100 ms low above still gives the - * host its disconnect; the ROM then enumerates as 303a:1001. */ + /* Put the pad back before resetting into the ROM: the bit survives the + * reset and the ROM never re-enables it. */ usb_serial_jtag_ll_phy_enable_pad(true); #endif #endif #ifdef ESP32_FORCE_DOWNLOAD_BOOT - /* Hand the internal USB PHY back to USB-Serial-JTAG, so the ROM download - * mode we just armed appears as 303a:1001 rather than ROM USB OTG. See - * the header comment on ESP32_FORCE_DOWNLOAD_BOOT for why this is needed - * and why it has to happen HERE -- after zephcore_usbd_detach(), which - * has to drop the D+ pull-up while OTG still owns the PHY, or the host - * never sees a clean disconnect. - * - * No-op on builds that never took the PHY (repeater/observer/debug keep - * USJ, so both bits are already 0). */ + /* Hand the internal USB PHY back to USB-Serial-JTAG, so the ROM appears as + * 303a:1001. After zephcore_usbd_detach(), which needs OTG to still own the + * PHY. */ REG_CLR_BIT(RTC_CNTL_USB_CONF_REG, RTC_CNTL_SW_HW_USB_PHY_SEL); REG_CLR_BIT(RTC_CNTL_USB_CONF_REG, RTC_CNTL_SW_USB_PHY_SEL); #endif diff --git a/zephcore/adapters/board/zephyr_poweroff.c b/zephcore/adapters/board/zephyr_poweroff.c index 4b58876..840d249 100644 --- a/zephcore/adapters/board/zephyr_poweroff.c +++ b/zephcore/adapters/board/zephyr_poweroff.c @@ -178,18 +178,9 @@ void zephcore_power_off(void) } #endif /* CONFIG_SOC_FAMILY_NORDIC_NRF && sw0 */ - /* 5. Release the board power latch — must be dead last. - * - * Only present on soft-power boards (see zephcore,poweroff-gpios). The - * rail is cut here rather than in step 2 because the pins are ordered - * loads-first/latch-last, and because step 4 must have observed the - * button release first: on these boards the button is also the power-on - * input, so dropping the latch while it is still held would let the I/O - * controller re-latch the rail immediately. - * - * On battery this does not return — the supply is gone mid-loop, which - * is the intended outcome. On USB the rail may be held up externally, in - * which case we fall through to System OFF. */ + /* 5. Release the board power latch, dead last: loads are already off and + * step 4 has seen the button released (it is also the power-on input). On + * battery this does not return; on USB we fall through to System OFF. */ #if DT_HAS_COMPAT_STATUS_OKAY(zephcore_poweroff_gpios) { #define _PWROFF_NODE DT_COMPAT_GET_ANY_STATUS_OKAY(zephcore_poweroff_gpios) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.c b/zephcore/adapters/clock/ZephyrRTCDiscover.c index 7f0b6b5..b701a4b 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.c +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.c @@ -197,14 +197,9 @@ static bool rtc_all_ff(const uint8_t blk[7]) return true; } -/* Decide whether the device at d is the RTC it declares. A first read that - * fails means nothing is there, and one that is all 0xFF is an erased EEPROM, - * though a real RTC can power up that way, so it is skipped for now. - * Otherwise the device is ruled out only when two reads each rule it out: a - * failed second read does not, an all-0xFF second read does, and an - * unreadable power-loss flag cannot vouch for the fields. RTC_FOUND leaves - * the block to decode in blk; RTC_FOUND_GARBLED is this RTC with no clean - * read to take a time from. */ +/* Decide whether the device at d is the RTC it declares: ruled out only when + * two reads each rule it out. RTC_FOUND leaves the block to decode in blk; + * RTC_FOUND_GARBLED is this RTC with no clean read to take a time from. */ static enum rtc_verdict rtc_identify(const struct rtc_desc *d, uint8_t blk[7]) { uint8_t again[7]; @@ -386,16 +381,10 @@ static uint8_t rv3028_cfg_backup_ram(const struct rtc_desc *d, uint8_t ram) return ram; } -/* Store the descriptor's rv3028-eeprom-config in the chip's EEPROM. The - * EEPROM is only touched with automatic refresh held off (4.6.7) and backup - * switchover disabled in RAM (3.15.6: BSM 00 for any EEPROM read or write). - * Each byte is compared against the EEPROM itself (4.6.6) and written only if - * it differs (4.6.5). A closing Refresh (4.6.4) reloads RAM from the EEPROM, - * switching back to the stored BSM, and the config is read back on every run. - * A triplet outside 35h-37h, masking an unimplemented bit, or repeating a - * register is ignored, so a devicetree mistake cannot write on every boot. - * Reports whether the config was confirmed, and if so whether EERD, which - * also stops the daily refresh, was cleared afterwards. */ +/* Store the descriptor's rv3028-eeprom-config in the chip's EEPROM, writing + * only bytes that differ, with refresh and backup switchover held off as the + * data sheet requires. Reports whether the config was confirmed, and if so + * whether EERD was cleared afterwards. */ enum rv3028_store { RV3028_STORED, RV3028_EERD_SET, RV3028_NOT_STORED }; static enum rv3028_store rv3028_store_config(const struct rtc_desc *d) diff --git a/zephcore/adapters/clock/ZephyrRTCDiscover.h b/zephcore/adapters/clock/ZephyrRTCDiscover.h index c2d22ae..42943c3 100644 --- a/zephcore/adapters/clock/ZephyrRTCDiscover.h +++ b/zephcore/adapters/clock/ZephyrRTCDiscover.h @@ -27,50 +27,20 @@ extern "C" { * Probe the declared RTC chips in order, stopping at the first that holds a * sane time (year >= 2025 and its power-loss flag clear): store its Unix * epoch in *epoch_out and return true. The first chip found (valid time or - * not) is remembered as the write-back target. A failed first read means - * nothing is there, and a first read of all 0xFF (an erased EEPROM) is - * skipped. Otherwise a device is passed over only if two reads each show it - * is not that RTC: a bit set that the data sheet shows as 0 (the - * descriptor's zero-mask), seconds, minutes, date or month out of range - * while the power-loss flag does not read as set (an unreadable flag counts - * as not set), or, on the second read, all 0xFF. A failed second read never - * rules a device out. A time is taken only from a clean read whose fields, - * hours and year are each in range (12-hour mode is not decoded). Returns - * false if none present or no trustworthy time is held. + * not) is remembered as the write-back target. Returns false if none is + * present or none holds a trustworthy time. * - * At a descriptor with rv3028-eeprom-config, the time is read once more - * with the chip's backup switch flag (BSF) cleared before and checked - * after, and is not taken if the read failed or BSF was set. A chip adopted - * on a read that identified it (not on a failed second read), with BSF - * clear, has that config stored at once: after the power-on refresh (up to - * ~66 ms), backup switchover is turned off while the EEPROM is read, each - * byte that differs is written, and a refresh from EEPROM restores it. - * Measured on a RAK4631: about 14 ms when nothing is written, 52 ms for two - * bytes, then up to 2 ms for DSM to react (4.2.2). If the store fails, the - * config is set in RAM, which lasts until the chip's next refresh from - * EEPROM (daily while EERD is clear), and only once EEbusy reads 0; - * otherwise switchover stays as the store left it, off once the store has - * disabled it, until a retry or that refresh. A failed store, or one - * skipped at boot, is retried on the system work queue every 10 minutes, at - * most 3 times, each retry identifying the chip again first. + * A descriptor with rv3028-eeprom-config also has that config stored in the + * chip's EEPROM, retried on the system work queue if it fails. */ bool zephcore_rtc_restore(uint32_t *epoch_out); /* * Persist an authoritative epoch to the discovered RTC chip and clear its - * power-loss flag. No-op if no RTC was discovered. Safe to call often, but - * intended only for real syncs (GPS/app/CLI), not per-packet clock nudges. - * If restore adopted nothing but skipped a candidate whose first read was all - * 0xFF, the first save probes again, once per boot, in the caller's context. - * A device ruled out on its second read is not probed again for that reason. - * - * At a descriptor with rv3028-eeprom-config the time is written with BSF - * cleared before and checked after, year 00h first and the real year last, - * so a write cut by a power loss reads at boot as "time not yet set". A - * write not confirmed is repeated every 5 s on the system work queue, with - * the time run on, until one is confirmed, a newer save replaces it, or 12 - * repeats (13 attempts in all) have failed. Call only from the system work - * queue: the repeat shares state with this call. + * power-loss flag. No-op if no RTC was discovered. Intended for real syncs + * (GPS/app/CLI), not per-packet clock nudges. Call only from the system work + * queue: an unconfirmed write is repeated there and shares state with this + * call. */ void zephcore_rtc_save(uint32_t epoch); diff --git a/zephcore/adapters/datastore/ZephyrDataStore.cpp b/zephcore/adapters/datastore/ZephyrDataStore.cpp index 04b350c..38b985a 100644 --- a/zephcore/adapters/datastore/ZephyrDataStore.cpp +++ b/zephcore/adapters/datastore/ZephyrDataStore.cpp @@ -301,19 +301,10 @@ bool ZephyrDataStore::hasOldSettingsFile() const void ZephyrDataStore::adoptVolume() { - /* /lfs/_zc_init is written after the first clean boot of ZephCore, so - * this runs once per volume. - * - * - No prefs, or prefs whose radio preset is implausible (a file in - * some other layout): format everything, bonds included. - * - Our prefs plus /lfs/settings: an upgrade from ZephCore <= 1.16.1 on - * nRF52, whose file-based bond store left old app code where the NVS - * bond partition now is. Bytes there can pass NVS sector validation - * and hang settings_load(), so BLE never advertises: erase the NVS - * only; identity, prefs and contacts stay, the phone re-pairs. - * (On nRF54L and MG24 /lfs/settings is the live bond store, but a - * volume that has booted this firmware once has the marker.) - * - Our prefs, no /lfs/settings: bonds already live in NVS, keep all. */ + /* Runs once per volume (/lfs/_zc_init marks a clean first boot): no prefs or + * implausible prefs = format everything; our prefs plus /lfs/settings (an + * upgrade from <= 1.16.1 on nRF52) = erase the bond NVS only; our prefs and no + * /lfs/settings = keep all. See docs/ARCHITECTURE.md 7.2. */ if (hasInitMarker()) { return; } @@ -499,23 +490,8 @@ void ZephyrDataStore::saveContacts(DataStoreHost *host) { const char *path = contactsFile(); - /* Atomic replace ONLY where there is external flash — deliberately, and - * not to be "fixed" later. - * - * zephcore_fs_atomic_write() needs room for a second full copy before the - * rename. contacts3 is by far the largest store here (152 B per record, - * ~47 KB at 313 contacts) and internal /lfs on these boards is 128 KB - * total, shared with identity, prefs and channels2. Two copies would sit - * at ~94 KB of 128 KB before LittleFS metadata, so the atomic path could - * fail with ENOSPC exactly when it is most needed — a worse failure than - * the one it prevents. - * - * channels2, identity and prefs are atomic everywhere because they are - * small enough for the second copy to be free. Only contacts is gated. - * - * The non-atomic branch below is therefore the constrained-board path, - * and it writes in place and truncates rather than unlinking first — see - * the note there. */ + /* Atomic replace only where there is external flash: internal /lfs (128 KB) + * cannot hold a second copy of the contacts file. Deliberate. */ bool use_atomic = _has_ext_fs; const char *save_mode = use_atomic ? "atomic" : "direct"; @@ -555,21 +531,8 @@ void ZephyrDataStore::saveContacts(DataStoreHost *host) }; write_ok = zephcore_fs_atomic_write(path, atomic_contacts_writer, &ctx, "saveContacts"); } else { - /* Overwrite in place, then truncate — never unlink first. - * - * This branch runs on boards with no external flash, i.e. the ones - * that cannot afford the atomic temp-file dance. It used to - * fs_unlink() the contacts file before recreating it, which left a - * window spanning the whole ~47 KB write where contacts3 did not - * exist at all: a power cut there lost every contact rather than - * corrupting some. The unlink was only ever a way to truncate. - * - * Truncating afterwards is the same guarantee without the window — - * the file is always present, and at worst briefly longer than its - * new contents (stale records past the end, which the truncate then - * removes). FS_O_TRUNC is NOT usable here: Zephyr's LittleFS - * backend maps only CREATE/READ/WRITE/APPEND and drops TRUNC - * silently, so asking for it would leave the stale tail in place. */ + /* Overwrite in place, then truncate; never unlink first (a power cut would + * lose every contact). FS_O_TRUNC is dropped by Zephyr's LittleFS backend. */ fs_file_t_init(&file); int rc = fs_open(&file, path, FS_O_CREATE | FS_O_WRITE); if (rc < 0) { diff --git a/zephcore/adapters/datastore/ZephyrFsFormat.c b/zephcore/adapters/datastore/ZephyrFsFormat.c index b65232e..1722099 100644 --- a/zephcore/adapters/datastore/ZephyrFsFormat.c +++ b/zephcore/adapters/datastore/ZephyrFsFormat.c @@ -39,19 +39,9 @@ static void flatten(uint8_t id, const char *tag) } #if PARTITION_EXISTS(qspi_storage_partition) -/* qspi-ext.dtsi makes the flash under /ext zephyr,deferred-init, so nothing - * probes it at boot. The flash sits behind a GPIO-switched rail on several - * boards, and the boot-time JEDEC read went out before a cold rail had come - * up: the driver failed, /ext stayed unmounted and contacts silently fell - * back to internal flash. A warm reboot keeps the rail charged; the first - * boot after a UF2 update (seconds in the bootloader, rail off) did not. - * Bringing the part up here, on first use, is long after the rail. Each role - * reaches this on its own path, so it is idempotent: -EALREADY means an - * earlier call already ran the init, whatever the result was. - * - * A role that does not use /ext parks the part in deep power-down - * (zephcore_fs_ext_power_down()); a format still has to reach it, so a - * suspended flash is resumed here. */ +/* Bring up the deferred-init flash under /ext on first use (a boot-time probe + * can beat a cold rail). Idempotent: -EALREADY means an earlier call ran the + * init. A flash parked in deep power-down is resumed. */ static void ext_flash_init(void) { const struct device *dev = PARTITION_DEVICE(qspi_storage_partition); @@ -143,14 +133,8 @@ bool zephcore_fs_format_all(bool *out_ext_mounted) *out_ext_mounted = false; } - /* Properly unmount from Zephyr's VFS before erasing flash. Clearing a - * local "mounted" flag is not enough — Zephyr would still hold /lfs - * mounted, so flash_area_flatten destroys the on-flash superblock while - * LittleFS considers itself active. Every subsequent file op then hits - * the erased blocks and logs "Corrupted dir pair at {0x0, 0x1}". - * FS_FSTAB_DECLARE_ENTRY exposes the non-static mount struct generated - * from the DTS fstab; fs_mount() on a blank partition auto-formats - * (littlefs_fs.c: lfs_mount fail -> lfs_format -> lfs_mount). */ + /* Unmount through the VFS before erasing: flattening under a mounted LittleFS + * corrupts it. fs_mount() on the blank partition then formats it. */ FS_FSTAB_DECLARE_ENTRY(DT_NODELABEL(lfs)); fs_unmount(&FS_FSTAB_ENTRY(DT_NODELABEL(lfs))); @@ -186,13 +170,8 @@ bool zephcore_fs_format_all(bool *out_ext_mounted) } #if DT_NODE_EXISTS(DT_NODELABEL(qspi_lfs)) - /* Remount external QSPI too. We unmounted it above and flattened its - * partition, so it must be re-mounted here — otherwise a runtime format - * (factory reset, or the first-boot "no prefs" auto-format) leaves /ext - * unmounted for the rest of the session. begin() then reads - * ext_lfs_mounted=false and the store falls back to internal /lfs, so - * contacts/channels save to /lfs and get needlessly migrated back to - * /ext on the next boot ("Migrating contacts to external storage"). */ + /* Remount /ext too, or a runtime format leaves it down for the session and + * the store falls back to /lfs. */ { bool ext_mounted = zephcore_fs_mount_ext(); diff --git a/zephcore/adapters/datastore/ZephyrFsFormat.h b/zephcore/adapters/datastore/ZephyrFsFormat.h index 7cccc04..5b27b06 100644 --- a/zephcore/adapters/datastore/ZephyrFsFormat.h +++ b/zephcore/adapters/datastore/ZephyrFsFormat.h @@ -18,15 +18,10 @@ extern "C" { /** * @brief Erase every ZephCore storage region and remount the filesystems. * - * Unmounts /lfs (and /ext when the board has QSPI), flattens lfs_partition, - * storage_partition (BLE bonds NVS) and qspi_storage_partition where each - * exists, then remounts. A blank LittleFS partition is auto-formatted by - * fs_mount(), so the volume comes back empty rather than corrupt. - * - * This is the only path that erases the LittleFS *volume*. Deleting files - * cannot recover a volume another firmware has written into — on nRF52840 the - * Adafruit core's InternalFileSystem lives at 0xED000, inside our 0xD4000 - * lfs_partition, and its format() scribbles our top 7 blocks. + * Unmounts /lfs (and /ext), flattens lfs_partition, storage_partition (BLE + * bonds) and qspi_storage_partition where each exists, then remounts; a blank + * partition is formatted by fs_mount(). The only path that erases the + * LittleFS volume itself. * * @param out_ext_mounted optional; receives whether /ext came back mounted. * Set to false on boards with no QSPI. diff --git a/zephcore/adapters/gps/ZephyrGPSManager.cpp b/zephcore/adapters/gps/ZephyrGPSManager.cpp index 4e6781b..0e298c2 100644 --- a/zephcore/adapters/gps/ZephyrGPSManager.cpp +++ b/zephcore/adapters/gps/ZephyrGPSManager.cpp @@ -229,13 +229,8 @@ static void gnss_data_cb(const struct device *dev, const struct gnss_data *data) ARG_UNUSED(dev); if (!gps_enabled || gps_current_state == GPS_STATE_STANDBY) { - /* GPS disabled or in standby — ignore NMEA data. - * The GNSS driver fires callbacks as long as the UART has data, - * even after we de-assert GPS_EN (module drains its buffer). - * On boards without GPS power control (e.g. RAK3401 where 3V3_S - * rail is shared with LoRa FEM), the GPS module stays powered in - * standby and keeps streaming NMEA — suppress those callbacks to - * avoid log spam and wasted CPU for the entire standby period. */ + /* Disabled or in standby: ignore NMEA. The driver keeps calling while the + * UART has data, and a module without power control keeps streaming. */ return; } @@ -250,20 +245,8 @@ static void gnss_data_cb(const struct device *dev, const struct gnss_data *data) k_mutex_lock(&gps_mutex, K_FOREVER); if (data->info.fix_status >= GNSS_FIX_STATUS_GNSS_FIX) { - /* Reject "Null Island" (0,0) fixes. Zephyr's NMEA parser splits - * data across callbacks: gnss_nmea0183_parse_gga fills altitude + - * fix_status but NOT lat/lon; gnss_nmea0183_parse_rmc fills - * lat/lon. A merged publish fires when both GGA and RMC share a - * UTC. If the chip emits GGA quality=1 while RMC is still 'V' - * (or reports null-island coords), parse_rmc's early-exit on 'V' - * leaves lat/lon at their previous value (zero at first boot, or - * stale) while altitude advances — the caller sees valid - * fix_status + altitude-only motion + (0,0) coords. Observed on - * AT6558R (RAK WisMesh Tag) during early acquisition; Air530Z - * (ThinkNode M1) doesn't desync GGA/RMC this way. (0,0) is never - * a real fix — skip so we don't poison current_pos (and with it - * telemetry and the node position) or promote - * consecutive_good_fixes. */ + /* Reject (0,0): the parser can publish a valid fix status with lat/lon not + * yet filled (GGA ahead of RMC). It is never a real fix. */ if (data->nav_data.latitude == 0 && data->nav_data.longitude == 0) { LOG_DBG("GPS: Ignoring (0,0) fix — GGA/RMC desync " "(fix=%d sats=%d alt_mm=%d)", @@ -391,13 +374,8 @@ GNSS_DATA_CALLBACK_DEFINE(NULL, gnss_data_cb); #ifdef CONFIG_ZEPHCORE_GPS_SAT_DIAG /* ========== Per-constellation satellite tally (diagnostic) ========== - * The Zephyr GSV parser fills gnss_satellite.system from the NMEA talker ID - * ($GPGSV/$GLGSV/$GAGSV/$GBGSV), so this is direct evidence of which - * constellations the module is actually tracking — the only way to confirm - * that the boot-time PMTK353 / UBX-CFG-GNSS configuration was accepted. - * A module still in its GPS-only default yields sats_gps only. - * - * Counts only tracked satellites (is_tracked), not merely visible ones. */ + * Tracked satellites per constellation, from the GSV talker ID: shows which + * constellations the module was actually configured for. */ static uint8_t sat_count[5]; /* gps, glonass, galileo, beidou, other */ static int64_t sat_seen_ms[5]; /* uptime when each bucket was last reported */ @@ -485,18 +463,9 @@ static void gps_hold_sleep_lock(bool hold) } } -/* Acquire-window timeout (ms) for the current phase. - * - Repeater: fixed 5-min time-sync window. - * - First acquisition after enable (cold start, no fix yet): a generous but - * bounded window so almanac download has time, without pinning the module - * on forever when there's no sky. Spent once (first_acquire_used set on the - * first standby), after which the node uses the normal duty cycle regardless - * of whether a fix was obtained. - * - A wake from a standby that was the full power-off: the same long window, - * every time. The module lost its state, so this is a cold start again, and - * in poor reception that does not fit the warm window. Decided from the - * interval like the power-off itself, never from the board. - * - All other windows: the normal (warm) acquire timeout. */ +/* Acquire-window timeout (ms) for the current phase: repeaters a fixed + * 5 minutes; the long first-fix window for the first acquisition after enable + * and after every full power-off; the warm timeout otherwise. */ static uint32_t gps_acquire_window_ms(void) { if (gps_repeater_mode) { @@ -538,14 +507,9 @@ static void gps_go_to_standby(void) * RTC survive the cut and re-acquisition is a warm/hot start, not cold. * Other non-GPIO boards: software sleep via UART commands (PMTK + UBX). */ gps_reapply_cancel(); - /* The module keeps its state (backup sleep, standby pin, VRTC) only for - * a short interval: that draws all the time, a cold start only for a - * few minutes, so past gps_standby_max_sec the full power-off is the - * cheaper one. Not a board property: every board takes the same - * decision, and gps_power_control() turns it into whatever the board's - * pins can do. Where they give one off state only (a bare supply - * switch, a bare standby pin, the UART sleep commands) both sides of - * the limit are that state. */ + /* Keep the module's state only for a short interval; past + * gps_standby_max_sec use the full power-off. The same decision on every + * board: gps_power_control() maps it onto the pins. */ standby_powered_off = wake_interval / 1000U > gps_standby_max_sec; gps_module_power(false, !standby_powered_off); @@ -563,15 +527,9 @@ static void gps_go_to_standby(void) k_work_schedule(&gps_wake_work, K_MSEC(wake_interval)); } -/* Wake GPS and start acquiring. - * GPIO boards: hardware power-on. - * Non-GPIO boards: UART wake byte (wakes L76K from standby, ZOE-M8Q from backup). - * Does NOT call gps_module_configure(). Calling modem_chat_run_script() here - * would deadlock: the chip needs ~300ms to boot after GPIO power restore, - * but modem_chat blocks the calling thread waiting for the system work - * queue which may be processing stale UART data. CASIC settings are NOT - * persisted (no PCAS00, see the air530z driver), so they are re-sent blind - * a second after power-on (CONFIG_ZEPHCORE_GPS_REAPPLY). */ +/* Wake GPS and start acquiring (power-on, or a UART wake byte). Must not + * run a modem_chat script here: it would deadlock. CASIC settings are re-sent + * a second later (CONFIG_ZEPHCORE_GPS_REAPPLY). */ static void gps_start_acquiring(void) { LOG_INF("GPS: Waking for %s", gps_repeater_mode ? "time sync" : "position fix"); @@ -664,18 +622,9 @@ static int gnss_init(void) } if (!device_is_ready(gnss_dev)) { - /* Root cause: GPS transmits NMEA immediately at power-up before - * modem_chat opens its DMA pipe. UARTE accumulates overrun/framing - * errors, causing modem_pipe_open() to fail and device_init to return - * an error. - * - * Strategy: use UARTE ERRORSRC as a real signal. Wait until errors - * appear (GPS is transmitting), clear them, then call device_init. - * This avoids arbitrary delays — we act when the hardware tells us - * conditions are ready, not after a fixed sleep. - * - * IMPORTANT: Do NOT use uart_poll_in() — it corrupts nRF52840 UARTE - * DMA state and breaks modem_pipe async receive. */ + /* The module sends NMEA before the modem pipe opens, and the UARTE errors + * that causes make device_init fail. Wait for those errors, clear them, then + * init. Do not use uart_poll_in() here. */ LOG_INF("GNSS device not ready — waiting for GPS activity on UART"); gps_power_control(true); @@ -748,13 +697,9 @@ static int gnss_init(void) } #ifdef CONFIG_PM_DEVICE - /* Some upstream GNSS drivers (gnss-nmea-generic) start suspended under - * CONFIG_PM_DEVICE and never open their modem pipe until resumed — no - * NMEA would ever flow (the old "PM broke GPS" trap). Resume once - * here: main thread at boot, the one safe context for the modem_chat - * scripts a resume may run. PM-less drivers (luatos,air530z) return - * -ENOSYS. Retried like device_init above — opening the pipe while - * the module is mid-sentence can fail transiently. */ + /* Some GNSS drivers start suspended under CONFIG_PM_DEVICE: resume once, + * here on the main thread. PM-less drivers return -ENOSYS. Retried like + * device_init above. */ int pm_ret = pm_device_action_run(gnss_dev, PM_DEVICE_ACTION_RESUME); for (int attempt = 1; pm_ret != 0 && pm_ret != -EALREADY && pm_ret != -ENOSYS && attempt < 3; attempt++) { @@ -1073,14 +1018,8 @@ void gps_request_fresh_fix(void) k_work_cancel_delayable(&gps_wake_work); gps_start_acquiring(); } else if (gps_current_state == GPS_STATE_ACQUIRING) { - /* Already acquiring — reschedule the timeout so the caller's - * fresh-fix request gets a full window from now. Otherwise, a - * telemetry request that arrives 25s into a 30s acquire window - * only has 5s left, which in marginal signal usually means the - * chip goes to standby before producing a fix the requester - * could use. Each duty phase has a bounded window - * (gps_acquire_window_ms); in always-on there's no timeout to extend - * (GPS is continuously acquiring and current_pos is always fresh). */ + /* Already acquiring: restart the timeout so the request gets a full window. + * Always-on has no timeout to extend. */ if (gps_duty_cycling()) { uint32_t timeout_ms = gps_acquire_window_ms(); LOG_INF("GPS: Fresh fix requested, extending acquire timeout to %u s", diff --git a/zephcore/adapters/gps/ZephyrGPSManager.h b/zephcore/adapters/gps/ZephyrGPSManager.h index 15cd451..76d3d89 100644 --- a/zephcore/adapters/gps/ZephyrGPSManager.h +++ b/zephcore/adapters/gps/ZephyrGPSManager.h @@ -39,14 +39,8 @@ struct gps_state_info { }; /* ===== GPS configuration diagnostics (runtime toggle, RAM only) ===== - * - * Module configuration (PMTK/UBX or the GNSS API) is sent blind at boot and - * runs exactly once. With diag on, the next GPS enable re-runs it and records - * what actually happened, so an operator can power-cycle the GPS ("gps off" - * then "gps on") and read the outcome back over the CLI on a release build — - * no debug logging, no reflash. - * - * Not persisted: a diagnostic, not a setting. Clears on reboot. */ + * With diag on, the next GPS enable re-runs the module configuration and + * records the outcome for `get gps diag`. Not persisted. */ void gps_set_diag(bool on); /* Render the last configuration attempt as a single CLI line. Always @@ -70,17 +64,9 @@ void gps_set_enable_callback(gps_enable_callback_t cb); typedef void (*gps_fix_callback_t)(double lat, double lon, int64_t utc_time); void gps_set_fix_callback(gps_fix_callback_t cb); -/* GPS event callback - called when the GPS state machine needs the main thread - * to process a state transition (wake from standby, timeout, etc.). - * - * IMPORTANT: GNSS drivers use modem_chat_run_script() which blocks on a - * semaphore signaled from the system work queue. GPS timer work items also - * run on the system work queue → calling blocking GNSS APIs from a timer - * work handler deadlocks the system work queue. - * - * Solution: timer work handlers signal this callback, which posts an event - * to the main mesh thread. The main thread then calls gps_process_event() - * which safely executes the blocking GNSS configuration. */ +/* GPS event callback: the state machine needs the main thread. Timer work + * handlers must not call blocking GNSS APIs (they would deadlock the system + * work queue), so they signal here and main calls gps_process_event(). */ typedef void (*gps_event_callback_t)(void); void gps_set_event_callback(gps_event_callback_t cb); diff --git a/zephcore/adapters/gps/gps_internal.h b/zephcore/adapters/gps/gps_internal.h index 5d6aa1c..847945c 100644 --- a/zephcore/adapters/gps/gps_internal.h +++ b/zephcore/adapters/gps/gps_internal.h @@ -29,16 +29,8 @@ #endif /* ========== GPS Feature Detection ========== - * Every HAS_GPS_* predicate is defined HERE, before first use. They are pure - * devicetree tests with no side effects, kept apart from the variables they - * gate so that ordering can never drift again. - * - * Why this block exists: HAS_GPS_UART used to be defined ~500 lines below its - * first `#if`, and an undefined identifier in `#if` is silently 0 — so the - * entire PMTK/UBX module-configuration path compiled to nothing on every - * board, and GPS ran at module defaults (GPS-only constellations, no AOP). - * If you add another HAS_GPS_* macro, define it in this block. - */ + * Every HAS_GPS_* predicate is defined here, before first use: an undefined + * identifier in #if is silently 0. Add new ones to this block. */ /* GNSS module hangs off a UART we can write to (any compatible). */ #if HAS_GNSS && DT_NODE_HAS_STATUS(DT_NODELABEL(gnss), okay) && \ diff --git a/zephcore/adapters/gps/gps_module_cfg.cpp b/zephcore/adapters/gps/gps_module_cfg.cpp index 07a6364..e1e6e44 100644 --- a/zephcore/adapters/gps/gps_module_cfg.cpp +++ b/zephcore/adapters/gps/gps_module_cfg.cpp @@ -21,17 +21,9 @@ static bool gps_diag_on = false; #if HAS_GNSS -/* Multi-constellation configuration — runs ONCE at boot. - * modem_chat_run_script() blocks on a semaphore signaled from the system - * work queue. Calling it after a GPIO power cycle can deadlock because: - * 1. The L76K needs ~300ms to boot after power restore - * 2. Meanwhile the modem_chat may be processing stale UART data - * 3. The script completion callback competes with NMEA processing - * - * Safe to call at boot because the driver init already ran and the chip - * is powered and outputting NMEA. PCAS settings are NOT persisted (nothing - * sends PCAS00), so what a power cut loses is re-sent blind after every - * power-on instead (gps_module_reapply_step, CONFIG_ZEPHCORE_GPS_REAPPLY). */ +/* Multi-constellation configuration, run once at boot: the one moment a + * modem_chat script is safe (the chip is up and streaming). Settings a power + * cut loses are re-sent blind after each power-on (gps_module_reapply_step). */ static bool gnss_configured = false; /* ========== Configuration Diagnostics ========== @@ -57,14 +49,9 @@ static struct { } gps_cfg_diag; -/* Module identification string, captured by the GNSS driver from the reply to - * its version query (CASIC parts answer in-band as a $GPTXT sentence). - * Weak so that boards whose driver has no version query still link — an - * absent symbol and an empty string mean the same thing to the report. - * - * Its real value is not the version text: on a transport where every command - * is written blind, a captured reply is the only positive proof that the - * MCU's TX line reaches the module at all. */ +/* Module identification string from the driver's version query; weak, so + * boards whose driver has none still link. A captured reply is also the only + * proof that our TX line reaches the module. */ extern "C" int zephcore_gnss_version_get(char *buf, size_t len) __attribute__((weak)); /* Count of NMEA sentences the GNSS driver has parsed. This is the one signal @@ -79,16 +66,8 @@ extern "C" uint32_t zephcore_gnss_rx_count(void) __attribute__((weak)); static bool gps_cfg_counting = false; /* ========== Vendor-Specific Configuration Commands ========== - * - * The RAK WisBlock GPS slot accepts multiple modules (L76K, ZOE-M8Q, etc.) - * and we use gnss-nmea-generic which is a passive NMEA listener — it has no - * GNSS API for configuration. - * - * Strategy: send BOTH Quectel PMTK and u-blox UBX configuration commands. - * Each module ignores the protocol it doesn't understand. - * - * This runs once at boot. Both modules persist config to internal flash, - * so these are effectively no-ops on subsequent boots. */ + * The generic-NMEA boards' GPS slot can hold any module, so the PMTK, PCAS and + * UBX configuration are all sent; each module ignores the others' protocol. */ #if HAS_GPS_UART @@ -96,15 +75,8 @@ static bool gps_cfg_counting = false; * regardless of compatible string. */ const struct device *const gps_uart_dev = DEVICE_DT_GET(DT_BUS(DT_NODELABEL(gnss))); -/* Send raw bytes to the GPS UART using blocking poll_out. - * Safe to call even though modem_chat/modem_ubx owns the UART pipe: - * uart_poll_out writes one byte at a time through the TX register, - * and GNSS modules are receive-only (no TX contention). - * - * Gated on HAS_GPS_UART alone — writing to the module is safe on every - * UART-attached GNSS. The narrower power-control gate below applies to the - * software *sleep* commands, which are only a fallback for boards that - * cannot cut GPS power. */ +/* Send raw bytes to the GPS UART with blocking poll_out. Safe beside the + * driver's pipe: GNSS modules do not contend for our TX line. */ void gps_uart_send(const uint8_t *data, size_t len) { if (!device_is_ready(gps_uart_dev)) { @@ -122,13 +94,7 @@ void gps_uart_send(const uint8_t *data, size_t len) } /* --- MediaTek-family (PMTK) configuration --- - * - * PMTK is MediaTek's protocol. It applies to genuine MTK parts (L76B and - * relatives). It does NOT apply to the Quectel L76K/L76KB or Air530Z, which - * are CASIC silicon and speak PCAS — their protocol specification contains no - * PMTK command at all, so these sentences are inert there. Those modules are - * driven by the air530z driver via the GNSS API instead and never reach this - * path. Kept because the RAK WisBlock GPS slot can hold an MTK part. */ + * For genuine MTK parts (L76B). Inert on CASIC parts (L76K, Air530Z). */ /* PMTK353: Enable GPS + GLONASS + Galileo + BeiDou (no QZSS). * Default is GPS-only. Multi-constellation dramatically improves TTFF @@ -148,15 +114,8 @@ static const char pmtk_easy[] = "$PMTK869,1,1*35\r\n"; static const char pmtk_aic[] = "$PMTK286,1*23\r\n"; /* --- CASIC (PCAS) configuration --- - * - * The Quectel L76K/L76KB (RAK12501) and Air530Z are CASIC silicon: they speak - * neither PMTK nor UBX, so without these sentences such a module in a - * WisBlock slot receives no configuration at all and sits on its factory - * defaults. Boards that always carry one use the air530z driver and the GNSS - * API instead; these are for the generic-NMEA boards whose GPS slot can hold - * any module. - * - * Inert on the other families, same as PMTK and UBX are here. */ + * For an L76K/L76KB or Air530Z in a generic-NMEA slot; boards that always + * carry one use the air530z driver instead. Inert on the other families. */ /* PCAS03: NMEA sentence selection. Field order is * GGA,GLL,GSA,GSV,RMC,VTG,ZDA,ANT,... — keep GGA + RMC (position, time) and @@ -210,23 +169,8 @@ static const char pcas_version_query[] = "$PCAS06,0*1B\r\n"; static const char pubx_version_query[] = "$PUBX,04*37\r\n"; /* --- u-blox NMEA output trim ($PUBX,40) --- - * - * THE LINK BUDGET IS THE CONSTRAINT, and it is easy to blow past it. At 9600 - * baud only ~960 bytes/s fit. With multi-GNSS enabled a u-blox emits, per - * second: GGA + RMC + GLL + VTG, one GSA per constellation, and a GSV burst - * that grows with satellite count — roughly 1030 bytes/s at ~26 SVs. The - * stream then cannot fit in the second it is generated in, sentences are - * truncated or dropped, and the symptom is not "slow GPS" but a receiver - * that appears to have stopped: no parseable GGA, so no fix, so no position. - * - * Enabling constellations without trimming output is therefore actively - * harmful on a 9600-baud link. We already do exactly this trim for CASIC - * parts via $PCAS03; u-blox had no equivalent, which is the asymmetry this - * fixes. GLL, GSA and VTG are dropped outright — nothing in the driver parses - * them — and GSV is kept only when the satellite tally needs it. - * - * Dropping GLL/GSA/VTG takes ~1030 -> ~650 bytes/s; dropping GSV as well - * takes it to ~160. */ + * At 9600 baud the multi-GNSS sentence set does not fit in a second, so GLL, + * GSA and VTG are switched off, and GSV unless the satellite tally needs it. */ static const char pubx_off_gll[] = "$PUBX,40,GLL,0,0,0,0,0,0*5C\r\n"; static const char pubx_off_gsa[] = "$PUBX,40,GSA,0,0,0,0,0,0*4E\r\n"; static const char pubx_off_vtg[] = "$PUBX,40,VTG,0,0,0,0,0,0*5E\r\n"; @@ -236,60 +180,18 @@ static const char pubx_off_gsv[] = "$PUBX,40,GSV,0,0,0,0,0,0*59\r\n"; /* --- u-blox ZOE-M8Q (UBX binary) configuration --- */ -/* UBX-CFG-PRT: force UART1 to 9600 8N1 with BOTH UBX and NMEA enabled in and - * out. Sent first, before anything that depends on the module talking to us. - * - * This exists because the module's port configuration is persistent and not - * necessarily ours. RAK's own RAK12500 example — and any host using the - * SparkFun u-blox library — calls setUART1Output(COM_TYPE_UBX) followed by - * saveConfiguration(), which stores "UBX only, NMEA off" in the module's - * flash. A module that has ever been driven that way stays silent on an - * NMEA-only host forever after, through power cycles and reflashes, and - * presents as a completely dead receiver: no GGA, no fix, no reply to any - * query. Re-asserting the port configuration costs one frame and removes a - * failure mode that is otherwise almost impossible to diagnose from the host. - * - * Limitation: if the module was also saved at a different baud rate, it will - * not parse this frame either. Recovering from that needs a baud scan, which - * the devicetree's fixed current-speed does not currently allow. */ +/* UBX-CFG-PRT: force UART1 to 9600 8N1 with UBX and NMEA in and out, sent + * first. A module saved as "UBX only" by another host is otherwise silent. */ static const uint8_t ubx_cfg_prt_uart1[] = { 0xB5, 0x62, 0x06, 0x00, 0x14, 0x00, 0x01, 0x00, 0x00, 0x00, 0xC0, 0x08, 0x00, 0x00, 0x80, 0x25, 0x00, 0x00, 0x03, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x8E, 0x95 }; -/* UBX-CFG-GNSS: Enable GPS + Galileo + GLONASS (+ QZSS) on u-blox M8. - * - * Config block layout is gnssId, resTrkCh, maxTrkCh, reserved0, flags[4] — - * EIGHT bytes, and the payload length must be exactly 4 + 8*numConfigBlocks - * or the receiver rejects the whole message. The previous version of this - * frame omitted reserved0, giving 7-byte blocks and a 39-byte payload where - * numConfigBlocks=5 demanded 44, so no u-blox module ever accepted it. - * - * Only THREE major GNSS (GPS/Galileo/GLONASS/BeiDou) can run concurrently on - * M8, so BeiDou is explicitly disabled rather than left alone: if the module - * came up with BeiDou on, enabling three others would make four and the - * message would be refused. - * - * QZSS is enabled even though it is Japan-regional and costs ~3 channels — - * u-blox require GPS and QZSS to be both enabled or both disabled (they - * share L1 C/A), and a mismatch is grounds for rejection. - * - * SBAS stays off: it needs 30-60 s to download corrections, useless for our - * quick-fix-then-sleep pattern (companions 30 s, repeaters 5 min). - * - * numTrkChHw = 0 and numTrkChUse = 0xFF (read-only / "use max available"). - * - * resTrkCh MUST be 0 on the disabled blocks. Reserving tracking channels for - * a system whose enable bit is clear is self-contradictory, and the receiver - * validates CFG-GNSS atomically — one bad block rejects all six. An earlier - * revision left SBAS at 1 and BeiDou at 8 and the whole frame was refused - * (observed on hardware: RAK12500/ZOE-M8Q stayed GPS-only, sys=G8/R0/E0/B0). - * - * NOTE — this is an M8 frame. u-blox 7 parts (MAX-7Q on the RAK1910) have no - * Galileo or BeiDou and use a different sigCfgMask, so they will refuse it - * and stay GPS-only. Sending an M7 frame as well is NOT safe blind: its - * sigCfgMask of 0 would be a signal-disabling value on M8. */ +/* UBX-CFG-GNSS for u-blox M8: GPS + Galileo + GLONASS + QZSS on, BeiDou and + * SBAS off. Blocks are 8 bytes each (payload 4 + 8*n); resTrkCh must be 0 on + * the disabled blocks, and GPS and QZSS go together. The receiver rejects + * the whole message for one bad block. u-blox 7 parts refuse it. */ static const uint8_t ubx_cfg_gnss[] = { 0xB5, 0x62, 0x06, 0x3E, 0x34, 0x00, 0x00, 0x00, 0xFF, 0x06, 0x00, 0x08, 0x10, 0x00, 0x01, 0x00, 0x01, 0x01, 0x01, 0x00, 0x03, 0x00, 0x00, 0x00, @@ -298,33 +200,16 @@ static const uint8_t ubx_cfg_gnss[] = { 0x01, 0x01, 0x06, 0x08, 0x0E, 0x00, 0x01, 0x00, 0x01, 0x01, 0xEE, 0x64 }; -/* UBX-CFG-NMEA: switch the NMEA output to version 4.10. - * - * Without this, Galileo and BeiDou satellites cannot be reported at all: the - * $GAGSV and $GBGSV talker IDs only exist from NMEA 4.10, and M8 firmware - * defaults lower. So a fully successful CFG-GNSS would still show zero - * Galileo in "get gps diag" — a reporting limit masquerading as a config - * failure. Mirrors Meshtastic's "enable NMEA 4.10" step (src/gps/ubx.h). - * - * gsvTalkerId = 0 (use the GNSS-specific talker per constellation) is what - * makes the per-constellation tally work — do not set it to 1, which forces - * every GSV onto the main talker and would collapse the tally into GPS. */ +/* UBX-CFG-NMEA: NMEA 4.10, without which Galileo and BeiDou cannot be + * reported. gsvTalkerId must stay 0 for the per-constellation tally. */ static const uint8_t ubx_cfg_nmea_410[] = { 0xB5, 0x62, 0x06, 0x17, 0x14, 0x00, 0x00, 0x41, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x75, 0x57 }; -/* UBX-CFG-NAV5: Set 5° minimum satellite elevation. - * Ignore satellites below 5° elevation — they have more atmospheric - * noise and multipath, degrading fix quality. The default 0° lets in - * everything including horizon-level junk. - * Dynamic model left at factory default (Portable) — works for fixed - * repeaters, walking companions, and vehicles alike. - * apply mask 0x0002 = minEl(bit1) only - * - * The trailing checksum was CK_B=0x37 (should be 0xE7) — a one-byte typo - * that made every receiver drop this frame silently, with no NAK. */ +/* UBX-CFG-NAV5: 5 degree minimum satellite elevation (apply mask 0x0002); + * the dynamic model stays at the factory default. */ static const uint8_t ubx_cfg_nav5_minelev[] = { 0xB5, 0x62, 0x06, 0x24, 0x24, 0x00, 0x02, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -450,13 +335,8 @@ static void gps_configure_via_uart(void) LOG_INF("GPS: PCAS config sent (sentences, constellations, nav mode, version query)"); /* --- u-blox ZOE-M8Q (UBX) --- - * NMEA 4.10 goes first: it governs whether the constellations enabled - * by the next frame can be *reported* at all. CFG-GNSS restarts the - * navigation engine, so it gets the long settle before the frames that - * follow it — at 50 ms they were being issued into a restarting - * receiver. CFG-CFG stays last so it only persists whatever was - * actually accepted; note that makes a rejected configuration sticky - * too, which is why a frame bug here survives power cycles. */ + * Order matters: NMEA 4.10 first, CFG-GNSS with a long settle (it restarts + * the navigation engine), CFG-CFG last so only accepted settings are saved. */ /* Make sure NMEA output is even switched on before anything else — a * module saved as UBX-only by a previous host is otherwise mute. */ gps_send_ubx(ubx_cfg_prt_uart1, sizeof(ubx_cfg_prt_uart1), GPS_CFG_RESTART_MS); @@ -487,14 +367,9 @@ static void gps_configure_via_uart(void) } #endif /* HAS_GPS_UART */ -/* Re-run module configuration on a GPS enable, but only when diagnostics are - * armed — this is a deliberate, operator-triggered action, not a normal path. - * - * Only the raw-UART path is re-runnable. gps_module_configure()'s API path goes - * through modem_chat_run_script(), which is safe at boot only: after a GPIO - * power restore the chip needs ~300 ms and calling it here deadlocks the main - * thread. So on API-driver boards this records nothing new and "get gps diag" - * keeps reporting the boot-time result, which is the honest answer. */ +/* Re-run module configuration on a GPS enable, only when diagnostics are + * armed. Only the raw-UART path can be re-run; on API-driver boards + * `get gps diag` keeps the boot-time result. */ void gps_diag_maybe_reconfigure(void) { if (!gps_diag_on || gnss_configured || gnss_dev == NULL) { @@ -568,25 +443,9 @@ void gps_module_configure(void) } #if HAS_GPS_UART && defined(CONFIG_ZEPHCORE_GPS_REAPPLY) -/* The boot-time settings a power cut loses (see the Kconfig help). One - * sentence per step, written with gps_uart_send() directly and no settle - * sleep: the caller waits each entry's gap with a work item, so the main - * thread only spends the UART time. - * - * Kept boot-only on purpose: - * - UBX: already saved to BBR + flash by CFG-CFG at boot, and repeating it - * would rewrite the module's flash on every wake; - * - PCAS02 (1 Hz is the default) and PCAS06 (a query, no state). - * - * Both constellation commands restart their chip's navigation engine, and a - * sentence written into a restarting engine is lost, so each is followed by - * the boot sequence's restart settle: PMTK353 first, PCAS04 last. Each only - * restarts its own family's chip (the other ignores it). On a MediaTek part - * that merely slept this costs one engine restart per wake, accepted - * (architect, 2026-09-27): one that lost power is otherwise left searching - * GPS-only, which is slower than a restart with every constellation. The - * PMTK sentences go only where the module is unknown (the generic-NMEA - * path); an API-driver board's module is known CASIC. */ +/* The boot-time settings a power cut loses, one sentence per step; the caller + * waits each entry's gap with a work item. UBX, PCAS02 and PCAS06 stay + * boot-only. Each constellation command is followed by its restart settle. */ struct reapply_cmd { const char *sentence; uint16_t gap_ms; /* wait before the next one */ diff --git a/zephcore/adapters/gps/gps_power.cpp b/zephcore/adapters/gps/gps_power.cpp index ac70bc2..e3750b9 100644 --- a/zephcore/adapters/gps/gps_power.cpp +++ b/zephcore/adapters/gps/gps_power.cpp @@ -15,44 +15,14 @@ LOG_MODULE_DECLARE(zephcore_gps, CONFIG_ZEPHCORE_GPS_LOG_LEVEL); /* ========== GPS Power Strategy ========== - * Module power is GPIO/regulator controlled — GNSS driver PM is not used - * for the module itself: - * - Wio Tracker L1 (L76K): P1.09 is the module's WAKEUP pin, not a supply - * switch. Per the L76K hardware design: WAKEUP is a digital input, active - * low with an internal pull-up, that "enters or exits Standby mode". In - * Standby the RF is powered off but the internal core and I/O power domain - * stay active, so VCC is never removed and ephemeris/almanac/RTC survive — - * every wake is a warm start, not a cold one. (Backup mode, the deeper - * state, requires cutting VCC while V_BCKP holds the RTC domain; this - * board has no VCC switch, so Standby is the floor available to us.) - * - T1000-E, MeshTracker X1 (AG3335): RTC backup sleep ($PAIR650,0), then - * GPS_EN de-asserted with VRTC kept; a GPS_RTC_INT pulse wakes it - * - All boards: gps-enable alias → GPIO power control - * - * CONFIG_PM_DEVICE is on globally, but nothing suspends automatically — - * system-managed suspend is compiled only under CONFIG_PM (off everywhere). - * This manager makes exactly two kinds of PM calls, both main-thread only: - * - a one-time RESUME of the GNSS device at boot (gnss-nmea-generic inits - * suspended under CONFIG_PM_DEVICE and never opens its pipe otherwise); - * - suspend/resume of the GNSS UARTE around standby/off (an armed UARTE RX - * holds HFCLK ≈0.5-1 mA on nRF52840 even with the module powered off). - * The old "PM broke GPS" deadlock was modem_chat_run_script() being reached - * from the system workqueue via driver PM hooks — the air530z driver is - * PM-less now and every PM call here stays on the main thread. */ + * The module is switched by GPIO or regulator, not by GNSS driver PM. The two + * PM calls made here, both on the main thread: a one-time RESUME of the GNSS + * device at boot, and suspend/resume of the GNSS UARTE around standby/off. */ /* ========== GPS Power GPIO Control ========== - * These are unconditional (not gated by HAS_GNSS) because - * gps_power_off_for_shutdown() must be available for System OFF - * even on boards without a GNSS driver. - * - * IMPORTANT: Do NOT touch GPIO during init! The GNSS driver needs the GPS - * to be powered and outputting NMEA for the modem pipe to work. - * We only configure GPIO lazily on first power-off request. - * - * Board-specific pins (defined in board overlays as gps-enable alias): - * - T1000-E: P1.11 (GPS_EN), P0.8 (GPS_VRTC_EN), P1.15 (GPS_RESET), P1.12 (GPS_SLEEP_INT) - * - Wio Tracker L1: P1.09 (GPS power, shared with luatos,air530z on-off-gpios) - */ + * Not gated by HAS_GNSS: gps_power_off_for_shutdown() must exist for System + * OFF on every board. No GPIO is touched during init (the GNSS driver needs + * the module powered); the lines are configured on the first power-off. */ #if HAS_GPS_POWER_CONTROL static const struct gpio_dt_spec gps_enable_gpio = GPIO_DT_SPEC_GET(DT_ALIAS(gps_enable), gpios); #endif @@ -144,13 +114,9 @@ static const struct gpio_dt_spec gps_resetb_gpio = GPIO_DT_SPEC_GET(DT_ALIAS(gps #define HAS_GPS_RESETB 0 #endif -/* Module standby pin (L76K WAKEUP: asserted = run, de-asserted = Standby, RF - * off with the core powered) on a board that ALSO has a supply switch - * (gps-enable): standby uses this pin and keeps the supply, so a wake is a - * hot start; a full off (`gps off`, a standby past prefs.gps_standby_max, - * boot with GPS disabled, System OFF) cuts the supply, since Standby still - * draws. A board whose only control is the standby pin (Wio Tracker L1) - * declares it as gps-enable instead. */ +/* Module standby pin (asserted = run) on a board that also has a supply + * switch: standby uses the pin, a full off cuts the supply. A board whose only + * control is the standby pin declares it as gps-enable instead. */ #if HAS_GPS_WAKEUP static const struct gpio_dt_spec gps_wakeup_gpio = GPIO_DT_SPEC_GET(DT_ALIAS(gps_wakeup), gpios); #endif @@ -192,16 +158,9 @@ static void gps_enter_backup_sleep(void) #endif /* The off levels, shared by gps_power_control(false) and System OFF: reset - * and resetb asserted (the module held in reset while its supply is gated, - * so it starts cleanly on the next power-on), the enable and rtcint - * de-asserted, VRTC de-asserted unless keep_vrtc (warm standby). - * - * The sleep line is the one difference: runtime standby/off leaves it - * asserted, System OFF drops it -- as upstream, whose T1000-E sleep_gps() and - * stop_gps() drive GPS_SLEEP_INT HIGH and only powerOff() drives it LOW. - * - * gpio_pin_configure_dt() on every line: a boot with the GPS off never ran - * the power-on path, so the pins may not be outputs yet. */ + * asserted, enable and rtcint de-asserted, VRTC de-asserted unless keep_vrtc. + * The sleep line stays asserted at runtime and is dropped for System OFF. + * Every line is configured here: a boot with the GPS off never ran power-on. */ static void gps_drive_off_levels(bool keep_vrtc, bool system_off) { #if HAS_GPS_WAKEUP @@ -249,15 +208,11 @@ static void gps_drive_off_levels(bool keep_vrtc, bool system_off) #endif } -/* GPS power control with warm standby support. +/* GPS power control. * @param on true = power on, false = power off - * @param keep_vrtc When powering off: true = the state-keeping off this board - * has (AG3335 backup sleep, the gps-wakeup standby pin, VRTC - * left on: ephemeris/almanac/RTC survive for a fast - * re-acquisition), false = full power-off (cold start on - * next wake). The caller decides from the standby length - * alone (prefs.gps_standby_max), never from the board; a - * board with one off state lands in it either way. */ + * @param keep_vrtc when powering off: true = the state-keeping off this board + * has, false = full power-off. A board with one off state + * lands in it either way. */ void gps_power_control(bool on, bool keep_vrtc) { #if HAS_GPS_POWER_REGULATOR @@ -275,15 +230,7 @@ void gps_power_control(bool on, bool keep_vrtc) } #endif #if HAS_GPS_POWER_CONTROL - /* Direct GPIO power control — works on all boards. - * We toggle the GPS power pin ourselves rather than using driver PM - * (driver PM can hang on modem_pipe_close / modem_chat_run_script). - * The GNSS driver's modem pipe stays open. - * - * T1000-E / X1 (HAS_GPS_BACKUP_SLEEP): standby up to - * prefs.gps_standby_max uses the AG3335 RTC backup sleep, as Arduino - * stop_gps(); a longer standby and `gps off` cut VRTC too. - * Simple boards (Wio etc.): Full power off/on via GPS_EN. */ + /* We switch the module ourselves; the GNSS driver's modem pipe stays open. */ if (on) { #if HAS_GPS_WAKEUP if (gpio_is_ready_dt(&gps_wakeup_gpio)) { @@ -291,22 +238,9 @@ void gps_power_control(bool on, bool keep_vrtc) } #endif #if HAS_T1000_GPS_CONTROL - /* T1000-E power-on sequence (from Arduino target.cpp start_gps()) - * Must follow this exact order with delays. Levels are ASSERTED / - * DE-ASSERTED, not physical: Arduino states them as HIGH/LOW because - * its pins are all active-high, while this block is also reached by - * boards whose lines are active-low (see the gate below). - * 1. GPS_EN asserted, delay 10ms - * 2. GPS_VRTC_EN asserted, delay 10ms (critical - RTC power) - * 3. GPS_RESET asserted, delay 10ms, then released - * 4. GPS_SLEEP_INT asserted - * - * Despite the name this is not a T1000-E-only path: - * HAS_T1000_GPS_CONTROL is (HAS_GPS_VRTC || HAS_GPS_RESET || - * HAS_GPS_SLEEP), so a bare gps-reset alias is enough to route a - * board here. heltec_wifi_lora32_v4, _v43 and thinknode_m9 all arrive - * this way, and all three declare gps-enable active-low. - */ + /* Power-on sequence, in this order with delays (Arduino start_gps()): + * enable, VRTC, reset pulse, sleep line. Levels are asserted/de-asserted, not + * physical; a bare gps-reset alias is enough to route a board here. */ if (gpio_is_ready_dt(&gps_enable_gpio)) { gpio_pin_configure_dt(&gps_enable_gpio, GPIO_OUTPUT_ACTIVE); } @@ -431,66 +365,18 @@ void gps_power_off_for_shutdown(void) #if HAS_GNSS /* ========== Software Sleep/Wake (no GPIO required) ========== - * - * On boards without dedicated GPS power control (e.g. RAK3401 where the - * 3V3_S rail is shared with the LoRa FEM), we send vendor-specific UART - * commands to put the GPS module into low-power mode. - * - * Strategy: send BOTH MediaTek and u-blox sleep commands — the module that - * isn't present simply ignores the bytes it doesn't understand. - * - * - MediaTek (e.g. L76B): $PMTK161,0*28\r\n → standby (~1mA), wake on UART - * - u-blox ZOE-M8Q (RAK12500): UBX-RXM-PMREQ → backup (~7µA), wake on UART - * - * Neither reaches a CASIC part (L76K/L76KB/Air530Z): those ignore PMTK, UBX - * and PCAS12 sleep commands alike — verified on hardware, which is why the - * boards carrying them duty-cycle with a power GPIO instead. Note also that - * the RAK1910 is a u-blox MAX-7Q, not an L76K, despite older comments here. - * - * Wake: any byte on UART wakes both modules from their low-power modes. - * After wake, the module resumes outputting NMEA autonomously. - */ + * Boards without GPS power control: both the MediaTek and the u-blox sleep + * command are sent, and the absent module ignores the other's bytes. Any byte + * on the UART wakes either. CASIC parts ignore both. */ /* HAS_GPS_UART, gps_uart_dev and gps_uart_send are defined near the top of * this file (see "GPS Feature Detection") — they are needed by the boot-time * module configuration, which runs long before this section. */ /* ========== GNSS UART Suspend/Resume (device PM) ========== - * nRF UARTE only. An armed UARTE RX holds HFCLK (~0.5-1 mA on nRF52840) - * even when the GPS module is powered off or silent, so standby/off - * suspends the UART device and every wake resumes it first. - * - * Verified symmetric in uart_nrfx_uarte.c under the still-open modem pipe: - * suspend saves the RX-interrupt state, STOPRXes, disables the peripheral - * and applies the sleep pinctrl; resume restores all of it. Other UART - * drivers (legacy nordic,nrf-uart on RAK4631, ESP32) are deliberately not - * gated in — their suspend/resume round-trip is unverified and the HFCLK - * cost is UARTE-specific. - * - * Every GPS UART node must carry a sleep pinctrl state (all boards do): - * without one, suspend fails *after* disabling RX while the PM state stays - * ACTIVE, so the next resume no-ops with -EALREADY — a dead GPS. - * - * Safe on boards WITHOUT GPS power control too, since patch 0010. - * uarte_pm_suspend() used to busy-wait for RXTO with no timeout after - * triggering STOPRX. In interrupt-driven mode RX runs on a 1-byte buffer - * with no ENDRX_STARTRX short, so the receiver stops after every byte until - * the ISR re-arms it — and suspend disables the ENDRX interrupt *before* - * STOPRX, removing the re-arm. Land in that window with bytes still arriving - * and STOPRX hits an already-stopped receiver, no RXTO is generated, and the - * caller spun forever on the main thread (observed: RAK3401 1W repeater on - * 1.16.6, CLI answering only "-> busy"). That is why this gate once required - * power control: boards with it cut the module first, so the line is quiet. - * Boards without it send gps_software_sleep()'s PMTK/UBX commands, which the - * module may ignore (CASIC parts; a MAX-7Q is protocol 14/15, older than the - * UBX-RXM-PMREQ we send) and keep streaming NMEA straight into the suspend. - * - * Patch 0010 bounds that wait (4 ms, then the unconditional - * nrf_uarte_disable() force-stops the receiver) and clears the RX events, so - * the race can no longer hang the caller, and resume re-arms RX from a clean - * state. So every nRF UARTE GPS board releases its UART in standby/off now, - * including an optional GPS port with nothing fitted (xiao_nrf54l15 uart21). - * Bench-tested by the streaming-into-suspend case: devdocs/lld/13. */ + * nRF UARTE only: an armed UARTE RX holds HFCLK even with the module off, so + * standby/off suspends the UART and every wake resumes it first. Every GPS + * UART node needs a sleep pinctrl state. Relies on patch 0010. */ #if HAS_GPS_UART && defined(CONFIG_PM_DEVICE) && \ DT_NODE_HAS_COMPAT(DT_BUS(DT_NODELABEL(gnss)), nordic_nrf_uarte) #define HAS_GPS_UART_PM 1 @@ -509,18 +395,8 @@ void gps_uart_set_power(bool on) return; } if (!on) { - /* Let the GNSS line go quiet before suspending. Every caller - * cuts module power (GPS_EN low / reset asserted / regulator - * off) immediately before this, but a byte can still be in - * flight. Settle so it finishes and the driver's RX ISR re-arms, - * leaving the receiver armed-and-idle when the suspend's STOPRX - * fires — that state yields RXTO, whereas a just-stopped, - * un-rearmed receiver can produce none and (pre-0010) hung the - * main thread. ~5 ms comfortably covers one character time at - * GNSS baud plus ISR latency; standby happens at most every few - * minutes, so the cost is negligible. Backstop: patch 0010 - * bounds the driver's RXTO wait so a missed RXTO can never hang - * us even if a byte still lands in the race window. */ + /* Let a byte in flight finish and the RX ISR re-arm before the suspend's + * STOPRX. */ k_msleep(5); } int ret = pm_device_action_run(gps_uart_dev, @@ -546,29 +422,9 @@ void gps_uart_set_power(bool on) { ARG_UNUSED(on); } * Inert on CASIC parts (L76K and relatives) — they have no such command. */ static const uint8_t pmtk_standby[] = "$PMTK161,0*28\r\n"; -/* u-blox ZOE-M8Q: UBX-RXM-PMREQ → enter backup mode - * UBX frame: B5 62 | 02 41 | 10 00 | payload(16) | CK_A CK_B - * Payload (protocol 23+, 16 bytes): - * version=0, reserved[3]=0, - * duration=0x00000000 (infinite), - * flags=0x00000006 (backup + force), - * wakeupSources=0x00000028 (uartrx bit3 | extint0 bit5) - * - * THE WAKE SOURCE BIT IS LOAD-BEARING. wakeupSources bit 3 is uartrx; bit 5 - * is extint0. This frame previously sent 0x20 — extint0 only — with a comment - * claiming that was "UART RX (bit 5)". It is not. EXTINT is not routed to the - * WisBlock connector on the RAK12500 (RAK's datasheet: only UART/I2C, 1PPS, - * RESET, VDD and GND are connected), so the module was told to sleep forever - * with a wake source that can never be asserted. duration=0 means infinite, - * so it never came back: no NMEA at any baud, and no I2C either, because - * backup mode powers down the DDC interface too. Only a physical power cycle - * recovered it — a reboot does not, since the 3V3_S rail stays up. - * Confirmed on hardware: reseat -> module answers -> one fix -> first standby - * -> gone again. - * - * 0x28 sets both, so a board that does wire EXTINT keeps that path as well. - * Module stops all output and draws ~20µA (ZOE-M8Q at 3V, datasheet Table 13; - * the 15µA hardware-backup figure needs VCC removed entirely). */ +/* u-blox ZOE-M8Q: UBX-RXM-PMREQ, enter backup mode until a wake source fires. + * wakeupSources = 0x28: uartrx (bit 3) | extint0 (bit 5). uartrx must be set: + * EXTINT is not wired on the RAK12500, and without it the module never wakes. */ static const uint8_t ubx_pmreq_backup[] = { 0xB5, 0x62, /* UBX sync chars */ 0x02, 0x41, /* Class: RXM, ID: PMREQ */ diff --git a/zephcore/adapters/radio/LoRaRadio.cpp b/zephcore/adapters/radio/LoRaRadio.cpp index 55d413f..46653f9 100644 --- a/zephcore/adapters/radio/LoRaRadio.cpp +++ b/zephcore/adapters/radio/LoRaRadio.cpp @@ -433,17 +433,9 @@ void LoRaRadio::configure(bool tx) return; } - /* Fast path: if only the TX/RX direction changed, skip the full - * hwConfigure → lora_config() call. The driver already has a valid - * config for the target direction (RadioSetRxConfig / RadioSetTxConfig - * with TxTimeout=4000) from a previous cycle — Radio.Rx(0) / Radio.Send() - * will use those register values directly. This avoids the - * modem_acquire → modem_release → Radio.Sleep() round-trip that wastes - * ~5 ms on every TX↔RX transition. - * - * Not used for loramac-node: Radio.SetTxConfig() and Radio.SetRxConfig() - * configure completely disjoint internal state (including TxTimeout). - * Skipping either on a direction change leaves that state uninitialized. */ + /* Fast path: only the TX/RX direction changed, so skip lora_config(); the + * driver keeps a valid config for each direction. Not for loramac-node, + * whose TX and RX configs are disjoint state. */ if (!_ops.loramac_node && _config_cached && onlyDirectionDiffers(cfg, _last_cfg)) { LOG_DBG("%s: direction-only change, skip hwConfigure", who); _last_cfg = cfg; diff --git a/zephcore/adapters/radio/radio_common.h b/zephcore/adapters/radio/radio_common.h index 64cbe2d..59f0a60 100644 --- a/zephcore/adapters/radio/radio_common.h +++ b/zephcore/adapters/radio/radio_common.h @@ -14,41 +14,15 @@ #include "radio_tuning.h" /* --- RSSI read timing (SX1261/2 DS rev 2.2 Table 13-82) --- - * - * The median-of-N above only rejects outliers if the N reads are actually - * independent. The chip updates RSSI once per "averaging window"; reads - * issued faster than that return the same underlying sample repeatedly, and - * the median degenerates into one read with extra SPI traffic. - * - * The published table is indexed by GFSK channel-filter bandwidth. Across - * all 19 rows the product window_us * BW_kHz lands in 921..938, so - * 936 / BW_kHz reproduces every published value to within a microsecond. - * The separate "RSSI delay" column (BUSY falling edge -> first valid sample) - * is 12x to 15x the window across the same rows. - * - * CAVEAT: Semtech documents this for GFSK only and publishes no LoRa - * equivalent. The RSSI path is the same analog/AGC chain, so these are used - * as the best available proxy — not as specified LoRa figures. `get cad` - * reports the measured burst spread so the assumption stays falsifiable on - * real hardware. - */ + * Reads faster than the chip's averaging window return the same sample. + * window_us = 936 / BW_kHz reproduces the table; it is specified for GFSK and + * used here for LoRa as the best available figure. */ #define RSSI_WINDOW_BW_PRODUCT 936U /* window_us * BW_kHz, DS Table 13-82 */ #define RSSI_SETTLE_WINDOWS 16U /* delay/window is 12..15; round up */ -/* Burst-stat rescale point. The counters behind `get cad`'s sp field are - * halved (all three together, so the mean and the zero-spread share are - * preserved exactly) once the burst count reaches this. - * - * Two reasons, and the display one is the hard constraint: the remote CLI - * reply is capped at CLI_REMOTE_REPLY_SIZE (161 B) and has to fit the header - * plus three level lines, so no field may grow without bound. At one burst - * per 15 s a free-running counter passes 500 000 in three months and would - * eat the level rows. 8192 keeps it to four digits forever. - * - * The second reason is that halving turns the totals into an exponential - * forgetting window, so the numbers describe recent conditions instead of - * being anchored to whatever the channel was doing at boot. Same idea as - * CAD_STATS_DECAY_MS below, which halves the CAD counters on a timer. */ +/* Burst-stat rescale point: the counters behind `get cad`'s sp field are + * halved together at this count, which keeps the reply inside the remote CLI + * limit and makes the totals describe recent conditions. */ #define RSSI_BURST_STATS_CAP 8192U static inline uint32_t rssi_avg_window_us(uint16_t bw_khz) @@ -71,16 +45,8 @@ static inline uint32_t rssi_settle_delay_us(uint16_t bw_khz) * the every-16th-sample unguarded bypass are counted in samples, so they scale * with this value — see the Kconfig help before changing it. */ #define NOISE_FLOOR_INTERVAL_MS CONFIG_ZEPHCORE_NOISE_FLOOR_INTERVAL_MS -/* A due sample that lands while the radio is mid-packet, transmitting, or in - * its duty-cycle sleep window is retried on this deadline rather than waiting a - * full interval. - * - * 5000 is not a tuned guess: before the deadline conversion a blocked sample - * simply waited for the next 5 s housekeeping tick, so this reproduces the old - * retry grid exactly. It matters under RX duty cycle, where the chip is in its - * sleep window a large fraction of the time and blocked attempts are the norm - * rather than the exception — a shorter retry there can push the wake rate - * ABOVE the fixed tick this conversion replaced, inverting the whole point. */ +/* A due sample that lands mid-packet, during TX or in the duty-cycle sleep + * window is retried after this long (the old housekeeping grid). */ #define NOISE_FLOOR_RETRY_MS 5000 /* Blocked attempts allowed before standing down to the next full interval. */ #define NOISE_FLOOR_MAX_RETRIES 2 @@ -144,14 +110,8 @@ static inline enum lora_signal_bandwidth bw_khz_to_enum(uint16_t bw_khz) case 125: return BW_125_KHZ; case 250: return BW_250_KHZ; case 500: return BW_500_KHZ; - /* Wide bandwidths, accepted under both spellings: the round name people - * type and the chip's true value they may read off a datasheet. Both - * select the same modem setting. - * - * Only the LR2021 implements these; every other driver here falls back - * to 125 kHz for an unmapped enum, which would be a silent mismatch. - * That is why the CLI only accepts a bandwidth above 500 on an LR2021 - * build — see the bw range check in CommonCLI.cpp. */ + /* Wide bandwidths, under both spellings (the round name and the chip's true + * value). LR2021 only; the CLI rejects them elsewhere. */ case 200: case 203: return BW_200_KHZ; case 400: case 406: return BW_400_KHZ; case 800: case 812: return BW_800_KHZ; @@ -160,22 +120,9 @@ static inline enum lora_signal_bandwidth bw_khz_to_enum(uint16_t bw_khz) } } -/* Lowest physically-possible noise floor for a given bandwidth, in dBm. - * - * This is raw thermal noise — kTB at 290 K, with NO noise-figure term: - * -174 dBm/Hz + 10*log10(BW_Hz) - * A passive receiver cannot read below it, so it is the one place a sanity - * clamp belongs: anything under this line is a bad RSSI read, not a quiet - * site. Adding a receiver noise figure here would clamp ABOVE what the - * hardware can legitimately report and manufacture a floor — which is exactly - * what the old fixed -120 rail did to BW 62.5 kHz, pinning it on every EMA - * update because -120 happens to be that preset's kTB+NF. - * - * Bandwidth is the only term that moves. SF changes the SNR the demodulator - * can decode at, not the noise power in the channel, so it must NOT appear. - * - * For reference, a typical SX126x (NF ~6 dB) reads about 6 dB above these: - * BW 62.5 kHz measures ~-120 dBm on a quiet site against a -126 kTB limit. */ +/* Lowest physically possible noise floor for a bandwidth, in dBm: thermal + * noise kTB at 290 K, -174 dBm/Hz + 10*log10(BW_Hz), with no noise figure and + * no SF term. A reading below it is a bad RSSI read. */ static inline int16_t noise_floor_min_dbm(uint16_t bw_khz) { switch (bw_khz) { diff --git a/zephcore/adapters/rng/ZephyrRNG.cpp b/zephcore/adapters/rng/ZephyrRNG.cpp index 20492e4..a44b733 100644 --- a/zephcore/adapters/rng/ZephyrRNG.cpp +++ b/zephcore/adapters/rng/ZephyrRNG.cpp @@ -41,21 +41,9 @@ void ZephyrRNG::random(uint8_t *dest, size_t sz) Utils::cryptoPanicReboot("CSPRNG unavailable after retries"); } -/* ===== Timing-entropy health check ======================================= - * - * Online health check (NIST SP 800-90B style) for the two-clock beat source - * below: repetition count + a distinct-value check tracked across all samples - * in the window with scalar state — no per-sample buffer needed. Detects - * stuck-source catastrophic failure (e.g. a frozen slow clock). Does not - * statistically prove entropy quality — that's what the selftest - * output-diversity run is for. - * - * (The former CPU-jitter fallback — Stephan Müller style k_cycle_get_32() - * delta sampling — was removed: it only ever carried entropy where the cycle - * counter was already cross-domain from the CPU, and every such board is - * exactly a board the beat covers. Where the beat is unavailable the counter - * is same-domain, the loop is deterministic, and jitter yields ~0 bits — - * measured dead on ESP32 hardware.) */ +/* ===== Timing-entropy health check ===== + * Repetition count plus a distinct-value check over the window, with scalar + * state. Detects a stuck source; it does not prove entropy quality. */ /* Health statistics for one beat window. Timing statistics only — never * pool contents or derived key material. Reporting these is standard practice @@ -90,45 +78,11 @@ struct beat_stats { bool ok; }; -/* ===== Universal two-clock beat entropy ================================== - * - * One physical entropy source for every board: count CPU cycles elapsed across - * a fixed interval of an INDEPENDENT low-frequency oscillator. The two clocks - * come from different sources, so the count fluctuates with the slow - * oscillator's phase noise — real physical entropy, not the deterministic - * same-domain loop that CPU-jitter degenerates to where the cycle counter and - * CPU share a clock (measured dead on ESP32: thousands of identical deltas). - * - * FAST counter = timing_counter_get() — portable CPU cycle counter (DWT on - * Cortex-M, CCOUNT on Xtensa; both at CPU frequency). Needs - * CONFIG_TIMING_FUNCTIONS and a one-time timing_init()/timing_start(). - * - * SLOW clock = an oscillator in a DIFFERENT domain from the CPU, selected by a - * principled rule so the choice is coherent across boards: - * - ESP32: the RTC-slow oscillator via esp_rtc_get_time_us() (internal - * ~136 kHz RC, independent of the XTAL->PLL CPU path). - * - Any board whose Zephyr system timer runs < 1 MHz: that timer IS a - * low-frequency oscillator cross-domain from the CPU (e.g. nRF's - * 32.768 kHz RTC off LFXO/LFRC), so k_cycle_get_32() is a valid slow - * clock. REQUIRES the LF clock to be LFXO/LFRC, not synthesised from - * HFCLK — true for every BLE-capable nRF config; the health check below - * catches it if a board ever violates that. - * - Otherwise (system timer at CPU frequency, e.g. bare SysTick): no - * independent slow clock is identified and the timing stages are - * SKIPPED — a same-domain counter measures a deterministic loop - * (~0 bits, measured), so sampling it would only pretend to add - * entropy. Such boards (STM32WL SysTick, nRF54L 1 MHz GRTC) rely on - * their true TRNG via the CSPRNG stages, which is what the removed - * CPU-jitter fallback effectively did anyway. - * - * Window = 500 us, from an on-hardware ESP32 sweep (memory/findings.md): - * 120/250/500/1000 us gave 1.85/3.04/3.81/5.14 bits/sample; 500 us is the knee. - * Full 32-bit delta is mixed. On ESP32 this is a SECONDARY source (the - * bootloader_random-seeded HWRNG is primary); on nRF it is the strong - * non-HWRNG leg. - * - * CAVEAT (memory/findings.md): the health stats show the beat VARIES, not that - * it is random — the selftest output-diversity run is what validates it. */ +/* ===== Universal two-clock beat entropy ===== + * CPU cycles counted across a fixed interval of an independent slow + * oscillator: ESP32's RTC-slow clock, or a system timer below 1 MHz (nRF's + * 32.768 kHz RTC). Where no independent slow clock exists the timing stages + * are skipped and the board relies on its TRNG. Window 500 us. */ #if defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) #define BEAT_SLOW_HZ 1000000ULL static inline uint64_t beat_slow_ticks(void) { return esp_rtc_get_time_us(); } @@ -236,15 +190,9 @@ static int distinct_bytes(const uint8_t *buf, size_t len) return n; } -/* One line per beat window. `distinct` is capped at 8 by the sampler, so 8/8 - * means "at least 8" — the pass threshold is 5. `maxrep` is the longest run of - * identical deltas; >=32 fails. The span/distinct/maxrep line IS the beat's - * health — per-sample entropy is characterised offline by the selftest window - * sweep, not estimated here (an in-path MCV estimate would need a 16k-slot - * histogram). Deliberately no statistics on the conditioned output: - * AES-256-CTR makes any input look uniform, so output statistics would read - * perfect even for a near-zero-entropy seed. Entropy is a property of the - * source. */ +/* One line per beat window: `distinct` is capped at 8 (pass at 5), `maxrep` + * is the longest run of identical deltas (fail at 32). No statistics on the + * conditioned output: AES-CTR makes any input look uniform. */ #ifdef HAVE_TWO_CLOCK_BEAT static void report_beat(const char *label, const struct beat_stats *st) { @@ -255,31 +203,10 @@ static void report_beat(const char *label, const struct beat_stats *st) } #endif /* HAVE_TWO_CLOCK_BEAT */ -/* ===== Entropy extraction via AES-256-CTR ================================ - * - * Per crypto consultant (MeshCore upstream PR#2280 author): the - * conditioning step is most correctly an XOF or stream cipher, not a - * truncated hash. For our 32-byte Ed25519-seed output the difference - * is design hygiene rather than security, but the cost is the same - * order of magnitude (~one SHA-512 vs SHA-256 + two AES-ECB blocks). - * - * Construction (NIST SP 800-108 KDF-in-Counter-Mode style): - * 1. Extract: SHA-256(pool) → 32-byte AES-256 key. - * 2. Expand: AES-256-ECB(counter_i) for counter_i = 0, 1, 2 ... - * output = concatenation of ciphertext blocks. - * Plaintext-XOR (true CTR mode) is omitted because plaintext would be - * all-zero — we want just the keystream. - * - * Uses PSA crypto API (already enabled via PSA_WANT_KEY_TYPE_AES + - * PSA_WANT_ALG_ECB_NO_PADDING in zephcore_common.conf). - * - * Every failure path fills `err` with the failing step and the raw - * psa_status_t. Failure here is a panic-reboot loop the user sees as a - * dead board (GH #88 on ttgo_tbeam), and the status code is the only - * thing that says WHICH step failed — a bare "extraction failed" costs a - * round trip to the reporter for a build that prints it. Sized for the - * caller's stack buffer; truncation is harmless. - */ +/* ===== Entropy extraction via AES-256-CTR ===== + * Extract: SHA-256(pool) = AES-256 key. Expand: AES-256-ECB over a counter, + * the ciphertext blocks concatenated. Every failure path fills `err` with the + * failing step and the psa_status_t. */ static int extract_via_aes_ctr(const uint8_t *pool, size_t pool_len, uint8_t *out, size_t out_len, char *err, size_t err_len) @@ -372,28 +299,9 @@ void ZephyrRNG::mixIdentitySeed(uint8_t *out, size_t out_len, uint8_t pool[512]; memset(pool, 0, sizeof(pool)); - /* ESP32 only: give the HWRNG a real entropy source for the duration of - * this function. - * - * WDEV_RANDOM is a PRNG that receives hardware entropy only "provided - * Wi-Fi or BT are enabled" (Zephyr drivers/entropy/entropy_esp32.c), and - * sys_csrand_get() maps straight to it. Every caller of this function - * runs before RF is up, and repeater / room-server builds never enable - * RF at all — so stages 1 and 5 below contributed NOTHING on ESP32, - * leaving CPU jitter as the only real source. That was observed failing - * its health check on ThinkNode M9 hardware while deriving a permanent - * identity key. - * - * bootloader_random_enable() puts the SAR ADC into continuous sampling - * and mixes its noise into the HWRNG; Espressif's header explicitly - * sanctions calling it from app code when RF is not up. It must be - * disabled again before anything else touches the ADC or RF — done at - * the end of the collection phase, before AES extraction, so the ADC is - * held for as short a window as possible. - * - * WARNING for future callers: this is unsafe if RF or the ADC is already - * in use. Do not call mixIdentitySeed() after bt_enable() or alongside a - * battery read on ESP32. */ + /* ESP32 only: bootloader_random_enable() feeds the HWRNG for the collection + * phase, because without RF it gets no hardware entropy. Unsafe once RF or + * the ADC is in use: do not call mixIdentitySeed() after bt_enable(). */ #if defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) bootloader_random_enable(); RNG_RPT("[RNG] === identity seed health ===\n"); @@ -435,18 +343,9 @@ void ZephyrRNG::mixIdentitySeed(uint8_t *out, size_t out_len, RNG_RPT("[RNG] stage3 extra : %d bytes\n", (int)n); } - /* Stage 4: hardware-timing entropy, 200ms — the two-clock beat. - * - * Skipped where no independent slow clock exists (see the beat header - * comment): a same-domain counter would sample a deterministic loop and - * only pretend to add entropy. Those boards rely on their true TRNG via - * stages 1 and 5. - * - * Also skipped on POSIX arch (native_sim / Linux): the simulated clock - * only advances when Zephyr threads yield, so k_uptime_get() is frozen - * while this loop spins → infinite loop. On Linux we have /dev/urandom - * (via sys_csrand_get in stages 1 and 5) which is a far stronger source - * than this sampling anyway. */ + /* Stage 4: hardware-timing entropy, 200 ms of the two-clock beat. Skipped + * where no independent slow clock exists, and on POSIX (the simulated clock + * does not advance while this loop spins). */ #if defined(HAVE_TWO_CLOCK_BEAT) && !defined(CONFIG_ARCH_POSIX) struct beat_stats js = {}; bool health_ok = sample_two_clock_beat(pool, sizeof(pool), 112, 200, &js); diff --git a/zephcore/adapters/rng/ZephyrRNG.h b/zephcore/adapters/rng/ZephyrRNG.h index 840368e..fb08021 100644 --- a/zephcore/adapters/rng/ZephyrRNG.h +++ b/zephcore/adapters/rng/ZephyrRNG.h @@ -15,27 +15,10 @@ class ZephyrRNG : public RNG { public: void random(uint8_t *dest, size_t sz) override; - /* Layered entropy mixer for one-time first-boot identity-key - * generation. Combines: - * 1. sys_csrand_get (early) — CSPRNG; on ESP32 only real - * because bootloader_random seeds - * the HWRNG (else pre-RF = 0 bits) - * 2. HWINFO unique device ID — per-device uniqueness (public) - * 3. Optional caller-supplied data — e.g. external noise samples - * 4. Hardware-timing entropy — two-clock beat (ESP32 RTC-slow, - * nRF/MG24 32 kHz RTC); skipped on - * boards with no independent slow - * clock (they rely on their TRNG) - * 5. sys_csrand_get (late) — second HWRNG draw - * 6. Hardware-timing entropy #2 — independent window - * Conditioned via AES-256-CTR (SHA-256(pool) → key, ECB on counter). - * On ESP32 the primary source is the bootloader_random-seeded HWRNG - * (stages 1+5); the beat (4+6) is a physical second source. On nRF the - * CSPRNG and the beat are both independently strong. Health-checked; - * reboots on degenerate output. Blocks ~450ms — first-boot identity only. - * - * Output is suitable as an Ed25519 seed regardless of platform - * TRNG state at boot. */ + /* Layered entropy mixer for the one-time identity key: CSPRNG draws early and + * late, the device ID, optional caller data and two windows of the two-clock + * beat, conditioned via AES-256-CTR. Health-checked; reboots on degenerate + * output. Blocks ~450 ms. */ static void mixIdentitySeed(uint8_t *out, size_t out_len, const uint8_t *extra = nullptr, size_t extra_len = 0); @@ -51,25 +34,13 @@ public: static void setTestKillHWRNG(bool kill); #endif - /* Silence mixIdentitySeed's per-stage entropy health report. - * - * Default OFF — the node prints it once, at first-boot identity - * generation, and it is the only record of what the entropy sources - * actually did for a key that is then permanent. Do not silence it there. - * - * Exists for tools/rng_selftest, which calls mixIdentitySeed thousands of - * times: without this the ~12 lines per seed bury the summary. */ + /* Silence mixIdentitySeed's per-stage health report. Default off; for + * tools/rng_selftest only. */ static void setSeedHealthQuiet(bool quiet); - /* End-to-end first-boot identity generation. Mixes a fresh seed, - * derives the Ed25519 keypair, and retries (up to 100×) if the - * MeshCore protocol-reserved 0x00/0xFF public-key prefix happens - * to land. Panics-and-reboots on cap exhaustion (essentially - * impossible with a working CSPRNG: P(100 reserved in a row) ≈ 10⁻²¹¹). - * Wipes the intermediate seed before return. - * - * Use this from main()'s `loadIdentity` fall-through path instead - * of open-coding the mix+derive+retry+zeroize sequence. */ + /* First-boot identity generation: mixes a seed, derives the Ed25519 keypair, + * retries (up to 100 times) on a reserved 0x00/0xFF public-key prefix, and + * wipes the seed. */ static void generateFirstBootIdentity(LocalIdentity &out_identity); }; diff --git a/zephcore/adapters/sensors/ZephyrEnvSensors.cpp b/zephcore/adapters/sensors/ZephyrEnvSensors.cpp index a6d1926..3c97380 100644 --- a/zephcore/adapters/sensors/ZephyrEnvSensors.cpp +++ b/zephcore/adapters/sensors/ZephyrEnvSensors.cpp @@ -53,20 +53,9 @@ struct sensor_entry { static struct sensor_entry s_sensors[MAX_SENSORS]; static uint8_t s_count; -/* Is this sensor usable — bringing it up first if its node deferred init? - * - * A part behind a switched rail cannot be probed at POST_KERNEL. Regulators - * come up at priority 75 and sensors at 90, typically microseconds later, and a - * regulator-boot-on rail never applies its startup-delay-us (regulator_common_init - * takes the refcount-only branch, so regulator_delay() never runs). Such a node - * is marked zephyr,deferred-init and initialised from here instead, where the - * rail has had the whole boot to settle. See the i2c0 comment in the - * MeshTracker X1 DTS for the failure this prevents. - * - * Safe to call for every candidate on every board: do_device_init() marks a - * device initialized even when its init function failed, so device_init() - * answers -EALREADY for anything that already ran at POST_KERNEL and this - * reduces to a plain device_is_ready() check. */ +/* Is this sensor usable? A node marked zephyr,deferred-init (a part behind a + * switched rail) is initialised here first. For any other device + * device_init() answers -EALREADY and this is a device_is_ready() check. */ static bool sensor_ready(const struct device *dev) { if (dev == NULL) { diff --git a/zephcore/adapters/sensors/t1000e_analog.c b/zephcore/adapters/sensors/t1000e_analog.c index 98f39c8..7eebd4a 100644 --- a/zephcore/adapters/sensors/t1000e_analog.c +++ b/zephcore/adapters/sensors/t1000e_analog.c @@ -26,18 +26,9 @@ LOG_MODULE_REGISTER(t1000e_analog, CONFIG_SENSOR_LOG_LEVEL); * 10 ms settle, so a few extra conversions are free noise rejection. */ #define T1000E_ADC_SAMPLES 4 -/* ================================================================ - * NTC thermistor - * - * Resistance in ohms at each degree from -30 C (index 0) to +105 C - * (index 135) — a 10k-at-25C part, effective beta about 3250. Seeed's - * firmware carries a parallel array of temperatures, which is just the - * index minus 30, so only the resistances are stored here. - * - * (Seeed's source also defines a beta of 4250 next to this table. It is - * dead code there — nothing reads it — and it does not describe this - * curve, so do not "simplify" the table into a beta formula with it.) - * ================================================================ */ +/* NTC thermistor: resistance in ohms at each degree from -30 C (index 0) to + * +105 C (index 135), a 10k-at-25C part. Do not replace the table by a beta + * formula. */ #define NTC_TABLE_LEN 136 #define NTC_TABLE_T_MIN (-30) @@ -59,17 +50,8 @@ static const uint32_t ntc_res[NTC_TABLE_LEN] = { 974, 949, 925, 902, 880, 858, }; -/* ================================================================ - * Photocell - * - * Seeed maps the divider voltage onto 0-100 with a dead band at each end. - * LIGHT_SPAN_MV happens to equal LIGHT_MAX_MV - LIGHT_MIN_MV today, but it is - * kept as its own constant rather than derived from them: the stock firmware - * treats the divisor as an independent number (subtract the 80 mV floor, - * divide by 2400, clamp at the top rather than reach it). Deriving it would - * let a future tweak to either endpoint silently move the scale factor away - * from what the stock firmware uses, and the readings would stop matching. - * ================================================================ */ +/* Photocell: the divider voltage mapped onto 0-100 with a dead band at each + * end, as Seeed's firmware. LIGHT_SPAN_MV is its own constant on purpose. */ #define LIGHT_MIN_MV 80 #define LIGHT_MAX_MV 2480 diff --git a/zephcore/adapters/usb/ZephyrCompanionUSB.cpp b/zephcore/adapters/usb/ZephyrCompanionUSB.cpp index 9b8eaf0..adb5887 100644 --- a/zephcore/adapters/usb/ZephyrCompanionUSB.cpp +++ b/zephcore/adapters/usb/ZephyrCompanionUSB.cpp @@ -47,16 +47,9 @@ enum usb_rx_state { #define USB_TEXT_LINE_MAX 128 -/* ---- Backend selection -------------------------------------------------- - * The companion byte transport is normally a native-USB CDC-ACM UART, but the - * same frame parser / TX ring / CLI runs unchanged over a plain UART too — for - * boards whose USB-C is a USB-UART bridge (e.g. Heltec V3 / CP2102) or that - * have no USB device controller at all. A board selects the backend with the - * `zephcore,companion-uart` chosen node; absent that we fall back to the sole - * cdc-acm-uart, so existing native-USB and nRF builds resolve identically. - * - * COMPANION_HAS_DTR is true only for the CDC backend — a plain UART has no DTR - * line, so a session there, once started, lasts until reboot. */ +/* ---- Backend selection ---- + * The chosen `zephcore,companion-uart` node, else the sole cdc-acm-uart. Only + * the CDC backend has DTR; a plain-UART session lasts until reboot. */ #if DT_HAS_CHOSEN(zephcore_companion_uart) # define COMPANION_UART_DEV DEVICE_DT_GET(DT_CHOSEN(zephcore_companion_uart)) # define COMPANION_HAS_DTR DT_NODE_HAS_COMPAT(DT_CHOSEN(zephcore_companion_uart), zephyr_cdc_acm_uart) @@ -104,10 +97,8 @@ K_MSGQ_DEFINE(usb_recv_queue, sizeof(struct frame), USB_RECV_QUEUE_DEPTH, 4); * bus reset). */ static bool usb_session_active; -/* The session can carry frames: it is open and the bus is not suspended (host - * asleep with the port open). Main hears each edge as a connect or a - * disconnect, so a suspended session is not a client: nothing is queued for - * it, and it does not hold up the last-client cleanup. */ +/* The session is open and the bus is not suspended. Main hears each edge as + * a connect or a disconnect. */ static bool usb_link_up; /* CLI text line callback — fired when a complete line arrives in text mode. */ @@ -244,13 +235,8 @@ static void usb_rx_work_fn(struct k_work *work) uint8_t byte; - /* Timeout partial input — if we've been mid-frame or mid-text-line too - * long without completing, reset the parser and resync. usb_frame_start_time - * is refreshed on every text byte (below), so for USB_RX_TEXT this acts as an - * inactivity watchdog: it recovers a stray printable byte back to IDLE (so a - * later binary frame parses) without truncating a line that is actively being - * typed. Note this only runs when bytes arrive — it is not a timer and never - * wakes a sleeping node. */ + /* A frame or text line left unfinished too long: resync. Checked only when + * bytes arrive, never by a timer. */ if (usb_rx_st != USB_RX_IDLE && (k_uptime_get_32() - usb_frame_start_time) > USB_FRAME_TIMEOUT_MS) { LOG_WRN("usb_rx: partial input timeout (state=%d, expected=%u), resync", @@ -531,13 +517,8 @@ void zephcore_usb_companion_init(const struct companion_link_cbs *link) { s_link = link; - /* COMPANION_UART_DEV resolves to the chosen `zephcore,companion-uart` node, - * or the sole cdc-acm-uart for back-compat (see the backend block above). - * The cdc_acm_uart DT node may be present without the class driver compiled - * (shared esp32s3_usb_otg.dtsi exposes the node unconditionally; the class - * is only enabled with esp32s3_usb.conf) — the COMPANION_HAS_BACKEND gate - * folds that case in, so DEVICE_DT_GET never references an undefined ordinal - * on, e.g., a debug ESP32-S3 companion built without esp32s3_usb.conf. */ + /* COMPANION_HAS_BACKEND is false when the cdc_acm_uart node exists without + * the class driver (a debug ESP32-S3 companion). */ #if COMPANION_HAS_BACKEND usb_dev = COMPANION_UART_DEV; if (device_is_ready(usb_dev)) { diff --git a/zephcore/adapters/usb/ZephyrUSBCDC.cpp b/zephcore/adapters/usb/ZephyrUSBCDC.cpp index ef3a349..81805b7 100644 --- a/zephcore/adapters/usb/ZephyrUSBCDC.cpp +++ b/zephcore/adapters/usb/ZephyrUSBCDC.cpp @@ -148,21 +148,14 @@ static void usbd_msg_callback(struct usbd_context *const ctx, s_dtr_cb(dtr_now); } } else if (msg->type == USBD_MSG_VBUS_REMOVED) { - /* Physical unplug — VBUS lost. On a device-side cable yank the - * host often never sends a clean DTR=0 line-state change, so the - * CONTROL_LINE_STATE release above won't fire and the companion - * would stay stuck on the USB interface (rejecting every BLE - * connection until reboot). Treat VBUS loss as a DTR drop so the - * interface is handed back to BLE. */ + /* Unplugged: the host often sends no DTR=0, so VBUS loss counts as + * a DTR drop. */ LOG_INF("CDC ACM: VBUS removed (device unplug)"); host_gone(); } else if (msg->type == USBD_MSG_RESET) { - /* Bus reset. At enumeration no port is open and this does nothing. - * Later it means the host is enumerating us again (a hub upstream - * was reset, or the host lost the device): the port it had open is - * gone, and no DTR=0 says so, since the class keeps the line state - * of the old session. VBUS can stay up throughout, so treat it as - * a DTR drop too. A reset also ends a suspend, with no RESUME. */ + /* The host is enumerating us again, so the port it had open is + * gone, and no DTR=0 says so: a DTR drop too. A reset also ends a + * suspend, with no RESUME. See docs/ARCHITECTURE.md 7.4. */ LOG_INF("CDC ACM: bus reset"); host_gone(); if (s_bus_cb) { @@ -310,19 +303,9 @@ extern "C" void zephcore_usbd_detach(void) return; } - /* usbd_disable() -> udc_disable() drops the D+ pull-up, which is the only - * thing that makes the host see an unplug. A soft reset does not: on the - * ESP32-S3, esp_restart_noos() resets WiFi/BT, timers, SPI, UART, DMA and - * crypto but nothing USB, so the PHY pad stays enabled across the reset - * and the host keeps talking to an endpoint the firmware has abandoned. - * (Same root cause as GH #43, which hit the USB-Serial-JTAG controller; - * this covers the USB OTG one that the CDC companion transport uses.) - * - * Safe from the usbd message callback: CONFIG_USBD_MSG_DEFERRED_MODE is - * on by default, so that callback runs on the system workqueue rather - * than in the device stack context, and usbd_disable() is not re-entered - * from the thread it stops. - */ + /* Drop the D+ pull-up so the host sees an unplug; a soft reset alone does + * not. Safe from the usbd message callback, which runs on the system work + * queue (CONFIG_USBD_MSG_DEFERRED_MODE). */ (void)usbd_disable(&zephcore_usbd); s_initialized = false; s_dtr_active = false; diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp index fea95bd..24169de 100644 --- a/zephcore/app/CompanionMesh.cpp +++ b/zephcore/app/CompanionMesh.cpp @@ -490,27 +490,10 @@ void CompanionMesh::confirmOfflineMessage() _offline_queue_count--; } -/* How long CMD_APP_START may suppress v-contact notice prompts. - * - * This deadline is also the worst-case freeze a mid-session CMD_APP_START can - * impose, because the app is under no obligation to follow one with the - * message sync that clears the latch — so it must be short enough that nobody - * notices, not merely short enough to recover eventually. - * - * It only has to cover the handoff between CMD_APP_START and the - * CMD_GET_CONTACTS that normally follows it, which is where a prompt could - * make the app fetch a message from a contact it has not been told about yet. - * The dump itself is gated directly by _contact_iter_active below, for however - * long it takes, and once the dump is done a prompt is harmless. Measured on - * hardware, a full 350-contact initial sync runs APP_START to - * PACKET_NO_MORE_MSGS in ~2.9 s, so the handoff is a small fraction of that. - * - * History, because the sizing is the whole bug: 60 s produced the ~54 s - * "v-contact is frozen" reports. 10 s only looked fixed — hardware logs showed - * the user's own settings-screen-to-message time landing at 10.5-11.7 s, so - * the latch was expiring inline microseconds before the prompt and the feature - * was winning a race by half a second. Do not raise this to "be safe"; raising - * it re-creates the bug, and the thing it guards does not need the time. */ +/* How long CMD_APP_START may suppress v-contact notice prompts: just the + * handoff to the CMD_GET_CONTACTS that follows (the dump has its own gate). + * It is also the worst freeze a mid-session APP_START can cause, so it stays + * short; do not raise it. */ #define VCONTACT_HOLD_MAX_MS 3000 bool CompanionMesh::vcontactMsgWaitHeld() @@ -543,16 +526,8 @@ void CompanionMesh::pushMsgWaiting() sendPush(PUSH_CODE_MSG_WAITING); } -/* Re-prompt cadence. - * - * Observed on hardware: the app ignores the first MSG_WAITING after a settings - * write (the GPS toggle) but honours a later identical one, so the reply is - * recovered rather than lost — it just arrives one cadence late. At the - * original 15 s that was a ~20 s wait with the 5 s housekeeping granularity on - * top, which reads as broken even though nothing is. The prompt is a 1-byte - * idempotent frame, so the cost of asking again sooner is nil, and the 5 s tick - * puts the real floor here anyway: 4 s means the first re-prompt lands on the - * first tick at least 4 s after the original, i.e. 4-5 s. */ +/* Re-prompt cadence: the app can ignore a first MSG_WAITING, and asking + * again is free. With the 5 s tick the first re-prompt lands at 4-5 s. */ #define MSGWAIT_REPROMPT_MS 4000 void CompanionMesh::msgWaitingWatchdog() @@ -736,16 +711,9 @@ void CompanionMesh::onDiscoveredContact(ContactInfo &contact, bool is_new, uint8 LOG_INF("onDiscoveredContact: '%s' is_new=%d path_len=%d num_contacts=%d", contact.name, is_new, path_len, getNumContacts()); - /* is_new is upstream's "NOT in contacts[]": auto-add declined it (type - * filter, hop limit, or a full table), so nothing that is stored changed - * and there is nothing to write -- upstream schedules its lazy write only - * for !is_new. A contact the base class has just added arrives with - * is_new false. - * - * Of those, an addition or a changed name, type or position is - * substantive and flushes on the short deadline; a known contact - * re-advertising (only the timestamps moved) is liveness and waits the - * long one, rather than costing a full-file rewrite per advert. */ + /* is_new is upstream's "not in contacts[]": nothing stored changed, so + * nothing to write. Otherwise an addition or a changed name, type or position + * flushes on the short deadline, a bare re-advert on the long one. */ if (!is_new) { bool substantive = !_advert_prev.known || _advert_prev.type != contact.type || @@ -760,13 +728,7 @@ void CompanionMesh::onDiscoveredContact(ContactInfo &contact, bool is_new, uint8 // Update advert path table if (path && mesh::Packet::isValidPathLen(path_len)) { /* Update this node's existing slot if it has one, else take the least - * recently heard (an empty slot has recv_timestamp 0 and wins first). - * - * This used to round-robin into a write cursor with no lookup, so a - * node heard N times occupied N of the slots, evicting other nodes, - * and findAdvertPath() -- which returns the first array match, not the - * newest -- could hand the app a stale path for a node whose route had - * since changed. */ + * recently heard (an empty slot has recv_timestamp 0 and wins first). */ AdvertPath *ap = &_advert_paths[0]; uint32_t oldest = 0xFFFFFFFF; for (int i = 0; i < ADVERT_PATH_TABLE_SIZE; i++) { @@ -1844,14 +1806,9 @@ bool CompanionMesh::handleCmdAppStart(const uint8_t *data, size_t len) return true; } LOG_INF("CMD_APP_START: session reset, %d queued", _offline_queue_count); - // Reset per-session state for a fresh app session. BLE/USB also run - // this cleanup on disconnect, but the serial transport has no - // disconnect event, so APP_START is its authoritative session-reset - // point: drop a stale contact iteration (a leftover PACKET_CONTACT_END - // would confuse the new session's sync state machine), a half-finished - // message sync, and free an abandoned Ed25519 sign buffer (else an 8KB - // leak if the previous session dropped mid-CMD_SIGN). Idempotent — all - // no-ops when the state is already clean. + // Reset per-session state: APP_START is the session reset for a transport + // with no disconnect event. Drops a stale contact iteration, a half-finished + // message sync and an abandoned sign buffer. Idempotent. _contact_iter_active = false; cancelSyncPending(); cleanupSignState(); @@ -1875,17 +1832,8 @@ bool CompanionMesh::handleCmdAppStart(const uint8_t *data, size_t len) * deferred v-contact and flush buffered notices for this session. */ vcontactClockSynced(); - /* Re-stamp the v-contact once per app session so it stays "fresh". - * _vcontact_lastmod feeds both lastmod and last_advert_timestamp, and - * it used to move only on boot / rename / identity import: the app - * showed an ever-growing "last seen" age, and — worse — the contact - * sync gate is `_vcontact_lastmod > _contact_iter_since`, so after the - * first sync the v-contact was never streamed again and app-side state - * (flags, name) could never be corrected. Bumping here, before the - * CMD_GET_CONTACTS that follows APP_START, means every session's sync - * carries a current timestamp; deferring it to the end of sync would - * always land one session late. Silent on purpose — no NEW_ADVERT push - * mid-handshake; the sync itself delivers it. */ + /* Re-stamp the v-contact once per app session, before the CMD_GET_CONTACTS + * that follows, so every sync carries it with a current timestamp. No push. */ if (vcontactReady() && vcontactClockValid()) { _vcontact_lastmod = (uint32_t)getRTCClock()->getCurrentTime(); } @@ -2699,14 +2647,8 @@ bool CompanionMesh::handleCmdImportPrivateKey(const uint8_t *data, size_t len) bool CompanionMesh::handleCmdSendRawData(const uint8_t *data, size_t len) { - /* Raw data packet: [cmd][path_len][path...][payload...] - * - * path_len is the encoded 2-bit (hash_size - 1) + 6-bit hop count, not - * a byte count, so it has to be decoded rather than added to an offset. - * Treating it as a length rejected every path with a hash size above 1: - * hash_size 2 sets bit 6 (path_len 0x42 for two hops read as 66 bytes, - * failing the length check) and hash_size 3 sets bit 7, which used to - * read back as a negative int8_t. */ + /* Raw data packet: [cmd][path_len][path...][payload...]. path_len is the + * encoded 2-bit (hash_size - 1) + 6-bit hop count, not a byte count. */ if (len >= 6) { /* min: cmd + path_len + 4 byte payload */ size_t i = 1; uint8_t path_len = data[i++]; diff --git a/zephcore/app/CompanionMesh.h b/zephcore/app/CompanionMesh.h index 20ac65b..61f09cf 100644 --- a/zephcore/app/CompanionMesh.h +++ b/zephcore/app/CompanionMesh.h @@ -338,16 +338,9 @@ private: int64_t _dirty_channels_expiry; static constexpr int64_t LAZY_WRITE_DELAY_MS = 5000; /* as upstream */ - /* Deadline for a liveness-only change (a known contact re-advertised with - * nothing but a newer timestamp): only last_advert_timestamp and lastmod - * moved. saveContacts() rewrites the whole file (~40 KB, ten 4 KB blocks - * on a T1000-E's 128 KB /lfs); on a busy mesh something is always - * re-advertising, so this deadline IS the rewrite rate. 5 s (upstream) - * was a rewrite per advert, 10 min was 144 a day, and one hour is 24, - * below the adv_blobs write per advert. Every clean reboot and power-off - * flushes first (flushPendingWrites()), so only a crash or a pulled - * battery loses the last hour of "last heard" times. A substantive - * change still pulls the deadline in. */ + /* Deadline for a liveness-only change (a known contact re-advertised): each + * flush rewrites the whole contacts file, so this is the rewrite rate on a + * busy mesh. Clean reboots and power-offs flush first. */ static constexpr int64_t LAZY_WRITE_LIVENESS_MS = 3600000; /* 1 hour */ void markContactsDirty(bool substantive = true); diff --git a/zephcore/app/ObserverMesh.cpp b/zephcore/app/ObserverMesh.cpp index 84c6c7e..7b4bd2f 100644 --- a/zephcore/app/ObserverMesh.cpp +++ b/zephcore/app/ObserverMesh.cpp @@ -61,16 +61,9 @@ void ObserverMesh::begin(RepeaterDataStore *store, struct ObserverCreds *creds) * saves these defaults as they are. */ _store->loadPrefs(_prefs); - /* Load or generate node identity. - * - * Use ZephyrRNG::generateFirstBootIdentity — the SAME hardened path the - * companion and repeater use (bootloader_random-seeded HWRNG + two-clock - * beat, conditioned via AES-256-CTR) — NOT LocalIdentity(_rng). The old - * form drew straight from ZephyrRNG::random() / sys_csrand_get, which on - * an ESP32 observer is unseeded (BLE never comes up to seed WDEV_RANDOM), - * so it derived a permanent key from a weak PRNG. generateFirstBootIdentity - * also owns the reserved-prefix retry (100 attempts + panic backstop), - * replacing the weaker 10-try loop that silently kept a reserved prefix. */ + /* Load or generate the node identity, through + * ZephyrRNG::generateFirstBootIdentity like the other roles: on an ESP32 + * observer the plain RNG is unseeded (BLE never comes up). */ if (!_store->loadIdentity(_self_id)) { LOG_INF("No identity found — generating new keypair"); mesh::ZephyrRNG::generateFirstBootIdentity(_self_id); diff --git a/zephcore/app/RepeaterDataStore.cpp b/zephcore/app/RepeaterDataStore.cpp index b424499..f24a56c 100644 --- a/zephcore/app/RepeaterDataStore.cpp +++ b/zephcore/app/RepeaterDataStore.cpp @@ -49,20 +49,9 @@ const char* RepeaterDataStore::getBasePath() const { return BASE_PATH; } bool RepeaterDataStore::hasRoleData() const { char path[64]; - /* Only THIS role's files count. A companion volume does not: the roles - * are deliberately not interchangeable, and a companion's contacts and - * blob cache would eat into the same 128 KB the repeater needs, so a - * repeater booting onto a companion volume formats it. The reverse - * already happens — ZephyrDataStore::hasPrefs() tests /lfs/new_prefs, - * which a repeater volume never has. - * - * Repeater, room server and observer DO share this store and base path; - * they use the same prefs layout, so switching among them keeps the - * node's identity, which is what an operator wants. - * - * Self-limiting: loadPrefs() persists defaults on boot 1 and main_*.cpp - * saves a generated identity on the same boot, so after one successful - * boot at least one of these exists and the check never fires again. */ + /* Only this role's files count: a repeater booting onto a companion volume + * formats it. Repeater, room server and observer share the store, so + * switching among them keeps the identity. */ static const char* const ours[] = { "prefs.json", "prefs", "_main.id" }; for (size_t i = 0; i < ARRAY_SIZE(ours); i++) { snprintf(path, sizeof(path), "%s/%s", BASE_PATH, ours[i]); diff --git a/zephcore/app/RepeaterMesh.cpp b/zephcore/app/RepeaterMesh.cpp index 59c5830..68d5416 100644 --- a/zephcore/app/RepeaterMesh.cpp +++ b/zephcore/app/RepeaterMesh.cpp @@ -181,16 +181,9 @@ uint8_t RepeaterMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t if (client == nullptr) { uint8_t perms; - /* Constant-time comparison: zero-pad BOTH the received password and - * the stored passwords into cleared buffers (copying only up to the - * NUL) before comparing full-width. Don't trust the stored buffer to - * be zero-padded: a password set over a longer previous value via the - * CLI leaves trailing garbage past the NUL (and such garbage may - * already be persisted in flash on upgraded devices). Comparing the - * raw stored buffer full-width would then fail to match a correct - * password. Compare both unconditionally so timing is identical for - * any wrong password regardless of which (admin/guest) it most - * resembles. */ + /* Constant-time comparison: zero-pad both the received and the stored + * passwords into cleared buffers (up to the NUL) and compare full-width. + * The stored buffer can carry garbage past the NUL. */ uint8_t received[sizeof(_prefs.password)] = {0}; uint8_t admin_pw[sizeof(_prefs.password)] = {0}; uint8_t guest_pw[sizeof(_prefs.guest_password)] = {0}; @@ -206,27 +199,9 @@ uint8_t RepeaterMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t guest_pw, sizeof(received)); - /* An empty stored guest password means OPEN GUEST ACCESS on a repeater, - * deliberately, matching Arduino MeshCore: its NodePrefs default is - * guest_password[0] = 0 (src/helpers/CommonCLI.h) and - * examples/simple_repeater/MyMesh.cpp compares with a plain strcmp, so a - * blank submitted password logs in as guest. Only a blank submission - * matches -- a wrong non-blank password still fails, because both - * buffers are zero-padded and compared full-width. - * - * This is deliberately NOT what RoomServerMesh.cpp does, and the two - * must not be "made consistent". There an empty guest password disables - * guest access, because on a room server that password is what gates - * posting and an accidentally open room is a real hole (the 2026-07-30 - * finding: every ZephCore room server shipped open). Arduino closes the - * same hole from the other side, by defaulting the room server's - * guest_password to ROOM_PASSWORD rather than leaving it empty. - * - * The blast radius on a repeater is small by construction: - * PERM_ACL_GUEST cannot run CLI commands -- the PAYLOAD_TYPE_TXT_MSG - * path in onPeerDataRecv() requires isAdmin() -- and cannot read the - * access list (REQ_TYPE_GET_ACCESS_LIST likewise). A guest gets login - * plus status/telemetry. Set a guest password to close it. */ + /* An empty stored guest password means open guest access on a repeater, as + * Arduino MeshCore. Deliberately not what RoomServerMesh does, where it + * disables guest access. A repeater guest gets login plus status/telemetry. */ if (admin_match) { perms = PERM_ACL_ADMIN; @@ -272,21 +247,9 @@ uint8_t RepeaterMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t memcpy(client->shared_secret, secret, PUB_KEY_SIZE); if (client->isAdmin() && (client->permissions != prev_perms || secret_changed)) { - /* Flush admin sessions now instead of leaving them on the lazy - * timer. The entry carries the shared secret this session's - * traffic is encrypted with; if power is lost before the timer - * fires, the client keeps a secret the repeater no longer knows - * and every later request decrypts to nothing, which presents as - * a wrong password or an unreachable node. A repeater is the - * device most likely to lose power unattended, so the few - * milliseconds are worth it. save() writes the whole ACL, so any - * other pending changes go out with it. - * - * Only when the stored entry changed, though. The secret is the - * ECDH of the two fixed keys, so a known admin logging in again - * carries the same one: rewriting an identical ACL on every login - * (a monitoring tool polling every few minutes) was pure wear. - * Upstream schedules a lazy write on every non-guest login. */ + /* Flush admin sessions now, not on the lazy timer: the entry carries the + * session's shared secret, and losing it to a power cut locks the client out. + * Only when the stored entry changed. */ if (_store) { acl.save(_store->getFS()); dirty_contacts_expiry = 0; @@ -1553,16 +1516,9 @@ void RepeaterMesh::handleRegionLoadLine(uint32_t sender_timestamp, char* command while (*np == ' ') np++; int indent = np - command; - /* An unindented, name-like line is a typed command, not a region row: - * real rows are indent >= 1 (load_stack[0] is the wildcard), and the - * one unindented line a client legitimately sends is the exported - * wildcard header "*", whose '*' is not a name char. Without this, - * `region load` is only escapable by a blank line -- which the USB - * reader discards (main_repeater.cpp) and a dead remote-admin client - * never sends, stranding the CLI until a reboot. Abort without - * committing temp_map and run the command. Must come BEFORE the - * name-terminator write below, which would truncate `set foo 1` to - * `set`. */ + /* An unindented, name-like line is a typed command, not a region row: abort + * the load without committing and run the command. Must come before the + * name-terminator write below. */ if (indent == 0 && RegionMap::is_name_char((uint8_t)*np)) { region_load_active = false; handleCommand(sender_timestamp, command, reply); diff --git a/zephcore/app/RoomServerMesh.cpp b/zephcore/app/RoomServerMesh.cpp index 30298bf..394a7d3 100644 --- a/zephcore/app/RoomServerMesh.cpp +++ b/zephcore/app/RoomServerMesh.cpp @@ -391,17 +391,9 @@ void RoomServerMesh::onAnonDataRecv(mesh::Packet* packet, const uint8_t* secret, } if (client == nullptr) { - /* Constant-time compare against both stored passwords. Admin grants - * ADMIN; the guest/room password grants READ_WRITE (so guests may - * post); allow_read_only downgrades any other login to GUEST. - * - * Zero-pad BOTH operands into cleared buffers first: the CLI's - * StrHelper::strncpy null-terminates but does NOT clear the rest of - * the 16-byte buffer, so a password set over a longer previous value - * leaves trailing garbage. Comparing the raw stored buffer full-width - * against the (zero-padded) received bytes would then fail to match a - * correct password. Copy only up to the NUL so the compare reflects - * the actual string while staying constant-time over the full width. */ + /* Constant-time compare against both stored passwords, zero-padded into + * cleared buffers first. Admin grants ADMIN; the guest/room password grants + * READ_WRITE; allow_read_only downgrades any other login to GUEST. */ uint8_t received[sizeof(_prefs.password)] = {0}; uint8_t admin_pw[sizeof(_prefs.password)] = {0}; uint8_t guest_pw[sizeof(_prefs.guest_password)] = {0}; @@ -1148,16 +1140,9 @@ void RoomServerMesh::handleRegionLoadLine(uint32_t sender_timestamp, char* comma while (*np == ' ') np++; int indent = np - command; - /* An unindented, name-like line is a typed command, not a region row: - * real rows are indent >= 1 (load_stack[0] is the wildcard), and the - * one unindented line a client legitimately sends is the exported - * wildcard header "*", whose '*' is not a name char. Without this, - * `region load` is only escapable by a blank line -- which the USB - * reader discards (main_repeater.cpp) and a dead remote-admin client - * never sends, stranding the CLI until a reboot. Abort without - * committing temp_map and run the command. Must come BEFORE the - * name-terminator write below, which would truncate `set foo 1` to - * `set`. */ + /* An unindented, name-like line is a typed command, not a region row: abort + * the load without committing and run the command. Must come before the + * name-terminator write below. */ if (indent == 0 && RegionMap::is_name_char((uint8_t)*np)) { region_load_active = false; handleCommand(sender_timestamp, command, reply); diff --git a/zephcore/app/VContact.cpp b/zephcore/app/VContact.cpp index 295a041..aa73c79 100644 --- a/zephcore/app/VContact.cpp +++ b/zephcore/app/VContact.cpp @@ -170,13 +170,8 @@ void CompanionMesh::vcontactFlushConfirm() void CompanionMesh::deriveVContactKey() { /* The Ed25519 public point of a keypair seeded from - * SHA256("zc-vcontact" || self prv_key || counter). - * - A real point: strict clients decompress peer keys and reject the ~50% - * of random 32-byte strings that are not valid points. - * - Seeded from the private key, so nobody else can link the v-contact to - * this node or derive its private half, which is dropped here unused. - * - The counter skips pub_key[0] of 0x00/0xFF, which MeshCore reserves - * (P = 2/256 per try, so still deterministic). */ + * SHA256("zc-vcontact" || self prv_key || counter): a real point, unlinkable + * to this node; the counter skips a reserved 0x00/0xFF first byte. */ static const char vc_salt[] = "zc-vcontact"; uint8_t material[PRV_KEY_SIZE + 1]; uint8_t seed[SEED_SIZE]; diff --git a/zephcore/app/main_observer.cpp b/zephcore/app/main_observer.cpp index 23ac57a..f83e6d9 100644 --- a/zephcore/app/main_observer.cpp +++ b/zephcore/app/main_observer.cpp @@ -326,17 +326,8 @@ int main(void) } #endif - /* First boot on a volume that is not this role's - a fresh chip, a - * companion, or a node that was running Arduino MeshCore, whose nRF52 - * filesystems overlap our lfs_partition - * (devdocs/HANDOVER_lfs_arduino_overlap.md). Erase everything so we - * start from a known state: Zephyr's automount only - * auto-formats the LittleFS volume when it fails to mount, and never - * touches storage_partition (BLE bonds NVS) or QSPI. - * - * Self-limiting, so it needs no "done" marker: the identity is generated - * and saved a few lines below, and loadPrefs() persists defaults on the - * same boot, so the next boot sees this role's data and skips this. */ + /* First boot on a volume that is not this role's: erase everything (LittleFS, + * bond NVS, QSPI). Self-limiting: this boot writes the role's data. */ if (!data_store.hasRoleData()) { LOG_WRN("Volume holds no data for this role - formatting before first boot"); if (!zephcore_fs_format_all(nullptr)) { @@ -360,19 +351,8 @@ int main(void) /* LoRa RX callback — observer never needs TX done */ lora_radio.setRxCallback(lora_rx_callback, nullptr); - /* Bind the radio to the mesh's NodePrefs BEFORE begin(). - * - * The radio reads freq/bw/sf/cr through this pointer, both during - * begin() → Dispatcher::begin() → Radio::begin() and on every later - * reconfigure(). Bound to a placeholder that was never loaded from - * flash, the radio stayed on the compiled-in defaults forever: `set freq/sf/bw/cr` wrote flash and - * updated the CLI/MQTT readback but never reached the hardware, so the - * setting looked accepted and then "reverted" on reboot. - * - * Binding before begin() is safe and required: begin() calls loadPrefs() - * first and Dispatcher::begin() last, so the object is populated by the - * time the radio reads through it. Mirrors main_repeater.cpp and - * main_room_server.cpp. */ + /* Bind the radio to the mesh's NodePrefs before begin(): the radio reads + * freq/bw/sf/cr through this pointer at begin() and on every reconfigure. */ lora_radio.setPrefs(observer_mesh.getNodePrefs()); /* Initialize and start mesh (loads prefs + identity from flash) */ diff --git a/zephcore/helpers/CommonCLI.cpp b/zephcore/helpers/CommonCLI.cpp index c25bf7c..47db72f 100644 --- a/zephcore/helpers/CommonCLI.cpp +++ b/zephcore/helpers/CommonCLI.cpp @@ -46,15 +46,9 @@ static uint32_t _atoi(const char* sp) { } /* ---- "default" keyword + strict numeric parsing for the `set` path ---- - * - * Bare atoi()/atof() fold every non-numeric string to 0, the word "default" - * included. On the knobs where 0 is itself legal and means "off" — - * probe.interval, cad.busycap, flood.max, rxduty — that silently switched the - * feature off and still answered OK. `set probe.interval default` disabling - * probing is the report that prompted this. - * - * Every `set` that has a default now takes the literal `default`, and rejects - * trailing garbage rather than turning it into a zero. */ + * atoi()/atof() fold any non-number to 0, which is "off" on several knobs. + * Every `set` with a default takes the literal `default` and rejects + * trailing garbage. */ /* The defaults are read out of initNodePrefs() itself rather than restated as * constants here, so `set default` cannot drift from what a factory-fresh @@ -73,14 +67,8 @@ static const NodePrefs* cliDefaults() { /* ---- leds.radio / leds.hb mode names ----------------------------------- */ /* - * Which LEDs this board actually has, so the CLI can tell the user when a - * setting it just accepted will not do anything here. The pref is still - * stored either way: the same prefs file follows a node onto a board that does - * have the LED, and silently dropping the value would be worse than storing a - * setting that is dormant. - * - * These mirror the aliases the drivers use — lora-tx-led in ZephyrBoard.cpp, - * led0 with an led1 fallback in helpers/ui/ui_common.c. + * Which LEDs this board has, so the CLI can say when a setting it stored + * does nothing here. Mirrors the aliases the drivers use. */ #define CLI_HAS_RADIO_LED DT_NODE_EXISTS(DT_ALIAS(lora_tx_led)) #define CLI_HAS_HB_LED (DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) || \ @@ -248,30 +236,12 @@ void CommonCLI::savePrefs() { _callbacks->savePrefs(); } -/* Drop everything the adaptive-CAD controller has learned. +/* Drop everything the adaptive-CAD controller has learned: on `set + * cad.reset` and on every change of preset. * - * Called both by `set cad.reset` and by every command that moves the radio to a - * different preset, because such a change invalidates all three pieces at once: - * detPeak's base table is per-SF and per-bandwidth, so the ladder the offset - * addresses is re-based under it (SF7->SF12 is 5 counts on SX126x and 16 on the - * LR11xx), and the per-level statistics that justified the offset describe a - * preset that is no longer on air. Re-converging from the new preset's own - * base costs an hour or two; carrying a stale offset across can leave the node - * either transmitting over live receptions or unable to transmit at all. - * - * The operating offset lives in two places — _prefs->cad_offset, and the radio's - * CadController, which applyCadPrefs() reloads through setCadParams(). - * - * preset_pending = the caller has already written the NEW freq/bw/sf/cr to - * _prefs but the radio is still running the OLD preset (frozen by - * freezeRadioParams until the reboot the caller is about to ask for). - * applyCadPrefs() stamps cad_base from the RUNNING radio, i.e. the preset being - * left, so that stamp has to be cleared again afterwards: a stale anchor in - * flash is precisely what makes setCadParams() re-anchor a freshly reset offset - * by (old_base - new_base) on the next boot, which is the bug this reset - * exists to avoid. 0 means "no anchor recorded", which that re-anchor - * deliberately treats as a no-op, and the next boot re-stamps it. - */ + * preset_pending = the new preset is in _prefs but the radio still runs the + * old one until the reboot. applyCadPrefs() then stamps cad_base from the + * preset being left, so the stamp is cleared again (0 = no anchor). */ void CommonCLI::resetCadState(bool preset_pending) { _prefs->cad_offset = cliDefaults()->cad_offset; _prefs->cad_base = 0; @@ -352,21 +322,9 @@ void CommonCLI::scheduleReboot(uint8_t type) k_work_schedule(&_reboot_work, K_SECONDS(2)); } -/* CLI commands are case-sensitive, matching upstream Arduino MeshCore. - * - * A case-insensitive normalizer lived here from 2026-07-12 until 2026-07-19. - * It lowercased the first two whitespace-delimited tokens before matching, on - * the assumption that a value never appears before the third token. That is - * false for "password ", whose value IS token 1 -- so any admin - * password containing uppercase was silently stored folded to lowercase and - * could never be used to log in again. ("set guest.password " was - * unaffected: three tokens.) - * - * Do not reintroduce input folding here. Any scheme that rewrites the buffer - * before dispatch has to guess where keywords end and arguments begin, and - * that guess is what broke. If case-insensitivity is wanted again, do it at - * the comparison sites so argument bytes are never touched. - */ +/* CLI commands are case-sensitive, matching upstream Arduino MeshCore. Do + * not fold the input buffer: a value can be the second token (`password + * `). */ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, char* reply) { if (handleRadioCmd(sender_timestamp, command, reply)) { // is a radio CLI command? return; @@ -377,18 +335,9 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch } if (strcmp(command, "start dfu") == 0) { - /* Reboot into the chip's own firmware-update mode. nRF52: the Adafruit - * UF2 bootloader. ESP32-S3: the ROM download mode, which is the only - * way to get the USB port back from the CDC companion transport for - * esptool (see ZephyrBoard::rebootToBootloader). - * - * Everything else has no bootloader this command can reach, so it must - * NOT reboot. rebootToBootloader() would just reset back into the app, - * dropping the node off the mesh for nothing while replying that it had - * gone somewhere it cannot go. C3/C6 and classic ESP32 do not need it - * anyway -- they have no DWC2 controller, so USB-Serial-JTAG never - * loses the port and esptool resets them itself; nRF54L15/MG24/STM32WL - * have no USB device peripheral at all and are flashed over SWD. */ + /* Reboot into the chip's own firmware-update mode: the UF2 bootloader on + * nRF52, the ROM download mode on ESP32-S3. Other platforms have no + * bootloader this command can reach, so they must not reboot. */ #if defined(CONFIG_SOC_SERIES_ESP32S3) strcpy(reply, "OK - rebooting to ESP32 download mode"); scheduleReboot(REBOOT_DFU); @@ -798,14 +747,9 @@ bool CommonCLI::handleRadioCmd(uint32_t sender_timestamp, const char* command, c /* Plain number, matching upstream Arduino MeshCore's "> %d". */ snprintf(reply, CLI_REPLY_SIZE, "> %d", (int)_prefs->tx_power_dbm); } else if (memcmp(config, "freqerr", 7) == 0) { - /* MUST stay above "freq" — that is a 4-char prefix match and - * would swallow this one. - * - * Carrier frequency error measured on received packets, LR2021 - * only. Diagnostic: nothing acts on it. The mean approximates - * THIS node's reference error only once averaged over many - * different peers (theirs cancel, ours does not), which is why - * the spread and packet count are shown alongside it. */ + /* MUST stay above "freq", a 4-char prefix match that would swallow this + * one. Carrier frequency error of received packets, LR2021 only; + * diagnostic. */ int n = snprintf(reply, CLI_REPLY_SIZE, "> "); if (_callbacks->formatFreqErrorStatus(reply + n, CLI_REPLY_SIZE - n) == 0) { @@ -821,13 +765,8 @@ bool CommonCLI::handleRadioCmd(uint32_t sender_timestamp, const char* command, c if (memcmp(command, "set ", 4) == 0) { const char* config = &command[4]; if (memcmp(config, "dutycycle ", 10) == 0) { - /* Floor is 10%, not Arduino's 1%: this writes airtime_factor as - * (100/dc)-1, and sanitizeNodePrefs() clamps that field to 0..9. - * A 1% duty cycle stores af=99, survives until the next load, and - * is then silently rewritten to 9 (=10%) — the node would run at - * one duty cycle and boot into another. 10% is the lowest value - * that round-trips, and it matches the 0..9 bound `set af` now - * enforces directly. */ + /* Floor is 10%, not Arduino's 1%: airtime_factor is clamped to 0..9, so + * a lower duty cycle would not survive a reload. */ float dc; if (!cliFloat(&config[10], 100.0f / (cliDefaults()->airtime_factor + 1.0f), &dc)) { strcpy(reply, "ERROR: dutycycle must be 10-100, or default"); @@ -875,13 +814,7 @@ bool CommonCLI::handleRadioCmd(uint32_t sender_timestamp, const char* command, c } } } else if (memcmp(config, "agc.reset.interval ", 19) == 0) { - /* Periodic AGC recalibration was removed: it reset the noise floor - * to its unseeded sentinel on every fire, forcing a fresh seed and - * a full EMA warmup, and it was already forced off under RX duty - * cycle. RX duty cycle is the supported way to cut RX current. - * The prefs BYTE is retained (read/written, never acted on) — the - * on-disk layout is byte-exact and shifting it would corrupt every - * existing node's prefs. */ + /* Removed. The prefs byte is kept for the legacy on-disk layout. */ strcpy(reply, "Removed - Automatic AGC reset is on"); } else if (memcmp(config, "multi.acks ", 11) == 0) { long val; @@ -1068,15 +1001,8 @@ void CommonCLI::handleGetCmd(uint32_t sender_timestamp, const char* command, cha zephcore_buzzer_mode_name(mode)); #endif #ifdef CONFIG_ZEPHCORE_ROLE_ROOM_SERVER - /* Room server only. RoomServerMesh.cpp is the sole consumer of - * allow_read_only; RepeaterMesh.cpp never reads it, so on a repeater - * this advertised a setting that silently did nothing. Note this is a - * deliberate divergence from Arduino MeshCore, whose shared CommonCLI - * exposes the knob on every role for the same reason ours used to. - * - * The pref itself stays unconditional -- it is byte 114 of the on-flash - * prefs layout, identical to Arduino's, so dropping it would shift every - * field after it and invalidate existing prefs files. */ + /* Room server only: nothing else reads allow_read_only. The pref itself + * stays for the legacy on-disk layout. */ } else if (memcmp(config, "allow.read.only", 15) == 0) { snprintf(reply, CLI_REPLY_SIZE, "> %s", _prefs->allow_read_only ? "on" : "off"); #endif @@ -1274,18 +1200,8 @@ void CommonCLI::handleGetCmd(uint32_t sender_timestamp, const char* command, cha (unsigned)_prefs->cad_busycap); } } else if (strcmp(config, "cad") == 0) { - /* Arduino exposes this as a boolean knob; ZephCore always does CAD, - * so the answer is a constant "on". Apps parse the word. - * - * EXACT match, not the 3-byte prefix this used to be. Every branch - * in this chain is a memcmp prefix test, so "cad" matched every - * `get cad.*` that had no branch of its own and answered "on" for - * all of them -- a percentage, a signed offset and an action all - * came back as a boolean. cad.stats escaped only by being ordered - * above it, which is a fragile way to stay correct. Anchoring this - * one fixes the whole family at once: anything under cad. without a - * getter now falls through to the honest "??:" reply instead of - * being answered wrongly. */ + /* ZephCore always does CAD, so the answer is a constant; apps parse the + * word. Exact match: a prefix test would answer every `get cad.*`. */ strcpy(reply, "> on"); } else if (memcmp(config, "extra.sf", 8) == 0) { /* No "> " prefix, and the empty case is a sentence -- both match @@ -1444,15 +1360,8 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, const char* command, cha strcpy(reply, "OK"); } } else if (memcmp(config, "cad.reset", 9) == 0) { - /* Clearing the probe statistics alone left the node re-converging - * FROM wherever the staircase had already walked detPeak, with no - * evidence left to justify sitting there — the opposite of a reset, - * and useless for the one job this command has (recovering after a - * base-table change). - * - * preset_pending = false: the radio is running the preset this - * reset is for, so the base applyCadPrefs() stamps is the right - * one to keep. */ + /* Reset the offset and the base anchor as well, not only the statistics. + * preset_pending = false: the radio runs the preset this reset is for. */ resetCadState(false); snprintf(reply, CLI_REPLY_SIZE, "OK - CAD probe stats cleared, detPeak offset reset to %d", @@ -1773,15 +1682,8 @@ void CommonCLI::handleSetCmd(uint32_t sender_timestamp, const char* command, cha strcpy(reply, "Error: must be 0, 1, on, or off"); } } else if (memcmp(config, "tz.offset ", 10) == 0) { - // Whole-hour offset from UTC for the ON-DEVICE CLOCK DISPLAY only. - // The RTC, `clock` and `time ` all stay UTC: they round-trip - // with each other, apps parse them, and a timezone that reached the - // clock would look like a backward jump to every timestamp consumer - // on the node (see NodePrefs::tz_offset). - // - // Whole hours, matching upstream MeshCore's command of the same - // name, so an app that speaks to both trees behaves identically. - // Half-hour zones (+5:30, -3:30) cannot be expressed. + // Whole-hour offset from UTC for the on-device clock display only; the RTC, + // `clock` and `time ` stay UTC. Whole hours as upstream. long val; if (!cliNum(&config[10], cliDefaults()->tz_offset, &val)) { snprintf(reply, CLI_REPLY_SIZE, "Error: expected %d..%d or default", diff --git a/zephcore/helpers/NodePrefs.h b/zephcore/helpers/NodePrefs.h index 2667e36..c5e2b28 100644 --- a/zephcore/helpers/NodePrefs.h +++ b/zephcore/helpers/NodePrefs.h @@ -17,27 +17,11 @@ * and this header is C++; see the note there. */ #include "led_gate.h" -/* ── Accepted radio parameter ranges ────────────────────────────────── - * - * One definition, used by everything that accepts or validates a radio - * preset: the USB CLI, the BLE companion protocol, the observer CLI, and the - * load-time sanity checks in both datastores. - * - * It lives here because these ranges were previously copy-pasted into six - * places and drifted. That drift was not theoretical: when 2.4 GHz support - * landed, the setters were widened and the *loaders* were not, so a node - * accepted `set freq 2450`, saved it, and then silently reverted to factory - * defaults on the next boot because the load-time guard still capped at - * 960 MHz. A setter must never accept what the loader will throw away. - * - * FREQ spans both LR2021 RF paths — the sub-GHz one (150-960 MHz) and the - * high band (1.9-2.5 GHz). Boards whose radio cannot reach the high band are - * limited by their own driver, not by this range. - * - * BW_MAX is the one value that is per-build. Only the LR2021 implements the - * wide 203/406/812/1000 kHz set; every other driver here maps an unknown - * bandwidth to 125 kHz instead of refusing it, so accepting 812 on those - * boards would put a node on a channel width it never reported. */ +/* ── Accepted radio parameter ranges ── + * One definition for everything that accepts or validates a radio preset + * (CLIs, the companion protocol, the load-time checks): a setter must never + * accept what the loader throws away. FREQ spans both LR2021 RF paths; + * BW_MAX is per build (only the LR2021 has the wide bandwidths). */ #define ZC_RADIO_FREQ_MIN_MHZ 150.0f #define ZC_RADIO_FREQ_MAX_MHZ 2500.0f #define ZC_RADIO_BW_MIN_KHZ 7.0f @@ -62,31 +46,15 @@ #define LOOP_DETECT_MODERATE 2 #define LOOP_DETECT_STRICT 3 -/* Adaptive-CAD operating detPeak offset range (levels from the family base). - * Wide on purpose: a dense hilltop can need a much higher detPeak than a quiet - * valley node. The per-family absolute clamp inside the driver (SX126x 15-40, - * LR11xx/LR20xx 48-90) is a firmware guardrail, NOT a chip limit — cadDetPeak - * is a full uint8_t (0-255). It just keeps the staircase from wandering into - * "CAD never fires" (too high) or "CAD always busy" (too low) territory. This - * offset range limits how far the staircase / manual offset may roam; MUST - * match CAD_LEVEL_MIN/MAX in adapters/radio/radio_common.h (they index the - * per-level stats array). */ +/* Adaptive-CAD detPeak offset range (levels from the family base): how far + * the staircase or a manual offset may roam. Must match CAD_LEVEL_MIN/MAX in + * adapters/radio/radio_common.h. */ #define CAD_OFFSET_MIN (-8) #define CAD_OFFSET_MAX 12 -/* leds_disabled, as stored in the repeater/room-server/observer prefs layout. - * - * It occupies the byte that used to hold agc_reset_interval (offset 120), - * retired when periodic AGC recalibration was removed. That byte is NOT - * reusable as a plain 0/1 boolean: the old command stored seconds/4, so a node - * upgrading from a build that had it configured has an arbitrary small integer - * sitting there, and a bare non-zero test would silently kill its LEDs. Hence - * a magic encoding — anything that is not one of these two values is a legacy - * AGC interval and decodes to the default (LEDs on). The first savePrefs() - * claims the byte for good. - * - * The companion layout is unaffected: it has always stored leds_disabled as a - * plain 0/1 at its own offset 93. */ +/* leds_disabled in the server prefs layout sits in the byte that held + * agc_reset_interval (offset 120), so it is magic-encoded: any other value is + * a legacy AGC interval and decodes to LEDs on. */ #define LEDS_PREF_ON 0xA0 #define LEDS_PREF_OFF 0xA1 @@ -152,14 +120,8 @@ struct NodePrefs { uint8_t rx_boost; // 1 = boosted RX gain (+3dB), 0 = power save uint8_t fem_rxgain; // 1 = external FEM LNA active during RX, 0 = off (power save) uint8_t rx_duty_cycle; // 1 = RX duty cycle, 0 = continuous RX - /* RESERVED — formerly apc_enabled / apc_margin (Adaptive Power Control, - * removed in 1.16.6). These two bytes are still read and written at their - * original offsets in both prefs layouts (companion new_prefs 94/95, server - * prefs 292/293; helpers/PrefsCodec.cpp) because every field after them - * is positional: dropping them would shift the rest of the layout and make - * every already-deployed node misparse its saved prefs on upgrade. - * Do not reuse for a new setting — an upgraded node still has the old APC - * values sitting in these bytes. */ + /* RESERVED: formerly apc_enabled / apc_margin. Still read and written at + * their offsets in both legacy layouts; do not reuse. */ uint8_t _reserved_apc_enabled; uint8_t _reserved_apc_margin; uint8_t meshtimesync; // 1 = mesh time-sync clock correction on (default off) @@ -180,49 +142,20 @@ struct NodePrefs { * driver rejects anything else. Ignored on non-LR2021 radios. */ uint8_t extra_sf[EXTRA_SF_MAX]; - /* Physical mounting orientation. Common to both roles: a repeater board - * with an OLED (RAK4631, Heltec) can be mounted upside down just as a - * companion can, and the joystick boards ship a repeater artifact too. - * - * display_rotate rotates the panel 180 degrees in hardware and is only - * honoured on SSD1306/SH1106 (see MC_DISPLAY_ROTATE_SUPPORTED); other - * panels report it unsupported rather than silently ignoring it. - * input_rotate swaps the joystick/D-pad axes to match, and is kept - * separate because the two are not always wanted together — a screen can - * be remounted without moving the stick. */ + /* Physical mounting orientation, both roles. display_rotate turns the panel + * 180 degrees where the panel supports it; input_rotate swaps the + * joystick/D-pad axes and is separate on purpose. */ uint8_t display_rotate; // 1 = panel rotated 180 degrees uint8_t input_rotate; // 1 = joystick up/down and left/right swapped - /* Whole-hour offset from UTC, -12..+14, applied ONLY when formatting a - * clock for the local display. It must never reach RTCClock or any - * timestamp that leaves this node: a negative offset applied to the clock - * itself reads as a backward jump to every timestamp consumer at once - * (advert timestamps, the repeater ACL's monotonic sender_timestamp gate, - * discovery_mod_timestamp, MeshTimeSync), and a backward clock is a silent - * mesh-wide mute. The CLI's `clock` / `time` commands stay UTC for the - * same reason -- they round-trip with each other and apps parse them. - * - * Whole hours only, matching upstream's `set tz.offset` so an app that - * speaks to both trees behaves the same. Half-hour zones (India +5:30, - * Newfoundland -3:30) therefore cannot be expressed. - * - * Common to both roles: a repeater with an OLED shows a clock too. On a - * headless repeater the field is simply inert. */ + /* Whole-hour offset from UTC, -12..+14, applied only when formatting a clock + * for the local display. It must never reach RTCClock or any timestamp that + * leaves this node. */ int8_t tz_offset; /* The family base detPeak that cad_offset was learned against (0 = never - * stored, i.e. a node upgrading from a build without this field). - * - * cad_offset is a SIGNED OFFSET from a per-SF/per-bandwidth base table - * that lives in the driver, so changing that table silently re-points a - * stored offset at a different absolute detPeak. The probe statistics - * behind the offset are RAM-only and die at the reboot a firmware upgrade - * involves, but the offset itself is persisted and survives — so after a - * table change a converged node quietly starts operating somewhere it - * never measured. Recording the base turns that into something the - * firmware can correct at boot (see LoRaRadio::setCadParams), instead - * of a "run set cad.reset after upgrading" line in the release notes that - * most users will not read. */ + * stored). Lets the firmware re-anchor the offset at boot when the driver's + * base table changes (LoRaRadio::setCadParams). */ uint8_t cad_base; /* ---- Companion-only fields ---- */ @@ -264,19 +197,10 @@ struct NodePrefs { uint8_t wifi_enabled; // 1 = join wifi_ssid when set (upstream default) }; -/* Range guards for prefs that came off flash. - * - * The atomic replace in every savePrefs() plus littlefs's own CRCs make a torn - * write impossible, so this is not about power loss — it is about a blob that - * is structurally intact and semantically wrong. Several of these fields make - * a node look bricked when they are: auto_shutdown_mv powers it off seconds - * after boot, ble_pin locks pairing out, and the char fields are stored as - * fixed-size blocks with no terminator in the file format, so an unterminated - * one runs every later %s off the end of the struct. - * - * Called by both prefs decoders (PrefsCodec.cpp). Fields whose whole range is - * legal (autoadd_config bitmask, discovery_mod_timestamp, the v_contact_* - * sentinels) are deliberately left alone. */ +/* Range guards for prefs that came off flash: a blob can be intact and + * semantically wrong, and several fields make a node look bricked then. + * Called by both prefs decoders. Fields whose whole range is legal are left + * alone. */ template static inline T clampPref(T v, T lo, T hi) { return v < lo ? lo : (v > hi ? hi : v); } @@ -483,14 +407,8 @@ static inline void initNodePrefs(NodePrefs* prefs) { prefs->flood_max_unscoped = 64; // un-scoped flood hop limit (defaults to flood_max) prefs->flood_max_advert = 8; // ADVERT flood hop limit (upstream default) /* 2-byte path hashes (mode + 1 = hash size) for everything this node - * originates. ZephCore's default, not upstream's -- Arduino MeshCore - * ships 0 (1-byte) for every role, which collides far more often on a - * dense mesh. It lived only in the RepeaterMesh and RoomServerMesh - * constructors until now, so a fresh companion quietly originated 1-byte - * floods and reported mode 0 to the app, and `set path.hash.mode default` - * wrote 0 even on a repeater that had booted at 1. Deployed nodes keep - * whatever they stored: path_hash_mode sits at a fixed prefs offset that - * always loads, so this only changes a factory-fresh node. */ + * originates. ZephCore's default for every role; upstream ships 0. Only + * affects a factory-fresh node. */ prefs->path_hash_mode = 1; // 2-byte path hashes prefs->interference_threshold = 0; prefs->leds_disabled = 0; // LEDs on @@ -521,14 +439,8 @@ static inline void initNodePrefs(NodePrefs* prefs) { prefs->v_contact_enabled = 1; // Default ON — v-contact loopback admin chat (companion) prefs->v_battery_alert_mv = 0xFFFF; // Sentinel: derive from board auto-shutdown threshold prefs->wifi_enabled = 1; // as upstream: WiFi still stays off until an SSID is set - /* Companion-only feature, and main_companion.cpp used to assign this by - * hand right after calling us — so no node ever ran without it. It lives - * here now because a default listed only at one call site is invisible to - * every other caller of initNodePrefs(), which is the exact drift that - * zeroed probe_interval and cad_auto in the past. Matches what - * sanitizeNodePrefs() already uses. - * 0 means disabled and is a legal stored value, so sanitize passes it - * through untouched — this default only applies to a fresh prefs struct. */ + /* Companion-only default, set here so every caller of initNodePrefs() sees + * it. 0 means disabled and is a legal stored value. */ #ifdef CONFIG_ZEPHCORE_AUTO_SHUTDOWN_MILLIVOLTS prefs->auto_shutdown_mv = CONFIG_ZEPHCORE_AUTO_SHUTDOWN_MILLIVOLTS; prefs->auto_shutdown_set = 1; diff --git a/zephcore/helpers/battery_curve.c b/zephcore/helpers/battery_curve.c index 02cb24a..67a5d53 100644 --- a/zephcore/helpers/battery_curve.c +++ b/zephcore/helpers/battery_curve.c @@ -5,13 +5,9 @@ #include "battery_curve.h" /* - * Generic single-cell LiPo OCV curve — 21 points at 5% steps. - * Derived from measured LiPo discharge data; 11-point base expanded to - * 21 points by linear interpolation for better knee resolution. - * - * Index 0 = 100% (4190 mV), index 20 = 0% (3100 mV). - * 100% is 4190 mV, not 4200 mV — reflects observed charge ceiling across - * multiple boards after the charger transitions to maintenance mode. + * Generic single-cell LiPo OCV curve, 21 points at 5% steps. + * Index 0 = 100% (4190 mV, the observed charge ceiling), index 20 = 0% + * (3100 mV). */ static const uint16_t ocv_generic[21] = { 4190, 4120, 4050, 4020, 3990, /* 100 .. 80% */ diff --git a/zephcore/helpers/boot_prefs.h b/zephcore/helpers/boot_prefs.h index 00bdd38..cb90e19 100644 --- a/zephcore/helpers/boot_prefs.h +++ b/zephcore/helpers/boot_prefs.h @@ -16,18 +16,9 @@ #include "ZephyrGPSManager.h" /* Push the prefs that drive hardware directly (LED gate, GPS duty) into their - * managers. Ordering is load-bearing: - * - after loadPrefs(): before it the struct still holds the initNodePrefs() - * defaults (leds on, gps_interval=300, the companion value). On - * repeater/room server loadPrefs() is also where the 48 h GPS default is - * set and persisted. Calling this too early is the bug class where CLI - * readback is correct and the hardware disagrees. - * - after ui_init(), so the heartbeat cycle exists to be stopped. - * - after the GPS fix/event callbacks are registered, because - * gps_enable() starts acquisition immediately. - * gps_time_sync: put GPS in repeater (time-sync-only) mode and apply - * prefs.gps_enabled, as upstream's applyGpsPrefs(). Only roles that own the - * GPS pass true. */ + * managers. Call after loadPrefs(), after ui_init() and after the GPS + * callbacks are registered. gps_time_sync: repeater (time-sync-only) GPS mode + * plus prefs.gps_enabled; only roles that own the GPS pass true. */ static inline void apply_boot_prefs(const NodePrefs *p, bool gps_time_sync) { bool leds_off = p->leds_disabled != 0; diff --git a/zephcore/helpers/input/stc8h_keypad.c b/zephcore/helpers/input/stc8h_keypad.c index 92a6428..413f16b 100644 --- a/zephcore/helpers/input/stc8h_keypad.c +++ b/zephcore/helpers/input/stc8h_keypad.c @@ -44,15 +44,9 @@ #include LOG_MODULE_REGISTER(stc8h_keypad, CONFIG_ZEPHCORE_BOARD_LOG_LEVEL); -/* Register map — see dts/bindings/input/zephcore,stc8h-keypad.yaml. - * - * The key register is 0x01, NOT the 0x05 "matrix key" register that the - * reference header documents: the recovered implementation bodies read the - * key from 0x01, and the 0x05 define is dead code nothing consumes. Reading - * 0x05 returns constant 0xFF — that was this driver's original bug (keys - * fully dead, phantom 0xFF at init). The overlap with the battery registers - * (0x01..0x04 little-endian) is odd but is what the shipped firmware does; - * it is also why key reads are IRQ-gated below. */ +/* Register map: see dts/bindings/input/zephcore,stc8h-keypad.yaml. The key + * register is 0x01, not the 0x05 the reference header names (0x05 reads + * 0xFF). It overlaps the battery registers, hence the IRQ-gated key reads. */ #define STC8H_REG_BATTERY 0x01 /* 0x01..0x04, little-endian millivolts */ #define STC8H_REG_KEY 0x01 #define STC8H_REG_STATE 0x06 diff --git a/zephcore/helpers/led_gate.h b/zephcore/helpers/led_gate.h index 020ce72..726dfac 100644 --- a/zephcore/helpers/led_gate.h +++ b/zephcore/helpers/led_gate.h @@ -40,18 +40,9 @@ void zephcore_leds_set_disabled(bool disabled); void zephcore_leds_ui_sync(bool disabled); /* - * Mode values for the two per-LED settings. They live here rather than in - * NodePrefs.h, where the rest of the prefs enums are, because both consumers of - * the heartbeat modes are C (helpers/ui/ui_common.c) and NodePrefs.h is C++. - * NodePrefs.h includes this header so the persisted fields and these values - * still have exactly one definition between them. - * - * leds.radio — what the LoRa activity LED (the `lora-tx-led` DT alias) reacts - * to. TX is deliberately value 0: it is what every node did before the setting - * existed, so an absent byte in an old prefs file, a short read, and a - * zero-filled byte all decode to the historical behaviour. Same reasoning - * applies to LEDS_HB_ALL. Boards without the alias have no activity LED at all - * and ignore this setting whatever it says. + * Mode values for the two per-LED settings (here, not in NodePrefs.h, because + * the consumers are C). Value 0 is the historical behaviour in both, so an + * absent or zero byte in an old prefs file decodes to it. */ #define LEDS_RADIO_TX 0 /* lit for the duration of each transmit (default) */ #define LEDS_RADIO_RX 1 /* short pulse on each packet received */ @@ -73,12 +64,8 @@ void zephcore_leds_ui_sync(bool disabled); #define LEDS_HB_MAX LEDS_HB_OFF /* - * Per-LED modes, below the master gate: "set leds off" still wins over both. - * - * Kept here rather than read from NodePrefs directly because the consumers are - * on different threads from the CLI that writes them: the heartbeat runs on the - * system work queue and the activity LED on the radio's TX path, so both need a - * lock-free snapshot rather than a pointer into a struct the main thread edits. + * Per-LED modes, below the master gate ("set leds off" wins). Lock-free + * snapshots: the consumers run on other threads than the CLI that writes. */ uint8_t zephcore_leds_radio_mode(void); void zephcore_leds_set_radio_mode(uint8_t mode); @@ -87,16 +74,9 @@ uint8_t zephcore_leds_hb_mode(void); void zephcore_leds_set_hb_mode(uint8_t mode); /* - * Shared-pin arbitration. On 8 of the supported boards the `lora-tx-led` alias - * IS `led0`, so the heartbeat cycle and the radio activity LED drive the same - * physical GPIO from two different modules. Without this the two clip each - * other: the heartbeat's off-work clears the pin in the middle of a transmit, - * and onAfterTransmit() truncates a heartbeat pulse. - * - * ZephyrBoard raises the hold for the transmit window and for each RX pulse; - * ui_common.c's heartbeat handlers skip lighting and clearing the pin while it - * is held, so radio activity wins and the liveness tick yields. Boards whose - * pins differ compile the check out entirely (see ZEPHCORE_LED_PIN_SHARED). + * Shared-pin arbitration: where `lora-tx-led` is `led0`, the radio holds the + * pin for a transmit or RX pulse and the heartbeat skips it meanwhile. + * Compiled out where the pins differ (ZEPHCORE_LED_PIN_SHARED). */ bool zephcore_led_radio_holds_pin(void); void zephcore_led_radio_hold_pin(bool held); diff --git a/zephcore/helpers/ui-button/ui_pages.c b/zephcore/helpers/ui-button/ui_pages.c index 058dcb6..ba4a487 100644 --- a/zephcore/helpers/ui-button/ui_pages.c +++ b/zephcore/helpers/ui-button/ui_pages.c @@ -35,18 +35,11 @@ LOG_MODULE_REGISTER(ui_pages, CONFIG_ZEPHCORE_BOARD_LOG_LEVEL); * the joystick UI's splash. See display.h for declaration. */ /* ========== Layout ========== */ -/* All layout is derived from the actual display and font dimensions - * queried at runtime, so the UI adapts to any resolution/font size. - * - * Naming convention: UPPER_CASE macros call display getters (cheap — - * they just return a cached static). This keeps page renderers - * readable while being resolution-independent. - * - * Layout (top to bottom): - * Row 0..FONT_H-1: Top bar (node name left, battery right) - * DOTS_Y..SEP_Y: Page indicator dots + separator line - * CONTENT_Y..end: Page-specific content - */ +/* Layout is derived from the display and font dimensions at run time. The + * UPPER_CASE macros call cached display getters. + * Row 0..FONT_H-1: top bar (node name left, battery right) + * DOTS_Y..SEP_Y: page indicator dots + separator line + * CONTENT_Y..end: page-specific content */ /* Full-scale end of the radio page's TX power bar. The board's configured * ceiling when it has one (same symbol CommonCLI clamps "set tx" against), * else the common SX126x/LR11xx maximum. */ @@ -601,17 +594,9 @@ static void render_tiny_title(void) } /* - * Per-page renderer split (UI profile pattern) - * -------------------------------------------- - * Each page's capability-divergent bodies live in dedicated _mono / _color - * functions. render_() is a thin dispatcher that picks one: - * - Mono / tiny / e-ink panels use render__mono(). - * - RGB565 color panels use render__color(), which is compiled ONLY - * when a `tft` node exists in devicetree (MC_DISPLAY_COLOR_PANEL). On a - * mono board the color body — and every color-only helper it references — - * drops out entirely, so no color RAM/flash is spent. - * The bodies are moved verbatim from the old combined function, so on-screen - * output is byte-identical to before. + * Per-page renderer split: render_() dispatches to render__mono() + * or, on an RGB565 panel, render__color(), which is compiled only when + * a `tft` node exists (MC_DISPLAY_COLOR_PANEL). */ static void render_messages_mono(void) { diff --git a/zephcore/helpers/ui-button/ui_task.c b/zephcore/helpers/ui-button/ui_task.c index 5047f30..61c1533 100644 --- a/zephcore/helpers/ui-button/ui_task.c +++ b/zephcore/helpers/ui-button/ui_task.c @@ -71,20 +71,12 @@ extern void ui_led_flash_msg(void); #include LOG_MODULE_REGISTER(ui_task, CONFIG_ZEPHCORE_BOARD_LOG_LEVEL); -/* Action feedback melodies. - * b=200, d=16 → each chirp ~75ms, rest ~75ms = clear separation. - * - * The chirp count identifies the action, not the tap count — the two are only - * equal on boards using the stock tap-codes order, and LED already differs - * there (2 taps, 5 chirps). Do not "fix" a count to match a gesture. - * - * advert sent (zero-hop or flood): chirp-chirp - * buzzer / notification mode: chirp×3 + high(ON) or low(OFF) - * GPS toggle: chirp×4 + high(ON) or low(OFF) - * LED heartbeat toggle: chirp×5 + high(ON) or low(OFF) - * - * ON tail: high E7 (~2637Hz) = "enabled" - * OFF tail: low G5 (~784Hz) = "disabled" */ +/* Action feedback melodies (b=200, d=16: ~75 ms chirps). The chirp count + * identifies the action, not the tap count. + * advert sent: chirp x2 + * buzzer / notification mode: chirp x3 + high (on) or low (off) + * GPS toggle: chirp x4 + high or low + * LED heartbeat toggle: chirp x5 + high or low */ #define MELODY_BEEP_2 "b2:d=16,o=7,b=200:c,p,c" #define MELODY_BUZZER_ON "bon:d=16,o=7,b=200:c,p,c,p,c,p,p,8e" @@ -542,28 +534,10 @@ static void action_deep_sleep(void) /* ========== Input Event Handler ========== */ #ifdef CONFIG_ZEPHCORE_UI_DISPLAY -/* Set when a button press woke the display; consumed by the resolved action - * code so the wake-up press does not also run its normal action. - * - * The display wakes on the raw press event (INPUT_KEY_0 from gpio-keys), but - * the action it maps to is emitted later by the longpress / multi-tap filters - * (up to tap-delay-ms or long-delay-ms afterwards). By then the display - * reports as on, so the resolved code used to fall through and run normally — - * a single tap woke the screen and immediately paged forward. - * - * The flag is matched against real action codes rather than "the next event" - * because the multi-tap filter passes an unhandled raw KEY_A through per tap - * before it resolves; swallowing that would leave a double tap still acting. - * - * No expiry is needed, but the flag must only be armed when the wake press was - * a raw code that still has an action code coming. ZEPHCORE_UI_DESIGN_JOYSTICK - * depends on ZEPHCORE_ROLE_COMPANION, so the repeater build of a joystick board - * (wio_tracker_l1, gat562_30s — both shipped as repeater artifacts) falls back - * to ZEPHCORE_UI_DESIGN_BUTTON and *does* reach this callback, with its - * joystick GPIOs wired straight to INPUT_KEY_UP/DOWN/LEFT/RIGHT/ENTER and no - * filter in between. There the wake press is already an action code and is - * consumed at the wake, so the flag stays clear and the next press acts. - */ +/* Set when a button press woke the display; the action code that press + * resolves to (emitted later by the longpress / multi-tap filters) is then + * swallowed. Armed only when the wake press was a raw code with an action + * code still to come. */ static bool display_woken_pending; static bool is_ui_action_code(uint16_t code) @@ -650,16 +624,8 @@ static void ui_input_cb(struct input_event *evt, void *user_data) } #ifdef CONFIG_ZEPHCORE_UI_DISPLAY - /* If display is off, wake it and consume the event. The action this - * press resolves to arrives later from the longpress / multi-tap filter; - * display_woken_pending makes the switch below swallow it. - * - * Only arm that flag when this press really does resolve into a *later* - * action code, i.e. when the code we just consumed is a raw one. Boards - * that wire an action code straight to the GPIO (joystick lines on - * wio_tracker_l1 / gat562_30s in repeater builds) already consumed their - * action here — arming the flag would make the next, genuinely separate - * press get swallowed instead. */ + /* Display off: wake it and consume the event. Arm display_woken_pending only + * if the consumed code is a raw one, whose action code arrives later. */ if (!mc_display_is_on()) { mc_display_on(); display_woken_pending = !is_ui_action_code(code); @@ -697,20 +663,9 @@ static void ui_input_cb(struct input_event *evt, void *user_data) } #endif - /* Map input key codes to UI actions. - * - * Two-code boards (RAK4631 / WisMesh Pocket / Heltec V3–V4.3 / T-Echo / - * ThinkNode M1 / T-Impulse): 1 tap KEY_1, 2 taps KEY_LEFT; long press - * KEY_ENTER (page enter). Five-code boards emit KEY_B/D/C/E as well, and - * KEY_POWER or KEY_F long → deep sleep. The gesture behind each code is - * the board's business (see the tap-codes note at the top of this file). - * KEY_RIGHT/LEFT/ENTER/UP/DOWN: joystick (Wio Tracker) - * - * NOTE: KEY_A (raw short-press from longpress filter) is NOT handled - * here. It feeds into the multi-tap filter which emits the tap codes. - * Since INPUT_CALLBACK_DEFINE(NULL) sees events from all devices, - * the raw KEY_A events fall through to default: break. - */ + /* Map input key codes to UI actions. Which gesture produces which code is + * the board's business (tap-codes note at the top of this file). Raw KEY_A + * is not handled here: it feeds the multi-tap filter. */ switch (code) { /* ===== Multi-tap outputs ===== */ case INPUT_KEY_1: diff --git a/zephcore/helpers/ui-joystick/joystick_defs.h b/zephcore/helpers/ui-joystick/joystick_defs.h index 1275f28..f11497d 100644 --- a/zephcore/helpers/ui-joystick/joystick_defs.h +++ b/zephcore/helpers/ui-joystick/joystick_defs.h @@ -9,27 +9,12 @@ #pragma once /* ===== Key codes ===== - * - * These are the UI's INTERNAL codes, carried as a single char through - * JoystickUITask::enqueueKey(). They are deliberately NOT the Zephyr - * INPUT_KEY_* codes — joystick_ui_input_cb() translates between the two — and - * they are never persisted or transmitted, so they can be renumbered freely. - * - * Key-space contract (keep this true): + * The UI's internal codes, one char through JoystickUITask::enqueueKey(); + * not the Zephyr INPUT_KEY_* codes, never persisted or transmitted. * 0x01-0x1F control keys (navigation, enter, cancel) - * 0x20-0x7E RESERVED for printable characters typed on a keyboard + * 0x20-0x7E reserved for printable characters typed on a keyboard * 0xF1-0xFF long-press and global action keys - * - * The printable range is reserved so a keyboard board (ThinkNode M9's STC8H - * matrix MCU at I2C 0x6C) can enqueue characters directly without colliding - * with a control code. The multi-tap action keys below used to sit at - * 0x42-0x45 — i.e. on 'B'..'E' — which would have made typing "BCDE" fire a - * flood advert, toggle GPS, mute the buzzer and kill the LED. - * - * Note KEY_ENTER and KEY_CANCEL are already ASCII CR and ESC, so a keyboard - * that reports plain ASCII (the convention for this class of I2C matrix MCU) - * produces correct enter/cancel with no translation at all. - */ + * KEY_ENTER and KEY_CANCEL are ASCII CR and ESC. */ #define KEY_ENTER 0x0D /* center/OK button click */ #define KEY_LEFT 0x01 /* joystick left */ #define KEY_RIGHT 0x02 /* joystick right */ diff --git a/zephcore/helpers/ui/display.c b/zephcore/helpers/ui/display.c index 8dc5466..84eefe6 100644 --- a/zephcore/helpers/ui/display.c +++ b/zephcore/helpers/ui/display.c @@ -384,13 +384,8 @@ static void auto_off_handler(struct k_work *work) } /* ========== Early blanking ========== - * OLED controllers (SSD1306, SH1106) turn the display ON during driver init, - * showing stale VRAM from before reset. Our mc_display_init() runs much later - * (after BLE, LoRa, etc.), so there's a visible garbage flash. - * - * Fix: SYS_INIT hook runs right after the driver, sending "Display OFF" before - * main() starts. This is harmless for non-OLED displays (blanking is a no-op - * or already blanked). */ + * OLED controllers switch the panel on during driver init and show stale + * VRAM. Send Display OFF right after the driver, before main(). */ static int display_early_blank(void) { const struct device *dev = NULL; @@ -478,13 +473,8 @@ int mc_display_init(void) return ret; } - /* Font selection. - * Default: smallest height for best text density — our custom 6x8 - * Latin-1 font typically wins on OLEDs. - * LARGE_FONT: smallest font whose height is >= 16 — picks Zephyr's - * built-in 10x16 (cfb_fonts.c) for larger e-paper panels where 6x8 - * is too small to read. Falls back to smallest-overall if no tall - * font is compiled in. */ + /* Font selection: the smallest height by default; with LARGE_FONT the + * smallest font at least 16 high, falling back to the smallest overall. */ const bool want_large = IS_ENABLED(CONFIG_ZEPHCORE_DISPLAY_LARGE_FONT); int num_fonts = cfb_get_numof_fonts(disp_dev); @@ -856,24 +846,9 @@ void mc_display_finalize(void) const int full_interval = CONFIG_ZEPHCORE_DISPLAY_EPD_FULL_REFRESH_INTERVAL; if (is_epd && full_interval > 0 && ++epd_partial_count >= (uint32_t)full_interval) { - /* Periodic full refresh to clear accumulated ghosting. - * - * Replicate the clean post-boot sequence: full-refresh the panel - * to WHITE, then redraw the current page as a partial. blanking_on - * selects the FULL profile; ssd16xx_fill_ram_white clears BOTH RAM - * banks to white; blanking_off runs the full-refresh waveform, - * leaving the panel (and both RAM banks) white. The trailing - * cfb_framebuffer_finalize then redraws the current frame — still - * held in the CFB buffer — as a partial against the all-white - * reference: every content pixel is actively driven and there is - * nothing stale to erase. - * - * Going through white is required. A partial after a full refresh - * of *content* leaves unchanged regions (e.g. the top bar) on the - * neutral waveform → they fade; and forcing the old-frame reference - * white while the panel still shows content fails to erase pixels - * that should clear → letters overlap. The brief white flash is - * the intended clean between frames. */ + /* Periodic full refresh to clear ghosting: full-refresh the panel to white, + * then redraw the current frame as a partial. Going through white is + * required. */ display_blanking_on(disp_dev); #if ZEPHCORE_DISPLAY_HAS_SSD16XX ssd16xx_fill_ram_white(disp_dev); diff --git a/zephcore/helpers/ui/display.h b/zephcore/helpers/ui/display.h index e7c724f..9e730e8 100644 --- a/zephcore/helpers/ui/display.h +++ b/zephcore/helpers/ui/display.h @@ -85,22 +85,9 @@ bool mc_display_is_on(void); */ bool mc_display_is_epd(void); -/* Color overlay support is compiled only when the devicetree points at a raw - * RGB565 TFT (the runtime probe still verifies pixel format and readiness). - * Boards without one get constant-false / mono-fallback inlines so every - * color code path — including the ~3.8 KB overlay op queue in display.c — - * is dropped at compile time. - * - * Two ways to name the panel, in priority order: - * - * 1. chosen { zephcore,color-tft = <&some_panel>; } - * 2. the `tft` nodelabel on the panel node - * - * (1) exists for boards whose panel node lives in an upstream Zephyr DTS: - * an overlay cannot add a nodelabel to an existing node, but it can always - * set a chosen property. (2) is kept because every in-tree board that had - * color before this indirection names its panel `tft:` — they need no edit. - */ +/* Color overlay support is compiled only when the devicetree names a raw + * RGB565 TFT, by the `zephcore,color-tft` chosen node (for a panel in an + * upstream DTS) or the `tft` nodelabel. Other boards get mono inlines. */ #if DT_HAS_CHOSEN(zephcore_color_tft) #define MC_DISPLAY_COLOR_NODE DT_CHOSEN(zephcore_color_tft) #elif DT_NODE_EXISTS(DT_NODELABEL(tft)) @@ -128,40 +115,10 @@ bool mc_display_is_epd(void); #endif /* - * 180-degree rotation support, decided at compile time from the panel. - * - * Deliberately an allow-list of the two families where the rotation is a - * hardware remap the driver actually implements: display_ssd1306.c flips - * SEGMENT_MAP + COM_OUTPUT_SCAN (two bytes on the wire, framebuffer - * untouched, no per-frame cost), and that driver backs both solomon,ssd1306 - * and sinowealth,sh1106. - * - * Other panel families are excluded on purpose rather than probed: - * - st7735r/st7789v (our mono-tft boards) return -ENOTSUP upstream for - * anything but NORMAL, so a probe would just fail; - * - ssd16xx e-paper *accepts* ROTATED_180 but implements it by flipping - * the RAM entry mode only, which reverses byte order without reversing - * bit order inside each byte — the 8 pixels a byte spans stay in their - * original order. It would report success and render wrong, which is - * worse than reporting unsupported. - * - * The geometry test on top of the family check matters as much as the family - * check itself. Both remaps reverse the controller's *entire* addressable - * range, not the part a given panel happens to use, so the flip only lands - * back on the glass when the visible window is centred in that range: - * - * - Vertically, that means the panel uses the full multiplex height from - * page 0. Boards that window a small panel into a larger controller do - * not: lilygo_timpulse_plus is a 64x32 glass on a 128x64 SSD1306 driven - * at page-offset 4 with multiplex-ratio 63, so its content sits on COM - * 32..63. Reversing the COM scan moves it to COM 31..0 — off the bonded - * region entirely, i.e. a blank screen. `page-offset == 0` and - * `height == multiplex-ratio + 1` is exactly the "uses the whole - * controller" condition, and it excludes that board. - * - Horizontally the surviving boards are already centred: the SSD1306 - * ones are a full 128 columns at segment-offset 0, and the SH1106 ones - * are 128 columns at segment-offset 2 in 132 columns of RAM — the - * standard 2-either-side layout these modules ship with. + * 180-degree rotation support, decided at compile time: an SSD1306/SH1106 + * panel that uses the controller's whole height from page 0 (page-offset 0, + * height == multiplex-ratio + 1). Other families and windowed panels are + * excluded on purpose. */ #if DT_NODE_HAS_COMPAT(MC_DISPLAY_NODE, solomon_ssd1306) || \ DT_NODE_HAS_COMPAT(MC_DISPLAY_NODE, sinowealth_sh1106) diff --git a/zephcore/helpers/ui/doom_game.c b/zephcore/helpers/ui/doom_game.c index 3f01742..04a175d 100644 --- a/zephcore/helpers/ui/doom_game.c +++ b/zephcore/helpers/ui/doom_game.c @@ -84,12 +84,8 @@ LOG_MODULE_REGISTER(doom_game, CONFIG_ZEPHCORE_BOARD_LOG_LEVEL); #define BOSS_CAST_PERIOD_RAGE 45 /* 2.25 s, phase 2 */ #define BOSS_CAST_MIN_DIST (FP_ONE * 5 / 2) /* nearer than this it swings */ /* - * Frames the boss spends backing off after a melee swing. Without this it - * closes once and camps in the player's face forever: the cast cycle is held - * below BOSS_CAST_MIN_DIST, so the tell never fires again and the fight - * collapses into a melee grind with nothing to read. Disengaging restores the - * loop — close, swing, back out, cast, close again — and at phase-1 speed 50 - * frames buys back about 1.5 tiles, just past casting range. + * Frames the boss backs off after a melee swing, so the cast cycle can + * start again instead of a melee grind. */ #define BOSS_RETREAT 50 @@ -394,16 +390,9 @@ static uint32_t isqrt64(uint64_t v) return (uint32_t)r; } -/* Turn by sin_r (signed) and put the view back on the unit circle. - * - * The rotation used to be cos = 1 - ROT/8, sin = ROT: a matrix with scale - * ~0.993, plus truncation in every fp_mul, so each turn tick shrank dir and - * plane. Half a view vector after ~1.3 full turns, 1/16 after ~5: walls read - * as ever farther away (the level "stretches for kilometres", steps get - * short), and around ten turns 1/ray_dir overflowed int32 in the raycaster - * and a zero wall height divided by zero - a usage fault that rebooted the - * node. Now: the small-angle cosine, then dir renormalised to exactly unit - * length and plane rebuilt perpendicular to it at its 2/3 field of view. */ +/* Turn by sin_r (signed) and put the view back on the unit circle: the + * small-angle cosine, then dir renormalised to unit length and plane rebuilt + * perpendicular to it. Without the renormalisation the vectors shrink. */ static void rotate_view(struct doom_player *p, fixed_t sin_r) { fixed_t cos_r = FP_ONE - fp_mul(sin_r, sin_r) / 2; @@ -682,16 +671,9 @@ static inline void clear_px(int x, int y) } /* - * Render one 16×16 sprite at world position (wx, wy), scale_pct percent of a - * wall's height with its feet on the floor. Shared by the portal, the enemies - * and the fireballs. - * - * Sprites are solid with a one-pixel black halo, not distance-dithered like - * the walls. On a 1-bit panel a dithered sprite ORed over the dithered floor - * band at the horizon has nothing to contrast against: the L2 boss, which - * starts 8-12 tiles away, was a few dots at best and a grey smudge in the - * floor's own pattern at 4 tiles. The halo pass clears first, then the fill - * sets; both honour the zbuffer, so a nearer wall is never cut into. + * Render one 16x16 sprite at world position (wx, wy), scale_pct percent of a + * wall's height with its feet on the floor. Solid with a one-pixel black + * halo; both passes honour the zbuffer. */ static void render_one_sprite(const uint16_t *sprite, fixed_t wx, fixed_t wy, diff --git a/zephcore/helpers/ui/haptic.c b/zephcore/helpers/ui/haptic.c index fd47389..f9c5cc7 100644 --- a/zephcore/helpers/ui/haptic.c +++ b/zephcore/helpers/ui/haptic.c @@ -21,15 +21,9 @@ LOG_MODULE_REGISTER(zephcore_haptic, CONFIG_ZEPHCORE_SENSORS_LOG_LEVEL); #define HAPTIC_NODE DT_COMPAT_GET_ANY_STATUS_OKAY(ti_drv2605) #if DT_PROP(HAPTIC_NODE, zephyr_deferred_init) -/* Wake a deferred-init DRV2605 and run its init here, where the part's supply - * rail has been up since boot instead of microseconds. - * - * The enable pin is asserted *before* device_init() on purpose. Upstream's - * drv2605_init() spends its DRV2605_POWER_UP_DELAY_US before it configures the - * GPIOs, so EN rises and the very next thing it does is read the status - * register over I2C — no settling time at all between the two. Raising EN - * ourselves and waiting turns that race into a plain sequence; the driver's own - * gpio_pin_configure_dt() to OUTPUT_ACTIVE afterwards is then a no-op. */ +/* Wake a deferred-init DRV2605 and run its init here. The enable pin is + * asserted, with a wait, before device_init(): the driver reads the status + * register right after raising EN. */ static void haptic_deferred_init(const struct device *dev) { static const struct gpio_dt_spec en = diff --git a/zephcore/helpers/ui/ui_common.c b/zephcore/helpers/ui/ui_common.c index c97dbd2..ca7865d 100644 --- a/zephcore/helpers/ui/ui_common.c +++ b/zephcore/helpers/ui/ui_common.c @@ -29,13 +29,8 @@ LOG_MODULE_REGISTER(ui_led, CONFIG_ZEPHCORE_BOARD_LOG_LEVEL); /* ========== Input axis flip ========== - * - * Shared by both UI variants so an upside-down mount only has to be - * configured once. Written from the mesh/CLI thread, read from the input - * callback. A plain bool needs no atomic here: it is a single aligned byte, - * and the only race — a keypress landing in the same instant the setting is - * toggled — costs that one keypress its direction, which is what toggling an - * axis swap does anyway. */ + * Shared by both UI variants. Written from the mesh/CLI thread, read from the + * input callback; a plain bool is enough. */ static bool input_flipped; @@ -77,13 +72,8 @@ void ui_play_startup_chime(void) /* ========== LED Heartbeat ========== */ /* - * Uses led0 (or led1 fallback) as a heartbeat indicator. - * Pulse width extends to LED_ON_MSG_MS when there are unread messages, - * driven by ui_led_get_msg_count() which the button variant overrides. - * - * led1 is also claimed as a message indicator in non-repeater companion builds - * when both led0 and led1 are present. The heartbeat cycle turns led1 on - * only when msg count > 0, giving a visual unread-message reminder. + * Heartbeat on led0 (or led1). The pulse widens while there are unread + * messages; with both LEDs present, companions light led1 for unread. */ #if DT_NODE_HAS_PROP(DT_ALIAS(led0), gpios) @@ -125,14 +115,9 @@ static struct k_work_delayable s_led_off_work; __attribute__((weak)) uint16_t ui_led_get_msg_count(void) { return 0; } /* - * Does the heartbeat LED light on this pass? "unread" is not a separate blink - * — it is this same cycle widening its pulse — so the modes are expressed as - * two questions over one cycle: may it light at all right now, and how wide. - * - * The cycle keeps running in every mode including LEDS_HB_OFF. That is on - * purpose: on companions with two LEDs the unread indicator is lit from inside - * this work chain, so stopping the chain would take unread indication down with - * the heartbeat. An idle pass costs one work item every 4 s. + * Does the heartbeat LED light on this pass? The cycle keeps running in + * every mode, LEDS_HB_OFF included, because the unread indicator is lit from + * inside it. */ static bool hb_should_light(uint16_t msg_count) { diff --git a/zephcore/helpers/ui/ui_task.h b/zephcore/helpers/ui/ui_task.h index dbd6fc7..f84a315 100644 --- a/zephcore/helpers/ui/ui_task.h +++ b/zephcore/helpers/ui/ui_task.h @@ -257,19 +257,9 @@ void ui_notify_channel_msg(const char *channel_name, const char *text, void ui_notify_packet_sent(void); /* ===== Input axis flip ===== - * - * A case that mounts the board upside down (e.g. the Meshnology N37E kit) - * rotates the joystick along with the screen, so "up" on the stick walks the - * menu down. This flips the two axes back. - * - * Kept separate from the display rotation on purpose: the two are not always - * wanted together — a panel can be remounted alone, and boards whose display - * cannot rotate can still benefit from the axis swap. - * - * The state lives in ui_common.c so both UI variants (button and joystick) - * share one source of truth; each variant's input callback runs its raw - * event code through zephcore_input_map_code() before decoding it. - */ + * For an upside-down mount: flips the two joystick/D-pad axes back. Separate + * from the display rotation on purpose. State in ui_common.c; each variant's + * input callback maps its raw code through zephcore_input_map_code(). */ /** * Enable or disable the joystick/D-pad axis swap. diff --git a/zephcore/src/ContentionTracker.cpp b/zephcore/src/ContentionTracker.cpp index a3aaeeb..ac51619 100644 --- a/zephcore/src/ContentionTracker.cpp +++ b/zephcore/src/ContentionTracker.cpp @@ -152,15 +152,8 @@ void ContentionTracker::tick(uint32_t now_ms) if (_last_retransmit_ms != 0 && now_ms - _last_retransmit_ms > STALE_MS && _ema_x256 > 0) { if (_last_decay_ms == 0) { - /* Arm one full period in the PAST, so the loop below - * applies a step on this very call. The old code decayed - * immediately on the first tick that observed staleness; - * arming at now_ms instead would make the first check - * (0 < DECAY_PERIOD_MS) break without decaying, deferring - * onset by a whole period on every stale transition — and - * would let the reset branch below starve decay entirely - * for traffic that goes stale and un-stale repeatedly. - * Unsigned wraparound makes this exact even near zero. */ + /* Arm one full period in the past, so the loop below applies a step on + * this call. Unsigned wraparound keeps it exact near zero. */ _last_decay_ms = now_ms - DECAY_PERIOD_MS; } for (int n = 0; n < MAX_DECAY_CATCHUP; n++) { diff --git a/zephcore/src/main_companion.cpp b/zephcore/src/main_companion.cpp index ffd4980..d86b4a7 100644 --- a/zephcore/src/main_companion.cpp +++ b/zephcore/src/main_companion.cpp @@ -200,12 +200,9 @@ extern "C" void zephcore_persist_before_off(void) #endif /* ========== Companion transports ========== - * * As upstream's companion_radio/main.cpp: every transport this build has is a - * BaseSerialInterface in one MultiSerialInterface, and CompanionMesh sees only - * that (through CompanionSerial, which keeps a lossless reply the transports - * could not take). Every connected transport gets every frame; a command from - * any of them is served. */ + * BaseSerialInterface in one MultiSerialInterface. Every connected one gets + * every frame; a command from any of them is served. */ static MultiSerialInterface interface_manager; static CompanionSerial companion_serial(interface_manager); #if IS_ENABLED(CONFIG_BT) @@ -337,14 +334,8 @@ static void process_companion_rx(void) } /* The contact dump, as far as every connected transport has room - * (MESH_EVENT_CONTACT_ITER): BLE and TCP below their 2/3 high-water mark and - * not congested, USB with room for a whole frame. A transport's TX idle - * brings us back. - * - * Only while a client is connected: with none, nothing is busy and no write - * succeeds, so the dump would not advance and this loop would not end. The - * last client can leave while we are in here (its callbacks preempt this - * thread), before MESH_EVENT_LINK has cancelled the dump. */ + * (MESH_EVENT_CONTACT_ITER); a transport's TX idle brings us back. Only while a + * client is connected: with none no write succeeds and the loop would not end. */ static void run_contact_iteration(void) { #ifdef ZEPHCORE_LORA diff --git a/zephcore/src/server_main_common.cpp b/zephcore/src/server_main_common.cpp index c8de5bd..1ea9351 100644 --- a/zephcore/src/server_main_common.cpp +++ b/zephcore/src/server_main_common.cpp @@ -36,13 +36,8 @@ extern "C" void bt_ctlr_assert_handle(char *file, uint32_t line) #endif /* USB CDC ACM init + 1200-baud DFU + DTR callbacks (shared with companion). - * - * Gate on the CDC-ACM class driver, NOT on DT_HAS_COMPAT_STATUS_OKAY alone: a - * board overlay may expose a cdc_acm_uart DT node unconditionally (the shared - * esp32s3_usb_otg.dtsi does), so the node can be present while the class driver - * — and therefore zephcore_usbd_* and the device object — is not compiled - * (e.g. an ESP32-S3 server built without esp32s3_usb.conf). This mirrors the - * CMake condition that compiles ZephyrUSBCDC.cpp. */ + * Gated on the class driver, as the CMake condition that compiles + * ZephyrUSBCDC.cpp: the DT node can exist without it. */ #define ZEPHCORE_USB_STACK \ (IS_ENABLED(CONFIG_USB_CDC_ACM) || IS_ENABLED(CONFIG_USBD_CDC_ACM_CLASS)) @@ -67,16 +62,8 @@ extern "C" void bt_ctlr_assert_handle(char *file, uint32_t line) #include "display.h" #endif -/* Headless servers link the weak no-op ui_* stubs (ui_headless_stubs.c), so - * the periodic UI refresh in the maintenance pass is pure work for nothing on - * them. Guard the hot path on a real ui_* implementation being linked; the - * one-shot calls at init and in the CLI reply path stay unguarded, matching - * the rest of the file. - * - * This MUST mirror the CMake condition that selects the stubs, not the display - * devicetree node: ZEPHCORE_UI_DESIGN_BUTTON is enabled by BUTTONS *or* - * DISPLAY *or* BUZZER, so a board with buttons/buzzer and no panel still links - * the real UI and still needs these updates. */ +/* A real ui_* implementation is linked (not the headless stubs). Mirrors the + * CMake condition that selects the stubs, not the display devicetree node. */ #define ZEPHCORE_HAS_UI (IS_ENABLED(CONFIG_ZEPHCORE_UI_DESIGN_BUTTON) || \ IS_ENABLED(CONFIG_ZEPHCORE_UI_DESIGN_JOYSTICK)) @@ -103,17 +90,9 @@ static const struct gpio_dt_spec led1 = GPIO_DT_SPEC_GET(LED1_NODE, gpios); #define MESH_EVENT_WAKE BIT(MESH_EVENT_ROLE_BASE + 1) /* Off-main state set; run loop() promptly */ #define MESH_EVENT_ALL (MESH_EVENT_LORA_RX | MESH_EVENT_LORA_TX_DONE | MESH_EVENT_CLI_RX | MESH_EVENT_MAINTENANCE | MESH_EVENT_GPS_ACTION | MESH_EVENT_TX_DRAIN | MESH_EVENT_INIT_ADVERT | MESH_EVENT_WAKE) -/* Maintenance is deadline-driven, not periodic: after every pass the loop asks - * the mesh when its soonest pending deadline is (msUntilNextMaintenance) and - * arms a single one-shot wake for exactly that moment. An idle node with - * nothing scheduled therefore sleeps until its next real deadline instead of - * waking on a fixed cadence. - * - * MAINTENANCE_BACKSTOP_MS bounds that: it caps how long we will go without a - * pass even when everything reports idle, so a deadline that is missed or - * mis-reported degrades to the old behaviour instead of wedging. - * MAINTENANCE_MIN_MS floors it, so an item that is due-but-blocked (radio - * mid-packet, duty-cycle sleep window) re-arms shortly rather than spinning. */ +/* Maintenance is deadline-driven: after every pass a one-shot is armed for the + * mesh's soonest deadline. MAINTENANCE_BACKSTOP_MS caps the wait, + * MAINTENANCE_MIN_MS floors it. */ #define MAINTENANCE_BACKSTOP_MS CONFIG_ZEPHCORE_MAINTENANCE_BACKSTOP_MS #define MAINTENANCE_MIN_MS 50 @@ -256,16 +235,9 @@ static bool cli_rx_bytes(void) const uint8_t prev = cli_prev_byte; cli_prev_byte = byte; - /* Process command on \r OR \n. Accepting both is what makes a piped - * `echo "cmd" > /dev/ttyACM0` (LF only) work. The cost is that a CRLF - * terminal sends two terminator bytes per Enter, so collapse the pair - * here: skip a terminator that directly follows a *different* one. - * "\r\r" and "\n\n" (two genuine blank lines) still dispatch twice. - * Blank lines ARE dispatched -- `region load` commits on one, and - * handleCommand() no-ops a blank line otherwise. The previous - * de-duplicator keyed on an empty line buffer instead: it ate every - * blank line (stranding `region load` until a reboot) and still let - * the CRLF tail fall through and emit a stray newline of its own. */ + /* Process the command on \r or \n, collapsing a CRLF pair: skip a terminator + * that directly follows a different one. Blank lines are dispatched + * (`region load` commits on one). */ if (byte == '\r' || byte == '\n') { if ((prev == '\r' || prev == '\n') && byte != prev) { continue; /* tail of a CRLF/LFCR pair */ @@ -342,17 +314,9 @@ static void maintenance_timer_fn(struct k_timer *timer) k_event_post(&mesh_events, MESH_EVENT_MAINTENANCE); } -/* Ask the mesh for its soonest pending deadline and arm the one-shot for it. - * Called after every loop pass, whatever woke us: any event may have created - * or cleared a deadline (a queued advert, a CLI tempradio command, a CAD probe - * that just ran), so the schedule is recomputed from scratch each time rather - * than tracked incrementally. - * - * The backstop below is an unconditional CEILING on the wait, not a fallback - * used only when everything reports idle. That means it must stay well above - * the shortest legitimate recurring deadline (the noise-floor sampler, and the - * CAD probe) or it becomes the effective period and the deadline scheduling - * buys nothing — see ZEPHCORE_MAINTENANCE_BACKSTOP_MS. */ +/* Ask the mesh for its soonest pending deadline and arm the one-shot for it, + * after every loop pass. The backstop is a ceiling on the wait, so it must + * stay well above the shortest recurring deadline. */ static void arm_maintenance_wake(void) { uint32_t delay = MAINTENANCE_BACKSTOP_MS; @@ -576,25 +540,9 @@ static void server_event_loop(void) #endif #ifdef ZEPHCORE_LORA - /* Radio maintenance runs OPPORTUNISTICALLY, on every pass, whatever - * woke us — not only when its own timer fired. - * - * Every item inside is deadline-gated internally, so this is nearly - * free when nothing is due. The point is what it does to a busy - * node: a hilltop repeater is already waking constantly for packets, - * so maintenance rides along on those wakes, its deadlines advance, - * and arm_maintenance_wake() below keeps pushing the timer out — the - * maintenance timer then almost never fires and costs no wakes at - * all. Running it only on its own event (as this did originally) - * inverted that: the busiest nodes, which can least afford it, paid - * the full timer cadence on top of their packet wakes, and every - * blocked sample burned a retry against a channel that is busy for - * sustained reasons (real traffic in isReceiving(), the RSSI - * prefilter) rather than the transient duty-cycle sleep window the - * retries were sized for. - * - * Ordering matters: this sits AFTER packet processing so an inbound - * frame is handled before we spend SPI time on an RSSI sweep. */ + /* Radio maintenance runs on every pass, whatever woke us: each item is + * deadline-gated, and on a busy node it rides along on packet wakes. After + * packet processing, so an inbound frame is handled first. */ if (server_role) { server_role->mesh->maintenanceLoop(); } @@ -673,17 +621,8 @@ int server_main(const ServerRole &role) } #endif - /* First boot on a volume that is not this role's - a fresh chip, a - * companion, or a node that was running Arduino MeshCore, whose nRF52 - * filesystems overlap our lfs_partition - * (devdocs/HANDOVER_lfs_arduino_overlap.md). Erase everything so we - * start from a known state: Zephyr's automount only - * auto-formats the LittleFS volume when it fails to mount, and never - * touches storage_partition (BLE bonds NVS) or QSPI. - * - * Self-limiting, so it needs no "done" marker: the identity is generated - * and saved a few lines below, and loadPrefs() persists defaults on the - * same boot, so the next boot sees this role's data and skips this. */ + /* First boot on a volume that is not this role's: erase everything (LittleFS, + * bond NVS, QSPI). Self-limiting: this boot writes the role's data. */ if (!data_store.hasRoleData()) { LOG_WRN("Volume holds no data for this role - formatting before first boot"); if (!zephcore_fs_format_all(nullptr)) { diff --git a/zephcore/tools/crypto_bench/src/main.cpp b/zephcore/tools/crypto_bench/src/main.cpp index aa5fcfc..af64879 100644 --- a/zephcore/tools/crypto_bench/src/main.cpp +++ b/zephcore/tools/crypto_bench/src/main.cpp @@ -82,16 +82,9 @@ static inline void sink(const uint8_t *p, size_t n) /* ---------------------------------------------------------------- timing */ -/* Zephyr's portable cycle counter (DWT on Cortex-M, CCOUNT on Xtensa) — the - * same mechanism ZephyrRNG's two-clock beat uses. See - * memory/zephyrrng-entropy.md for the platform traps. - * - * MIN, not mean, is the headline figure: every perturbation (interrupt, - * cache miss, flash wait state) can only ADD time, so the minimum over N runs - * is the closest estimate of the true cost. Mean is printed alongside because - * a large min/mean gap is itself information — it says the operation is being - * interfered with, which matters on a node where it shares a CPU with the - * radio ISR. */ +/* Zephyr's portable cycle counter (DWT on Cortex-M, CCOUNT on Xtensa). MIN + * is the headline figure (perturbations only add time); the mean is printed + * beside it because a large gap shows interference. */ struct BenchResult { const char *name; uint64_t min_ns; @@ -388,15 +381,8 @@ int main(void) /* ---- composites: real call paths -------------------------------- */ print_header("Per-packet totals — the real RX/TX call paths"); - /* Dedup as implemented today. SimpleMeshTables::wasSeen() and - * markSeen() each call packet->calculatePacketHash() independently, - * and since the 2026-07-02 hasSeen split every caller invokes markSeen - * right after wasSeen returns false — so a NEW packet hashes twice. - * The linear memcmp scan those functions also do is not crypto and is - * not timed here; it is a 160-entry 8-byte compare, nowhere near the - * hash. Packet.cpp is compiled from the main tree, so the buffer - * assembly (the memcpy into a stack buffer) is included, as it is on a - * node. */ + /* Dedup as implemented: wasSeen() and markSeen() each hash the packet, so a + * new packet hashes twice. The table scan is not crypto and is not timed. */ /* Not `static`: a function-local static with a non-trivial constructor * emits __cxa_guard_acquire/release, which Zephyr's minimal C++ runtime * does not provide. ~260 bytes on the stack instead. */ diff --git a/zephcore/tools/lr1110_updater/src/lr11xx_hal_updater.c b/zephcore/tools/lr1110_updater/src/lr11xx_hal_updater.c index af46515..2c7eabb 100644 --- a/zephcore/tools/lr1110_updater/src/lr11xx_hal_updater.c +++ b/zephcore/tools/lr1110_updater/src/lr11xx_hal_updater.c @@ -26,16 +26,9 @@ LOG_MODULE_REGISTER(lr1110_hal, LOG_LEVEL_INF); /* SPI bus device */ static const struct device *spi_dev = DEVICE_DT_GET(DT_BUS(LR1110_NODE)); -/* Flashing SPI clock cap. - * - * In bootloader mode the chip runs off its internal RC oscillator (the XOSC - * needs either a crystal or a powered TCXO), so it has far less timing - * margin than during normal operation. Flashing pushes ~1000 back-to-back - * 256-byte writes, and the images are encrypted+signed: a SINGLE corrupted - * byte anywhere makes the whole image fail its integrity check at boot, - * with every write still reporting OK (the bootloader never reads back). - * Cap the operational 8-16 MHz down to a conservative rate — the entire - * 239 KB image still takes ~1 s of SPI time at 2 MHz. */ +/* Flashing SPI clock cap. In bootloader mode the chip runs on its RC + * oscillator with less timing margin, and one corrupted byte fails the whole + * signed image with every write reporting OK. */ #define UPDATER_SPI_MAX_HZ 2000000 /* SPI config — manual CS (we toggle NSS via GPIO) */ @@ -102,22 +95,9 @@ static int wait_on_busy(uint32_t timeout_ms) } /* - * Wait for a command the chip has just been given to actually COMPLETE. - * - * The chip does not raise BUSY the instant NSS deasserts — it needs a few - * microseconds. Polling only for "BUSY is low" therefore has a race: on a - * fast host (ESP32-S3 at 240 MHz drives GPIO in nanoseconds) the poll can - * observe the *stale* pre-command LOW and conclude the command is already - * finished. The next transaction then starts clocking while the chip is - * still writing flash, and because WriteFlashEncrypted is fire-and-forget - * — no read-back, no status check — the resulting corruption is silent. - * One bad chunk anywhere invalidates the whole signed image, so the odds - * of a clean flash fall off a cliff as the image grows: a 19 KB loader is - * 77 transactions, a 239 KB firmware is 959. - * - * So: first watch for the rising edge (bounded — a command that finishes - * faster than we can look is fine and simply never appears busy), then - * wait for the fall. + * Wait for a command the chip has just been given to complete: first watch + * for BUSY rising (bounded: a fast command may never show busy), then wait + * for it to fall. Polling for low alone can read the stale pre-command level. */ /* ── Per-command instrumentation ─────────────────────────── * @@ -230,15 +210,9 @@ int lr1110_updater_reset_to_bootloader(void) { printk("Resetting LR1110 into bootloader (BUSY held LOW)...\n"); - /* Semtech lr1110_updater_tool pattern: - * 1. Drive BUSY LOW as output during reset - * 2. Pulse RESET - * 3. Wait 500ms - * 4. Release BUSY back to input - * 5. Wait 100ms + BUSY low - * - * When BUSY is held LOW by the host during reset, the LR1110 - * enters bootloader mode instead of executing flash firmware. */ + /* Semtech lr1110_updater_tool pattern: hold BUSY low as an output across + * the RESET pulse, which makes the LR1110 enter its bootloader; wait 500 ms, + * release BUSY, wait 100 ms and for BUSY low. */ /* Drive BUSY to physical LOW (pin_busy has GPIO_ACTIVE_HIGH, * so we use raw GPIO to be explicit about physical level) */ @@ -266,25 +240,10 @@ int lr1110_updater_reset_to_bootloader(void) return ret; } -/* ── SD card presence probe ─────────────────────────────────── - * - * On boards where the SD slot shares the radio's SPI bus, an inserted card - * breaks LR1110 flashing: every write still reports OK and the programmed - * image fails its integrity check at boot, which is indistinguishable from a - * dozen other faults and cost days to track down. There is no card-detect pin - * wired on the M9, so presence is established over the bus. - * - * Standard SPI-mode detection: >=74 dummy clocks with CS high to bring the - * card up in SPI mode, then CMD0 (GO_IDLE_STATE). A present card answers R1 - * with the MSB clear (0x01 = idle). An empty slot leaves MISO pulled high, so - * every byte reads 0xFF and nothing else. False "absent" is possible if a card - * ignores CMD0 — no worse than not probing; false "present" essentially cannot - * happen, since 0xFF is all an empty slot can produce. - * - * Runs at 400 kHz (SD init is specified at 100-400 kHz; the radio path stays - * at its own clock) and is safe on the shared bus: the LR1110's NSS is parked - * inactive by hal_init, and the TFT is held in reset and is write-only. - */ +/* ── SD card presence probe ── + * An inserted card on the radio's SPI bus silently corrupts LR1110 flashing, + * and the M9 has no card-detect pin. SPI-mode detection at 400 kHz: 74+ dummy + * clocks with CS high, then CMD0; a card answers R1, an empty slot reads 0xFF. */ #if defined(CONFIG_BOARD_THINKNODE_M9) /* No DT node exists for the slot — the base board DTS only parks its CS with * a gpio-hog. GPIO48 = gpio1 pin 16. */ @@ -385,21 +344,8 @@ lr11xx_hal_status_t lr11xx_hal_write(const void *context, const uint8_t *command return LR11XX_HAL_STATUS_ERROR; } - /* Send the command and its payload as ONE contiguous buffer. - * - * Passing them as two spi_bufs makes Zephyr's ESP32 SPI driver walk - * the set buffer-by-buffer (spi_context_max_continuous_chunk() never - * spans a buffer boundary), so a 6-byte command and a 256-byte payload - * become separate hardware transactions — and each is further split at - * SOC_SPI_MAXIMUM_BUFFER_SIZE (64 bytes on the S3, no DMA). - * - * That matters here because WriteFlashEncrypted is the ONLY command - * this tool issues with a payload: every command known to work - * (GetVersion, GetStatus, EraseFlash, the EUI reads) is a single - * sub-64-byte frame. Keeping the frame contiguous — together with a - * flash chunk size chosen so command+payload stays under 64 bytes — - * makes the write path look exactly like the paths already proven - * good on this hardware. */ + /* Send the command and its payload as one contiguous buffer: as two + * spi_bufs the ESP32 SPI driver makes them separate hardware transactions. */ static uint8_t txbuf[LR11XX_HAL_MAX_FRAME]; if ((size_t)command_length + (size_t)data_length > sizeof(txbuf)) { diff --git a/zephcore/tools/lr1110_updater/src/main.c b/zephcore/tools/lr1110_updater/src/main.c index 09c2c07..7d3e496 100644 --- a/zephcore/tools/lr1110_updater/src/main.c +++ b/zephcore/tools/lr1110_updater/src/main.c @@ -39,15 +39,8 @@ #include "lr1110_bootloader.h" #include "lr1110_bl_updater.h" /* - * Which transceiver image to flash. 0x0402 is the current release (and the - * only one Semtech's own tool will attempt on bootloader 0x1001). - * - * 0x0401 is selectable to TEST whether the downgrade block is enforced by - * the CHIP or merely by the host tool: the compatibility table that pairs - * 0x0401 with bootloader 0x6500 lives in Semtech's reference *application* - * (lr11xx_update_utils.c), not in the silicon. Since 0x0402 is a CVE fix, - * anti-rollback in the new bootloader is plausible — but unverified, and a - * chip that runs 0x0401 is fully usable by ZephCore (T1000-E ships it). + * Which transceiver image to flash. 0x0402 is the current release; 0x0401 is + * selectable to test whether the chip itself blocks a downgrade. * * west build ... -- -DUPDATER_TARGET_FW=0x0401 */ @@ -57,19 +50,8 @@ /* * Perform the 0x6500 -> 0x1001 chip-bootloader update when firmware 0x0402 is - * the target (0x0402 will not run on the old bootloader). - * - * Verified end-to-end on a T1000-E 2026-07-21: Stage A then Stage B, chip - * afterwards reports TYPE=0x01 FW=0x0402 and runs from flash. An earlier - * belief that this update bricks radios came from a sample where every - * observed 0x1001 chip was on one board (ThinkNode M9) — that board has a - * separate, still-open problem, so a board fault was being read as a - * bootloader fault. - * - * It IS one-way: Semtech ships loaders in the forward direction only, and the - * new bootloader lives inside the encrypted loader payload, so there is - * nothing to flash back. Set this to 0 to flash firmware only and leave the - * chip bootloader untouched. + * the target (0x0402 does not run on the old bootloader). One-way: there is + * no loader back. Set to 0 to flash firmware only. */ #ifndef UPDATER_ALLOW_BOOTLOADER_UPDATE #define UPDATER_ALLOW_BOOTLOADER_UPDATE 1 @@ -124,32 +106,16 @@ static void led_off(void) {} static void led_toggle(void) {} #endif -/* Idle gap inserted after every flash page program. - * - * Diagnostic for a marginal supply. A page program pulls current for ~3.6 ms - * and we issue 959 of them back to back, so a rail that cannot sustain that - * burst will sag — and a sagging rail corrupts what gets programmed while the - * write still reports OK (the bootloader never reads back). Spacing the pages - * lets the supply recover between them. 10 ms stretches a full image write - * from ~6.6 s to ~16 s, which costs nothing. - * west build ... -- -DUPDATER_CHUNK_DELAY_MS=10 - */ +/* Idle gap after every flash page program: a diagnostic for a marginal + * supply. + * west build ... -- -DUPDATER_CHUNK_DELAY_MS=10 */ #ifndef UPDATER_CHUNK_DELAY_MS #define UPDATER_CHUNK_DELAY_MS 0 #endif -/* Refuse to flash with an SD card in the slot. - * - * On the M9 the slot shares SPI2 with the radio, and a card present during - * flashing produces a silently corrupt image: every write reports OK, the - * chip spends real time programming, and the result fails its integrity check - * at boot with no error anywhere to point at. Field-confirmed 2026-07-21 — - * removing the card was what finally made that board flash. - * - * Gated rather than warned because the asymmetry is stark: the cost of the - * gate is ejecting a card, the cost of missing it is days of debugging. - * west build ... -- -DUPDATER_IGNORE_SDCARD=1 - */ +/* Refuse to flash with an SD card in the slot (M9: the slot shares SPI2 with + * the radio and a card corrupts the image silently). + * west build ... -- -DUPDATER_IGNORE_SDCARD=1 */ #ifndef UPDATER_IGNORE_SDCARD #define UPDATER_IGNORE_SDCARD 0 #endif @@ -235,27 +201,11 @@ static uint8_t tcxo_code_from_mv(uint16_t mv) return LR1110_TCXO_CTRL_1_6V; } -/* Power the TCXO and switch the chip onto it before touching flash. - * - * On a crystal board the chip can start its 32 MHz XOSC by itself. On a - * TCXO board the oscillator is powered from DIO3, which stays OFF until - * SetTcxoMode is issued — so the chip would otherwise run the ENTIRE - * update on its internal RC oscillator, which is both slower and far less - * accurate. Flash program/erase pulse timing and the charge-pump sequencing - * derive from that clock, so a marginal clock can produce writes that - * report OK but do not survive the image integrity check at boot. - * - * Not in Semtech's update documentation (their reference hardware does not - * need it), but field-reported to make LR1110 flashing succeed on boards - * where it otherwise fails. Best-effort: a chip that rejects the command - * simply stays on RC, exactly as before. */ +/* Power the TCXO and switch the chip onto it before touching flash, so a + * TCXO board does not run the update on its RC oscillator. Best-effort. */ /* - * Off by default. Neither Semtech's SWTL001 nor RadioLib touches the TCXO - * while flashing, and SetTcxoMode (0x0117) / Calibrate (0x010F) are *system* - * opcodes that the bootloader command set does not include — so issuing them - * here is unverified behaviour on a bootloader we need to keep in a - * well-defined state. The field report that TCXO helps was not reproducible - * on this board (the failure predates and survives it). Set to 1 to re-test. + * Off by default: SetTcxoMode and Calibrate are not in the bootloader's + * command set. Set to 1 to re-test. */ #define UPDATER_ENABLE_TCXO 0 @@ -409,15 +359,9 @@ static void erase_and_flash_image(void *ctx, const char *what, const int64_t write_start = k_uptime_get(); - /* MUST be 64 words (256 bytes = one flash page). A 14-word write is - * rejected outright with PERR at chunk 0 — the chip validates the byte - * count and only accepts whole pages (a short final chunk is fine). - * - * NOTE: that PERR result does NOT clear the SPI path, though it was - * once read that way. A 14-word write is a 62-byte frame — one hardware - * transaction on the ESP32-S3 — so it says nothing about whether a - * 262-byte frame survives being split across five. Matches Semtech's - * LR11XX_FLASH_DATA_MAX_LENGTH_UINT32 = 64 regardless. */ + /* Must be 64 words (256 bytes = one flash page): the chip accepts whole + * pages only (a short final chunk is fine). Semtech's + * LR11XX_FLASH_DATA_MAX_LENGTH_UINT32. */ const uint32_t chunk_size = 64; uint32_t num_chunks = (total + chunk_size - 1) / chunk_size; uint32_t progress_step = num_chunks / 10; /* Print every 10% */ @@ -508,28 +452,9 @@ static void loader_rewrite_and_verify(void *ctx) lr1110_bl_updater_report_t report = { 0 }; lr1110_status_t rc; - /* No configure_tcxo() here, deliberately. The 0x8100 rewrite below is a - * flash write and the chip is back on its RC oscillator after the reboot - * into the loader, so this looks like a hole in the TCXO workaround that - * erase_and_flash_image() applies — it is not: - * - * - SWTL001 configures no TCXO at ANY point of the update (its - * lr11xx_update_firmware() is just erase + write; the driver's - * set_tcxo_mode is never called by the update application). Our - * configure_tcxo() is a ZephCore-only field workaround with no - * counterpart in the reference flow. - * - The loader image's documented command set is only 0x8100/01/02 plus - * GetVersion/GetStatus. It exposes no system commands, so SetTcxoMode - * (0x0117) is undocumented against a RUNNING loader — sending it is a - * guess, not a fix. - * - The failure modes are not symmetric. A bad transceiver flash leaves - * the chip falling back to the bootloader, which is retryable. A bad - * bootloader rewrite may not be recoverable at all. That asymmetry is - * what decides it: do not fire an undocumented opcode at the chip in - * the seconds before the one irreversible operation in this tool. - * - * If M9 bring-up ever shows the rewrite failing on a TCXO board, revisit - * with the GetStatus output below as evidence — do not add it blind. */ + /* No configure_tcxo() here, deliberately: SetTcxoMode is undocumented + * against a running loader, and this is the one irreversible operation in + * the tool. */ /* Ask the loader to rewrite the bootloader — BUSY is held for the * duration; the HAL grants opcode 0x8100 the long timeout. */ @@ -761,16 +686,9 @@ int main(void) fatal_error("Unknown chip type — cannot proceed"); } - /* Enter the bootloader the way RadioLib does: the SOFTWARE reboot - * command with stay_in_bootloader=true. - * - * We previously used the BUSY-held hardware reset (what SWTL001 - * does). Both are documented, but RadioLib is the reference that - * demonstrably flashes this chip family from an ESP32 host, and the - * entry method is the last structural difference left between its - * flow and ours — opcodes, offsets, byte order and 64-word pages are - * all now verified identical. The BUSY-held reset stays as the - * fallback for a chip that will not answer commands at all. */ + /* Enter the bootloader as RadioLib does: the software reboot command with + * stay_in_bootloader=true. The BUSY-held hardware reset stays as the + * fallback for a chip that does not answer. */ printk("[4/8] Entering bootloader (software reboot, stay=true)...\n"); lr1110_bootloader_reboot(ctx, true); k_msleep(500); @@ -863,19 +781,9 @@ int main(void) erase_and_flash_image(ctx, "transceiver firmware", lr11xx_firmware_image, LR11XX_FIRMWARE_IMAGE_SIZE); - /* Boot the freshly written firmware with the bootloader's own reboot - * command (0x8005, stay=false) — and then DO NOT TOUCH NRESET. - * - * The LR1110 samples BUSY as NRESET is released and enters the - * bootloader whenever it reads LOW. Nothing holds that line high while - * the chip is in reset, so a hardware reset here lands back in the - * bootloader every time — which is precisely what the old "re-reset for - * a clean state" did, making this step report "firmware not running" - * no matter how good the flash was. Stage A reboots the very same way - * *without* a trailing hardware reset, and its image always came up - * running: that asymmetry is what exposed this. - * - * Firmware boot takes ~273 ms (datasheet); 500 ms is comfortable. */ + /* Boot the new firmware with the bootloader's reboot command (0x8005, + * stay=false) and do not touch NRESET afterwards: the chip samples BUSY as + * NRESET is released and would re-enter the bootloader. Boot takes ~273 ms. */ printk(" Rebooting LR1110 into new firmware...\n"); lr1110_bootloader_reboot(ctx, false); k_msleep(500); diff --git a/zephcore/tools/rng_selftest/src/main.cpp b/zephcore/tools/rng_selftest/src/main.cpp index a3b1c1c..c851ab8 100644 --- a/zephcore/tools/rng_selftest/src/main.cpp +++ b/zephcore/tools/rng_selftest/src/main.cpp @@ -38,14 +38,8 @@ #include #if defined(CONFIG_SOC_FAMILY_ESPRESSIF_ESP32) -/* RTC-slow-clock beat diagnostic. ESP32's k_cycle_get_32() is CCOUNT, same - * clock domain as the CPU, so CPU-jitter sampling measures a deterministic - * loop (proven on hardware: maxrep in the thousands, ~0 bits). The RTC slow - * clock is a SEPARATE oscillator (internal ~136 kHz RC, or a 32 kHz crystal) - * that drifts independently of the main-crystal->PLL->240 MHz path — so the - * number of CPU cycles that elapse across a fixed RTC interval fluctuates, and - * that fluctuation is real physical entropy. This is what nRF gets for free - * (its system timer IS the 32 kHz RTC). This diagnostic MEASURES that beat; it +/* RTC-slow-clock beat diagnostic (ESP32): measures the CPU cycles elapsed + * across a fixed interval of the RTC slow clock, a separate oscillator. It * does not touch identity generation. */ #include #define HAVE_RTC_BEAT 1