From 9ba23afbe25258a9e414b40043e7acef3a66f67f Mon Sep 17 00:00:00 2001 From: liquidraver <504870+liquidraver@users.noreply.github.com> Date: Sat, 22 Aug 2026 10:50:02 +0200 Subject: [PATCH] vcontact improvements --- releasenotes/RELEASE_NOTES_1.17.2-zephcore.md | 212 +++++++++--------- zephcore/adapters/ble/ZephyrBLE.cpp | 12 + zephcore/adapters/ble/ZephyrBLE.h | 6 + zephcore/adapters/usb/ZephyrCompanionUSB.cpp | 13 ++ zephcore/adapters/usb/ZephyrCompanionUSB.h | 4 + zephcore/app/CompanionMesh.cpp | 59 ++++- zephcore/app/CompanionMesh.h | 35 +++ zephcore/helpers/CommonCLI.cpp | 31 ++- zephcore/helpers/CommonCLI.h | 11 +- zephcore/src/main_companion.cpp | 16 ++ 10 files changed, 286 insertions(+), 113 deletions(-) diff --git a/releasenotes/RELEASE_NOTES_1.17.2-zephcore.md b/releasenotes/RELEASE_NOTES_1.17.2-zephcore.md index 74128b8..2661d62 100644 --- a/releasenotes/RELEASE_NOTES_1.17.2-zephcore.md +++ b/releasenotes/RELEASE_NOTES_1.17.2-zephcore.md @@ -1,139 +1,143 @@ # ZephCore 1.17.2-zephcore -A **minor release.** Six changes, two of which only affect specific hardware, plus a hardening pass on -how the node's identity is stored and a fix for repeater guest logins that 1.17.1 broke. +A small release. Most of it is fixes, and a few of them only matter on specific boards. > [!NOTE] -> **Most people can skip this.** Two groups should not. **XIAO nRF52840 and Ikoka Nano 30 dBm** owners — -> those boards get twice the charge current and a battery-sense pin that no longer sits at the nRF52840's -> absolute maximum. And **anyone running a repeater**, where 1.17.1 stopped accepting guest logins that -> use no password. Everyone else: safe to skip and pick these up with the next real release. +> **Worth updating if you have:** a **XIAO nRF52840** or **Ikoka Nano 30 dBm** (charges twice as fast now), +> a **T1000-E** (temperature and light sensors work again), or **any repeater** (1.17.1 broke guest logins +> without a password). Everyone else can wait for the next release. > -> Straight upgrade from 1.17.1 either way — bonds and data survive, no re-pairing. +> Upgrading from 1.17.1 is straightforward — your contacts, settings and phone pairing all survive. --- -## XIAO nRF52840 / Ikoka Nano 30 dBm: 100 mA charging, and a pin fix +## Commands to your own node get their tick straight away -The BQ25100 on the XIAO module selects its charge current from the HICHG pin, and left floating — the -reset default — it charges at **50 mA**. That pin is now driven, so these boards charge at **100 mA**, -roughly halving the time to fill a pack. Matches Arduino MeshCore's `variants/xiao_nrf52` and -`variants/ikoka_nano_nrf`. +When you send a command to your node's own chat contact (the one named `v` plus your node name), the app +now marks it delivered immediately. -The battery ADC divider enable (P0.14) is now held low permanently instead of toggled per reading. -Driving it high puts about **3.6 V on the AIN7 node with a 4.2 V cell** — the nRF52840 GPIO absolute -maximum, with no margin, and over it on a warm pack. Holding it low parks the node near 1.4 V and costs -about 2.8 µA, which is nothing beside BLE and LoRa receive. +Before, the tick took a few seconds to appear, and while the app was waiting it quietly sent the command +a second time. Mostly harmless — but `reboot` ran twice, so your node rebooted again right after coming +back up. + +## XIAO nRF52840 and Ikoka Nano 30 dBm charge twice as fast + +These boards were charging at 50 mA. They now charge at **100 mA**, so a battery fills in roughly half +the time. + +A battery-measurement pin was also putting close to the maximum voltage the chip is rated for onto one of +its inputs whenever it took a reading. It no longer does. Battery readings are unchanged. > [!WARNING] -> **If your XIAO sits on a carrier board with its own charger** — a TP4056 module, for example — delete -> the `hichg` GPIO hog from the board DTS before flashing. Two chargers must not drive the same pack. +> **Only if your XIAO sits on a carrier board with its own charger** (a TP4056 module, for example): +> remove the `hichg` GPIO hog from the board DTS before flashing. Two chargers must never charge the same +> battery at once. -## Less receiver downtime when the channel is busy — LR2021 only +## T1000-E reports temperature and light again -On the MeshTracker X1, every transmit that listen-before-talk refused rebuilt the receiver **twice** -before retrying, because the driver restored RX and then the layer above it did too. One of those is now -a no-op, halving the time the node spends deaf per refused attempt. +The T1000-E has a real temperature sensor and a light sensor built in, and ZephCore was ignoring both. +The app only showed battery voltage and the chip's own internal temperature — which reads about **3 °C +too warm**, because it measures the chip, not the room. Both sensors are now read and sent along with +everything else. -No effect on any other board. +The readings match Seeed's original firmware, so your node reports the same numbers after reflashing as +it did before. -## A muted node now says so +> [!NOTE] +> Light is reported as **0–100, not in lux**. That is what the sensor and the original firmware produce. +> Normal indoor lighting sits in the bottom few percent and only direct light moves it much — that is the +> sensor's real range, not a stuck reading. -If listen-before-talk keeps refusing to transmit, that is reported after four seconds of continuous -refusal and flagged in the node's error status. Previously it was logged below debug level with no -counter, so a node that could hear the mesh but never got a word in looked completely idle. +The T1000-E also boots about **3.5 seconds faster** and has around **7 KB more free space**. It had been +looking for plug-in sensors that physically cannot be attached to it — it is a sealed tracker with no +connector — and each one took half a second to give up on. -All roles, all boards. - -## A node can no longer advertise a key it cannot sign for - -Your identity is a private key plus the public key derived from it. Those were stored side by side and -read back on trust, so if the two halves ever stopped matching — a stale file, a foreign write — the node -would advertise a public key it had no private key for and keep doing so through every reboot. Nothing -ever re-checked. In that state adverts verify nowhere, direct messages to you cannot be decrypted, and -the companion app is handed an exported key that contradicts what the node says about itself. - -One node in the field turned up like this after moving from Arduino MeshCore, which is how it was found. -The public half is redundant — it can always be recomputed from the private key — so it is now treated as -a checksum rather than as data: the node derives its public key on every boot and never reads the stored -copy. If the two disagree, that is now visible in the log instead of silent. - -Storage also moves to Arduino MeshCore's byte order (`pub || prv`), so all three roles finally agree on -one format and an identity file can be moved between the two projects. Every layout either project has -ever written is still readable, including the 64-byte prv-only files older repeaters wrote — nothing -needs migrating and no existing file is rewritten. - -If a file cannot be made sense of at all, the node tries each half as a private key and adopts one only -if exactly one is genuine. Failing that it keeps the bytes as `_main.id.bad` and generates a fresh -identity, so a broken key is never silently thrown away. - -Found and diagnosed by **Marcel Verdult** ([@marcelverdult](https://github.com/marcelverdult)) — -[#70](https://github.com/liquidraver/ZephCore/pull/70). His writeup is what made the byte-order -difference findable. - -All roles, all boards. - -> [!WARNING] -> **Downgrading below 1.17.2 after a factory reset or a fresh install.** Nodes carried over from 1.17.1 -> keep their existing identity file untouched and are unaffected. But an identity *generated* on 1.17.2 -> (or re-imported into it) uses the new byte order, and 1.17.1 and earlier will misread it — the node -> comes up with a broken identity. Nothing is lost: older firmware misreads the file but does not rewrite -> it, so flashing 1.17.2 again restores the identity exactly as it was. +Reported by **Kimotu** ([#71](https://github.com/liquidraver/ZephCore/issues/71)). T1000-E only; no other +board loses its sensor detection. ## Repeaters accept guest logins without a password again -1.17.1 changed an empty guest password to mean "guest access disabled" on both repeaters and room -servers. That was right for room servers and wrong for repeaters: Arduino MeshCore ships an empty guest -password by default and treats a blank password as a valid guest login, so a repeater with no guest -password set is supposed to be open to guests. On 1.17.1 those logins were refused. +1.17.1 made an empty guest password mean "guests not allowed". That is correct for room servers, but +wrong for repeaters — a repeater with no guest password is supposed to be open to guests, the same way +Arduino MeshCore does it. Those logins were being refused. They work again. -Repeaters now match Arduino again. Guests on a repeater cannot run CLI commands or read the access -list — they get login plus status and telemetry. Set `guest.password` if you want one required. +Guests on a repeater get login, status and telemetry. They cannot run commands or see the access list. +Set `guest.password` if you want one required. -**Room servers are deliberately unchanged.** An empty guest password still disables guest access there, -so a room is never accidentally left open; that hole is what the 1.17.1 change was for. Use -`allow.read.only on` to run an open room. +**Room servers are deliberately left as they are:** no guest password still means no guest access, so a +room is never accidentally left wide open. Use `allow.read.only on` if you want an open room. -`allow.read.only` no longer appears in the repeater CLI either. Only the room server ever consulted it, -so on a repeater it was a setting that silently did nothing. Stored configuration is untouched. +`allow.read.only` no longer shows up in the repeater's command list. Only room servers ever used it, so +on a repeater it was a setting that did nothing. Your saved configuration is untouched. -## Duty-cycled nodes spend more time with the radio off +## A node that cannot get a word in now tells you -Receive duty cycling derates its sleep budget to absorb drift in the radio's internal sleep clock. That -derating was 15 % and is now 5 %, which the radio can afford because that clock is recalibrated every -time the chip temperature moves 5 °C — so the drift it has to absorb is what accumulates inside a 5 °C -window, not since power-on. +If your node keeps finding the channel busy and can never transmit, it now says so after four seconds and +flags it as an error. Before, a node that could hear everyone but never got heard back looked completely +idle, with nothing in the log to explain it. -On a typical SF7 preset this moves radio-off time from about **31 % to about 35 %**. All three radio -families pick it up. +All roles, all boards. + +## Duty-cycled nodes keep the radio off a bit longer + +Nodes using receive duty cycling now spend more time with the radio asleep — on a typical SF7 setup, +radio-off time goes from about **31 % to about 35 %**. All radio types benefit. > [!NOTE] -> If a node starts dropping packets for no visible reason — random losses while signal strength looks -> perfectly healthy — raise `CONFIG_ZEPHCORE_LORA_DC_MARGIN_PCT` back toward 15 before investigating -> anything else. This kind of loss does not depend on signal strength, so RSSI will not show it. +> If a node suddenly starts losing packets for no obvious reason — random losses while the signal looks +> perfectly fine — set `CONFIG_ZEPHCORE_LORA_DC_MARGIN_PCT` back to 15 before looking at anything else. +> This kind of loss has nothing to do with signal strength, so signal readings will not show it. -## An LR1110 too old to join the mesh now says so +## Your node can no longer advertise an identity it cannot prove -An LR1110 running firmware older than 0x0303 cannot change the LoRa sync word, which leaves the node on -the public one — transmitting and receiving fine, but invisible to everyone else on the mesh. That is now -reported in the log instead of looking like a dead radio. +Your identity is a private key and a matching public key. If those two ever stopped matching — a damaged +file, a bad write — your node would keep announcing a public key it could not actually use, forever, and +nothing would notice. In that state nobody can verify your adverts and direct messages to you cannot be +opened. + +One node in the field turned up exactly like this after moving over from Arduino MeshCore. The public key +can always be recalculated from the private one, so the node now recalculates it at every boot instead of +trusting the stored copy. A mismatch shows up in the log instead of passing silently. + +Identity files are now stored in the same format Arduino MeshCore uses, so a file can be moved between +the two projects. Every older format is still readable — nothing needs converting and no existing file is +rewritten. If a file is too damaged to make sense of, the node keeps it as `_main.id.bad` and generates a +fresh identity rather than throwing the old one away. + +Found and diagnosed by **Marcel Verdult** ([@marcelverdult](https://github.com/marcelverdult)) — +[#70](https://github.com/liquidraver/ZephCore/pull/70). + +> [!WARNING] +> **Only if you go back to 1.17.1 or older after a factory reset or fresh install.** An identity created +> on 1.17.2 uses the new format, and older firmware will misread it — the node comes up with a broken +> identity. Nothing is lost: older firmware misreads the file but never rewrites it, so flashing 1.17.2 +> again brings the identity back exactly as it was. Nodes simply upgraded from 1.17.1 keep their existing +> file and are not affected at all. + +## MeshTracker X1: less time deaf when the channel is busy + +Every time the X1 held off transmitting because the channel was busy, it rebuilt its receiver twice +instead of once. Now it does it once, halving the time it spends unable to hear anything. + +LR2021 boards only. + +## An LR1110 too old to reach the mesh now says so + +An LR1110 running firmware older than 0x0303 cannot be moved off the public LoRa channel. It transmits +and receives perfectly well, but nobody on your mesh can see it. That now appears in the log instead of +looking like a broken radio. --- -## Known limitations +## Not fully tested yet -- **The radio changes are only partly on-air validated.** The duty-cycle margin change is computed and - build-verified, not measured: the loss it could cause is signal-strength-independent, and confirming - it needs a delivery-ratio comparison against `rxduty 0` on a marginal link, which has not been run. - The LR2021 listen-before-talk change is likewise reasoned from the datasheet only. -- **The repeater guest-login change is build-verified on both roles but not exercised on hardware.** -- **The identity repair and regenerate paths are not exercised on hardware.** Every layout and failure - case is covered by host tests against the same crypto library the firmware uses, and all three roles - build clean, but no physical node has been put into a broken state and recovered. The normal path — a - healthy node loading a healthy identity — is unchanged apart from one extra key derivation at boot. -- **How that one node's key halves stopped matching is still unexplained.** The byte-order difference - reproduces the symptom exactly, but it should not have been reachable on that hardware, so something - else may have put it there. What this release fixes is that the state can no longer go unnoticed. -- **The reported X1 difficulty logging in to repeaters is not fixed here** and remains under - investigation. The listen-before-talk reporting above will make that failure visible in the log if it - is the cause, which is one of several open hypotheses. +- **The radio timing changes have not been measured on air.** Both are worked out on paper and build + clean, but neither has been checked against real traffic. +- **The repeater guest-login fix has not been tried on hardware**, only verified to build on both roles. +- **The T1000-E sensor readings are confirmed on a real device**; the faster boot time is measured from a + before-log only, with no after-log yet. +- **The identity repair paths have not been tried on hardware.** Every case is covered by tests on a PC, + but no real node has been broken and recovered. Normal healthy nodes are unaffected. +- **Why that one node's identity broke is still unknown.** This release makes sure the same thing can no + longer go unnoticed, but the original cause has not been found. +- **The reported X1 trouble logging in to repeaters is not fixed here** and is still being looked into. diff --git a/zephcore/adapters/ble/ZephyrBLE.cpp b/zephcore/adapters/ble/ZephyrBLE.cpp index db977bd..9485be0 100644 --- a/zephcore/adapters/ble/ZephyrBLE.cpp +++ b/zephcore/adapters/ble/ZephyrBLE.cpp @@ -1420,6 +1420,18 @@ bool zephcore_ble_is_congested(void) return ble_tx_congested; } +bool zephcore_ble_tx_idle(void) +{ + /* Nothing connected — nothing can be in flight, and nothing ever will be. */ + if (!current_conn) { + return true; + } + return k_msgq_num_used_get(&ble_send_queue) == 0 && + !ble_tx_in_progress && + !tx_retry_pending && + !overflow_pending; +} + bool zephcore_ble_is_advertising(void) { return adv_running; diff --git a/zephcore/adapters/ble/ZephyrBLE.h b/zephcore/adapters/ble/ZephyrBLE.h index f01adeb..110a903 100644 --- a/zephcore/adapters/ble/ZephyrBLE.h +++ b/zephcore/adapters/ble/ZephyrBLE.h @@ -62,6 +62,12 @@ bool zephcore_ble_is_connected(void); /** True if TX queue is full and overflow retry is active. */ bool zephcore_ble_is_congested(void); +/** True when every queued frame has been transmitted and link-layer acked: + * send queue empty, nothing in flight, no retry or overflow frame held back. + * True when disconnected (nothing to wait for). Reboot-class CLI commands + * poll this so a reset cannot cut off a reply or delivery-ack mid-flight. */ +bool zephcore_ble_tx_idle(void); + /** True if the controller is currently broadcasting advertising PDUs. * Returns FALSE during an active connection (Zephyr stops adv when the * BT_MAX_CONN=1 slot is consumed) and FALSE after any explicit stop. diff --git a/zephcore/adapters/usb/ZephyrCompanionUSB.cpp b/zephcore/adapters/usb/ZephyrCompanionUSB.cpp index 20699d8..1df5dd0 100644 --- a/zephcore/adapters/usb/ZephyrCompanionUSB.cpp +++ b/zephcore/adapters/usb/ZephyrCompanionUSB.cpp @@ -444,6 +444,19 @@ size_t zephcore_usb_companion_write_frame(const uint8_t *src, size_t len) /* True if the TX ring can hold one more frame of `payload_len` (+3 framing). * The pump checks this before each contact so write_frame can't fail mid-dump. */ +/* True when the TX ring has drained — every framed byte handed to the CDC + * interrupt writer. Mirrors zephcore_ble_tx_idle() for the USB transport. */ +bool zephcore_usb_companion_tx_idle(void) +{ + if (!usb_dev) { + return true; + } + k_spinlock_key_t key = k_spin_lock(&usb_tx_lock); + bool idle = ring_buf_is_empty(&usb_tx_ring_buf); + k_spin_unlock(&usb_tx_lock, key); + return idle; +} + bool zephcore_usb_companion_tx_has_space(size_t payload_len) { if (!usb_dev) { diff --git a/zephcore/adapters/usb/ZephyrCompanionUSB.h b/zephcore/adapters/usb/ZephyrCompanionUSB.h index 23abd15..9473b3e 100644 --- a/zephcore/adapters/usb/ZephyrCompanionUSB.h +++ b/zephcore/adapters/usb/ZephyrCompanionUSB.h @@ -43,6 +43,10 @@ size_t zephcore_usb_companion_write_frame(const uint8_t *src, size_t len); */ bool zephcore_usb_companion_tx_has_space(size_t payload_len); +/** True when the companion TX ring has fully drained. True when no USB device + * is bound (nothing to wait for). Counterpart to zephcore_ble_tx_idle(). */ +bool zephcore_usb_companion_tx_idle(void); + /** * Register a callback fired when the TX ring fully drains (USB analogue of the * BLE on_tx_idle notify-complete event). Used to resume the contact pump. May diff --git a/zephcore/app/CompanionMesh.cpp b/zephcore/app/CompanionMesh.cpp index 1cc87b8..a72bf92 100644 --- a/zephcore/app/CompanionMesh.cpp +++ b/zephcore/app/CompanionMesh.cpp @@ -219,6 +219,9 @@ CompanionMesh::CompanionMesh(mesh::Radio &radio, mesh::MillisecondClock &ms, mes _vcontact_pending_count = 0; _vcontact_hold_msgwait = false; _vcontact_app_hidden = false; + _vcontact_confirm_ack = 0; + _vcontact_confirm_work.self = this; + k_work_init_delayable(&_vcontact_confirm_work.work, vcontactConfirmWorkHandler); memset(&prefs, 0, sizeof(prefs)); prefs.node_lat = 0; prefs.node_lon = 0; @@ -990,6 +993,46 @@ void CompanionMesh::vcontactNotify(const char *text) vcontactQueueText(text); } +/* How long the delivery-ack is held back after its PACKET_SENT. + * + * A plain timer on purpose. The tempting trigger — "flush as soon as the app + * sends us another frame, that proves it is done with the response" — is + * useless here: we emit MSG_WAITING for the CLI reply a few lines further + * down, and the app answers that with CMD_SYNC_NEXT_MESSAGE inside one + * connection interval. That inbound frame would arrive ~50 ms in and flush + * the confirm almost as early as sending it inline did, which is the bug. + * + * Bounded on the other side by the 3000 ms est_timeout we advertise in + * PACKET_SENT: the confirm must land well before the app gives up and resends, + * or the resend races it. */ +#define VCONTACT_CONFIRM_DELAY_MS 300 + +void CompanionMesh::vcontactConfirmWorkHandler(struct k_work *work) +{ + struct k_work_delayable *dwork = k_work_delayable_from_work(work); + ConfirmWork *cw = CONTAINER_OF(dwork, ConfirmWork, work); + cw->self->vcontactFlushConfirm(); +} + +void CompanionMesh::vcontactFlushConfirm() +{ + /* Single 32-bit read/clear: a concurrent flush from the work handler and + * the frame path can at worst emit the push twice, which the app treats as + * idempotent. Losing it is the failure that matters, so no lock. */ + uint32_t ack = _vcontact_confirm_ack; + if (ack == 0) { + return; + } + _vcontact_confirm_ack = 0; + k_work_cancel_delayable(&_vcontact_confirm_work.work); + + uint8_t ack_push[8]; + memcpy(ack_push, &ack, 4); + memset(&ack_push[4], 0, 4); /* trip time: 0 ms — never left the box */ + LOG_DBG("vcontact: emitting deferred ack 0x%08x", ack); + sendPush(PUSH_CODE_SEND_CONFIRMED, ack_push, 8); +} + void CompanionMesh::deriveVContactKey() { /* v-contact pubkey = SHA256("zc-vcontact" || self pubkey). Re-run whenever @@ -1057,8 +1100,13 @@ bool CompanionMesh::vcontactHandleFrame(const uint8_t *data, size_t len) * the real send path. The old code sent a RANDOM ack, which never * matched the value the app derives locally, so the app treated the * loopback message as un-acked and resent it once (attempt=1) a few - * seconds later → duplicate reply. Emitting the correct SENT + - * CONFIRMED up front (before the CLI runs) settles the app at once. */ + * seconds later → duplicate reply. + * + * The value is right, but the CONFIRMED that carries it is NOT sent + * inline — see _vcontact_confirm_ack. Sent here it lands + * sub-millisecond after the PACKET_SENT response to the same write, + * before the app has committed the outgoing message, and the app + * drops it. Deferred by a few hundred ms it sticks first time. */ uint8_t hbuf[5 + MAX_TEXT_LEN]; memcpy(hbuf, &data[3], 4); /* timestamp, on-wire LE bytes */ hbuf[4] = (data[2] & 3); /* attempt & 3 */ @@ -1069,10 +1117,9 @@ bool CompanionMesh::vcontactHandleFrame(const uint8_t *data, size_t len) if (ack == 0) ack = 1; sendPacketSent(MSG_SEND_SENT_DIRECT, ack, 3000); - uint8_t ack_push[8]; - memcpy(ack_push, &ack, 4); - memset(&ack_push[4], 0, 4); /* trip time: 0 ms */ - sendPush(PUSH_CODE_SEND_CONFIRMED, ack_push, 8); + _vcontact_confirm_ack = ack; + k_work_reschedule(&_vcontact_confirm_work.work, + K_MSEC(VCONTACT_CONFIRM_DELAY_MS)); /* Dedupe app resends: a retry reuses the message timestamp (only * the attempt byte changes). The correct ack above should stop most diff --git a/zephcore/app/CompanionMesh.h b/zephcore/app/CompanionMesh.h index 0f4947d..bf802e1 100644 --- a/zephcore/app/CompanionMesh.h +++ b/zephcore/app/CompanionMesh.h @@ -182,6 +182,10 @@ public: * carry sane timestamps. Self-gating; safe to call speculatively. Hooked * at CMD_APP_START, CMD_SET_DEVICE_TIME, and GPS time sync. */ void vcontactClockSynced(); + /** True while a v-contact delivery-ack is still waiting to be emitted. + * Reboot-class CLI commands gate on this (plus transport TX idle) so the + * ack is not cut off by the reset it just scheduled. */ + bool vcontactConfirmPending() const { return _vcontact_confirm_ack != 0; } /** * Continue contact iteration (call each main loop iteration). @@ -514,6 +518,37 @@ private: * the USB CLI. The pref is node-side state: `set v.contact off` is the only * durable disable. Cleared at CMD_APP_START (the session reset). */ bool _vcontact_app_hidden; + /* Deferred delivery-ack for a v-contact chat message. + * + * The ack itself is computed and PACKET_SENT emitted synchronously (the app + * is waiting on that as the response to its own write), but the + * PUSH_CODE_SEND_CONFIRMED that marks the bubble delivered is held back. + * Emitted inline it lands sub-millisecond after the response to the very + * same write -- before the app has committed the outgoing message to its + * own state -- and the app drops it. It then resends at est_timeout, the + * dedup ring above suppresses the CLI re-run, and the resend's ack is the + * one that finally sticks: one reply, two acks, delivery mark seconds late. + * + * No radio ack can arrive that fast, so the real send path never provokes + * this; the v-contact is the only sub-millisecond acker in the system. + * + * 0 = nothing pending. Emitted by the timer only — see the note on + * VCONTACT_CONFIRM_DELAY_MS for why an inbound frame is not the trigger. */ + uint32_t _vcontact_confirm_ack; + /* CONTAINER_OF is offsetof underneath, and offsetof on a non-standard-layout + * type is only conditionally supported — CompanionMesh has base classes and + * virtuals, so it does not qualify (CommonCLI does, which is why the same + * trick is clean there). Wrap the work item in a POD that carries its own + * back-pointer: offsetof stays inside a standard-layout struct, and the + * owner comes from the pointer rather than from pointer arithmetic. */ + struct ConfirmWork { + struct k_work_delayable work; + CompanionMesh *self; + }; + ConfirmWork _vcontact_confirm_work; + static void vcontactConfirmWorkHandler(struct k_work *work); + /** Emit a deferred SEND_CONFIRMED now, if one is pending. Idempotent. */ + void vcontactFlushConfirm(); bool vcontactClockValid(); bool vcontactReady() { return isVContactEnabled() && !_vcontact_app_hidden && _vcontact_lastmod != 0; diff --git a/zephcore/helpers/CommonCLI.cpp b/zephcore/helpers/CommonCLI.cpp index 1fcff98..ce048ef 100644 --- a/zephcore/helpers/CommonCLI.cpp +++ b/zephcore/helpers/CommonCLI.cpp @@ -296,11 +296,31 @@ uint8_t CommonCLI::buildAdvertData(uint8_t node_type, uint8_t* app_data) { } } +/* How long past the initial delay we keep waiting for the transport to drain, + * and how often we look. The poll MUST re-schedule rather than sleep: this + * handler runs on the system work queue, which is the same queue that drains + * the BLE TX ring — blocking here would stall the very thing being waited on + * and guarantee the timeout. */ +#define REBOOT_TX_DRAIN_GRACE_MS 3000 +#define REBOOT_TX_POLL_MS 20 + void CommonCLI::rebootWorkHandler(struct k_work *work) { struct k_work_delayable *dwork = k_work_delayable_from_work(work); CommonCLI *self = CONTAINER_OF(dwork, CommonCLI, _reboot_work); + /* Hold the reset until the companion app has actually been told what + * happened — the CLI reply and, more importantly, the delivery-ack for the + * command that asked for this reboot. Losing that ack is what makes the + * app resend the command, and a resend of "reboot" arrives after RAM has + * been zeroed, so the v-contact dedup ring cannot recognise it and the + * command runs a second time. */ + if (!self->_callbacks->transportTxIdle() && + k_uptime_get() < self->_reboot_deadline_ms) { + k_work_reschedule(&self->_reboot_work, K_MSEC(REBOOT_TX_POLL_MS)); + return; + } + switch (self->_pending_reboot) { case REBOOT_DFU: static_cast(self->_board)->rebootToBootloader(); @@ -320,7 +340,10 @@ void CommonCLI::rebootWorkHandler(struct k_work *work) void CommonCLI::scheduleReboot(uint8_t type) { _pending_reboot = type; - /* 2 second delay - enough for LoRa reply to be transmitted */ + /* 2 second delay - enough for LoRa reply to be transmitted. On a + * companion the handler then keeps deferring in REBOOT_TX_POLL_MS steps + * until the BLE/USB transport has drained, up to the grace below. */ + _reboot_deadline_ms = k_uptime_get() + 2000 + REBOOT_TX_DRAIN_GRACE_MS; k_work_schedule(&_reboot_work, K_SECONDS(2)); } @@ -377,7 +400,11 @@ void CommonCLI::handleCommand(uint32_t sender_timestamp, const char* command, ch scheduleReboot(REBOOT_NORMAL); } else if (memcmp(command, "clkreboot", 9) == 0) { getRTCClock()->setCurrentTime(1715770351); // 15 May 2024, 8:50pm - _board->reboot(); + /* Deferred like every other reboot path: called inline this reset the + * board before the reply — and before any delivery-ack — could leave + * the TX queue. */ + strcpy(reply, "OK - clock reset, rebooting"); + scheduleReboot(REBOOT_NORMAL); } else if (memcmp(command, "advert.zerohop", 14) == 0) { _callbacks->sendSelfAdvertisement(1500, false); // 0-hop (direct) advert strcpy(reply, "OK - zerohop advert sent"); diff --git a/zephcore/helpers/CommonCLI.h b/zephcore/helpers/CommonCLI.h index 22123d3..c39e3fd 100644 --- a/zephcore/helpers/CommonCLI.h +++ b/zephcore/helpers/CommonCLI.h @@ -58,6 +58,12 @@ public: virtual void removeNeighbor(const uint8_t* pubkey, int key_len) { (void)pubkey; (void)key_len; } + /* True when the companion transport has nothing left to deliver: TX queue + * drained AND no delivery-ack still held back. Reboot-class commands poll + * this before resetting so the reply and its ack are not cut off by the + * reset they just scheduled. Default true — the repeater's LoRa reply is + * covered by the fixed pre-reboot delay instead. */ + virtual bool transportTxIdle() { return true; } virtual void formatStatsReply(char* reply) { strcpy(reply, "not available"); } virtual void formatRadioStatsReply(char* reply) { strcpy(reply, "not available"); } virtual void formatPacketStatsReply(char* reply) { strcpy(reply, "not available"); } @@ -119,6 +125,9 @@ class CommonCLI { /* Deferred reboot - lets LoRa reply be sent before rebooting */ struct k_work_delayable _reboot_work; uint8_t _pending_reboot; + /* Uptime (ms) past which the reboot goes ahead even if the transport is + * still busy — a stalled or dropped link must not wedge the reset. */ + int64_t _reboot_deadline_ms; static void rebootWorkHandler(struct k_work *work); mesh::RTCClock* getRTCClock() { return _rtc; } @@ -129,7 +138,7 @@ public: CommonCLI(mesh::MainBoard& board, mesh::RTCClock& rtc, ClientACL& acl, NodePrefs* prefs, CommonCLICallbacks* callbacks) : _board(&board), _rtc(&rtc), _acl(&acl), _prefs(prefs), _callbacks(callbacks), - _pending_reboot(REBOOT_NONE) + _pending_reboot(REBOOT_NONE), _reboot_deadline_ms(0) { k_work_init_delayable(&_reboot_work, rebootWorkHandler); } diff --git a/zephcore/src/main_companion.cpp b/zephcore/src/main_companion.cpp index b1a45d5..047ae72 100644 --- a/zephcore/src/main_companion.cpp +++ b/zephcore/src/main_companion.cpp @@ -718,6 +718,22 @@ public: void updateAdvertTimer() override {} void updateFloodAdvertTimer() override {} + /* Reboot gate — true only when the app has been told everything we owe it. + * A held-back v-contact delivery-ack counts as outstanding even though it + * has not reached the TX queue yet, so a `reboot` typed into the v-contact + * waits for its own ack before resetting. */ + bool transportTxIdle() override { + if (companion_mesh.vcontactConfirmPending()) { + return false; + } +#if ZEPHCORE_USB_STACK + if (zephcore_ble_get_active_iface() == ZEPHCORE_IFACE_USB) { + return zephcore_usb_companion_tx_idle(); + } +#endif + return zephcore_ble_tx_idle(); + } + /* Log control — no log file on companion. */ void setLoggingOn(bool enable) override { (void)enable; } void eraseLogFile() override {}