mirror of
https://github.com/meshtastic/firmware.git
synced 2026-10-30 20:09:18 -04:00
develop
266
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
95ff510f47 |
feat(beacon): per-target and offered frequency slots (#12126)
* feat(beacon): per-target and offered frequency slots Implements the slot fields from meshtastic/protobufs#1059 on top of the broadcast_targets scheme from #11646: - BroadcastTarget.frequency_slot pins a target's slot. Unset derives it the way a node on that channel would: the target region's override slot, else the hash of the target channel's name on the target preset. A target on the home radio keeps the home slot. A pin the target region does not hold skips the target. The deprecated ChannelSettings.channel_num is no longer read, and the home node's channel_num no longer leaks into other presets. - broadcast_offer_frequency_slot is advertised as MeshBeacon.offer_frequency_slot only when a receiver could not derive it. An offer pinned to a slot its region lacks is withheld; the text still goes out. - The listener keeps a received offer's slot. - Admin clears slot 0, and a slot past an explicit region and preset. - userPrefs: USERPREFS_MESH_BEACON_OFFER_FREQUENCY_SLOT and USERPREFS_MESH_BEACON_TARGET_<N>_FREQUENCY_SLOT. Also from #11662: - A beacon is addressed at its target's channel index, so perhapsEncode() keys it like any other packet; the primary channel is never overwritten. The TX hook writes the target preset's channel hash for a blank-named channel on another preset. - A beacon whose send() did not queue releases its target entry on every failure, not only ERRNO_SHOULD_RELEASE. - A beacon still queued a broadcast interval after it was armed, or whose entry was evicted from a full table, is dropped at TX instead of going out on the home radio. Supersedes #11662. Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com> * fix(beacon): address review of frequency slots - TX gate checks the target preset against its region or EU sibling, not the home slot - Targets and offers run an EU sibling's preset in that sibling, as the switch does - Switch clears override_frequency and restores tx_power; restore keeps fields edited mid-switch - An unusable channel_index skips the target instead of falling back to the primary - Beacon config validation moved to MeshBeaconModule::sanitizeConfig(), also run on load; message cap no longer ends mid-character - Evicted-id ring sized to the TX queue * fix(radio): frequencySlotCount() reports no slots for UNSET UNSET's table span is a placeholder; an unknown region code resolves to it too. * fix(beacon): slot count and restore for custom bandwidth - frequencySlotCount() counts a custom bandwidth wider than the region at the region default, as the clamp runs it - resolveFrequencySlot() falls back to the hash when the region override slot exceeds the count - The beacon switch restores a custom bandwidth applyModemConfig() clamped for the target region * fix(beacon): run a target's named preset on a node with custom modem params The switch sets use_preset for a target that names a preset and restores it after TX. A target naming no preset keeps the custom params. * fix(beacon): keep an out-of-range target channel_index sanitizeConfig() cleared has_channel_index for an index past the channel table. That turns the entry into an unset-index target, and sendBeacon() sends those on the primary channel - under a name and key the operator never named. sanitizeConfig() now also runs from loadFromDisk(), so a stored or userPrefs index did that at every boot, not only on the write that set it. Keep the index and only warn. sendBeacon() already bounds-checks it against channels.getNumChannels(), which is tighter than MAX_NUM_CHANNELS, before getByIndex(), and skips the target - so nothing can index out of bounds, which is what the clear was there for. test_adminValidation_targetChannelIndexOutOfRange_isCleared becomes _isKeptAndSkipped: the index survives the write, and the stored config emits no beacon rather than one on the primary. Reported by NomDeTom on #12126. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SwNPp17REf1c9je42aWyJa --------- Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com> Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5.5
|
34cb8d5828 |
Rework XEdDSA signing policy: Compatible stops signing, AES-CCM overrides, Strict exemptions (#12191)
* Rework XEdDSA signing policy: Compatible stops signing, AES-CCM overrides Signing costs 66 bytes per packet. On a crowded mesh that matters, and packets on an AES-CCM channel were paying it on top of the CCM tag. Sending (perhapsEncode): - Compatible, the default, no longer signs. Balanced and Strict sign broadcasts as before; licensed (ham) nodes also sign unicasts, but now only under Balanced or Strict. - Packets on an AES-CCM (use_aead) channel are never signed. This also removes the case where a signed CCM packet passed signedDataFits() but then failed the CCM size check with TOO_LARGE. Receiving (checkXeddsaReceivePolicy): - Only Strict drops a packet for lacking a signature. Balanced no longer drops unsigned broadcasts from known signers; that downgrade rule and its canonical-sizing helper are removed. - An AES-CCM channel overrides Strict: its unsigned or unverifiable packets are accepted, since the CCM tag authenticates them. - A malformed or failing signature still drops the packet in every mode. Identity (new NodeDB::refusesUnsignedIdentity, used by updateUser, NodeInfoModule and TrafficManagementModule): - Balanced and Strict still refuse an unsigned NodeInfo that would change a known signer's identity, except on an AES-CCM channel. - Compatible accepts it, since firmware before 2.8 strips signatures when relaying. Channels::isAEADEnabled() now returns false for an out-of-range index instead of logging, since receive paths pass a packet's index unchecked. XEDDSA_SIGNATURE_FIELD_BYTES lost its only user and is removed. Tests: test_packet_signing drops the cases that pinned the removed downgrade sizing (A7-A9, A18-A19, E5, E10-E13, D1), flips A4 and E1 to acceptance, and adds AES-CCM, Compatible and ham cases (A18-A20, B18-B20, N12-N13). test_mqtt and test_traffic_management pin the same rules on the plaintext-MQTT and NodeInfo-cache paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Sign NodeInfo unicasts, forward signed requests, check signed NodeInfo keys A NodeInfo request or reply carries the key a peer will use for PKI direct messages, but off ham mode only broadcasts were signed. A Strict node therefore dropped every NodeInfo reply and could only learn a new peer's key from a signed broadcast, hours apart. - perhapsEncode: under Balanced and Strict, NodeInfo is signed even when unicast (still never on an AES-CCM channel, never in Compatible). - TrafficManagementModule: a NodeInfo request that carries a signature is forwarded instead of answered from the cache. Its sender may need a signed reply, and a reply sent on the target's behalf can never be signed. - checkXeddsaReceivePolicy: a signed NodeInfo verified against a stored key must advertise that same key, and the key must hash to the sender's node number, as first contact already requires. Otherwise it is dropped. This stops a valid signature vouching for another key, and stops a key planted on trust under the wrong node number proving itself with fresh signatures. Tests: B7 now expects NodeInfo unicasts signed; B21 pins signing per policy; A21-A23 pin the key-consistency check; a traffic-management case pins that a signed request is forwarded while the same request unsigned is answered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Strict: accept unsigned acks and traceroutes; never sign traceroutes Strict dropped every unsigned packet that was not PKI-encrypted. Acks and traceroutes are never signed, so a Strict node saw no acks, retransmitted every reliable packet to exhaustion, and could not take part in traceroutes. - checkXeddsaReceivePolicy: Strict no longer drops an unsigned ROUTING_APP packet; an ack carries its own proof (AckProof.h). Traceroutes are accepted in every mode, and any signature on one is ignored rather than verified. - perhapsEncode: a traceroute is never signed, including in ham mode, which signs other unicasts. Every relay rewrites the route, so a signature fails verification from the second hop on, and a bad signature drops the packet in every mode. - TraceRouteModule::alterReceivedProtobuf: strip any signature when rewriting the route, so a traceroute signed by older firmware is not dropped by older receivers further along. Tests: A24-A26 (Strict accepts unsigned acks and traceroutes; a traceroute signature is ignored), B22 (traceroutes never signed), B7 updated, and test_traceroute_nexthop pins the signature strip on rewrite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Drop the traceroute signature strip Only 2.8.0 and 2.8.1 pre-release nodes ever signed a traceroute: ham mode, which signs every unicast, and client-sent broadcast traceroutes. Stripping the signature on rewrite only helped those pre-releases interoperate, which does not justify the code. Receivers still ignore a traceroute signature, and traceroutes are still never signed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Sign unicast positions under Balanced and Strict A position request or reply is PKI-excluded, so a signature is its only authentication, but off ham mode only broadcasts were signed. Strict dropped every position request and reply, and under Balanced any channel member could fake another node's location with an unsigned reply. perhapsEncode now signs POSITION_APP unicasts under Balanced and Strict, as it does NodeInfo. Nothing rewrites a position in flight - precision is applied by the originator before signing - so the signature survives relaying, and it binds request_id so a reply cannot be retargeted. Compatible and AES-CCM channels still sign nothing. Tests: B23 pins position unicast signing per policy and Strict acceptance; B7 expects position unicasts signed; B2 and B5 use PRIVATE_APP as their unsigned unicast now that position is signed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Strict: accept unsigned key verification messages Key verification's bootstrap messages (M1, M2) go out channel-encrypted because the peer's key is not known yet, so they cannot carry a signature the receiver could verify. Strict dropped them, so two Strict nodes that did not already hold each other's keys could never run key verification. The handshake does not rely on the transport: hash1 binds both public keys to the security number the users exchange, and a key is only stored after the numbers match and the user accepts. A forged request can at most raise a verification prompt, bounded by the single session slot and the 60 s remote cooldown - as it already can under Balanced. Test: A27 pins that Strict accepts an unsigned key verification message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * test_nodedb_identity_hygiene: pin the identity refusal to Balanced Two cases expected updateUser() to refuse an unsigned update from a known signer under a zeroed config. Zeroed is Compatible, which now accepts it, so they failed in CI. Run them under Balanced, and add the Compatible counterpart so both halves of the rule are pinned at the updateUser level. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Ham mode: sign in place of PKI, authenticate acks with the ack proof (#12200) Ham mode signed every unicast under Balanced and Strict, acks included. A signed ack from a node whose key a Strict receiver lacks is dropped, so the sender retransmits to exhaustion, and the signature costs 66 bytes where the ack proof costs about 10. Ham mode now signs the unicasts PKI would cover in regular mode, plus broadcasts, NodeInfo and position as before. Acks are no longer signed; they carry the ack proof, which needs only the two nodes' keys, not PKI. Strict already accepts unsigned acks. Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK Co-authored-by: Claude <noreply@anthropic.com> * Notify the client when an unsigned identity update is refused Balanced and Strict refuse an unsigned NodeInfo that would change a known signer's name or role, but the refusal only reached the device log, so the user saw a stale name with no reason. Send a ClientNotification naming the node (by the name already held, never the refused claim) and how to accept its updates again: remove the node. Rate limited to once per node per boot, over the last 8 nodes, and to one notice per minute overall, so a signer relayed through pre-2.8 firmware or a burst of them cannot flood the client. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Refused identity notice: only for a name or role change, drop the warnings Notify only when the refused NodeInfo would have changed the stored long name, short name or role, compared through CopyUserToNodeInfoLite so truncation and sanitizing cannot read as a change. A signer held only in the warm tier has no stored name to compare and is not reported. Remove the two LOG_WARNs at the refusal sites; the notice replaces them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Refused identity notice: name the security setting that refused it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK * Refused identity notice: one global hourly throttle, no per-node tracking Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
329b7ea918 |
Fail the native simulator job on a sanitizer fault, and fix the one it found (#12205)
* Fail the native simulator job on a sanitizer fault, and fix the one it found The integration step backgrounded meshtasticd and ended in `wait "$PID" 2>/dev/null || true`, which discards the simulator's exit status. The coverage env is built with -fsanitize=address, so a fault there prints a full report and exits non-zero on the way out, after the Python harness has already printed its own success - and the job concluded green with an AddressSanitizer heap-use-after-free sitting in the log. Gate it: capture the simulator's output, read the status `wait` reports, and scan the output for a sanitizer report. The pattern moves to bin/lib/sanitizer.sh so bin/run-tests.sh and the new bin/check-sanitizer-log.sh share one matcher rather than drifting apart; the output is uploaded as an artifact, since a fault on the exit path prints after the last test has passed. Leak detection is off, as in bin/test-config-check.sh - exit_simulator exit()s with the node's globals still live. The fault it had been hiding: OSThread::run() points currentThread at the running thread for the whole of runOnce(), and exit_simulator calls exit(0) from AdminModule while ServerAPI is that thread. ServerAPI is declared `public StreamAPI, private OSThread`, so its Thread base frees ThreadName first and PhoneAPI::~PhoneAPI's own LOG_DEBUG then reads it back through currentThread. Clear the pointer in ~OSThread when it names the thread being destroyed, which covers any thread deleted from inside its own runOnce, not just this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JjAvRt6xuztC3hKqWkW9Dw * Detach observers when an Observable is destroyed The new sanitizer gate turned the simulator job red on a second heap-use-after-free on the same exit path, this one in Observable::removeObserver. ~Observer removes itself from every observable it watches, but Observable has no destructor and dies with its watchers still pointing at it. exit_simulator exit()s from AdminModule, and static destruction takes the global xModem - whose packetReady is an Observable that PhoneAPI::handleStartConfig attached to - before the heap-allocated ServerAPI that is watching it. PhoneAPI::close() then calls unobserve(&xModem.packetReady) on the dead observable, and ~Observer would walk it again afterwards. Both halves of the contract, so neither side has to outlive the other: ~Observable drops itself from each watcher's list, and unobserve() checks membership before reading through the pointer it is given. Also take CodeRabbit's grep -m 4 in bin/check-sanitizer-log.sh: with more than four matching lines, head closing the pipe is a SIGPIPE that pipefail reports and set -e exits on, so the script gave 141 where it documents 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JjAvRt6xuztC3hKqWkW9Dw --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5.5
|
e069a8d491 |
LR2021: time received frames on the configured coding rate (#12183)
* LR2021: time received frames on the configured coding rate The LR2021's reported RX coding rate can pick up extra bits at a bad status read right after our own TX and keep them until a hard reset. Explicit-header frames still decode, since the rate travels in the header, but the wrong value inflates RX airtime: a latched 4/8 against a configured 4/5 times a 200-byte frame about 55% long. That feeds channel utilisation, the CSMA contention window and the reliable-router retransmit timers. computePacketTime() gains useRxHeaderInfo; LR20x0Interface passes false, so received frames are timed on the configured coding rate and CRC, and the readout task no longer reads the header info on that chip. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTiifXrVc4YdonmLRCeWhw * LR2021: describe the coding-rate accumulator in the comments The reported RX coding rate was described as latching after a post-TX status read. That attribution was wrong: GetLoraPacketStatus accumulates the bitwise OR of every received header's coding rate since reset, which an SX1262 injector reproduces on demand - frames at 4/5, 4/6 and 4/7 each OR their bit in, a frame at the configured rate changes nothing, and the field reaches values DS rev 2.2 Table 9-4 does not define. It is invisible on a single-coding-rate mesh, since cr | cr == cr. No behaviour change: comments only. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude
|
20dc8e31e5 |
Key verification: refuse in licensed (ham) mode (#12201)
The handshake's third message must be PKI-encrypted, and licensed operation neither sends nor decrypts PKI. A licensed node still opened a session, raised the security-number prompt, and waited until it timed out. A licensed node now refuses to start a session, from the admin message or the screen, and tells the client why. It ignores a remote node's request without a NAK, as a busy node does. The screen's node menu hides the Key Verification entry in licensed mode. Claude-Session: https://claude.ai/code/session_011DjtCrF8NLMGs9DtEVQkUK Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
dc9999280f |
Don't let a stale RX flag re-arm RX over a live frame (#12153)
* Don't let a stale HEADER_ERR re-arm RX over a live frame
Before: a header that fails on noise, a collision or a foreign frame
latches HEADER_ERR and no RX_DONE follows it. `checkStaleRxFlags()`
counts HEADER_VALID and HEADER_ERR as one boolean, so the error starts
the max-packet window that re-arms RX. A good frame arriving inside that
window raises HEADER_VALID beside the error, which does not restart the
window - a header was already up. One max packet after the error was
first seen, the poll re-arms, and `startReceive()`'s standby aborts the
frame still on the air.
After: the window is timed from the valid header alone. A HEADER_ERR
beside a HEADER_VALID is retired at once and decides nothing, because one
header raises one flag or the other (SX1262 DS rev 2.1 table 13-29), so
the two together are two frames and the error is the older one.
A lone stale HEADER_ERR past its window is now cleared rather than
re-armed on every chip but SX128x. The re-arm exists for the SX1280 wedge
of DS rev 3.3 16.2, which a clear does not unwedge; on SX126x, LR11x0 and
LR20x0 the demodulator is already hunting for a preamble again, and a
clear never aborts a frame that may be in one. `headerErrWedgesRx()` is
the per-chip hook, true only on SX128x. SX126x already clears HEADER_ERR
unconditionally in `resumeRunningReceive()` for the same reason.
The busy decision is unaffected: `receiveDetected()` reads only
HEADER_VALID and PREAMBLE_DETECTED, so the cost of the bug was a dropped
reception, never a talk-over.
How: `staleRxFlagAction()` takes HEADER_VALID and HEADER_ERR separately
plus the per-chip wedge flag, and gains a ClearHeaderErr verdict.
`checkStaleRxFlags()` keeps a second stamp bit for "the window was
started by a valid header", so a valid header restarts a failed one's
window the way develop already has a header restart a bare preamble's.
Five native cases cover it, two of them the regression itself.
Found and analysed by Tom (NomDeTom), who also wrote the commit the
behaviour came from,
|
||
|
Jonathan BennettandClaude Opus 5
|
e9c046547d |
LoRa: size the preamble hold for the worst-case frame, and let the CAD settle an old bare preamble (#12125)
* LoRa: size the preamble hold for the frame that could be arriving The hold ran for getPacketTime(DATA_PAYLOAD_LEN + sizeof(PacketHeader)): 249 bytes at our own coding rate. A sender's header carries its own CR, up to 4/8, and that header is unreadable when the hold starts, so a legal 255-byte frame at CR 4/8 with CRC outlives the hold and we key up over it. That is the case the hold exists for, since LoRa we cannot decode - a foreign sync word - never produces HEADER_VALID at all. maxRxFrameMsec() times the 255-byte PHY frame at CR 4/8 with CRC from the configured modem, through the radio's own calculateTimeOnAir(), and falls back to getPacketTime(MAX_LORA_PAYLOAD_LEN) if the radio answers an error code (#11935). At SHORT_TURBO that is 159 ms rather than 100 ms, at LONG_FAST 3.3 s rather than 2.1 s. The HEADER_VALID ageing in receiveDetected() takes the same bound, for the same reason. checkStaleRxFlags() and checkCadHandoffTimeout() keep the shorter window: they decide a latched flag is dead and recover the chip, where waiting longer delays the recovery rather than protecting a frame. Three test cases in test/test_radio pin the sizing, the error fallback and that the hold outlives the flag clear that starts it. FakeIrqRadio now answers calculateTimeOnAir() and honours a clear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CStem6K6MQRBNR8YCFgtCE * LoRa: let the pre-TX CAD settle an old bare-preamble hold A hold runs for one max frame whether or not anything is really arriving: 159 ms at SHORT_TURBO, but 2.1 s at LONG_FAST and 14 s at LONG_SLOW. For all of it the TX path only re-rolls its backoff - canSendImmediately() is false, so the pre-TX CAD never runs and nothing re-examines the channel. A noise preamble therefore costs seconds of TX and can overflow the Tx queue, which is what @GUVWAF objected to on #11968. A frame we could decode latches HEADER_VALID within the rest of the preamble plus the ~12.25 symbols of sync word, SFD and explicit header. Once that grace has passed with no header, the sighting is noise or LoRa we cannot read, and neither deserves the rest of the hold: preambleHoldPeekable() lets the TX path fall through to its CAD and send if the channel is clear. At LONG_FAST the grace is 248 ms against the old code's own 262 ms deadline, and 8 ms at SHORT_TURBO, so a spurious preamble now blocks TX for slightly less than develop did before the hold existed - and then actually listens. HEADER_VALID keeps the full hold: that is a real frame on our own modem. receiveDetected() notes a header even while the hold short-circuits, so a frame whose header arrives mid-hold is never peeked past, and a new hold forgets the previous one's header. Our own transmit ends the hold, since the CAD has just judged the channel. preambleHeaderGraceMsec() computes the grace from sf and bw rather than RadioInterface::preambleTimeMsec, which is whole milliseconds: above 2 GHz a 12-symbol preamble is 0.95 ms, so that value is 0 and the grace would be 0, making every sighting peekable the moment it was seen. Six more cases in test/test_radio: the grace at wide-LoRa and sub-GHz presets, its rounding, and the peek inside the grace, after it, behind a header and across a new hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CStem6K6MQRBNR8YCFgtCE * LoRa: ask for the frame bound before reading back the CR it timed test_preambleHold_isSizedForTheWorstCaseFrame read phy.lastCodingRate straight after the first preamble sighting, where nothing has yet asked the chip to time anything: preambleHoldActive() short-circuits on the unset stamp, so the sighting that starts a hold never reaches calculateTimeOnAir() and the fake still reports a coding rate of 0. Ask maxRxFrameMsec() for the bound first, then assert the CR and CRC it was asked for. The case pins the same thing it always meant to - that the hold is timed at CR 4/8 with CRC rather than at our own CR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CStem6K6MQRBNR8YCFgtCE * LoRa: scale the preamble hold's airtime fallback to CR 4/8 When calculateTimeOnAir() cannot time the worst-case frame, maxRxFrameMsec() falls back to getPacketTime(), which times it at our own coding rate. A configured rate below 4/8 then produced a hold shorter than the frame the hold exists to cover. Scale that figure up to 4/8; the preamble does not scale with CR, so the result runs long, which is the safe direction for a hold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013mdEBL5t2xdysijGG6ejQR --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
1c366c2d05 |
Radio: end a CAD handoff whose RX hits a damaged header (#12144)
* Radio: end a CAD handoff whose RX hits a damaged header When CAD hands off to RX and that RX ends on a damaged header (CRC_ERR or HEADER_ERR with no RX_DONE), the RX readout task cleared those flags and returned. Nothing was notified and cadHandoffRxStart stayed set, so the radio thread never re-armed. Where the chip had stopped listening, the node stayed deaf until the main loop's backstop logged "CAD>RX timeout" and re-armed: 200 ms to over a second later. Before the readout task, the same edge went to handleReceiveInterrupt(), which read the error, cleared the flags and re-armed. The bench saw this on an LR2021 (X1) running develop: 5 "CAD>RX timeout" lines across 3 runs, where pre-#11983 develop logged none in 4 runs. Now the task hands every frameless end of a handoff's RX (TIMEOUT, CRC_ERR, HEADER_ERR) to the radio thread, which clears the flags through handleReceiveInterrupt() and re-arms, as it already did for TIMEOUT. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 * Radio: don't strand a damaged header's flags when its notification is dropped Three follow-ups to the handoff end, from review of the first commit. The readout task's notify(ISR_RX, !rxArmedBeforeTxDone) deliberately does not overwrite a pending TX notification, so the RX notification can be dropped. That is pre-existing for the TIMEOUT case, and the ISR_TX path does put the chip back into RX, but the first commit moved the clearing of CRC_ERR / HEADER_ERR to the radio thread, so in the dropped case nothing clears them until checkCadHandoffTimeout(). notify() returns false exactly when it dropped, so clear them at that site in that case, as the code did before, and leave the open handoff to the backstop. handleReceiveInterrupt() logged "CAD>RX pkt" for a frameless end, and read a length that is not a frame's. A damaged header now takes the same exit as an empty window, with its own log line, so a capture says which of the two happened and getPacketLength() is not asked about a packet that never arrived. rxBad is unchanged either way: neither case counted before. An empty window inside a handoff can carry a damaged header's flags alongside the timeout, both routed to DIO1, so that exit clears them together. Plain RX is untouched there and still leaves HEADER_ERR to checkStaleRxFlags(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GTiifXrVc4YdonmLRCeWhw --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
c7df2e85bc |
fix(traffic): round the default position dedup window to 6h, derive the floor from it (#11765)
The 5 h dedup window and the 6 h stationary broadcast floor were paired by a hand-picked one-hour margin. Round the window to 6 h and derive the floor from it: one dedup tick (6 min) above the window, which is the exact margin a stationary node's refresh needs to clear its neighbours' tick-quantised window regardless of tick phase. A static_assert enforces the pairing. Role caps (tracker 1 h, lost-and-found 15 min) are unchanged. Saved configs keep their value, as with the 11 h -> 5 h change in #11606; the new default applies when traffic management config is first installed. Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Tom <116762865+NomDeTom@users.noreply.github.com> |
||
|
|
1e3a38fc8a |
Add native Portduino mDNS discovery with Avahi and Bonjour (#12032)
* Add native Portduino mDNS discovery via Avahi and Bonjour * Guard native discovery in Portduino WASM builds * Add linux dependencies in packaging, support "buildroot" Adds libavahi dependencies for Debian, RPM, Docker and CI. Moves the libavahi definition to the shared `build_flags` (which is only consumed on Linux targets) * Fix platformio formatting * fix(portduino): disable Avahi when daemon unavailable --------- Co-authored-by: Jake-B <jake-b@users.noreply.github.com> Co-authored-by: vidplace7 <vidplace7@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5.5
|
48d812536d |
LR20x0: empty the RX FIFO when a frame is dropped unread
The LR2021 receives into a FIFO that RadioLib empties only at the end of readData(). Every path that gives up on a frame without reading it (the readout task's bad length and full ring, the thread's early outs) cleared only the IRQ flags, so the dropped frame's bytes stayed at the front of the FIFO and the next frame was read out on top of them. On an X1, 37 of 37 forced drops were followed by a "Decryptable packet failed decoding", so each drop cost two frames. clearReadIrqs(), which every such path already calls, now also asks the driver to discard the frame. LR20x0 sends CLEAR_RX_FIFO (private in RadioLib); the other chips read from a buffer offset and need nothing. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 |
||
|
Jonathan BennettandClaude Opus 5.5
|
dbc7daecc6 |
feat(portduino): Adafruit Arcade Bonnet input driver (#12116)
* feat(portduino): Adafruit Arcade Bonnet input driver
Add an InputBroker source for the Adafruit Arcade Bonnet: six buttons
(1A-1F) and a 4-way joystick on an MCP23017 I2C expander.
The expander is polled over I2C every 20 ms rather than serviced from its
interrupt line, because the bonnet ties INT to GPIO17, which is the HUB75
clock on the adafruit-hat mapping. The driver sets INT to open-drain and
enables no interrupt sources so the line never drives against the panel.
Buttons emit once per press and carry a gamepad button code in kbchar
(1E = BTN_SELECT, 1F = BTN_START, so Start pauses games). The joystick
emits directions with the same auto-repeat as LinuxJoystick. The analog
stick bits are ignored; they float with no stick attached.
Config:
Input:
ArcadeBonnet: 0x26
ArcadeBonnetButtons: # optional
select: [1A, 1F]
cancel: 1B
--check validates both keys. The action-name parser moves from
LinuxJoystick.cpp to InputBroker.h so both drivers share it.
Breakout now polls the bonnet joystick's held direction the same way it
polls the USB gamepad, so the paddle glides instead of stepping at the
auto-repeat rate.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vaqwGPoD64pqsnPYbvnUn
* fix(portduino): Arcade Bonnet buttons act in games; address review
1F reported BTN_START, which GamesModule treats as pause, so a bonnet
button bound to select paused Tetris and Breakout instead of dropping or
serving. 1E/1F now report BTN_C/BTN_Z; no bonnet button is Start.
From review:
- Clear the held joystick zones when an I2C read fails, so Breakout's
paddle doesn't keep moving after the bonnet stops answering.
- Set Input.ArcadeBonnet only when the key is present, so a later
config.d file with its own Input section doesn't disable the bonnet.
- Reject addresses outside the MCP23017's 0x20-0x27 in the driver and
in --check, instead of truncating them to a uint8_t.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vaqwGPoD64pqsnPYbvnUn
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
|
9084439392 |
test(nodedb): drive the deferred node save from a real peer (#12114)
The deferred-save cases added in #11930 set the dirty flag through the test shim. These two go through NodeDB::updateUser() with a new peer inside the one-minute throttle window, check the peer is not yet in nodes.proto, then read nodes.proto back after the minute passes and after a reboot and assert the peer is there under its name. Both fail if updateUser() stops marking the database dirty. Co-authored-by: marshallumsted <bot@marshallumsted.dev> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Jonathan BennettandClaude Opus 5.5
|
8c97a11613 |
Read received LoRa frames out from a FreeRTOS task (#11983)
* Read received LoRa frames out from a FreeRTOS task
The radio holds one received frame, and the next one overwrites it.
Until now the frame was read by the radio thread, which runs only when
the main loop gets to it, so a long hold anywhere in the loop (PKI key
derivation, serial console, display) lost the frames that ended during
it.
On FreeRTOS targets with a DIO1 interrupt the RX_DONE interrupt now
wakes a task above the main loop, pinned to the loop's core. It reads
the frame with the same RadioLib calls as before (readData, getSNR,
getRSSI) into an 8-frame ring, and notifies the radio thread, which
delivers the frames and re-arms RX as before. The interrupt stays
enabled, so frames that end back to back are all read.
It works for every RadioLib radio (SX126x, SX127x, SX128x, LR11x0,
LR20x0). -DMESHTASTIC_EXCLUDE_READOUT_TASK=1 turns it off. Portduino,
STM32WL and LORA_DIO1_SOFTWARE_POLL boards keep the thread readout.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Initialize the RX readout ring in the constructor
cppcheck's uninitMemberVar failed every FreeRTOS check job: rxRing is an
array of plain structs with no member initializers, so the constructor left
it indeterminate. The ring is only read behind rxRingTail != rxRingHead, so
nothing depended on its contents, but zeroing it costs nothing - the
interfaces are globals, so the array sits in .bss either way.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuWbtYhL797NBX6kjw449H
* Keep the RX readout task out of RadioLib call sequences
The readout task runs above the radio thread on the same core, and the SPI
lock is taken per transaction, so a readout could land between the RadioLib
calls of a sequence the radio thread was partway through. Every driver's
readData() ends by clearing the chip's IRQ flags, and SX128x's drops to
standby first, so a readout inside a channel scan, an RX arm, a transmit
setup or a reconfigure could clear flags that sequence was about to rely on,
or move the chip out from under it. The scan-to-transmit window in
TRANSMIT_DELAY_COMPLETED is the sharpest case: it is deliberately kept as
tight as possible, and startTransmit() would run on flags a readout had just
wiped.
Add a recursive, priority-inheriting mutex the task and the radio thread
share, held across each whole sequence: the two re-arms and the scan-to-
transmit pair in onNotify(), the recovery ladder, and each driver's
reconfigure(). Recursive because the sequences nest; priority-inheriting so a
waiting readout lifts the thread holding it rather than queueing behind it.
Packet delivery stays outside the lock - deliverCapturedFrames() makes no
RadioLib calls, and holding it there would delay the next readout for
nothing. This bounds a readout by the radio sequence in flight, not by the
main loop, so the latency the task exists to remove stays removed.
wakeRxReadout() loses its spin. It is called from inside driver RX arms,
which now hold the lock, so waiting there for a task that needs the same
lock would deadlock; the lock also makes the wait redundant, since the task
cannot reach the radio until the arm finishes. The guard records whether it
actually took the lock, because the mutex is created alongside the task from
within startReceive() - a sequence already in flight at that moment has
nothing to release.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuWbtYhL797NBX6kjw449H
* Do not clear an unread RX_DONE when resuming a continuous RX
resumeRunningReceive() cleared RX_DONE, CRC_ERR, HEADER_ERR and TIMEOUT
before picking the RX back up. In continuous RX the chip keeps listening, so a
second frame can complete while the first is being handled - between
readData() and the resume - and that clear took its RX_DONE without anyone
reading the frame. The checkRxDoneIrqFlag() on the next line could then no
longer find it, so the frame was lost.
The clear was there for the paths that give up on a frame without reading it,
which leave the flags latched where readData() would have cleared them; a
latched RX_DONE holds DIO1 high past the re-arm. Put the clear in those paths
instead, so it happens only where readData() never ran: not in rx mode, a bad
readback length, region unset, and readData()'s own early error returns (an Rx
timeout or a failed SPI read come back before it clears). Never on the
captured path, where the readout task has already read the frame and a latched
flag is the next one's.
With that, a latched RX_DONE at the resume is by construction a frame nobody
has read, so resumeRunningReceive() stops clearing anything and its
rxReadoutActive() special case goes away. Uses RadioLib's generic
PhysicalLayer clearIrq() rather than the SX126x flag names, so the helper
works for every radio.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuWbtYhL797NBX6kjw449H
* Clear the RX flags only where readData() left them latched
The previous commit cleared the terminal flags on every readData() error, which
put back the bug it was meant to fix, in the case that matters most. Each
driver decides a CRC or damaged-header verdict before its clear and returns it
afterwards - SX126x asserts crcState one line past clearIrqStatus(), and SX127x
has no early return at all - so those errors come back with the flags already
gone. A CRC mismatch is the routine error on a noisy channel, so that clear
fired often, and each time it could take the RX_DONE of a frame that completed
while the error was being logged.
Gate both on the returned code: RADIOLIB_ERR_CRC_MISMATCH and
RADIOLIB_ERR_LORA_HEADER_DAMAGED mean the flags are already clear, and every
other error - an Rx timeout, a failed SPI stream check or buffer read, a wrong
modem - returned before the clear and leaves them to be dropped by hand.
The readout task needs the same test for the opposite reason. When its own
readData() fails early the flags stay latched, and the radio thread cannot
clear them for a captured frame, because it has no way to tell whether the task
got past RadioLib's clear. So RX_DONE stayed set for a frame nobody would ever
read, and checkRxDoneIrqFlag() woke the task again for the same dead event,
round after round. The task now clears its own failed reads, where the answer
is known, and the ring-full path reuses the helper rather than repeating the
flag list.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuWbtYhL797NBX6kjw449H
* LR20x0: re-arm RX at TX_DONE from the readout task
After TX_DONE the LR2021 sat in standby until the radio thread restarted
RX, which a main-loop hold can stretch. Now the TX_DONE interrupt asks the
readout task to restart RX through RadioLib (an interrupt cannot call it),
and the radio thread adopts that RX instead of restarting it. On by
default wherever the readout task runs; -DLR2021_RX_REARM_AT_TX_DONE=0
opts out.
The task re-arms inside a radio sequence, and the thread's adopt runs
inside the post-TX sequence, so the two cannot interleave: if the thread
gets there first it takes the re-arm over. A stale re-arm from a TX that
the poll completed is cleared when the next TX is configured.
Measured on Jonathan's bench (Seeed Mesh-Tracker X1, LR2021, SHORT_TURBO,
DMShell workload): the X1 went from ~612 post-TX deaf windows per 300 s
run and a 3.7% board miss rate to better than the T1000-E (LR1110), which
already re-arms this way.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* LR20x0: read frames that end before the radio thread adopts the re-arm
The TX_DONE interrupt detaches the radio interrupt, and the task's re-arm
left it detached until the radio thread adopted the RX. During a long
main-loop hold the chip listened but nothing woke the readout task, so a
second frame overwrote the first in the chip.
The task now attaches the RX interrupt once its re-arm is in. Until the
thread handles that TX_DONE, a frame the task reads notifies ISR_RX
without overwriting the pending ISR_TX, which would otherwise skip
completeSending() and stall the TX queue; onNotify() delivers the
captured frames ahead of the ISR_TX as it already does for an overwritten
ISR_RX.
Found by CodeRabbit on #11983.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: re-arm RX at TX_DONE from the readout task
After TX_DONE the SX126x sat in standby until the radio thread restarted
RX, which a main-loop hold can stretch. Now, as on the LR20x0, the TX_DONE
interrupt asks the readout task to restart RX through RadioLib (an
interrupt cannot call it), and the radio thread adopts that RX instead of
restarting it. The task sends the same continuous or duty-cycled RX
command startReceive() would, now shared through startRxCommand().
On by default wherever the readout task runs except nRF52, where #11984
re-arms from the TX_DONE interrupt itself; -DSX126X_RX_REARM_AT_TX_DONE=0
opts out.
Measured on Jonathan's bench: the T-Beam Supreme (ESP32-S3, SX1262,
SHORT_TURBO, DMShell workload) loses less with it than without, in a
same-session A/B.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Take the radio-sequence lock around the IRQ-status read in canSendImmediately()
canSendImmediately() called isActivelyReceiving() with no lock, and that reads the
chip's IRQ status over SPI in every driver - getIrqFlags() on SX126x, getIrqStatus()
on LR11x0/LR20x0/SX128x, isReceiving() on RF95. The readout task can preempt the main
loop between that command and its response read, leaving the chip mid-command with
BUSY asserted; both then spin in RadioLib's BUSY wait until its 1000 ms timeout
fires, and the radio, which holds one frame, loses whatever arrives meanwhile.
Measured on a T1000-E (LR1110), 300 s sessions, two arms interleaved, 3 runs each,
with this lock and the same one on the LR11x0 early TX stage (a bench-only path not on
this branch): BUSY waits >=20 ms 5-13 -> 0, 1 s radio stalls 5/11/13 -> 0/0/0, frames
the chip decoded but the firmware never fetched 7/16/18 -> 0/0/0, total loss
0.59/1.23/1.30% -> 0.00/0.00/0.07%.
The sequence mutex is recursive, so a caller already holding it re-enters, and
lockRadioSequence() is a no-op without the readout task.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Hold the radio-sequence lock on the main loop's radio paths
The lock exists so a readout cannot land between the calls of a RadioLib
sequence, where it would clear flags the sequence is about to use. Three
paths reached from the main loop still made such sequences without it:
- checkCadHandoffTimeout()'s startReceive() after a handoff RX that never
completed.
- checkStaleRxFlags(), which reads the IRQ flags, draws a verdict from
them and then re-arms or clears - all of which a readout in between
invalidates, since it retires the very flags being judged.
- resetAGC(), which is a sequence end to end: warm sleep, RC standby,
CALIBRATE_ALL, the re-applied DIO2/boosted-gain/0x8B5 settings and the
startReceive() that resumes. Its own mid-packet bail is a point check,
so it cannot exclude a readout that starts just after it.
All three predate the readout task, so this is the task's own lock
coverage catching up rather than a regression.
Guarding resetAGC() at its call site in periodicRadioMaintenance() covers
every driver's override. It can hold the lock for the length of a
calibration, which delays a readout, but the chip is not readable for any
of that anyway - the RX that would read a frame is the startReceive() at
the end.
checkRxDoneIrqFlag() is deliberately left unguarded: it is a single flag
read and a wake, not a sequence, and serialising it behind a readout in
flight would only delay the handoff it exists to make.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* Readout task: re-arm after a CAD handoff's RX expires empty
With the readout task, an empty handoff RX raises TIMEOUT alone. The
task returned on the missing RX_DONE, so nothing re-armed the chip and it
sat in standby until checkCadHandoffTimeout() ran from the ~1 s missed-IRQ
poll. The task now hands that TIMEOUT to the radio thread, which logs
"CAD>RX empty" and re-arms; an ISR_RX that finds the handoff RX still
running leaves it alone instead of restarting RX over it.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Ask for continuous RX when the duty cycle has nothing to sleep through
RadioLib's startReceiveDutyCycleAuto(preambleLength, minSymbols) listens
for 2 x minSymbols and sleeps through whatever preamble is left over. We
pass preambleLength 16 (12 above 2 GHz) and minSymbols 8, so the sleep
window is zero or negative on every board and every modem preset: the
chip sits in continuous RX whichever of the two commands we send.
rxArmedContinuous recorded the command instead of the state, so it was
false everywhere but a CH341 host, and resumeRunningReceive() refused to
resume an RX that was in fact still running. Every RX_DONE therefore paid
for a standby and a full RX restart - the window in which a packet
already arriving is lost - for no power saved.
Have continuousRxWanted() ask for continuous RX in that case, which makes
rxArmedContinuous describe the RX the chip is really in and lets
resumeRunningReceive() pick it back up. If the preamble ever grows past
2 x minSymbols the duty cycle starts sleeping again and this goes back to
the duty-cycled command on its own.
Measured on the RAK4631, where resuming tied or slightly beat the TX_DONE
ISR re-arm. CH341 hosts are unchanged - they already took the continuous
branch. LR11x0/LR20x0/SX128x have no resumeRunningReceive() override, so
nothing changes for them.
The ISR_RX comment claimed rearmReceive() avoids the standby; that was
only ever true where the resume could fire. Say so.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* SX126x: re-arm RX at TX_DONE from the readout task on nRF52 too
The readout task's re-arm was held off nRF52 because the TX_DONE interrupt
there can re-arm RX itself, with raw SPI, which the task does with RadioLib
calls on every other platform. On the bench the two tied once the resume was
honest about the RX the chip was already in, so there is no reason to carry
two paths: one path for every board the task runs on is the alternative.
-DSX126X_RX_REARM_AT_TX_DONE=0 still opts out.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* LR11x0: re-arm RX at TX_DONE from the readout task
The LR11x0 was the one chip the readout task's TX_DONE re-arm skipped, so on
a T1000-E the chip still sat in standby from TX_DONE until the radio thread
next ran, which a main-loop hold can stretch by hundreds of ms. Same shape as
the LR20x0's: the interrupt marks a re-arm pending and wakes the task, the
task sends what startReceive() sends less the standby, and the radio thread
adopts the RX already running instead of restarting it. No raw SPI - the
interrupt only sets a flag, and the task re-arms with RadioLib calls under the
radio-sequence lock.
-DLR11X0_RX_REARM_AT_TX_DONE=0 opts out.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* LR11x0, LR20x0: resume a running continuous RX after a frame
Both drivers restart RX after every frame: a standby and the whole RX
setup again, deaf throughout. We arm RX with RX_TIMEOUT_INF, so the chip
is still listening at that point, and resumeRunningReceive() picks it back
up instead - what SX126xInterface already does.
The chip is asked first. Only a chip that reports RX is resumed: one that
left it unseen, through a reset or an RX that did not survive the frame,
would otherwise stay deaf with nothing left to re-arm it, and the restart
also runs the chip-state recovery. RadioLib's LRxxxx::getStatus() is
protected, and reads its reply at the wrong offset, so readChipMode()
sends the same NOP transfer itself and takes the mode out of Stat2.
-DLR11X0_RESUME_CONTINUOUS_RX=0 and -DLR2021_RESUME_CONTINUOUS_RX=0 opt
out and take the restart every time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014NeGVaLzYEgfHdj2hcXiSr
* Drop the per-chip opt-out flags for the re-arm and the resume
SX126X_RX_REARM_AT_TX_DONE, LR11X0_RX_REARM_AT_TX_DONE,
LR2021_RX_REARM_AT_TX_DONE, LR11X0_RESUME_CONTINUOUS_RX and
LR2021_RESUME_CONTINUOUS_RX were bench levers. Nothing in the tree sets any
of them to 0, and neither path needs a build-time escape hatch: a re-arm that
the chip does not take falls back to the radio thread's checked
startReceive(), and a resume only happens when the chip reports it is still
in RX, taking the restart otherwise.
So both are now unconditional wherever the readout task runs, and the gates
collapse to #ifdef MESHTASTIC_RX_READOUT_TASK. LR11X0_READ_CHIP_MODE and
LR2021_READ_CHIP_MODE went with them, having existed only to compile
readChipMode() out when the resume was off.
MESHTASTIC_EXCLUDE_READOUT_TASK stays: one switch for a board that cannot
afford the task. So does the MESHTASTIC_RX_READOUT_TASK condition itself,
which is what keeps the re-arm off Portduino and software-polled DIO1, where
there is no task to re-arm from.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* Guard updateNoiseFloor()'s chip reads, and clear a handoff's error flags
Two findings from CodeRabbit's re-review of #11983.
updateNoiseFloor() runs from the main loop on every iteration and made three
RadioLib calls - isActivelyReceiving(), isIRQPending() and getCurrentRSSI() -
with no RadioSequence, which is the same gap
|
||
|
|
40a0178157 |
fix(nodedb): keep deferred node saves pending instead of dropping them (#11930)
* fix(nodedb): persist newly discovered nodes saveNodeDatabaseToDisk() is reached from the deep sleep path (sleep.cpp), the shutdown path (Power.cpp) and the classic screen's menu handler. A node that runs with power.is_power_saving disabled and a UI other than the classic screen reaches none of them, and a graceful reboot does not save either: AdminModule::reboot() only arms rebootAtMsec, and Power::reboot() notifies observers (only InkHUD listens) before ESP.restart(). nodes.proto therefore keeps whatever was written on first boot, usually just the local node, and every peer learned since is dropped on restart. Because the peer's public key lives in its node entry, PKI direct messages to that peer then fail until it is heard again: Loaded saved nodedatabase v25: 1 nodes Received a NAK, error reason: PKI_SEND_FAIL_PUBLIC_KEY Save when a node is first added, rate limited to once a minute. * fix(nodedb): keep deferred node saves pending instead of dropping them The previous commit misread the save paths: updateUser() already saves the node DB when a peer's user info changes. But it saves at most once a minute, and a change inside that window is dropped, not deferred: it logs "Defer NodeDB saveToDisk" and nothing retries it. lastNodeDbSave starts at 0, so every change in the first minute after boot is dropped, and a graceful reboot has no save point. A later save writes the whole DB and catches up, so the loss shows when nothing else changes before the restart. This removes the save on admission (nodes without user info are dropped by cleanupMeshDB() at boot anyway) and instead: - marks the node DB dirty when updateUser() records a change; - clears the flag only after nodes.proto is actually written, so a save that was skipped or deferred (no keypair yet, xmodem busy, unsafe power) stays pending; - retries a pending save from the main loop as a single attempt that shares updateUser()'s lastNodeDbSave budget, so no new write trigger is added and the worst case stays one write a minute; - saves a pending change from a notifyReboot observer on graceful reboot. Addresses both CodeRabbit findings on #11930. * fix(nodedb): skip the deferred node save while encrypted storage is locked; test it saveNodeDatabaseIfDirty() and the reboot observer call saveNodeDatabaseToDisk() directly, so they missed the locked-storage skip in saveToDiskNoRetry() and logged a failed encrypted write every minute on a locked MESHTASTIC_ENCRYPTED_STORAGE build. Both now apply the same check. test_nodedb_save_retry pins the deferred save: a change held back by the one-minute throttle is written once the minute passes, and a reboot writes it at once. --------- Co-authored-by: marshallumsted <bot@marshallumsted.dev> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Jonathan BennettandClaude
|
364a111f4a |
Uplink PKI DMs the gateway relays but cannot decrypt to MQTT (#12110)
Since the packet authenticity gate (
|
||
|
James Rich
|
023164a7ea |
Clamp an imported private key before deriving its public key (#12095)
Curve25519::eval uses the scalar as given while XEdDSA::priv_curve_to_ed_keys clamps it, so a private key set through admin, an app or a backup restore without clamping derived an X25519 public key that disagreed with the signing key, and peers rejected every signature the node sent. regeneratePublicKey now clamps the key in place, so the stored key, public key and node number agree. |
||
|
|
95c57c27b2 |
ch341 server fixes (#12016)
* LoRa LBT: fix SX128x/SX126x CAD encoding + slot model, ESP32 sleep race
Three fixes from the "are we really listening?" LBT audit:
- SX128x CAD: pass RADIOLIB_SX128X_CAD_ON_4_SYMB instead of the raw count 4.
The raw value set bits 7:5 = 000, i.e. a 1-symbol scan, leaving LBT nearly
blind on 2.4 GHz. Now runs the intended 4-symbol CAD.
- SX126x CAD / slot time: name the symNum value explicitly as CAD_ON_4_SYMB
(identical 0x02 raw value, so no on-air change) and add a virtual
getCadSymbolCount() so computeSlotTimeMsec() models the real ~4.5-symbol
scan for the SX126x/LLCC68/STM32WL family only, leaving LR11x0/SX127x at
their correct 2.5 symbols. Fixes the too-short contention-window slots.
- ESP32 light sleep: if the DIO1/RF95_IRQ line is already asserted just before
esp_light_sleep_start(), skip the sleep so the main loop services the pending
packet instead of stranding it for up to a full sleep chunk (the
detachInterrupt-clears-the-GPIO-wake disarm race).
clod helped too
* LoRa LBT: keep software-poll preamble flags latched while idle (anti-stomp)
Finding #10 from the "are we really listening?" audit, LORA_DIO1_SOFTWARE_POLL
boards (meshnology-w10).
The 1 ms poll cleared latched PREAMBLE_DETECTED|HEADER_VALID whenever they were
the only bits set, so for the rest of a reception isActivelyReceiving() read idle.
A TX queued during the payload phase then passed the send gate and startTransmit
overwrote the shared chip buffer, losing the inbound packet.
That clear only exists to stop a noise preamble from permanently blocking our own
TX (starvation). Starvation can only happen when we actually have something to
send, so gate the clear on a non-empty txQueue. While the queue is empty the bits
stay latched, isActivelyReceiving() reports the in-progress packet, and a later TX
defers instead of stomping it. A queued TX still gets the escape hatch. The poll
cadence is a fixed 1 ms reschedule either way, so latched bits don't spin the loop;
readData() clears everything once a real RX_DONE arrives.
clod helped
* LoRa LBT: optional RSSI energy second-gate before TX (default off)
Findings #4/#5 of the "are we really listening?" audit.
CAD only detects a preamble. A neighbour's packet whose preamble already aired -
during our own TX, a CAD scan, or a standby gap - is mid-payload and invisible to
CAD, so we key up on top of it. Add an RSSI energy check as a second listen-before-
talk gate in the TX-decision path: channelBusyByRSSI() compares the instantaneous
in-RX RSSI against the rolling noise floor (getNoiseFloor() + margin) and defers if
the channel is hot. It reads RSSI before isChannelActive()'s setStandby() via the
|| short-circuit, so the sample is taken while still in RX where GET_RSSI_INST is
valid - no mode juggling.
Orthogonal to CAD: CAD has processing gain and catches weak preambles below the
noise floor; RSSI catches strong energy at any packet phase, including the mid-
payload case CAD is blind to. Neither subsumes the other.
DEFAULT OFF. A mis-set threshold makes a node read "busy" on noise and stop
transmitting entirely, which is worse than the bug, so it must be tuned on hardware
before it ships live. Compiled out unless LORA_RSSI_LBT_MARGIN_DB is defined at
build time; when off it is a compile-time `return false` (zero behaviour change).
Fails open: returns not-busy unless in RX with a full noise-floor window and a sane
dBm reading, so it can never persistently silence a node.
Native build (gate off) links clean.
clod helped
* LoRa LBT: RSSI margin profiler to tune RSSI_LBT_MARGIN_DB (default off)
Instrumentation to pick the #4/#5 RSSI second-gate margin from measured data
instead of guessing. Build with -DLORA_RSSI_LBT_PROFILE; compiled out otherwise.
It accumulates (RSSI - rolling noise floor) in two channel states:
- idle (reuses updateNoiseFloor()'s existing RX sample, no extra SPI): the noise-
jitter distribution. Its high tail is the floor the margin must clear.
- signal (decoded-packet RSSI, hooked in handleReceiveInterrupt): the real-traffic
rise. Its low tail is the ceiling the margin must stay under.
A throttled log dump prints both distributions (min/mean/max + percentiles) and the
derived usable window (idle p99, signal p05).
Placement: the stats/histogram engine is a dependency-free, radio-agnostic header
(RssiProfiler.h) so it stays out of the core file and is testable on its own; the
two hooks and the RSSI/log I/O live on RadioLibInterface where the state is.
Independent of the gate flag (LORA_RSSI_LBT_MARGIN_DB) on purpose: profile first to
choose the margin, then build the gate on. Native build (profiler on) compiles clean.
clod helped
* docs: describe the RSSI LBT margin profiler
Feature doc for the RSSI listen-before-talk margin profiler
(-DLORA_RSSI_LBT_PROFILE): what it measures (idle noise-jitter vs decoded-signal
distributions of RSSI-minus-noise-floor), how it's wired (RssiProfiler.h engine +
RadioLibInterface hooks, idle sample reused from updateNoiseFloor), how to build and
run it, and how to read the log output to choose RSSI_LBT_MARGIN_DB.
clod helped
* LoRa LBT: draft #3/#6 fixes behind MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE (inactive)
Firmware side of the CAD->RX handoff (#3) and ISR_RX re-arm (#6) from the LBT
audit. Both need to re-arm RX WITHOUT the standby() that startReceive() forces
(which aborts an in-flight reception) - unavailable in stock RadioLib, provided by
resumeReceive() in NomDeTom/RadioLib@rxResume.
Guarded by MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE, left UNDEFINED: against stock
RadioLib these paths compile out and behaviour is unchanged. Define it (via
build_flags) only when linking a RadioLib that has resumeReceive().
- resumeReceiveInPlace(bool startRx): base-class helper - iface->resumeReceive()
(remap RX IRQ, optional SetRx, no standby) + re-attach the MCU ISR + base RX
bookkeeping. One place, chip-agnostic via iface.
- #3: SX126x/LR11x0 isChannelActive() CAD exitMode -> GOTO_RX; the TX-decision path
splits the busy check into rssi/cad flags and, on a CAD detect, hands the packet
to RX via resumeReceiveInPlace(false) instead of startReceive(). Macro-off, the
split is exactly the old `channelBusyByRSSI() || isChannelActive()` short-circuit.
- #6: ISR_RX re-arms via resumeReceiveInPlace(true).
Open (flagged in-code, for review + HW validation): whether re-issuing SetRx (#6)
disturbs an on-chip in-flight reception, and the #3 handoff's interaction with a
beacon-target packet on the home config.
clod helped
* LoRa LBT #4: synthesize GPIO wake cause when light sleep is skipped
doLightSleep() skips esp_light_sleep_start() when a LoRa IRQ is already
asserted, but still read esp_sleep_get_wakeup_cause() unconditionally - so
the skip path returned the previous real sleep's cause (e.g. TIMER) and
missed the ESP_SLEEP_WAKEUP_GPIO branch that sets runASAP. Capture whether we
skipped and return a synthetic GPIO cause in that case so PowerFSM takes the
packet-service path. Reads the ESP-IDF cause only after an actual sleep.
clod helped too
* LoRa LBT #10: keep HEADER_VALID latched in the software IRQ poll
The LORA_DIO1_SOFTWARE_POLL anti-starvation escape hatch cleared both
PREAMBLE_DETECTED and HEADER_VALID whenever a TX was queued. HEADER_VALID is
evidence of a real inbound frame, so clearing it made isActivelyReceiving()
read idle mid-payload and let the queued TX stomp the frame. Gate the hatch on
a bare-preamble IRQ (PREAMBLE set, HEADER_VALID clear) and clear only
PREAMBLE_DETECTED. A stuck header still can't block TX forever - receiveDetected()
ages it out after maxPacketTimeMsec - and readData() clears everything on RX_DONE.
clod helped too
* LoRa LBT #3/#6: CAD->RX handoff + continuous-RX re-arm on stock RadioLib
Reimplements the two audit fixes that previously needed the NomDeTom/RadioLib
@rxResume fork (MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE), using only stock
RadioLib, behind a new default-off flag MESHTASTIC_LBT_CAD_TO_RX. Per the
RadioLib maintainer's feedback, no new RX API is needed.
(GOTO_RX) with the RX IRQs pre-mapped in the scan config, so the chip's own
CAD->RX transition delivers RX_DONE with no library call in between. irqFlags
(status-enable) carries the CAD verdict + full RX set incl. PREAMBLE/HEADER so
isActivelyReceiving() can read them; irqMask (DIO trigger) carries the CAD
verdict + terminal RX events only - NOT PREAMBLE/HEADER, which would fire the
ISR mid-frame and run readData() on an incomplete packet. On detection it hands
off in place: clear the latched CAD verdict, re-attach the MCU ISR, mark
receiving - no standby (which would abort the packet just detected).
standby), so re-attach the MCU ISR instead of re-issuing SetRx (which could
disturb a second packet already arriving on-chip).
Removes resumeReceiveInPlace() and the fork macro. SX126x-only: LR11x0's
startChannelScan conflates irqFlags/irqMask (drives the DIO from irqFlags), so
it keeps the safe GOTO_STDBY path, re-arming on detect to stay coherent with
onNotify when the flag is on.
Default-off: needs on-hardware validation and assumes continuous RX (the
default preamble=16/minSymbols=8 degenerates startReceiveDutyCycleAuto to
continuous). Native builds pass both flag-off and flag-on.
clod helped too
* LoRa LBT #1: re-arm RX unconditionally when deferring a busy channel
The post-CAD/RSSI defer path wrapped its startReceive() re-arm in a
!hasTargetRadioSettings(txp) guard, so a beacon-target packet that hit a busy
channel took setTransmitDelay() with the radio left in standby. When the
beacon's target config coincides with the home config the radio is on the home
channel, so the node went deaf on home traffic until the next TX cycle - and
kept skipping the re-arm every cycle the channel stayed busy. Drop the guard:
re-arm unconditionally and keep listening while we wait to key up. A beacon's
job is to get a packet out, but it should still mirror the main RX path.
(No effect under MESHTASTIC_LBT_CAD_TO_RX, where the chip is already left in RX.)
clod helped too
* docs: fix RSSI profiler fences (MD040) + RAM figure, note negative-bin limit
- Tag the formula and sample-output fenced blocks as `text` so markdownlint
MD040 passes.
- Correct the RAM figure: RssiDeltaStats is 148 bytes (12 scalar + 34*4
histogram), so two instances are ~296 bytes, not ~280.
- Add a limitations note that all negative deltas collapse into bin[0], so a
percentile landing there returns the minimum sample. This does not affect the
margin (keyed off the always-positive idle p99) but can understate signal p05
when a node decodes below-floor packets; never biases toward a smaller margin.
clod helped too
* docs: RSSI profiler - record histogram bucketing design choices
Add a "Histogram bucketing" section laying out why a fixed histogram is used
over exact percentiles, what the axis has to resolve (the margin window sits at
roughly -a few..+20 dB), and the arguments for five bucketing schemes: the
current positive-only layout (A), a cost-neutral re-centred layout with
negative-range resolution (B, recommended if signal p05 must be trusted),
wider full-range bins (C), more bins (D), and exact/off-box (E). Explains the
current choice and when to switch.
clod helped too
* LoRa LBT: move RSSI profiler + energy second-gate to listening-now-rssi
Split the RSSI work off this branch so the active CAD/sleep fixes and the #3/#6
CAD->RX rework can land without the tuning-dependent RSSI pieces. Removed here
(preserved intact on branch listening-now-rssi):
- RSSI margin profiler: RssiProfiler.h, the LORA_RSSI_LBT_PROFILE hooks, and
docs/rssi_lbt_margin_profiler.md.
- RSSI energy second-gate: channelBusyByRSSI() and the LORA_RSSI_LBT_MARGIN_DB
gate.
The pre-existing noise-floor sampler (updateNoiseFloor/getNoiseFloor, already on
develop and consumed by telemetry) is untouched. The TX-decision block reverts
to a plain CAD scan; #3/#6 (behind MESHTASTIC_LBT_CAD_TO_RX) and #1 unchanged.
Native build green.
clod helped too
* reduce to bare essentials
* LoRa LBT: calibrate CAD symbol count and detPeak against vendor data
NUM_SYM_CAD carried two incompatible meanings: computeSlotTimeMsec() read it
as a literal symbol count, while the drivers passed it into scanChannel(),
where symNum is a register encoding on SX126x/SX128x. On SX126x the raw 2
encodes CAD_ON_4_SYMB, so the chip has scanned 4 symbols since #5749/#5772
while the slot time was priced at 2.5 - a 1.8x mis-scaling of the CSMA
backoff unit, live since January 2025.
Give the constant one meaning (a literal count of 4) and have each driver
translate at its own call site. getCadSymbolCount() reports the real scan
length so slotTimeMsec matches it; RF95 overrides it because SX127x CAD is
fixed in hardware and its legacy scanChannel() takes no symNum.
AN1200.48 Table 1 recommends 4 symbols for SF9-SF12, which is where most
shipped presets sit, so 4 is the right target. The comment claiming 2 was
reading that note's SF7/SF8 row.
detPeak is calibrated per (SF, symbol count), and on LR11x0 per bandwidth
as well. Both LR drivers ran thresholds meant for a 2-symbol scan, leaving
them less sensitive than measured-best and biased toward missed detections.
Now sourced explicitly:
LR20x0 LR20xx datasheet rev 2.2 Table 6-19, indexed by symbols x SF
LR11x0 Semtech SWSD003 lr11xx/apps/cad/main_cad.c optimized_parameters,
indexed by symbols x bandwidth x SF
SX126x RadioLib's SF+13, which sits at or below every AN1200.48 value and
so errs toward deferring rather than transmitting
Combinations the vendor data does not characterise fall back to the RadioLib
default rather than guessing. LR2021 also pins fastCad, which scanChannel()
reads off the radio object rather than the config struct.
Also fix the 2.4 GHz slot term: (2 * sf + 3) / 32 is integer division that
truncates to 0 for every legal SF, so the AN1200.22 fractional term never
contributed anything.
Not hardware-verified.
* LoRa LBT: drop audit-doc pointers and cross-chip asides from comments
The LBT comments referenced an external audit document by item number (#1,
#3, #3/#6, #10, and a see-.notes/lbt pointer), which means nothing to anyone
reading the tree on its own. They also described what sibling drivers do,
which goes stale silently the moment one of them changes and belongs in that
driver's own file anyway.
Keep the reasoning, drop the pointer and the comparison. Where a comment
explained a rule that genuinely spans drivers - NUM_SYM_CAD's register-
encoding caveat - state it as a rule rather than a roll-call of chips.
No behaviour change; comments only.
* LoRa LBT: don't index the LR11x0 CAD table with a 2.4 GHz bandwidth
The detPeak lookup picked its bandwidth row by range, so wideLora's 406.25 kHz
fell into the sub-GHz 500 kHz row - SWSD003's table is the sub-GHz LoRa modem
(BW enums 62.5-500), and 2.4 GHz is a separate modem with its own calibration.
Match the four bandwidths exactly instead, so anything unmatched falls back to
the RadioLib default rather than borrowing a row it has no claim to.
Those four are the whole sub-GHz set today - LR11x0::setBandwidth() rejects
anything else - so note that a newly unlocked bandwidth, or a symbol count
outside 2/4/8/16, needs a row adding here and fails silently if it doesn't get
one. Same note on LR20x0 for symbol counts above 4.
Also correct two citations. The 2.4 GHz slot term is the SX1280 datasheet's own
CAD duration formula, and the CAD symbol-count guidance is AN1200.77, not
AN1200.22 - which is "LoRa Modulation Basics" from 2015 and predates the part.
Record that SX1280 ignores detPeak/detMin/exitMode entirely: SetCadParams
carries only cadSymbolNum, and the PNR threshold lives in register 0x942.
* LoRa LBT: set the SX1280 CAD PNR threshold and scan 8 symbols
SetCadParams on this part carries only cadSymbolNum - detPeak/detMin in the
scan config are inert - so CAD sensitivity is governed entirely by register
0x942, which nothing was writing. The scan therefore ran at the chip's reset
value of 0x32, against the 19-36 that AN1200.77 Table 1 calibrates per SF and
bandwidth: at LONG_FAST wideLora the recommendation is 27 and we were using 50.
A threshold that high needs a much stronger correlation peak, so 2.4 GHz LBT
was under-detecting on top of the symbol-count bug already fixed.
Write the table value before each scan, indexed by bandwidth x SF. Unmatched
bandwidths write nothing and leave the chip's default, as on the LR parts.
This goes through our own Module rather than lora.writeRegister(), which
RadioLib keeps protected unless RADIOLIB_LOW_LEVEL unlocks raw register access
across every driver. SX128x::writeRegister() is a straight SPIwriteRegisterBurst
on the same Module we already own, and begin() has pointed spiConfig at the
0x18 WriteRegister opcode long before any scan runs, so the transaction is
identical - no subclass needed.
Also raise the CAD window from 4 to 8 symbols, matching RadioLib's own SX128x
default. AN1200.77 measures a longer window as moving P(Detection) further from
P(False Detection); its thresholds are stated to hold for any window duration,
so the table above is unaffected. NUM_SYM_CAD_24GHZ moves in step so the slot
time still reflects the real scan.
Not hardware-verified.
* LoRa LBT: hand CAD straight to RX on the LR parts
CAD exited to standby, and the busy-channel path then issued a full
startReceive() - which standbys the radio and re-enters RX from scratch,
throwing away the reception CAD had just detected. Both LR families expose a
CAD-to-RX exit mode that avoids the round trip, and it reaches the chip through
the public startChannelScan(cfg), so nothing here needs a library patch.
A note in an earlier commit claimed this was impossible because RadioLib
collapses irqFlags and irqMask into one set. That reasoning is SX126x's, where
irqMask is a status gate and an event omitted from it never latches at all.
The LR parts have no such gate: SetDioIrqParams takes a mask per interrupt pin
and SetDioIrqConfig maps IRQs to one DIO, while getIrqStatus() reads the status
bytes returned on every SPI transaction and consults no pin mapping. So the CAD
IRQ set now carries the verdict plus terminal RX events only - preamble and
header stay off the pin, where they would fire the ISR mid-frame and run
readData() on a partial packet - and isActivelyReceiving() still sees them.
rearmReceive() cannot be unconditionally standby-free the way SX126x's is.
cad_timeout 0 means "RX until one packet arrives, then standby", so the two
re-arm paths leave the chip in different states: already listening after the
handoff, back in standby after that packet's RX_DONE. It branches on a flag set
at detection and cleared on use, so the first re-arm skips the standby and the
next restores continuous RX. Forcing continuous RX through the timeout instead
is not reliable - RadioLib converts it to RTC ticks through a float divide, so
the 0xFFFFFF "no timeout" encoding is not addressable.
On a CAD false positive the chip now waits in single-shot RX rather than
returning to standby. Still listening, but a state the old code never entered.
Not hardware-verified.
* LoRa LBT: size the 2.4 GHz CAD slot per chip, not per band
computeSlotTimeMsec() branches on myRegion->wideLora, which is a region rather
than a part. LR1120 and LR2021 both run there, and both scan NUM_SYM_CAD
symbols - but that branch read the flat NUM_SYM_CAD_24GHZ, so raising it to 8
for SX128x moved the slot for two families whose scan never changed. At
LONG_FAST wideLora that priced ~22 ms of CAD against a ~10 ms scan.
getCadSymbolCount() exists precisely so the slot tracks the real scan, but only
the sub-GHz branch consulted it. Use it in both, and give SX128x an override
returning NUM_SYM_CAD_24GHZ. That constant stops being a fleet-wide 2.4 GHz
value and becomes what one driver reports; SX128x is unchanged at 8.78 symbols,
while the LR parts on 2.4 GHz drop from a wrong 8.78 to 4.78.
The error was conservative - slots too long, so throughput lost rather than
collisions caused - but it is the same scan/slot mismatch the rest of this work
removes.
* LoRa LBT: declare CAD symbols per chip and per band, 8 on 2.4 GHz
A 2.4 GHz region is shared: an LR1120 and an SX1280 can sit on the same mesh.
Leaving the LR parts at 4 symbols there while SX1280 scanned 8 gave them
different slot times, and so a different CSMA backoff scale between neighbours
- the cross-family asymmetry this branch removed for sub-GHz, reintroduced one
band up. Both LR families now scan 8 on 2.4 GHz, matching SX1280.
Rather than switch on the band inside a shared constant, each driver states its
own count for each band it works in, and derives the symNum it passes from the
same accessor - so the scan and the slot cannot drift apart:
SX126x 4 sub-GHz
SX128x 8 on 2.4 GHz
LR11x0 4 sub-GHz, 8 on 2.4 GHz
LR20x0 4 sub-GHz, 8 on 2.4 GHz
RF95 2 sub-GHz, its CAD being fixed in hardware and not settable
NUM_SYM_CAD and NUM_SYM_CAD_24GHZ become base-class defaults nothing falls
through to; they now record the fleet convention rather than any part's
behaviour.
LR20xx Table 6-19 stops at 4 symbols, so an 8-symbol scan reuses that row -
the last measured point, erring high and so slightly less sensitive than a
true 8-symbol value. Noted at the table.
Also a comment pass over the branch: drop retrospective phrasing that describes
what the code used to do, cut trailing justification clauses, and fix comments
the per-band change left stale - one still claimed the LR parts scan fewer
symbols than SX1280 on 2.4 GHz, which is now the opposite of the truth.
* LoRa LBT: sweep up a CAD->RX handoff that delivers nothing
A handoff leaves the chip listening by its own exit mode, so nothing in the
driver arms the radio and no return value says whether it worked. If the
detection was false, or the exit mode was not honoured, the node waits in a
state we never verified - and on the LR parts that is single-shot RX, which
does not return to continuous listening on its own.
Stamp the handoff and let the existing ~1s poll re-arm if a whole max-length
airtime passes with no packet, the same bound receiveDetected() uses. Clearing
it on a real RX_DONE is obvious; clearing it in startSend() is not, but a TX
can intervene before any packet arrives and its completion re-arms RX anyway,
so leaving the marker set would have the poll re-arm a radio already listening.
Reads the clock through Time::getMillis(), the same source Throttle compares
against, so the site stays testable across the millis() wrap. 0 is the
"none outstanding" sentinel and is tested before the elapsed compare.
Also document on rearmReceive() that an override must never call the chip's own
startReceive() - its standby+SetRx aborts the packet CAD just detected, which
is the entire reason the override exists.
* define txen pin to allow sx128x testing
* LoRa LBT: bound the CAD->RX handoff in hardware, and re-arm after it delivers
cadTimeout only applies to the CAD->RX exit mode. Every driver on develop
exited CAD to standby, so the field was ignored and its value - 0, which no
datasheet defines - was dead data. Switching the exit mode to RX made it
load-bearing while the line itself never changed, so the diff showed nothing.
All three handoff parts now bound that RX at one max-length airtime, taken
from getPacketTime() so it tracks the preset. A detection that delivers
nothing expires to standby and raises TIMEOUT, already mapped to the pin, so
the existing ISR re-arms and the queued TX retries on its own backoff.
handleReceiveInterrupt() returns early on TIMEOUT without RX_DONE; otherwise
each dud detection would reach readData() and be logged as a bad packet.
That RX is single-shot, so the arm after it delivers must be a full one.
SX126x re-armed unconditionally without a standby, which left the chip in
standby while the interface believed it was listening - and nothing swept it,
since RX_DONE has already cleared the handoff stamp. A node that received a
handoff packet then went deaf until its next transmit. It now carries the
same cadHandedToRx flag the LR drivers already had.
Sourcing, since the parts disagree and RadioLib's uniform ChannelScanConfig_t
hides it: SX126x DS_SX1261-2 p98 (15.625us steps), LR11x0 UM Table 8-8
(31.25us), LR2021 lr20xx_driver cad_timeout_in_pll_step (31.25us, contra the
datasheet's "32 MHz periods"). RadioLib scales both LR parts as 30.52us, so
the programmed value lands ~2% long - harmless, and left for upstream.
Also marks the three SWSD003 cells at 2 symbols/BW500/SF10-12 as that SDK's
fallback rather than measurements.
* LoRa LBT: own the CAD->RX handoff in one place, and void it on standby
The handoff state was split across two classes: cadHandoffRxStart in
RadioLibInterface, cadHandedToRx in each of SX126x/LR11x0/LR20x0, whose
rearmReceive() overrides were byte-identical. Neither field was cleared by
setStandby(), so a standby taken while a handoff was outstanding could strand
cadHandedToRx set - and the next rearmReceive() would then re-attach the ISR to
a chip sitting in standby, marking the interface as receiving with no RX_DONE
ever to come and no recovery path. A stale cadHandoffRxStart was milder but
still re-armed the radio over whatever was arriving at the time.
Hold both in the base, arm them together in noteCadHandoffToRx(), clear both in
setStandby(), and collapse the three overrides into one non-virtual base
rearmReceive(). SX128x and RF95 never arm the flag, so they keep taking the
plain startReceive() branch. Document on isChannelActive() that a true return
must be followed by rearmReceive() - the invariant the whole scheme rests on.
Two fixes fall out of the shape:
- The handoff re-arm attaches a rising-edge ISR to a line that is not known to
be low, unlike startReceive() which clears the IRQ status first. Follow it
with checkRxDoneIrqFlag(), as startReceive() already does.
- checkCadHandoffTimeout() used one max-length airtime, the same figure the
chip's own cadTimeout now uses, so the two expired together. Losing that race
left a queued ISR_RX to run against flags the software path had just cleared,
falling past the TIMEOUT guard into readData() on an empty buffer. Double it
so the hardware always expires first; the backstop then only covers an RX
whose timer stopped on a header that never completed.
* LoRa LBT: re-arm RX when a queued packet is dropped for tx_enabled=false
Pre-existing on develop, not introduced by this branch, but it is the same
failure this series exists to remove. TRANSMIT_DELAY_COMPLETED reaches
startSend() only through isChannelActive(), which leaves the chip in standby
for the transmit about to follow. When that transmit is then dropped because
LoRa TX is disabled, the early return frees the packet and returns without
arming anything: isReceiving stays false, so pollMissedIrqs() skips the radio
too, and the node is deaf until something else re-arms it.
Narrow - tx_enabled has to change between enqueue and dequeue, since send()
checks it as well - but the fix is one call on a path that already exists.
* LoRa LBT: say what the CAD slot time actually does per chip
Two comments in RadioInterface.h described intentions the code does not carry
out.
NUM_SYM_CAD/NUM_SYM_CAD_24GHZ claimed the per-band values keep the CW backoff
scale equal across a mesh. They do not: the slot has to match the scan each part
really runs, and RF95's CAD is fixed in hardware at 2.25 symbols while
SX126x/LR scan 4. A mixed sub-GHz mesh therefore runs two slot lengths, which
is correct per part and worth stating rather than implying otherwise.
The slotTimeMsec initializer runs during RadioInterface's own construction, so
the virtual getCadSymbolCount() it calls always resolves to the fallbacks here,
never a driver override. That is harmless only because applyModemConfig()
recomputes it before anything reads it - left as-is deliberately, since
initializing to 0 would make random(0, 2^CW) * slotTimeMsec fire an immediate
transmit if anything ever did read it early.
* LoRa LBT: set the SX128x CAD threshold at config time, not per scan
isChannelActive() looked up the AN1200.77 PNR row and wrote register 0x0942 on
every scan, but sf and bw only change in applyModemConfig(). That is an SPI
round trip between the channel scan and the transmit that follows it, on the
one path this file's own comments ask to keep short.
Move it to applyCadDetPeak(), called from reconfigure() after the sf/bw setters
it depends on, and from init() as well - init() does not route through
reconfigure(), so it needs its own call.
* ESP32: stop skipping light sleep forever on a stuck LoRa IRQ
The skip added earlier in this series returns instead of sleeping whenever
DIO1 already reads high, so the main loop can service the packet rather than
strand it for the sleep chunk. It had no bound: a line that latches high with
nothing clearing it makes every subsequent call skip too, and the node quietly
stops sleeping at all, announced only by a LOG_INFO each time.
Count consecutive skips - warn at 3, sleep through at 5, reset on any real
sleep. If the IRQ has survived five attempts the main loop is not going to
clear it, and never sleeping costs more than one stranded packet.
Also drop the LORA_DIO1_EXTENDED_IO branch in loraWakeIrqAsserted(): those
boards return from doLightSleep() before it is ever called.
* nrf52 promicro: gate the P1.06 TXEN pin, and add an optional SX1280 build
P1.06 was wired to SX126X_TXEN unconditionally earlier in this series, to drive
an external PA on a bench unit. That is not safe to ship: three envs share this
variant.h, and P1.06 is PIN_WIRE_SCL on the EasyProMicro pinout and
PIN_EINK_BUSY on the InkHUD one. The default builds go back to RADIOLIB_NC.
Put it behind -D PROMICRO_SX126X_TXEN, refused wherever the pin is already
claimed. The EasyProMicro arrangement has its own SX126X_TXEN on P1.00 in the
#else branch and never reaches this, so only the e-ink exclusion is needed.
The pin was wanted for SX1280 in the first place - every other SX128X line is
already assigned on this board and TXEN was the one missing - so add that too,
behind -D PROMICRO_SX1280 and a new nrf52_promicro_diy-sx1280 env. Both opt-ins
claim P1.06, so asking for both is an #error rather than a silent pick.
Costs about 10.7 KB: the image ends at 0xE5140, 19 KB clear of the warm region,
against 0xE2710 and 30 KB for the base env.
* LoRa LBT: fix two defects in the CAD->RX handoff found in review
Both reported by Copilot against 1fcd219af, both confirmed against the code as
it stands after the handoff was hoisted into RadioLibInterface.
rearmReceive() armed the MCU ISR before marking the interface as receiving. An
ISR firing in that window queues ISR_RX while isReceiving is still false, so
handleReceiveInterrupt() bails with "called while not in rx mode" and the packet
is dropped - the exact packet the CAD handoff existed to catch. Reversed, which
also makes this path match the order the drivers' own startReceive() has always
used: base state, then arm, then checkRxDoneIrqFlag().
noteCadHandoffToRx() stored Time::getMillis() raw, but 0 is the "no handoff
outstanding" sentinel that checkCadHandoffTimeout() tests before its elapsed
comparison. A handoff armed on the one millisecond per wrap where getMillis()
returns 0 reads as "none outstanding" and the backstop never fires. Step past
it to 1, the same hand-remap Throttle.h's own TODO records GPS.cpp using for
fixHoldEnds. One millisecond of error, once per 49.7 days.
* ESP32: wake on SX128X_DIO1, which five SX1280 boards define instead of LORA_DIO1
enableLoraInterrupt() only ever looked at LORA_DIO1 and RF95_IRQ. RadioInterface
builds an SX1280Interface from SX128X_DIO1, and several variants name the IRQ
pin only that way, or put it on a different pin from LORA_DIO1:
tlora_v2_1_18 LORA_DIO1 undefined SX128X_DIO1 26
betafpv_2400_tx_micro LORA_DIO1 undefined SX128X_DIO1 4
t-eth-elite LORA_DIO1 16 SX128X_DIO1 8
tlora_t3s3_v1 LORA_DIO1 33 // TCXO_EN? SX128X_DIO1 9
tlora_t3s3_epaper LORA_DIO1 33 // TCXO_EN? SX128X_DIO1 9
Every branch of the function is gated on defined(LORA_DIO1), so on the first two
it preprocessed to an empty function: esp_sleep_enable_gpio_wakeup() still
enabled GPIO as a wake class, but no pin was ever armed for the radio. On the
other three it armed the wrong pin - GPIO33 there being the line the variant's
own comment questions as TCXO_EN. Those boards could not be woken from light
sleep by an incoming packet; they waited out the 30s chunk and pollMissedIrqs()
recovered the latched RX_DONE about a second after waking. Delayed, not lost,
but deaf for the duration.
loraWakeIrqAsserted(), added earlier in this series, inherited the same blind
spot and was dead code on the two boards with no LORA_DIO1 at all - so the
skip-the-sleep fix never once fired on an SX1280.
Resolve the pin per radio in loraWakeDio1Pin() and use it for arming, polling
and teardown alike, so the three cannot drift apart again, and widen their
guards to accept either macro. Behaviour is unchanged wherever it already
worked: for any non-SX1280 radio the helper returns LORA_DIO1, including on
RF95 boards where the SOC_PM_SUPPORT_EXT_WAKEUP branch arms it regardless of
radio type. LORA_DIO1_EXTENDED_IO still short-circuits first.
Note this is a power-profile change for those five boards, not only a fix: they
gain a LoRa wake source they have never had, so on a busy channel their light
sleep will now be cut short by traffic like every other board's.
* LoRa LBT: trim three-line comments to the repo's two-line limit
copilot-instructions.md:340 says one or two lines; the P1.06 availability note
and the LR2021 DIO routing note were both three. CodeRabbit flagged both.
* LoRa LBT: trace the CAD scan, its exits and the handoff outcome
Eight short lines, all in the base class so they cost one set of strings
rather than one per chip driver. The scan logs arm/busy/free; the in-chip
GOTO_RX handoff logs started, then exactly one of pkt, empty, timeout or
void - so a dropped or double-counted handoff is visible by inspection.
void and timeout are LOG_WARN because both mean an invariant broke: a
standby discarded a live handoff, or the chip's own timer failed to fire
and the software backstop had to re-arm. The rest are LOG_DEBUG, so a node
at normal log level stays quiet unless something is actually wrong.
The two pre-existing handoff strings were 44 and 41 bytes of flash; cut to
"CAD>RX timeout" and "CAD>RX empty", they pay for most of the new set. The
shared "CAD" prefix does not dedup - only string suffixes merge - and
splitting it out behind a %s measured 8 bytes worse on Cortex-M once the
extra pointer argument per call site is counted.
Also fixes the delivered-packet trace, which sat above the TIMEOUT check
and so claimed a packet on a window that expired empty.
* LoRa LBT: restore the SX1280 CAD threshold after a chip-state recovery
The calibrated AN1200.77 PNR threshold moved from a per-scan register write to
a config-time one, so it now has to be re-applied wherever the chip's state is
rebuilt. recoverChipStateLoss() hardware-resets the chip and reprograms only
the modem params, which left a recovered SX1280 doing CAD against the vendor
default threshold until the next reconfigure() - and on a node whose config
never changes, that is the rest of its uptime.
* LoRa LBT: retire RX flags that plain RX never completes
Plain RX runs with no chip timeout (startReceiveDutyCycleAuto(16, 8)
falls back to RX_TIMEOUT_INF) and the DIO mask carries RX_DONE only, so
a PREAMBLE_DETECTED, HEADER_VALID or HEADER_ERR that never completes
stays latched until something re-arms RX. On a node with nothing to
send, nothing does. The CAD->RX handoff already has this recovery
(cadTimeout plus checkCadHandoffTimeout()); plain RX had none.
checkStaleRxFlags() is its twin on the ~1 s pollMissedIrqs() tick. It
notes when flags are first seen, and if no RX_DONE has consumed them one
max-length packet later:
- a header (valid or errored) re-arms RX. RX_DONE clears every flag in
readData(), so a header still latched has no frame behind it. SX1280
DS rev 3.3 section 16.2: a header error leaves RX wedged with no
RX_DONE or timeout, and only a re-arm recovers it.
- a bare preamble is only cleared. ClearIrqStatus/ClearIrq changes the
IRQ register alone on SX126x, SX1280, LR11x0 and LR20x0, so it never
aborts a reception; startReceive() goes through standby and would.
HEADER_VALID is never cleared on its own: RadioLib's readData() reads
it at RX_DONE, and on LR2021 its absence reports the packet as header
damaged.
Skipped while a CAD handoff is outstanding (its own timeout owns that
RX) and while RX_DONE is pending. RF95 opts out: its carrier sense reads
modem status rather than latched flags, and SX127x maps neither
PREAMBLE_DETECTED nor HEADER_ERR.
The decision is staleRxFlagAction() in RadioInterface.h, pinned in
test_radio at both sides of the window and for each action.
* LoRa LBT: hold TX on a seen preamble instead of judging it by a deadline
receiveDetected() declared a bare PREAMBLE_DETECTED false once
2 * preambleTimeMsec passed without HEADER_VALID - about 8 ms at
SF7/BW500. Nothing reads the flags on that cadence: they are latched
status bits, never routed to DIO, and the only reader is the TX path,
once per CSMA backoff (0-56 ms at SF7/BW500). The verdict was usually
reached by a read older than its own deadline, and the queued TX went
out over the frame. Reported in #11933 by @jp-bennett, whose two-node
bench measured own TX a median 43 ms after the verdict, inside a ~102 ms
frame. This branch's longer CAD slot widens it to LONG_FAST: 0-308 ms of
backoff at CWmin against a 262 ms deadline.
A read that sees a bare preamble now clears PREAMBLE_DETECTED and holds
TX for one max-length airtime. The clear touches only the IRQ register
(ClearIrqStatus/ClearIrq on SX126x, SX1280, LR11x0, LR20x0), so it never
aborts a reception. Reads during the hold report busy, so the normal
random backoff still spreads retries, and any RX interrupt ends the hold
early. A preamble that fires again during the hold stays latched, and
the first read after it holds again: a source that keeps triggering keeps
deferring TX, one airtime at a time, while a single stale flag cannot
block it. HEADER_VALID is never cleared here - RadioLib's readData()
reads it at RX_DONE, and on LR2021 its absence reports the packet as
header-damaged - and keeps its one-airtime ageing.
The SX126x software IRQ poll routes its bare-preamble case through the
same hold instead of clearing on every tick while a TX is queued, which
would have released TX before the next read could see a refire.
That poll read the private txQueue directly, so meshnology_w10 - the
only env that defines LORA_DIO1_SOFTWARE_POLL - did not compile.
hasQueuedTx() exposes the one fact it needs.
#11937 addresses the same defect by deferring the verdict when the
previous read was stale; the two conflict in receiveDetected().
* LoRa: re-arm RX before logging the packet that just went out
After TX_DONE the chip sits in standby, deaf, until startReceive(). The ISR_TX
path used to log airtime ("Packet TX") and printPacket() the sent packet
first, which kept it deaf for ~2-5 ms more depending on where the log goes.
Measured on an nRF52 SX1262 bench node: the TX_DONE-to-RX gap drops from
~8.8 ms to ~3.7 ms with USB serial logging (~11.3 to ~6.2 ms with the log
also going to a phone over BLE).
completeSending() is split into detachSentPacket(), which clears sendingPacket
and runs packetReleased() so RX re-arms on the home config, and
finishSentPacket(), which does the airtime, counters, log and pool release.
setStandby() callers keep the combined completeSending().
* SX126x: keep the TCXO powered in standby
RadioLib leaves standbyXOSC off, so standby and the post-TX/RX fallback both
use STDBY_RC, which powers a DIO3-controlled TCXO down. Every SetRx and SetTx
after that first waits out the TCXO start-up delay programmed into the chip
(RadioLib's setTCXO() default, 5000 us). On an nRF52 SX1262 bench node the RX
start after TX_DONE took ~6.0 ms at SF7, SF9 and SF11 alike; with
setStandbyXOSC(true) it takes ~0.95 ms, and the TX_DONE-to-RX gap falls to
~1.1 ms together with the previous commit. The result held across the
periodic AGC reset.
Only boards that drive a TCXO from DIO3 change. The cost is the TCXO and XOSC
current while the chip sits in standby, which the firmware keeps brief;
duty-cycled RX still sleeps, and powers the TCXO down, between windows.
* LoRa: skip the periodic AGC reset while a packet is queued
resetAGC() ends in startReceive(), and trySetStandby() first runs
checkNotification() to drain pending events. With a packet queued that can be
TRANSMIT_DELAY_COMPLETED: CAD passes, startSend() keys up, and trySetStandby()
then puts the chip in standby over the transmission it just started.
completeSending() still reports the packet as sent and counts its airtime.
A bench node logged this twice in 20 minutes; one was a packet's last retry,
so it never went on air.
Skip the reset for this tick when a TX is queued. The queue drains within
seconds, and the reset comes round again on the next interval.
* GPS: re-enable on a LoRa config save only when the region was unset
#10386 keeps the GPS off until a region is set and turns it on when a LoRa
config save sets one. The check never looked at the old region, so a GPS the
probe had disabled ("GPS not detected; not present this boot") was
re-enabled by every LoRa config save, preset changes included, and the probe
ran again. The probe blocks the cooperative main loop: on a bench node with
no GPS fitted, TX_DONE handling stalled by 0.34-0.89 s on 4 of 9 packets
after a preset change, leaving the radio deaf in standby for that long.
* SX126x: retry a standby that reports a stale command timeout
After a CAD-to-RX handoff's bounded RX times out, the status byte that
comes back with the next SET_STANDBY still carries command status 0x3
(timeout). RadioLib reports that as RADIOLIB_ERR_SPI_CMD_TIMEOUT although
the chip took the command, so every "CAD>RX empty" was followed by a
failed standby, a failed RX start and a full chip re-init. Send
SET_STANDBY once more on that error; the second one sees a fresh status.
Measured on a CH341 host (SHORT_TURBO): chip re-inits went from 3 to 0,
and all 7 retries returned 0.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: on a CH341 host, pick a running RX back up instead of restarting it
In continuous mode the SX126x stays in RX after RX_DONE, and RadioLib's
readData() never puts it in standby. rearmReceive() restarted it anyway:
a standby and a full startReceive(), about ten commands. On a CH341 host
each command is a USB round trip, and the chip is deaf for all of them.
On a CH341 host (irqPolledOverUsb(): Lora.spidev is ch341), startReceive()
now arms continuous RX and records that in rxArmedContinuous. Any standby
clears it, including the scan's before a CAD-to-RX handoff, so the next
re-arm after one takes the full path. While it holds, rearmReceive()
resumes instead: it clears the terminal IRQ flags, re-attaches the ISR
and checks for an RX_DONE that beat the arm. PREAMBLE_DETECTED and
HEADER_VALID stay latched, since they can belong to the next frame.
Hosts with a real DIO1 interrupt are unchanged.
Measured on a CH341 host (SHORT_TURBO, 3 runs per arm): post-RX deaf
time 17.8 s -> 0.8 s per run, frame miss rate 11.4% -> 8.5% (p=0.024).
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Portduino: settable SX126x TCXO start-up delay
A new Lora.DIO3_TCXO_DELAY_US key reprograms the DIO3 TCXO start-up
delay, which begin() leaves at RadioLib's 5000 us. It applies to an
SX126x with DIO3_TCXO_VOLTAGE set, and takes 1 to 10000 us. Absent or 0
keeps 5000 us, so no existing config changes.
The delay matters after a clear CAD: the chip falls back to STDBY_RC,
which stops the TCXO, so the SET_TX that follows waits out the full
start-up before the PA ramps. That wait sits between the channel
verdict and the air, where another node can key up unheard.
Measured on a CH341 host at 1000 us (SHORT_TURBO):
the launch's BUSY wait 5 -> 1 ms, frame miss rate 6.28% -> 3.63%
(p=0.003), with no decode cost at the observer.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: on a CH341 host, write the TX payload before the scan's verdict
Between a clear CAD verdict and the air, RadioLib's TX staging sends
about ten commands: the buffer write, the buffer base, packet params,
the IQ and sensitivity register fixes, the DIO IRQ map, an IRQ clear and
SET_TX. On a CH341 host each is a USB round trip, and the channel goes
unwatched for all of them.
On a CH341 host, isChannelActive() now writes the payload of the packet
it is scanning for (scanForTx) into the chip's buffer in the scan's
standby, before SET_CAD. The CAD leaves the buffer alone. After a clear
verdict, launchTransmit() sends only SET_PACKET_PARAMS,
SET_DIO_IRQ_PARAMS, CLEAR_IRQ_STATUS and SET_TX, then waits on BUSY as
RadioLib's launchMode() does.
RadioLib's full staging still runs:
- on a busy verdict, a failed standby, or a packet that is not the one
staged;
- for the first TX after a chip reset, sleep or AGC reset, so the
register fixes are in place before any short launch relies on them.
beginSending() is split so that encodeRadioBuffer() can fill radioBuffer
without claiming sendingPacket. Hosts with a real DIO1 interrupt are
unchanged.
Measured on a CH341 host (SHORT_TURBO): the post-verdict staging went
6 -> 1 ms and the verdict-to-air window 15 -> 10 ms; overlapping frames
fell from 32 to 18 a run (p=0.032).
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: on a CH341 host, start the channel scan in three commands
RadioLib's scanChannel() sends six commands before the CAD starts: a
packet-type read, a standby, the DIO IRQ map, an IRQ clear, the CAD
parameters and SET_CAD. Its verdict read adds a second packet-type read.
On a CH341 host each is a USB round trip, and the chip hears nothing
from the scan's standby until the CAD runs.
On a CH341 host the scan now sends the DIO IRQ map, the IRQ clear and
SET_CAD, and the CAD parameters only when they differ from the last ones
sent (they are sent again after a chip reset, sleep or AGC reset). The
standby is already done by trySetStandby() just before. The verdict is
read from the IRQ flags alone. The parameter bytes are the ones
RadioLib's setCad() would send.
Hosts with a real DIO1 interrupt still call lora.scanChannel().
Measured on a CH341 host (SHORT_TURBO), together with the staging in RX
that follows: the deaf part of the scan went 4.87 -> 1.48 ms, and board
frames starting in it 23 -> 0 a run.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: on a CH341 host, stage the TX payload during its backoff, while RX runs
Staging in the scan's standby still leaves the payload write between
the backoff's end and the air. On a CH341 host a 250-byte write takes a
few milliseconds, and in that time another node's frame can start and
be missed, or the two nodes' key-ups can line up.
On a CH341 host the payload is now written while RX still runs:
- A backoff of 5 ms or more is brought forward: the TX timer fires at
once, the radio thread writes the payload, then waits out the rest.
At the scan, the packet is encoded again and compared, and a match
skips the write.
- Otherwise the scan writes it while RX runs, and skips the scan
(reporting busy, with no standby) if a frame is arriving or unread
then, since the standby would abort that frame.
- The launch points the TX base at the staged payload.
SX126x continuous RX writes each frame right after the last one and
wraps at the end of the 256-byte buffer. The payload goes just behind
RX's write point, the last place RX reaches again, or at the top of the
buffer when it does not fit there. Each readout reads the frame's
offset to track the write point, and an RX start resets it to 0.
A frame long enough will still reach the payload. Two checks cover it:
- A frame that lands on a staged payload drops the stage, which is
written again at the next backoff or at the scan.
- A frame that was arriving or unread when a write ended is checked at
its readout. The chip writes a frame at about 0.37 ms a byte, far
slower than our write sweeps, so any damage starts at the first byte
the two share. If that byte is ours the frame is dropped with a WARN;
if it is the frame's, the frame is whole and kept.
Hosts with a real DIO1 interrupt are unchanged.
Measured on a CH341 host (SHORT_TURBO, T1000-E peer, 6 v 6):
- Staging during the backoff: the server's CAD arm to air 10.23 -> 6.23
ms, total loss 4.12% -> 2.77% (p=0.002), the peer's miss of the
server's frames 6.00% -> 1.92%. The server's own miss rose 1.16% ->
4.12%, from frames the stage overwrote at the top of the buffer.
- Then staging behind the write point with the byte check: total loss
2.66% -> 1.51%, the server's miss 3.97% -> 1.16%, frames dropped for a
stage 17.3 -> 0 a run. The byte check matched the observer's copy of
the frame in 30 of 30 cases.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* SX126x: drop a stale stage-overlap marker when RX restarts
A TX stage written while a frame was arriving marks that frame for the
first-shared-byte check at its readout. If RX is restarted from standby
first (startReceive(), or a CAD that hands off to RX), that frame is
gone and the marker outlived it, so a later intact frame could be
checked against the old stage and dropped on a 1-in-256 byte match.
Clear it wherever the write pointer is reset.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* LoRa: time a stale RX header from when the header shows
checkStaleRxFlags() stamps rxFlagsSeenMs at the first poll that sees
PREAMBLE_DETECTED, HEADER_VALID or HEADER_ERR, and re-arms once a header
has been latched for one max packet. If a bare preamble set the stamp
and a real frame's header then arrived, the header was judged against
the older stamp: a poll more than one max packet later re-armed RX in
the middle of that frame and lost it. Restart the stamp when a header
first shows, so a header is only re-armed after it alone has been
latched for a full max packet.
Found by CodeRabbit on #12019.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* test: time a stale RX header from when it shows, through checkStaleRxFlags()
The staleRxFlagAction() cases only pin the decision. This drives
checkStaleRxFlags() with a fake IRQ source and the test clock: a header
that shows after a bare preamble restarts the window, so it is not
re-armed one max packet after the preamble.
Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
* Radio: drop a duplicated STDBY_XOSC block from the develop merge
The merge added the STDBY_XOSC-keeps-the-TCXO-powered block twice at
different offsets in reinitChip(), so every init and recovery sent
SET_STANDBY_XOSC twice and logged it twice - one extra USB round trip on a
CH341 host. Keep one copy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112S3QXU8WUGSCcMQHnwRKZ
* SX126x: keep a staged-over frame's RX_DONE across the resume
A CAD skipped because a frame was in flight set keepRxIrqsAtResume only
when RX_DONE or CRC_ERR was already up. With a header but no RX_DONE yet,
the resume cleared RX_DONE, CRC_ERR, HEADER_ERR and TIMEOUT - and on a
CH341 host the flag read in stageTxInRx() and that clear are a USB round
trip apart, so a frame completing in between lost its RX_DONE. The poll
then had no edge to deliver and the frame waited for checkStaleRxFlags()
to retire the latched HEADER_VALID, one max packet later.
Keep RX_DONE and CRC_ERR for every busy verdict, so the resume's
checkRxDoneIrqFlag() drives the readout. HEADER_ERR and TIMEOUT are still
cleared unconditionally: no readout waits on them, and a stale one costs
checkStaleRxFlags() a re-arm. Only RX_DONE is mapped to DIO1, so leaving
the other terminal flag latched holds no interrupt line.
Found by CodeRabbit on #12016; the blanket keep it proposed would have left
a stale HEADER_ERR or TIMEOUT latched instead.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112S3QXU8WUGSCcMQHnwRKZ
* SX126x: record the stage over a bare preamble, and name the keep's catch
Two changes the r105 bench round asked for, both common to its arms.
noteStagedOverFrame() was gated on HEADER_VALID or a terminal flag, so a
busy verdict on PREAMBLE_DETECTED alone - which the bench observes - recorded
nothing. That frame can still grow into the staged bytes, no later look
records the check, and its readout then has nothing to compare against: a
frame our write damaged is accepted silently instead of being dropped with
the WARN. Every busy verdict now records it. The record is spent on the next
readout, so it cannot outlive the frame it was taken for, and the stage sits
behind the write point where an unrelated frame reaches it only after the
wrap.
"caught missed RX_DONE" cannot say which look found the flag: pollMissedIrqs(),
rearmReceive(), startReceive() and resumeRunningReceive() all log it, and the
first fires on a timer, so the line counts the ordinary backstop. The resume
now logs its own line when the kept RX_DONE is what it caught, which is the
only case the keep is responsible for. checkRxDoneIrqFlag() returns whether it
notified, so this costs no extra IRQ read - a USB round trip on a CH341 host.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112S3QXU8WUGSCcMQHnwRKZ
* SX126x: never clear a frame's RX_DONE at the RX resume
resumeRunningReceive() cleared RX_DONE and CRC_ERR unless a busy verdict had
asked it not to, and readData() has already cleared the flags of the frame it
read. So the clear could only ever erase a *later* frame's RX_DONE - one that
finished during the readout, which on a CH341 host is several USB round trips
wide, the staged-payload overlap check among them. The poll then had no edge to
deliver and the frame waited for checkStaleRxFlags() to retire its latched
HEADER_VALID, one max packet later.
|
||
|
TomandTom
|
07008058c0 |
nrf52 promicro: LR2021 LF PA table from the LR20xx datasheet (#11980)
* nrf52 promicro: LR2021 LF PA table from the LR20xx datasheet RadioLib's default LR2021 LF PA table never programs tx_power above 20.5 dBm, so output stops rising above 14 dBm. This board table takes 10-22 dBm from LR20xx DS rev 2.2 Table 7-19 and keeps RadioLib's rows below 10 dBm, where that table stops. * LR2021: install the board LF PA table only at 500-1000 MHz The datasheet table is the 915 MHz reference design, but applyCustomLfPaTable() installed it for every sub-GHz frequency, so 433 and 490 MHz regions used it too. Outside 500-1000 MHz the LF table now goes back to RadioLib's default. RadioLib keeps the table pointer, so the same-band reconfigure path, which retunes without a full begin(), also reselects the table before setOutputPower(): a 915 -> 433 MHz region change would otherwise have kept the 915 MHz table. isLr20x0CustomLfPaBand() carries the range and is covered in test_radio. * fix(lr2021): restore receive metadata after develop merge Restore the virtual method lost in the merge; CI native and LR2021 links otherwise fail. --------- Co-authored-by: Tom <116762865+Nestpebble@users.noreply.github.com> |
||
|
Jonathan BennettandClaude Opus 5.5
|
f7b2d258d1 |
Stop NeighborInfo relays rewriting the payload (#12090)
NeighborInfoModule::alterReceivedProtobuf set last_sent_by_id to the relay's node number and re-encoded the payload. The XEdDSA signature covers Data.payload, so receivers that know the sender's key dropped the relayed copy. Relays now forward the payload unmodified, and senders no longer set last_sent_by_id. Receivers attribute a NeighborInfo to the node they heard it from using the packet header: the sender when heard directly, otherwise the relay named by relay_node when it resolves to a single known direct neighbor. A last_sent_by_id rewritten by an older relay is still used when it matches relay_node. Claude-Session: https://claude.ai/code/session_01NmhtcXBe3B3NvS9RttzwHW Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
0fc168d875 |
Improved LBT behaviour - CAD-to-RX, correct symbol times, better thresholds (#11137)
* LoRa LBT: fix SX128x/SX126x CAD encoding + slot model, ESP32 sleep race Three fixes from the "are we really listening?" LBT audit: - SX128x CAD: pass RADIOLIB_SX128X_CAD_ON_4_SYMB instead of the raw count 4. The raw value set bits 7:5 = 000, i.e. a 1-symbol scan, leaving LBT nearly blind on 2.4 GHz. Now runs the intended 4-symbol CAD. - SX126x CAD / slot time: name the symNum value explicitly as CAD_ON_4_SYMB (identical 0x02 raw value, so no on-air change) and add a virtual getCadSymbolCount() so computeSlotTimeMsec() models the real ~4.5-symbol scan for the SX126x/LLCC68/STM32WL family only, leaving LR11x0/SX127x at their correct 2.5 symbols. Fixes the too-short contention-window slots. - ESP32 light sleep: if the DIO1/RF95_IRQ line is already asserted just before esp_light_sleep_start(), skip the sleep so the main loop services the pending packet instead of stranding it for up to a full sleep chunk (the detachInterrupt-clears-the-GPIO-wake disarm race). clod helped too * LoRa LBT: keep software-poll preamble flags latched while idle (anti-stomp) Finding #10 from the "are we really listening?" audit, LORA_DIO1_SOFTWARE_POLL boards (meshnology-w10). The 1 ms poll cleared latched PREAMBLE_DETECTED|HEADER_VALID whenever they were the only bits set, so for the rest of a reception isActivelyReceiving() read idle. A TX queued during the payload phase then passed the send gate and startTransmit overwrote the shared chip buffer, losing the inbound packet. That clear only exists to stop a noise preamble from permanently blocking our own TX (starvation). Starvation can only happen when we actually have something to send, so gate the clear on a non-empty txQueue. While the queue is empty the bits stay latched, isActivelyReceiving() reports the in-progress packet, and a later TX defers instead of stomping it. A queued TX still gets the escape hatch. The poll cadence is a fixed 1 ms reschedule either way, so latched bits don't spin the loop; readData() clears everything once a real RX_DONE arrives. clod helped * LoRa LBT: optional RSSI energy second-gate before TX (default off) Findings #4/#5 of the "are we really listening?" audit. CAD only detects a preamble. A neighbour's packet whose preamble already aired - during our own TX, a CAD scan, or a standby gap - is mid-payload and invisible to CAD, so we key up on top of it. Add an RSSI energy check as a second listen-before- talk gate in the TX-decision path: channelBusyByRSSI() compares the instantaneous in-RX RSSI against the rolling noise floor (getNoiseFloor() + margin) and defers if the channel is hot. It reads RSSI before isChannelActive()'s setStandby() via the || short-circuit, so the sample is taken while still in RX where GET_RSSI_INST is valid - no mode juggling. Orthogonal to CAD: CAD has processing gain and catches weak preambles below the noise floor; RSSI catches strong energy at any packet phase, including the mid- payload case CAD is blind to. Neither subsumes the other. DEFAULT OFF. A mis-set threshold makes a node read "busy" on noise and stop transmitting entirely, which is worse than the bug, so it must be tuned on hardware before it ships live. Compiled out unless LORA_RSSI_LBT_MARGIN_DB is defined at build time; when off it is a compile-time `return false` (zero behaviour change). Fails open: returns not-busy unless in RX with a full noise-floor window and a sane dBm reading, so it can never persistently silence a node. Native build (gate off) links clean. clod helped * LoRa LBT: RSSI margin profiler to tune RSSI_LBT_MARGIN_DB (default off) Instrumentation to pick the #4/#5 RSSI second-gate margin from measured data instead of guessing. Build with -DLORA_RSSI_LBT_PROFILE; compiled out otherwise. It accumulates (RSSI - rolling noise floor) in two channel states: - idle (reuses updateNoiseFloor()'s existing RX sample, no extra SPI): the noise- jitter distribution. Its high tail is the floor the margin must clear. - signal (decoded-packet RSSI, hooked in handleReceiveInterrupt): the real-traffic rise. Its low tail is the ceiling the margin must stay under. A throttled log dump prints both distributions (min/mean/max + percentiles) and the derived usable window (idle p99, signal p05). Placement: the stats/histogram engine is a dependency-free, radio-agnostic header (RssiProfiler.h) so it stays out of the core file and is testable on its own; the two hooks and the RSSI/log I/O live on RadioLibInterface where the state is. Independent of the gate flag (LORA_RSSI_LBT_MARGIN_DB) on purpose: profile first to choose the margin, then build the gate on. Native build (profiler on) compiles clean. clod helped * docs: describe the RSSI LBT margin profiler Feature doc for the RSSI listen-before-talk margin profiler (-DLORA_RSSI_LBT_PROFILE): what it measures (idle noise-jitter vs decoded-signal distributions of RSSI-minus-noise-floor), how it's wired (RssiProfiler.h engine + RadioLibInterface hooks, idle sample reused from updateNoiseFloor), how to build and run it, and how to read the log output to choose RSSI_LBT_MARGIN_DB. clod helped * LoRa LBT: draft #3/#6 fixes behind MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE (inactive) Firmware side of the CAD->RX handoff (#3) and ISR_RX re-arm (#6) from the LBT audit. Both need to re-arm RX WITHOUT the standby() that startReceive() forces (which aborts an in-flight reception) - unavailable in stock RadioLib, provided by resumeReceive() in NomDeTom/RadioLib@rxResume. Guarded by MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE, left UNDEFINED: against stock RadioLib these paths compile out and behaviour is unchanged. Define it (via build_flags) only when linking a RadioLib that has resumeReceive(). - resumeReceiveInPlace(bool startRx): base-class helper - iface->resumeReceive() (remap RX IRQ, optional SetRx, no standby) + re-attach the MCU ISR + base RX bookkeeping. One place, chip-agnostic via iface. - #3: SX126x/LR11x0 isChannelActive() CAD exitMode -> GOTO_RX; the TX-decision path splits the busy check into rssi/cad flags and, on a CAD detect, hands the packet to RX via resumeReceiveInPlace(false) instead of startReceive(). Macro-off, the split is exactly the old `channelBusyByRSSI() || isChannelActive()` short-circuit. - #6: ISR_RX re-arms via resumeReceiveInPlace(true). Open (flagged in-code, for review + HW validation): whether re-issuing SetRx (#6) disturbs an on-chip in-flight reception, and the #3 handoff's interaction with a beacon-target packet on the home config. clod helped * LoRa LBT #4: synthesize GPIO wake cause when light sleep is skipped doLightSleep() skips esp_light_sleep_start() when a LoRa IRQ is already asserted, but still read esp_sleep_get_wakeup_cause() unconditionally - so the skip path returned the previous real sleep's cause (e.g. TIMER) and missed the ESP_SLEEP_WAKEUP_GPIO branch that sets runASAP. Capture whether we skipped and return a synthetic GPIO cause in that case so PowerFSM takes the packet-service path. Reads the ESP-IDF cause only after an actual sleep. clod helped too * LoRa LBT #10: keep HEADER_VALID latched in the software IRQ poll The LORA_DIO1_SOFTWARE_POLL anti-starvation escape hatch cleared both PREAMBLE_DETECTED and HEADER_VALID whenever a TX was queued. HEADER_VALID is evidence of a real inbound frame, so clearing it made isActivelyReceiving() read idle mid-payload and let the queued TX stomp the frame. Gate the hatch on a bare-preamble IRQ (PREAMBLE set, HEADER_VALID clear) and clear only PREAMBLE_DETECTED. A stuck header still can't block TX forever - receiveDetected() ages it out after maxPacketTimeMsec - and readData() clears everything on RX_DONE. clod helped too * LoRa LBT #3/#6: CAD->RX handoff + continuous-RX re-arm on stock RadioLib Reimplements the two audit fixes that previously needed the NomDeTom/RadioLib @rxResume fork (MESHTASTIC_RADIOLIB_HAS_RESUME_RECEIVE), using only stock RadioLib, behind a new default-off flag MESHTASTIC_LBT_CAD_TO_RX. Per the RadioLib maintainer's feedback, no new RX API is needed. (GOTO_RX) with the RX IRQs pre-mapped in the scan config, so the chip's own CAD->RX transition delivers RX_DONE with no library call in between. irqFlags (status-enable) carries the CAD verdict + full RX set incl. PREAMBLE/HEADER so isActivelyReceiving() can read them; irqMask (DIO trigger) carries the CAD verdict + terminal RX events only - NOT PREAMBLE/HEADER, which would fire the ISR mid-frame and run readData() on an incomplete packet. On detection it hands off in place: clear the latched CAD verdict, re-attach the MCU ISR, mark receiving - no standby (which would abort the packet just detected). standby), so re-attach the MCU ISR instead of re-issuing SetRx (which could disturb a second packet already arriving on-chip). Removes resumeReceiveInPlace() and the fork macro. SX126x-only: LR11x0's startChannelScan conflates irqFlags/irqMask (drives the DIO from irqFlags), so it keeps the safe GOTO_STDBY path, re-arming on detect to stay coherent with onNotify when the flag is on. Default-off: needs on-hardware validation and assumes continuous RX (the default preamble=16/minSymbols=8 degenerates startReceiveDutyCycleAuto to continuous). Native builds pass both flag-off and flag-on. clod helped too * LoRa LBT #1: re-arm RX unconditionally when deferring a busy channel The post-CAD/RSSI defer path wrapped its startReceive() re-arm in a !hasTargetRadioSettings(txp) guard, so a beacon-target packet that hit a busy channel took setTransmitDelay() with the radio left in standby. When the beacon's target config coincides with the home config the radio is on the home channel, so the node went deaf on home traffic until the next TX cycle - and kept skipping the re-arm every cycle the channel stayed busy. Drop the guard: re-arm unconditionally and keep listening while we wait to key up. A beacon's job is to get a packet out, but it should still mirror the main RX path. (No effect under MESHTASTIC_LBT_CAD_TO_RX, where the chip is already left in RX.) clod helped too * docs: fix RSSI profiler fences (MD040) + RAM figure, note negative-bin limit - Tag the formula and sample-output fenced blocks as `text` so markdownlint MD040 passes. - Correct the RAM figure: RssiDeltaStats is 148 bytes (12 scalar + 34*4 histogram), so two instances are ~296 bytes, not ~280. - Add a limitations note that all negative deltas collapse into bin[0], so a percentile landing there returns the minimum sample. This does not affect the margin (keyed off the always-positive idle p99) but can understate signal p05 when a node decodes below-floor packets; never biases toward a smaller margin. clod helped too * docs: RSSI profiler - record histogram bucketing design choices Add a "Histogram bucketing" section laying out why a fixed histogram is used over exact percentiles, what the axis has to resolve (the margin window sits at roughly -a few..+20 dB), and the arguments for five bucketing schemes: the current positive-only layout (A), a cost-neutral re-centred layout with negative-range resolution (B, recommended if signal p05 must be trusted), wider full-range bins (C), more bins (D), and exact/off-box (E). Explains the current choice and when to switch. clod helped too * LoRa LBT: move RSSI profiler + energy second-gate to listening-now-rssi Split the RSSI work off this branch so the active CAD/sleep fixes and the #3/#6 CAD->RX rework can land without the tuning-dependent RSSI pieces. Removed here (preserved intact on branch listening-now-rssi): - RSSI margin profiler: RssiProfiler.h, the LORA_RSSI_LBT_PROFILE hooks, and docs/rssi_lbt_margin_profiler.md. - RSSI energy second-gate: channelBusyByRSSI() and the LORA_RSSI_LBT_MARGIN_DB gate. The pre-existing noise-floor sampler (updateNoiseFloor/getNoiseFloor, already on develop and consumed by telemetry) is untouched. The TX-decision block reverts to a plain CAD scan; #3/#6 (behind MESHTASTIC_LBT_CAD_TO_RX) and #1 unchanged. Native build green. clod helped too * reduce to bare essentials * LoRa LBT: calibrate CAD symbol count and detPeak against vendor data NUM_SYM_CAD carried two incompatible meanings: computeSlotTimeMsec() read it as a literal symbol count, while the drivers passed it into scanChannel(), where symNum is a register encoding on SX126x/SX128x. On SX126x the raw 2 encodes CAD_ON_4_SYMB, so the chip has scanned 4 symbols since #5749/#5772 while the slot time was priced at 2.5 - a 1.8x mis-scaling of the CSMA backoff unit, live since January 2025. Give the constant one meaning (a literal count of 4) and have each driver translate at its own call site. getCadSymbolCount() reports the real scan length so slotTimeMsec matches it; RF95 overrides it because SX127x CAD is fixed in hardware and its legacy scanChannel() takes no symNum. AN1200.48 Table 1 recommends 4 symbols for SF9-SF12, which is where most shipped presets sit, so 4 is the right target. The comment claiming 2 was reading that note's SF7/SF8 row. detPeak is calibrated per (SF, symbol count), and on LR11x0 per bandwidth as well. Both LR drivers ran thresholds meant for a 2-symbol scan, leaving them less sensitive than measured-best and biased toward missed detections. Now sourced explicitly: LR20x0 LR20xx datasheet rev 2.2 Table 6-19, indexed by symbols x SF LR11x0 Semtech SWSD003 lr11xx/apps/cad/main_cad.c optimized_parameters, indexed by symbols x bandwidth x SF SX126x RadioLib's SF+13, which sits at or below every AN1200.48 value and so errs toward deferring rather than transmitting Combinations the vendor data does not characterise fall back to the RadioLib default rather than guessing. LR2021 also pins fastCad, which scanChannel() reads off the radio object rather than the config struct. Also fix the 2.4 GHz slot term: (2 * sf + 3) / 32 is integer division that truncates to 0 for every legal SF, so the AN1200.22 fractional term never contributed anything. Not hardware-verified. * LoRa LBT: drop audit-doc pointers and cross-chip asides from comments The LBT comments referenced an external audit document by item number (#1, #3, #3/#6, #10, and a see-.notes/lbt pointer), which means nothing to anyone reading the tree on its own. They also described what sibling drivers do, which goes stale silently the moment one of them changes and belongs in that driver's own file anyway. Keep the reasoning, drop the pointer and the comparison. Where a comment explained a rule that genuinely spans drivers - NUM_SYM_CAD's register- encoding caveat - state it as a rule rather than a roll-call of chips. No behaviour change; comments only. * LoRa LBT: don't index the LR11x0 CAD table with a 2.4 GHz bandwidth The detPeak lookup picked its bandwidth row by range, so wideLora's 406.25 kHz fell into the sub-GHz 500 kHz row - SWSD003's table is the sub-GHz LoRa modem (BW enums 62.5-500), and 2.4 GHz is a separate modem with its own calibration. Match the four bandwidths exactly instead, so anything unmatched falls back to the RadioLib default rather than borrowing a row it has no claim to. Those four are the whole sub-GHz set today - LR11x0::setBandwidth() rejects anything else - so note that a newly unlocked bandwidth, or a symbol count outside 2/4/8/16, needs a row adding here and fails silently if it doesn't get one. Same note on LR20x0 for symbol counts above 4. Also correct two citations. The 2.4 GHz slot term is the SX1280 datasheet's own CAD duration formula, and the CAD symbol-count guidance is AN1200.77, not AN1200.22 - which is "LoRa Modulation Basics" from 2015 and predates the part. Record that SX1280 ignores detPeak/detMin/exitMode entirely: SetCadParams carries only cadSymbolNum, and the PNR threshold lives in register 0x942. * LoRa LBT: set the SX1280 CAD PNR threshold and scan 8 symbols SetCadParams on this part carries only cadSymbolNum - detPeak/detMin in the scan config are inert - so CAD sensitivity is governed entirely by register 0x942, which nothing was writing. The scan therefore ran at the chip's reset value of 0x32, against the 19-36 that AN1200.77 Table 1 calibrates per SF and bandwidth: at LONG_FAST wideLora the recommendation is 27 and we were using 50. A threshold that high needs a much stronger correlation peak, so 2.4 GHz LBT was under-detecting on top of the symbol-count bug already fixed. Write the table value before each scan, indexed by bandwidth x SF. Unmatched bandwidths write nothing and leave the chip's default, as on the LR parts. This goes through our own Module rather than lora.writeRegister(), which RadioLib keeps protected unless RADIOLIB_LOW_LEVEL unlocks raw register access across every driver. SX128x::writeRegister() is a straight SPIwriteRegisterBurst on the same Module we already own, and begin() has pointed spiConfig at the 0x18 WriteRegister opcode long before any scan runs, so the transaction is identical - no subclass needed. Also raise the CAD window from 4 to 8 symbols, matching RadioLib's own SX128x default. AN1200.77 measures a longer window as moving P(Detection) further from P(False Detection); its thresholds are stated to hold for any window duration, so the table above is unaffected. NUM_SYM_CAD_24GHZ moves in step so the slot time still reflects the real scan. Not hardware-verified. * LoRa LBT: hand CAD straight to RX on the LR parts CAD exited to standby, and the busy-channel path then issued a full startReceive() - which standbys the radio and re-enters RX from scratch, throwing away the reception CAD had just detected. Both LR families expose a CAD-to-RX exit mode that avoids the round trip, and it reaches the chip through the public startChannelScan(cfg), so nothing here needs a library patch. A note in an earlier commit claimed this was impossible because RadioLib collapses irqFlags and irqMask into one set. That reasoning is SX126x's, where irqMask is a status gate and an event omitted from it never latches at all. The LR parts have no such gate: SetDioIrqParams takes a mask per interrupt pin and SetDioIrqConfig maps IRQs to one DIO, while getIrqStatus() reads the status bytes returned on every SPI transaction and consults no pin mapping. So the CAD IRQ set now carries the verdict plus terminal RX events only - preamble and header stay off the pin, where they would fire the ISR mid-frame and run readData() on a partial packet - and isActivelyReceiving() still sees them. rearmReceive() cannot be unconditionally standby-free the way SX126x's is. cad_timeout 0 means "RX until one packet arrives, then standby", so the two re-arm paths leave the chip in different states: already listening after the handoff, back in standby after that packet's RX_DONE. It branches on a flag set at detection and cleared on use, so the first re-arm skips the standby and the next restores continuous RX. Forcing continuous RX through the timeout instead is not reliable - RadioLib converts it to RTC ticks through a float divide, so the 0xFFFFFF "no timeout" encoding is not addressable. On a CAD false positive the chip now waits in single-shot RX rather than returning to standby. Still listening, but a state the old code never entered. Not hardware-verified. * LoRa LBT: size the 2.4 GHz CAD slot per chip, not per band computeSlotTimeMsec() branches on myRegion->wideLora, which is a region rather than a part. LR1120 and LR2021 both run there, and both scan NUM_SYM_CAD symbols - but that branch read the flat NUM_SYM_CAD_24GHZ, so raising it to 8 for SX128x moved the slot for two families whose scan never changed. At LONG_FAST wideLora that priced ~22 ms of CAD against a ~10 ms scan. getCadSymbolCount() exists precisely so the slot tracks the real scan, but only the sub-GHz branch consulted it. Use it in both, and give SX128x an override returning NUM_SYM_CAD_24GHZ. That constant stops being a fleet-wide 2.4 GHz value and becomes what one driver reports; SX128x is unchanged at 8.78 symbols, while the LR parts on 2.4 GHz drop from a wrong 8.78 to 4.78. The error was conservative - slots too long, so throughput lost rather than collisions caused - but it is the same scan/slot mismatch the rest of this work removes. * LoRa LBT: declare CAD symbols per chip and per band, 8 on 2.4 GHz A 2.4 GHz region is shared: an LR1120 and an SX1280 can sit on the same mesh. Leaving the LR parts at 4 symbols there while SX1280 scanned 8 gave them different slot times, and so a different CSMA backoff scale between neighbours - the cross-family asymmetry this branch removed for sub-GHz, reintroduced one band up. Both LR families now scan 8 on 2.4 GHz, matching SX1280. Rather than switch on the band inside a shared constant, each driver states its own count for each band it works in, and derives the symNum it passes from the same accessor - so the scan and the slot cannot drift apart: SX126x 4 sub-GHz SX128x 8 on 2.4 GHz LR11x0 4 sub-GHz, 8 on 2.4 GHz LR20x0 4 sub-GHz, 8 on 2.4 GHz RF95 2 sub-GHz, its CAD being fixed in hardware and not settable NUM_SYM_CAD and NUM_SYM_CAD_24GHZ become base-class defaults nothing falls through to; they now record the fleet convention rather than any part's behaviour. LR20xx Table 6-19 stops at 4 symbols, so an 8-symbol scan reuses that row - the last measured point, erring high and so slightly less sensitive than a true 8-symbol value. Noted at the table. Also a comment pass over the branch: drop retrospective phrasing that describes what the code used to do, cut trailing justification clauses, and fix comments the per-band change left stale - one still claimed the LR parts scan fewer symbols than SX1280 on 2.4 GHz, which is now the opposite of the truth. * LoRa LBT: sweep up a CAD->RX handoff that delivers nothing A handoff leaves the chip listening by its own exit mode, so nothing in the driver arms the radio and no return value says whether it worked. If the detection was false, or the exit mode was not honoured, the node waits in a state we never verified - and on the LR parts that is single-shot RX, which does not return to continuous listening on its own. Stamp the handoff and let the existing ~1s poll re-arm if a whole max-length airtime passes with no packet, the same bound receiveDetected() uses. Clearing it on a real RX_DONE is obvious; clearing it in startSend() is not, but a TX can intervene before any packet arrives and its completion re-arms RX anyway, so leaving the marker set would have the poll re-arm a radio already listening. Reads the clock through Time::getMillis(), the same source Throttle compares against, so the site stays testable across the millis() wrap. 0 is the "none outstanding" sentinel and is tested before the elapsed compare. Also document on rearmReceive() that an override must never call the chip's own startReceive() - its standby+SetRx aborts the packet CAD just detected, which is the entire reason the override exists. * define txen pin to allow sx128x testing * LoRa LBT: bound the CAD->RX handoff in hardware, and re-arm after it delivers cadTimeout only applies to the CAD->RX exit mode. Every driver on develop exited CAD to standby, so the field was ignored and its value - 0, which no datasheet defines - was dead data. Switching the exit mode to RX made it load-bearing while the line itself never changed, so the diff showed nothing. All three handoff parts now bound that RX at one max-length airtime, taken from getPacketTime() so it tracks the preset. A detection that delivers nothing expires to standby and raises TIMEOUT, already mapped to the pin, so the existing ISR re-arms and the queued TX retries on its own backoff. handleReceiveInterrupt() returns early on TIMEOUT without RX_DONE; otherwise each dud detection would reach readData() and be logged as a bad packet. That RX is single-shot, so the arm after it delivers must be a full one. SX126x re-armed unconditionally without a standby, which left the chip in standby while the interface believed it was listening - and nothing swept it, since RX_DONE has already cleared the handoff stamp. A node that received a handoff packet then went deaf until its next transmit. It now carries the same cadHandedToRx flag the LR drivers already had. Sourcing, since the parts disagree and RadioLib's uniform ChannelScanConfig_t hides it: SX126x DS_SX1261-2 p98 (15.625us steps), LR11x0 UM Table 8-8 (31.25us), LR2021 lr20xx_driver cad_timeout_in_pll_step (31.25us, contra the datasheet's "32 MHz periods"). RadioLib scales both LR parts as 30.52us, so the programmed value lands ~2% long - harmless, and left for upstream. Also marks the three SWSD003 cells at 2 symbols/BW500/SF10-12 as that SDK's fallback rather than measurements. * LoRa LBT: own the CAD->RX handoff in one place, and void it on standby The handoff state was split across two classes: cadHandoffRxStart in RadioLibInterface, cadHandedToRx in each of SX126x/LR11x0/LR20x0, whose rearmReceive() overrides were byte-identical. Neither field was cleared by setStandby(), so a standby taken while a handoff was outstanding could strand cadHandedToRx set - and the next rearmReceive() would then re-attach the ISR to a chip sitting in standby, marking the interface as receiving with no RX_DONE ever to come and no recovery path. A stale cadHandoffRxStart was milder but still re-armed the radio over whatever was arriving at the time. Hold both in the base, arm them together in noteCadHandoffToRx(), clear both in setStandby(), and collapse the three overrides into one non-virtual base rearmReceive(). SX128x and RF95 never arm the flag, so they keep taking the plain startReceive() branch. Document on isChannelActive() that a true return must be followed by rearmReceive() - the invariant the whole scheme rests on. Two fixes fall out of the shape: - The handoff re-arm attaches a rising-edge ISR to a line that is not known to be low, unlike startReceive() which clears the IRQ status first. Follow it with checkRxDoneIrqFlag(), as startReceive() already does. - checkCadHandoffTimeout() used one max-length airtime, the same figure the chip's own cadTimeout now uses, so the two expired together. Losing that race left a queued ISR_RX to run against flags the software path had just cleared, falling past the TIMEOUT guard into readData() on an empty buffer. Double it so the hardware always expires first; the backstop then only covers an RX whose timer stopped on a header that never completed. * LoRa LBT: re-arm RX when a queued packet is dropped for tx_enabled=false Pre-existing on develop, not introduced by this branch, but it is the same failure this series exists to remove. TRANSMIT_DELAY_COMPLETED reaches startSend() only through isChannelActive(), which leaves the chip in standby for the transmit about to follow. When that transmit is then dropped because LoRa TX is disabled, the early return frees the packet and returns without arming anything: isReceiving stays false, so pollMissedIrqs() skips the radio too, and the node is deaf until something else re-arms it. Narrow - tx_enabled has to change between enqueue and dequeue, since send() checks it as well - but the fix is one call on a path that already exists. * LoRa LBT: say what the CAD slot time actually does per chip Two comments in RadioInterface.h described intentions the code does not carry out. NUM_SYM_CAD/NUM_SYM_CAD_24GHZ claimed the per-band values keep the CW backoff scale equal across a mesh. They do not: the slot has to match the scan each part really runs, and RF95's CAD is fixed in hardware at 2.25 symbols while SX126x/LR scan 4. A mixed sub-GHz mesh therefore runs two slot lengths, which is correct per part and worth stating rather than implying otherwise. The slotTimeMsec initializer runs during RadioInterface's own construction, so the virtual getCadSymbolCount() it calls always resolves to the fallbacks here, never a driver override. That is harmless only because applyModemConfig() recomputes it before anything reads it - left as-is deliberately, since initializing to 0 would make random(0, 2^CW) * slotTimeMsec fire an immediate transmit if anything ever did read it early. * LoRa LBT: set the SX128x CAD threshold at config time, not per scan isChannelActive() looked up the AN1200.77 PNR row and wrote register 0x0942 on every scan, but sf and bw only change in applyModemConfig(). That is an SPI round trip between the channel scan and the transmit that follows it, on the one path this file's own comments ask to keep short. Move it to applyCadDetPeak(), called from reconfigure() after the sf/bw setters it depends on, and from init() as well - init() does not route through reconfigure(), so it needs its own call. * ESP32: stop skipping light sleep forever on a stuck LoRa IRQ The skip added earlier in this series returns instead of sleeping whenever DIO1 already reads high, so the main loop can service the packet rather than strand it for the sleep chunk. It had no bound: a line that latches high with nothing clearing it makes every subsequent call skip too, and the node quietly stops sleeping at all, announced only by a LOG_INFO each time. Count consecutive skips - warn at 3, sleep through at 5, reset on any real sleep. If the IRQ has survived five attempts the main loop is not going to clear it, and never sleeping costs more than one stranded packet. Also drop the LORA_DIO1_EXTENDED_IO branch in loraWakeIrqAsserted(): those boards return from doLightSleep() before it is ever called. * nrf52 promicro: gate the P1.06 TXEN pin, and add an optional SX1280 build P1.06 was wired to SX126X_TXEN unconditionally earlier in this series, to drive an external PA on a bench unit. That is not safe to ship: three envs share this variant.h, and P1.06 is PIN_WIRE_SCL on the EasyProMicro pinout and PIN_EINK_BUSY on the InkHUD one. The default builds go back to RADIOLIB_NC. Put it behind -D PROMICRO_SX126X_TXEN, refused wherever the pin is already claimed. The EasyProMicro arrangement has its own SX126X_TXEN on P1.00 in the #else branch and never reaches this, so only the e-ink exclusion is needed. The pin was wanted for SX1280 in the first place - every other SX128X line is already assigned on this board and TXEN was the one missing - so add that too, behind -D PROMICRO_SX1280 and a new nrf52_promicro_diy-sx1280 env. Both opt-ins claim P1.06, so asking for both is an #error rather than a silent pick. Costs about 10.7 KB: the image ends at 0xE5140, 19 KB clear of the warm region, against 0xE2710 and 30 KB for the base env. * LoRa LBT: fix two defects in the CAD->RX handoff found in review Both reported by Copilot against 1fcd219af, both confirmed against the code as it stands after the handoff was hoisted into RadioLibInterface. rearmReceive() armed the MCU ISR before marking the interface as receiving. An ISR firing in that window queues ISR_RX while isReceiving is still false, so handleReceiveInterrupt() bails with "called while not in rx mode" and the packet is dropped - the exact packet the CAD handoff existed to catch. Reversed, which also makes this path match the order the drivers' own startReceive() has always used: base state, then arm, then checkRxDoneIrqFlag(). noteCadHandoffToRx() stored Time::getMillis() raw, but 0 is the "no handoff outstanding" sentinel that checkCadHandoffTimeout() tests before its elapsed comparison. A handoff armed on the one millisecond per wrap where getMillis() returns 0 reads as "none outstanding" and the backstop never fires. Step past it to 1, the same hand-remap Throttle.h's own TODO records GPS.cpp using for fixHoldEnds. One millisecond of error, once per 49.7 days. * ESP32: wake on SX128X_DIO1, which five SX1280 boards define instead of LORA_DIO1 enableLoraInterrupt() only ever looked at LORA_DIO1 and RF95_IRQ. RadioInterface builds an SX1280Interface from SX128X_DIO1, and several variants name the IRQ pin only that way, or put it on a different pin from LORA_DIO1: tlora_v2_1_18 LORA_DIO1 undefined SX128X_DIO1 26 betafpv_2400_tx_micro LORA_DIO1 undefined SX128X_DIO1 4 t-eth-elite LORA_DIO1 16 SX128X_DIO1 8 tlora_t3s3_v1 LORA_DIO1 33 // TCXO_EN? SX128X_DIO1 9 tlora_t3s3_epaper LORA_DIO1 33 // TCXO_EN? SX128X_DIO1 9 Every branch of the function is gated on defined(LORA_DIO1), so on the first two it preprocessed to an empty function: esp_sleep_enable_gpio_wakeup() still enabled GPIO as a wake class, but no pin was ever armed for the radio. On the other three it armed the wrong pin - GPIO33 there being the line the variant's own comment questions as TCXO_EN. Those boards could not be woken from light sleep by an incoming packet; they waited out the 30s chunk and pollMissedIrqs() recovered the latched RX_DONE about a second after waking. Delayed, not lost, but deaf for the duration. loraWakeIrqAsserted(), added earlier in this series, inherited the same blind spot and was dead code on the two boards with no LORA_DIO1 at all - so the skip-the-sleep fix never once fired on an SX1280. Resolve the pin per radio in loraWakeDio1Pin() and use it for arming, polling and teardown alike, so the three cannot drift apart again, and widen their guards to accept either macro. Behaviour is unchanged wherever it already worked: for any non-SX1280 radio the helper returns LORA_DIO1, including on RF95 boards where the SOC_PM_SUPPORT_EXT_WAKEUP branch arms it regardless of radio type. LORA_DIO1_EXTENDED_IO still short-circuits first. Note this is a power-profile change for those five boards, not only a fix: they gain a LoRa wake source they have never had, so on a busy channel their light sleep will now be cut short by traffic like every other board's. * LoRa LBT: trim three-line comments to the repo's two-line limit copilot-instructions.md:340 says one or two lines; the P1.06 availability note and the LR2021 DIO routing note were both three. CodeRabbit flagged both. * LoRa LBT: trace the CAD scan, its exits and the handoff outcome Eight short lines, all in the base class so they cost one set of strings rather than one per chip driver. The scan logs arm/busy/free; the in-chip GOTO_RX handoff logs started, then exactly one of pkt, empty, timeout or void - so a dropped or double-counted handoff is visible by inspection. void and timeout are LOG_WARN because both mean an invariant broke: a standby discarded a live handoff, or the chip's own timer failed to fire and the software backstop had to re-arm. The rest are LOG_DEBUG, so a node at normal log level stays quiet unless something is actually wrong. The two pre-existing handoff strings were 44 and 41 bytes of flash; cut to "CAD>RX timeout" and "CAD>RX empty", they pay for most of the new set. The shared "CAD" prefix does not dedup - only string suffixes merge - and splitting it out behind a %s measured 8 bytes worse on Cortex-M once the extra pointer argument per call site is counted. Also fixes the delivered-packet trace, which sat above the TIMEOUT check and so claimed a packet on a window that expired empty. * LoRa LBT: restore the SX1280 CAD threshold after a chip-state recovery The calibrated AN1200.77 PNR threshold moved from a per-scan register write to a config-time one, so it now has to be re-applied wherever the chip's state is rebuilt. recoverChipStateLoss() hardware-resets the chip and reprograms only the modem params, which left a recovered SX1280 doing CAD against the vendor default threshold until the next reconfigure() - and on a node whose config never changes, that is the rest of its uptime. * LoRa LBT: retire RX flags that plain RX never completes Plain RX runs with no chip timeout (startReceiveDutyCycleAuto(16, 8) falls back to RX_TIMEOUT_INF) and the DIO mask carries RX_DONE only, so a PREAMBLE_DETECTED, HEADER_VALID or HEADER_ERR that never completes stays latched until something re-arms RX. On a node with nothing to send, nothing does. The CAD->RX handoff already has this recovery (cadTimeout plus checkCadHandoffTimeout()); plain RX had none. checkStaleRxFlags() is its twin on the ~1 s pollMissedIrqs() tick. It notes when flags are first seen, and if no RX_DONE has consumed them one max-length packet later: - a header (valid or errored) re-arms RX. RX_DONE clears every flag in readData(), so a header still latched has no frame behind it. SX1280 DS rev 3.3 section 16.2: a header error leaves RX wedged with no RX_DONE or timeout, and only a re-arm recovers it. - a bare preamble is only cleared. ClearIrqStatus/ClearIrq changes the IRQ register alone on SX126x, SX1280, LR11x0 and LR20x0, so it never aborts a reception; startReceive() goes through standby and would. HEADER_VALID is never cleared on its own: RadioLib's readData() reads it at RX_DONE, and on LR2021 its absence reports the packet as header damaged. Skipped while a CAD handoff is outstanding (its own timeout owns that RX) and while RX_DONE is pending. RF95 opts out: its carrier sense reads modem status rather than latched flags, and SX127x maps neither PREAMBLE_DETECTED nor HEADER_ERR. The decision is staleRxFlagAction() in RadioInterface.h, pinned in test_radio at both sides of the window and for each action. * LoRa LBT: hold TX on a seen preamble instead of judging it by a deadline receiveDetected() declared a bare PREAMBLE_DETECTED false once 2 * preambleTimeMsec passed without HEADER_VALID - about 8 ms at SF7/BW500. Nothing reads the flags on that cadence: they are latched status bits, never routed to DIO, and the only reader is the TX path, once per CSMA backoff (0-56 ms at SF7/BW500). The verdict was usually reached by a read older than its own deadline, and the queued TX went out over the frame. Reported in #11933 by @jp-bennett, whose two-node bench measured own TX a median 43 ms after the verdict, inside a ~102 ms frame. This branch's longer CAD slot widens it to LONG_FAST: 0-308 ms of backoff at CWmin against a 262 ms deadline. A read that sees a bare preamble now clears PREAMBLE_DETECTED and holds TX for one max-length airtime. The clear touches only the IRQ register (ClearIrqStatus/ClearIrq on SX126x, SX1280, LR11x0, LR20x0), so it never aborts a reception. Reads during the hold report busy, so the normal random backoff still spreads retries, and any RX interrupt ends the hold early. A preamble that fires again during the hold stays latched, and the first read after it holds again: a source that keeps triggering keeps deferring TX, one airtime at a time, while a single stale flag cannot block it. HEADER_VALID is never cleared here - RadioLib's readData() reads it at RX_DONE, and on LR2021 its absence reports the packet as header-damaged - and keeps its one-airtime ageing. The SX126x software IRQ poll routes its bare-preamble case through the same hold instead of clearing on every tick while a TX is queued, which would have released TX before the next read could see a refire. That poll read the private txQueue directly, so meshnology_w10 - the only env that defines LORA_DIO1_SOFTWARE_POLL - did not compile. hasQueuedTx() exposes the one fact it needs. #11937 addresses the same defect by deferring the verdict when the previous read was stale; the two conflict in receiveDetected(). * LoRa: re-arm RX before logging the packet that just went out After TX_DONE the chip sits in standby, deaf, until startReceive(). The ISR_TX path used to log airtime ("Packet TX") and printPacket() the sent packet first, which kept it deaf for ~2-5 ms more depending on where the log goes. Measured on an nRF52 SX1262 bench node: the TX_DONE-to-RX gap drops from ~8.8 ms to ~3.7 ms with USB serial logging (~11.3 to ~6.2 ms with the log also going to a phone over BLE). completeSending() is split into detachSentPacket(), which clears sendingPacket and runs packetReleased() so RX re-arms on the home config, and finishSentPacket(), which does the airtime, counters, log and pool release. setStandby() callers keep the combined completeSending(). * SX126x: keep the TCXO powered in standby RadioLib leaves standbyXOSC off, so standby and the post-TX/RX fallback both use STDBY_RC, which powers a DIO3-controlled TCXO down. Every SetRx and SetTx after that first waits out the TCXO start-up delay programmed into the chip (RadioLib's setTCXO() default, 5000 us). On an nRF52 SX1262 bench node the RX start after TX_DONE took ~6.0 ms at SF7, SF9 and SF11 alike; with setStandbyXOSC(true) it takes ~0.95 ms, and the TX_DONE-to-RX gap falls to ~1.1 ms together with the previous commit. The result held across the periodic AGC reset. Only boards that drive a TCXO from DIO3 change. The cost is the TCXO and XOSC current while the chip sits in standby, which the firmware keeps brief; duty-cycled RX still sleeps, and powers the TCXO down, between windows. * LoRa: skip the periodic AGC reset while a packet is queued resetAGC() ends in startReceive(), and trySetStandby() first runs checkNotification() to drain pending events. With a packet queued that can be TRANSMIT_DELAY_COMPLETED: CAD passes, startSend() keys up, and trySetStandby() then puts the chip in standby over the transmission it just started. completeSending() still reports the packet as sent and counts its airtime. A bench node logged this twice in 20 minutes; one was a packet's last retry, so it never went on air. Skip the reset for this tick when a TX is queued. The queue drains within seconds, and the reset comes round again on the next interval. * GPS: re-enable on a LoRa config save only when the region was unset #10386 keeps the GPS off until a region is set and turns it on when a LoRa config save sets one. The check never looked at the old region, so a GPS the probe had disabled ("GPS not detected; not present this boot") was re-enabled by every LoRa config save, preset changes included, and the probe ran again. The probe blocks the cooperative main loop: on a bench node with no GPS fitted, TX_DONE handling stalled by 0.34-0.89 s on 4 of 9 packets after a preset change, leaving the radio deaf in standby for that long. * LR20x0: CAD exit to RX is 0x01 on the LR2021 RadioLib (510e00cf) defines RADIOLIB_LR2021_CAD_EXIT_MODE_RX as 0x02. The LR2021 takes SetLoraCadParams exit modes 0x00 CAD only, 0x01 RX on detection and 0x10 TX when clear, as the LR11x0 does, and refuses 0x02 with a processing error (-706). With the CAD>RX handoff passing 0x02, every scan failed before the CAD ran and isChannelActive() reported the channel clear: on a Seeed Mesh-Tracker X1, 539 scans against live traffic gave no busy verdict, so the node transmitted with no carrier sense. Pass 0x01, and warn when a scan fails rather than reporting it clear in silence. * LoRa LBT: take out the preamble hold on a seen preamble Revert |
||
|
|
6362e05424 |
Stop NodeInfo relays rewriting the User payload (#12088)
* Stop NodeInfo relays rewriting the User payload NodeInfoModule::alterReceivedProtobuf rewrote user.id and re-encoded the User payload of every NodeInfo it saw, including ones it relays. The XEdDSA signature covers Data.payload, so a rewritten NodeInfo failed verification at receivers that know the sender's key. Relays now forward the payload unmodified. The user.id coercion moves to the local copies that consumed it: phone delivery (MeshService::sendToPhone) and the unencrypted MQTT uplink. It only re-encodes when the id does not match the sender. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmhtcXBe3B3NvS9RttzwHW * Keep signed NodeInfo unmodified on MQTT uplink A coerced copy keeps the sender's XEdDSA signature, which no longer covers the payload, so downlink gateways would drop it. Signed NodeInfo is now published as sent. Also assert that a re-encoded User always fits Data.payload. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NmhtcXBe3B3NvS9RttzwHW --------- Co-authored-by: Jonathan Bennett <jonathan@meshtastic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Thomas Göttgens
|
c7bc29e19a | fix(admin): cap the beacon message at the generated field size instead of a hard-coded index (#12066) | ||
|
Jonathan BennettandClaude Opus 5
|
c7826791d4 |
feat(portduino): admin keys in config.yaml, beyond the protobuf's three (#12056)
* feat(portduino): admin keys in config.yaml, beyond the protobuf's three meshtasticd can now list the public keys authorized to administer the node in its own config, as Security.AdminKeys, and list more than the three config.security.admin_key holds. The host's file is the authority: it is root-owned, so unlike the persisted protobuf an authorized remote cannot rewrite it, and applyHostKeys() re-asserts it at boot and after every remote write of the security config. A key removed from the file loses access on the next boot instead of lingering in flash. The protobuf array stays at three. It is not an arbitrary cap: a security set_config already encodes to ~207 bytes against the 221 a PKC admin packet can carry (233 payload bytes less 12 of PKC overhead), so a fourth key would not fit in one admin message on any platform. Four sites decided who may administer a node, each unrolling admin_key[0..2] by hand - the PKC gate in AdminModule, the same gate in DMShell, the is_managed guard, and the admin-key trial decrypt in Router::perhapsDecode. They now share AdminKeys::isAuthorized(), so a key source reaches all four at once, and enumeration skips empty slots rather than trusting admin_key_count. Each configured key costs one trial decryption per PKI packet from an unknown sender, so the host list is capped at 16 and duplicates are dropped. Files in config.d append to the list, one per operator. A key that is not exactly 32 canonical base64 bytes stops meshtasticd rather than silently authorizing something else, and --check reports it with its line instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SP5hpFwdTBC2K9ggGFLpcs * test(portduino): config-check fixtures for Security.AdminKeys bin/test-config-check.sh drives a built meshtasticd against the fixtures, which is the only place the two process-level behaviours here are reachable: what --check reports, and that a normal boot refuses the file. A clean two-key list, a key three characters short of 32 bytes, and one key written as a scalar instead of a list - the last two as errors under --check and as a non-zero exit on a normal boot, since a truncated admin key would authorize something other than what was written and a file whose only key is unreadable would otherwise boot a node nobody can administer. adminkeys-append/ pins the append-across-config.d rule by value through --output-yaml, with the first key in config.yaml so the effective list does not depend on the filesystem's order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SP5hpFwdTBC2K9ggGFLpcs --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
977b1d775c |
Dmshell (#11976)
* Very WIP dmshell
* Dumb fixes
* Interactive mode
* Try to re-request missed sequences
* troubleshoot dropped packets
* Don't firehose missing packets
* Attempt at better responsiveness
* DMShell heartbeat
* dmshell client serial support and tweaks
* Remove some dead code and LLM overcomplication
* Trunk
* Use RemoteShell in protobufs
* Make the DMShell tests compile
* Harden against possible memory overflows
* Minor cleanups
* No Child Left Behind
* simplify pt 1
* Refactor and Simplify
* Make new protobuf value consistent
* dmshell reliability tweaks
* Add real Lock handling on Portduino
* talking stick
* latest test changes
* Guard the DMShell TX-queue check against a null RadioLibInterface
runOnce() dereferenced RadioLibInterface::instance unconditionally. It is
null on any node whose radio is not RadioLib-backed - meshtasticd with
Lora.Module sim, and the serialHal path - so the first runOnce after a
session went active segfaulted the process.
* Revert the DMShell half-duplex turn-taking ("talking stick") scheme
Before: a DMShell session gated every transmission on a token. The server
only sent shell output while it held the turn; with no token it emitted an
RTS and waited (rate-limited to one per 250 ms) for the client to grant one.
Each grant, yield and re-grant was its own LoRa frame, so an unprompted burst
of shell output cost a full RTS -> grant round trip before the first byte
moved, and a long burst paid another yield/grant pair every 8 frames. On a
link where a single frame already takes a noticeable fraction of a second,
that turned interactive use into seconds of latency per exchange.
After: both ends transmit as soon as they have something to send, which is
how the module behaved before the scheme landed. The flags field stays in the
wire format but is written as 0 and ignored, so nothing in the protobuf
changes and a peer still running the turn-taking build is simply not answered
in kind.
The scheme was introduced in
|
||
|
|
34c9457c69 |
Remove TMM relayed-position precision clamp (#12039)
The clamp rewrote the payload of relayed position broadcasts. The XEdDSA signature covers Data.payload, so receivers that know the sender's key dropped the rewritten packet. Relays now forward position payloads unmodified. Removes USERPREFS_TMM_APPLY_TO_PRIVATE_CHANNELS, which only gated the clamp. Claude-Session: https://claude.ai/code/session_01NmhtcXBe3B3NvS9RttzwHW Co-authored-by: Jonathan Bennett <jonathan@meshtastic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
e691bd3790 |
Claude/dmshell lock (#12024)
* Lock: give Portduino a real mutex instead of the empty fallback Lock.cpp has a FreeRTOS implementation and an empty one, and Portduino takes the empty one: every lock() and unlock() on a Linux build is a no-op, so concurrency::Lock protects nothing there. TrafficManagementModule's cacheLock and SPILock are both built on it, and native meshtasticd runs the radio and the API on separate threads. Add a pthread implementation under ARCH_PORTDUINO. The timed lock(uint32_t) blocks rather than returning early, because there is no portable timed pthread_mutex_lock across Linux and macOS and returning true without acquiring would leave a caller such as SPILock unlocking a mutex it never took. Targets that have neither FreeRTOS nor pthreads, such as STM32WL, keep the existing empty implementation byte for byte. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 * test: create spiLock in the shared setup, which NodeDB needs and no test had Making Portduino's Lock real turns a latent null dereference into a crash. spiLock is a bare pointer that initSPI() fills in, and only main.cpp calls that, so in a test binary it stays null. NodeDB's constructor reaches it through loadFromDisk(), and while Lock::lock() was an empty function the call never touched `this`, so 23 suites have been calling a method on a null pointer and getting away with it. With a pthread mutex behind it the same call reads through the null pointer and takes SIGSEGV at offset 0x10, which is what test_phone_api_config_dump, test_muted_source, test_nodeinfo_send_window and test_module_config hit. Create it once in initializeTestEnvironment(), which every affected suite calls as the first statement of setup(), before any of them constructs a NodeDB. The guard is the idiom test_xmodem and test_nodedb_identity_hygiene already use; theirs stay correct and become no-ops. test_safefile called initSPI() bare right after the harness, which would now trip its assert, so that call goes away. No firmware behaviour changes: main.cpp still calls initSPI() exactly once, and nothing outside the test harness is touched. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 * test: create cryptLock where a suite reaches it without building a Router Second instance of the same latent null dereference the previous commit fixed for spiLock. cryptLock is a bare pointer that Router's constructor creates (Router.cpp:246); AdminModule::setPassKey takes a LockGuard on it, and a suite that exercises an admin path without standing up a Router leaves it null. While Lock::lock() was empty on Portduino the guard never touched `this`; with a pthread mutex it reads through null, which is test_tak_config's SIGSEGV in handleGetModuleConfig. It cannot go in initializeTestEnvironment() the way spiLock did, because Router asserts cryptLock is unset before allocating its own, so creating it for every suite would break the ones that do build a Router. It is a named helper instead, testEnsureCryptLock(), called by the six suites that reach a cryptLock path with no Router: test_ack_proof, test_admin_session_repro, test_fuzz_packets, test_hop_scaling, test_module_config and test_tak_config. The three that define setup() twice behind a PKI #if get the call only in the branch that compiles the tests in. No firmware behaviour changes; nothing outside test/ is touched. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 * test: create cryptLock in the harness instead of chasing suites Router's constructor asserted cryptLock was unset, then allocated it. That assert is why ten suites carry a mock-router destructor whose only job is to delete the global and null it so the next router can be built. While Lock::lock() was an empty function on Portduino a null cryptLock cost nothing, so those null windows were invisible; with a real mutex, anything reaching perhapsDecode() or the ack-proof paths after one of those destructors runs dereferences null. The fix is the idiom already on the next line of the same constructor, which routingAuthCacheLock has used all along: reuse the lock if one exists. Nothing in src/ ever deleted cryptLock, so a Router that finds one is finding the process's only one. initializeTestEnvironment() can then create it for every suite, the way it now does for spiLock, and the ten teardowns and the per-suite helper from the previous commit all go away. Replaces the six testEnsureCryptLock() call sites with one creation point, and removes the null windows in test_admin_radio, test_mesh_beacon, test_mesh_module, test_mqtt, test_nexthop_routing, test_nodeinfo_send_window, test_traffic_management, test_event_channel_phone_api, test_event_channel_router and test_phone_api_config_dump. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
8c0abbd522 |
feat(portduino): BLE peripheral support via BlueZ for meshtasticd (Raspberry Pi) (#11396)
* feat(portduino): BLE peripheral support via BlueZ for meshtasticd on Linux Adds the standard Meshtastic BLE service (toRadio/fromRadio/fromNum/logRadio) to the Linux native target, so a Raspberry Pi running meshtasticd can be paired and used over BLE like any other Meshtastic device. Implementation: a new LinuxBluetooth backend registers a GATT application, LE advertisement and pairing agent with bluetoothd over the org.bluez D-Bus APIs, using sdbus-c++ (both the 1.x and 2.x major versions, via a small compat shim - Debian bookworm/Ubuntu 24.04 ship 1.x, trixie/Fedora ship 2.x). When the sdbus-c++ dev package is absent the whole backend compiles out via __has_include, the same optional-dependency idiom as the ulfius webserver. Threading follows the NimbleBluetooth model, simplified: the sdbus event loop runs its own thread, and all PhoneAPI calls happen on the main thread. Writes queue to the main loop; reads park the D-Bus reply and are completed from the main thread after queued writes, so write-then-read clients see their answer without any busy-waiting. Enablement is a double opt-in: a new `Bluetooth:` config.yaml section (Enabled, default false; AdapterId, default hci0) must turn BLE on for the host, and the regular device config bluetooth.enabled must be on. The config-check schema and fixtures cover the new section. Pairing honors config.bluetooth.mode: NO_PIN maps to a NoInputNoOutput just-works agent; RANDOM_PIN to DisplayOnly with the kernel-generated passkey shown on screen/log via the existing BluetoothStatus plumbing. FIXED_PIN falls back to random-passkey semantics with a warning - BlueZ does not support forcing a passkey. PIN modes enforce encrypt-authenticated-read/write on all mesh characteristics. Packaging: install a D-Bus system policy so the meshtasticd user may talk to org.bluez, add it to the bluetooth group, order the unit after bluetooth.service, and add libsdbus-c++-dev to debian/rpm/docker/CI deps. Verified in-container against a mock bluetoothd: registration flow, GATT tree enumeration, advertisement properties, and a full config download (ToRadio wantConfig -> 47 FromRadio packets) through the D-Bus bridge. Real-hardware pairing/notify testing on a Pi still pending. Known limitations (v1): meshtasticd must be restarted if bluetoothd restarts; FIXED_PIN degrades to a random passkey; getRssi() returns 0 (same as nRF52). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): BLE fixes from first real-hardware pass (Pi CM5 + RAK6421) Findings from testing PR #11396 on a Raspberry Pi CM5 (Pi OS trixie, BlueZ/sdbus-c++ 2.1 - the v2 compat path) with a RAK6421 HAT and an Android phone: - getMacAddr() leaked its HCI socket on every call and never closed it, and on failure returned without touching the caller's buffer - which getDeviceName() passed in uninitialized. Close the socket on all paths and cache the MAC after the first successful read; it cannot change at runtime and this now runs on every bluetoothd property read. - getDeviceName() zero-initializes its MAC buffer, and LinuxBluetooth snapshots the name once at setup() on the main thread: the advertisement's LocalName getter runs on the D-Bus event-loop thread and getDeviceName()'s static buffer is not thread-safe. - Restore NimBLE-style config-phase packet prefetch (depth 3). The initial port answered every FromRadio read with a D-Bus -> main-loop round trip, which made the config download noticeably slow; ReadValue now answers straight from the prefetch queue on the event-loop thread, with NimBLE's safety rules (never in STATE_SEND_PACKETS, writes always observed before reads, queue cleared on disconnect). - Set advertising MinInterval/MaxInterval to 20-100ms (BlueZ >= 5.71; older versions ignore the properties). btmon showed the kernel default of 1.28s otherwise, and Android's background-connect scan windows are sparse enough that tap-to-connect took 8-14s; 20ms is the same floor NimBLE uses on ESP32. Verified on hardware: scan, passkey pairing, connect, config download, reconnect after bond wipe. Also diagnosed (no code change): the node identity MAC comes from the RAK HAT EEPROM by design, so the BLE name suffix follows the HAT rather than the BT adapter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): address CodeRabbit review on BLE support - setBluetoothEnable: handle disable before the config gate, so a running BLE stack is always stoppable even after the device config turns Bluetooth off underneath it - getMacAddr: read the adapter configured as Bluetooth.AdapterId instead of hardcoding hci0, falling back to hci0 for unparseable names - systemd unit: Wants=bluetooth.service so bluetoothd is pulled up when present (After= only orders, it does not start it) - debian/rpm: Recommends: bluez as the runtime contract for BLE - dbus policy: document why the org.bluez rule is destination-wide rather than a per-interface allowlist Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): show the BLE pairing code on BaseUI screens onDisplayPasskey published the passkey to bluetoothStatus and triggered PowerFSM, but never called screen->startAlert(), so on BaseUI the code only ever reached the log. BluetoothStatus has no BaseUI consumer -- only InkHUD's PairingApplet and StatusLEDModule read it -- so a Pi driving a HUB75/OLED panel showed nothing while BlueZ sat waiting for the user to type a code they could not see. NimBLE and nRF52 draw it via startAlert(); this adds the missing half for Linux. The agent callbacks run on the sdbus event-loop thread while the screen is owned by the main thread, so the passkey is handed over as a pending flag and drawn from runOnce(), matching the existing disconnectCleanupPending pattern rather than reaching into the screen from the event loop. Dismissed on all four exits, so a stale code cannot stick on an always-on panel: Paired -> true (newly watched in PropertiesChanged, which previously only looked at Connected), agent Cancel, peer disconnect (moved out of the lastGone branch so a peer leaving mid-pairing clears the code even when another device is still connected), and doDeinit() -- applied inline there because runOnce() may never be scheduled again after teardown. Verified on a Pi 5 + BlueZ 5.66 in RANDOM_PIN mode: the code renders on a HUB75 panel and clears once the phone completes pairing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: install libsdbus-c++-dev for the native test build setup-native-test landed on develop while this branch was adding libsdbus-c++-dev to setup-native, so the new action's "full setup-native list" of C libraries is missing it. Without the package the test job builds with HAS_BLUETOOTH 0 and never compiles LinuxBluetooth.cpp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): warn when a factory reset cannot clear BLE bonds factoryReset(eraseBleBonds) silently did nothing on Linux when the BLE backend was not running, so the reset reported success while the host's pairings stayed. Removing them needs a live connection to bluetoothd that a disabled backend never opened, so say so rather than imply they went. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): gate the factory-reset bond clear on an enabled backend setup() leaves linuxBluetooth allocated with its bus torn down when it throws, so a pointer check alone let factoryReset log "Clear bluetooth bonds" for a clear that clearBonds() then declined to perform. isEnabled() is only true after setup() completes, which routes that case to the warning that says the bonds were left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * style: reformat under clang-format 20 #11909 moved trunk from clang-format 16 to 20, which spaces C-style casts differently and reindents the comment above the HAS_WIFI block. Both files are ones this branch already touches, and trunk's fmt linter grades whole files, so its check fails until they are reformatted. No behaviour change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0184v7MyCLuJHW2ebZ9r8NmQ * fix(portduino): three BLE config and lifecycle fixes from review Bluetooth config keys are now assigned individually rather than per section. loadConfig() runs once for every file in config.d, so reading an absent key as its default let a later file that named only one of them silently reset the other: `AdapterId: hci1` alone turned Bluetooth off, and `Enabled: true` alone dragged the adapter back to hci0. Only what a file actually states should override what an earlier one set. A backend that failed to come up is now retried. setup() can leave linuxBluetooth non-null but disabled - bluetoothd not ready, adapter missing, policy refusing - and every later enable then called resumeAdvertising(), which returns immediately while disabled. A transient failure at boot kept BLE off until the process restarted. doSetup() already opens with `if (enabled) return` and tears the bus down on every failure path, so calling it again is safe. Bluetooth.AdapterId is now checked for the hci<digits> form. LinuxBluetooth uses the value verbatim as the BlueZ object path while the MAC fallback reads only the leading hciN, so "hci1junk" looks plausible, yields a MAC, then finds no adapter and BLE never comes up. Covered by a new fixture and suite case, which is the kind of silent no-op that directory exists to catalogue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
e0c76fd41e |
Improve performance of encrypted packets with a shared secret cache (#11979)
* Improve performance of encrypted packets with a shared secret cache Every PKI encrypt, decrypt and ack-proof ran Curve25519::dh2 plus a SHA256 to derive the pairwise key, so a node in a conversation paid a full X25519 per packet, on the main loop, under cryptLock. On a RAK4631 that is ~96 ms of the ~210 ms it takes to handle a DM. setCryptoSharedSecret() derives the key only when it is not already held for that peer, keeping the last MAX_CACHED_SHARED_SECRETS derivations (8 on nRF52, 2 on STM32WL, 10 elsewhere, under 400 bytes) and evicting the least recently used. encryptCurve25519, decryptCurve25519 and ackProofCompute all go through it, so the ack proof gets the same cache without a second DH path. The cache is emptied whenever our own private key changes, since every secret in it is then stale. The lookup key is the first 4 bytes of the peer's public key. A collision makes us derive against the wrong cached secret, which costs a failed decrypt for that pair; it cannot disclose either secret. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLHxcWJuvSoSSMpz3LWdt3 * Do not let an empty cache slot answer for a zero-prefixed peer key The cache used lookup_key == 0 to mean "slot unused", so a peer key whose first 4 bytes are zero matched every unused slot and was handed that slot's zeroed secret as a hit. The all-zero key is exactly such a key, which is how test_proof_rejects_weak_peer_key caught it: dh2's weak-point check never ran. Entries carry an explicit valid flag instead, which the struct's existing padding absorbs. A weak peer key is also now rejected before the cache is consulted, since with 4 bytes of lookup key it could otherwise collide with a cached peer and be served that peer's secret rather than being refused. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLHxcWJuvSoSSMpz3LWdt3 * Key the shared secret cache by the whole peer public key A four-byte lookup key is grindable: anyone can generate a keypair whose public key shares those bytes with a peer they want to shadow, get their own entry cached, and then be handed the secret this node uses to talk to that peer - readable by them, since they hold the matching private key. That is disclosure of traffic meant for the peer, and a forgeable ack proof in its name, not the failed exchange a chance collision would cause. Entries hold the peer key itself and are matched on all 32 bytes, so the weak-key check dh2 does on a miss can no longer be skipped either and the explicit isWeakPoint guard goes away with it. The cache costs 66 bytes per entry: 528 on nRF52, 660 where the default 10 entries apply. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLHxcWJuvSoSSMpz3LWdt3 * Allowlist the cache's uptime shift for the millis deadline guard The guard's regex reads `millis() >> 22` as a comparison against the uptime clock. It is a right shift, coarsening uptime into the ~1.165 hour units the cache stamps entries with, and the eviction arithmetic handles that stamp's 8-bit wrap itself. Co-Authored-By: Jonathan Bennett <jbennett@incomsystems.biz> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JLHxcWJuvSoSSMpz3LWdt3 --------- Co-authored-by: Jason B. Cox <contact@jasonbcox.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tom
|
7fe3176a40 |
Radiolib recording hal tests (#11997)
* test: exercise RadioLib's drivers through a recording HAL, pinned to RadioLib 7.8.0 test_radiolib_drivers drives the RadioLib calls Meshtastic's radio interfaces make against RecordingHal, which logs every SPI transaction and answers with a success status, a few scripted replies (the packet type the LoRa setters check) and, for SX127x, a register file. One set per chip family - SX126x, SX127x, SX128x, LR11x0, LR2021 - each ending in a list of the reply-dependent tests still to write. SX126x and SX128x get the SPI framing their begin() would set. RadioLib is pinned to 7.8.0 to show the failure: the LR2021 DC-DC workaround (jgromes/RadioLib#1864) passes sizeof(uint32_t) as a word count, and under [env:coverage] ASan aborts the LR2021 set with a stack-buffer-overflow in readRegMem32. The other four sets pass. * test: pin RadioLib to the DC-DC word-count fix NomDeTom/radiolib@3509dbc8e (branch lr2021-dcdc-regmem32-len, on RadioLib master after 7.8.0) passes 1 as the word count at the three DC-DC workaround call sites. The overrun is gone and the LR2021 set runs to completion. test_lr2021_dcdc_freq_lf_write_sends_one_word still fails, on a second RadioLib bug: LRxxxx::writeCommon() sends 4 + 4*len bytes whatever the address width, so every LR2021 WriteRegMem32 (24-bit address) carries one uninitialised trailing byte - 10 bytes where 9 are meant. * test: keep RecordingHal reachable across a failed assertion A failed TEST_ASSERT longjmps out of the test, skipping destructors, so each test's local RecordingHal leaked its transaction log. LeakSanitizer then reported it at exit, and under [env:coverage] a plain assertion failure came out as an ERRORED suite (run 36412456860: 26 Tests 1 Failures, then a 384-byte leak from RecordingHal::spiTransfer). One static HAL, reset at the start of each test, stays reachable, so a failure reports as FAILED. * test: run the RadioLib driver suite against develop's RadioLib pin Revert the pin to develop's 510e00cfb (RadioLib 7.7.1). That RadioLib predates the LR2021 DC-DC workaround, so its three tests would only fail on their own premise there. They now register only when LR2021 has the public setRegulatorDCDC() that jgromes/RadioLib#1864 added with the workaround: any RadioLib carrying it runs them, and none is left ignored. Against 7.8.0 the suite shows the DC-DC overrun (ASan, run 36412379592). Against the DC-DC fix (NomDeTom/radiolib@3509dbc8e) it shows a second bug: every LR2021 WriteRegMem32 sends one byte too many, since LRxxxx::writeCommon() still sizes the frame for a 4-byte address after jgromes/RadioLib@e3af85867 made it 3 on the LR2021 (run 36412456860). Neither is reachable at this pin. * test: pin the coding-rate byte the radio interfaces send LR11x0Interface, LR20x0Interface and SX128xInterface all call setCodingRate(cr, cr != 7), so 4/5 goes out with the long interleaver. The modulation tests used the driver default (short), and only counted frames. Pass the flag the firmware passes, and add a test per chip that reads the coding-rate field back from the last modulation-params frame: 5 for 4/5 long, 3 for 4/7 short (RadioLib 7.7.1's encoding). Also correct test_main.cpp: at the 7.7.1 pin the LR2021 DC-DC tests are not registered, so the set does not fail; the 7.8.0 overrun is now stated as conditional on a RadioLib that carries it. |
||
|
James Rich
|
20284840a8 |
Report the ack proof verdict to the client (#11965)
* Report the ack proof verdict to the client * Test that a proven ack reaches the phone as VALID * Say why the verdict clear and the pending gate sit where they do |
||
|
Tom
|
f90b48ea6c |
chore: trunk fmt --all (#11938)
Whitespace and comment-alignment only: the output of `trunk fmt --all` on
develop @
|
||
|
|
95906609db |
Sign the whole Data envelope, in one unambiguous layout (#11422)
* Bind request_id and reply_id into the XEdDSA signing buffer
A signed reply can be re-pointed at a different message today. The client sets
reply_id on an outgoing text to make a tapback, firmware signs the resulting
broadcast, but reply_id lives in the Data envelope rather than the payload the
signature covers - and channel crypto is AES-CTR with no MAC, so anyone holding
the PSK can rewrite it in flight and the signature still verifies. request_id
has the same shape and is bound with it.
Packets carrying neither field keep the existing [from|id|portnum|payload]
layout, byte-identical to what v2.8.0 alphas are signing today, so the bulk of
signed traffic - broadcasts - stays verifiable in both directions across the
upgrade. Only packets that actually carry one of the two fields use the extended
layout. Both sides pick the layout from the packet's own decoded fields, so
nothing is transmitted to select it.
Open question for review, deliberately not decided here: a format/version byte
in the buffer would be cleaner than a conditional layout, because the safety
argument for the conditional form has to be re-derived whenever a portnum is
added. It costs nothing on the wire since the buffer is never transmitted, but
it changes every signature and so breaks verification against the alphas that
are already signing. If we want it, better done once and before 2.8.0 leaves
alpha.
Tests: request_id/reply_id flips alongside the existing from/id/portnum negative
cases; a hand-built alpha-format signature that must still verify, and must not
be reinterpretable as the extended layout or vice versa; and receive-path cases
for a retargeted tapback, a retargeted response, and an ordinary signed
broadcast that must be unaffected.
* Sign the whole Data envelope, in one unambiguous layout
Replaces the conditional two-layout signing buffer with a single fixed one:
version(1) | from(4) | id(4) | to(4) | portnum(4) | request_id(4)
| reply_id(4) | emoji(4) | bitfield(4) | flags(1) | payload(N)
The conditional scheme was ambiguous. Base was header || arbitrary payload, so
any byte string the extended layout emitted was also a legal base payload: an
attacker could move eight payload bytes into request_id/reply_id and truncate a
signed message while its signature still verified. No marker placed only in the
extended layout fixes that, because base can always reproduce it. A fixed-length
header does - the payload boundary is total - XEDDSA_SIGNED_HEADER_LEN and never
depends on content.
Binding reply_id alone was also not enough, because the fields around it are just
as malleable:
emoji a reaction is a text packet with the emoji in the payload,
reply_id naming the parent, and this flag telling the client to
render it as a reaction. Flipping it turns a signed reply into a
signed reaction, so it has to travel with reply_id.
bitfield bit 0 is OK_TO_MQTT, the sender's consent to upload to a public
broker, and the exploitable direction is the one that leaks. The
whole uint32 is signed so bits 2..31 are covered in advance, and
presence is signed separately so stripping it is not the same as
sending it zero.
want_response bit 1 of bitfield mirrors it and Router merges the two with |=,
so signing either alone protects neither.
to without it a signed broadcast can be re-addressed as a direct
message and still verify, delivering a public statement as an
apparent private one. Relays rewrite hop_limit, next_hop and
relay_node, never `to`.
Left out: dest and source (one write in the tree, no readers), channel (the wire
carries a hash where the decoded packet carries an index), and the hop fields,
which relays rewrite by design. Signing the encoded Data wholesale is not an
option either - a relay that holds the channel key decodes and re-encodes it, so
byte fidelity is lost and unknown fields are stripped. The ack_proof excision
trick does not transfer for the same reason: Routing survives because it rides
inside the opaque payload, which Data itself does not.
sign/verify now take the Data rather than a field list, so adding to the covered
set cannot silently miss a call site. Integers are explicitly little-endian, as
in ackProofCompute. The buffer is sized from the schema's own maximum payload
rather than from what the fits-on-air gate currently admits, because an overflow
makes buildSigningBuffer return 0 and signing fail with no error; MAX_BLOCKSIZE
is left alone so the AES-CTR guard and scratch buffers sharing it are unaffected.
This changes every signature, so it is not compatible with the v2.8.0 alphas. It
is deliberately being done while 2.8.0 is still prerelease: a verify failure
against an authoritative key is an unconditional drop, so the same change after
2.8.0 goes stable would split signed traffic across versions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
|
||
|
Jonathan BennettandClaude Opus 5
|
57bdedf324 |
Bind the ack proof to the node we addressed, not the ack's sender (#11932)
* Bind the ack proof to the node we addressed, not the ack's sender The MAC proves only that its author holds a pairwise key with us, and every keyed peer holds one. ackProofVerify looks the key up by getFrom(p) - the ack's claimed sender - and nothing compared that against the node we actually sent to, so C, whose authoritative key we hold, could read our packet id out of the cleartext header and mint a receipt for a packet that went to B. It verified VALID. "An authenticated delivery receipt from the actual recipient" was not what the code delivered. Guard on orig->packet->to before verifying. Since that establishes getFrom(p) == orig->packet->to, the existing key lookup is then correct and AckProof.cpp is untouched. Bailing out rather than verifying against the recipient key and reporting INVALID is deliberate twice over: it skips the X25519 an attacker would otherwise choose when we pay, and a third-party ack is "not a receipt" rather than "a forged receipt" - naks from intermediates (NO_CHANNEL, PKI_UNKNOWN_PUBKEY, MAX_RETRANSMIT) legitimately come from a node that is not the destination and must not be logged as proof mismatches. A broadcast original has no single recipient, so there is nothing to bind to. Also correct the header doc. It claimed channel (non-PKI) traffic gets nothing from this, but isProvableAck tests only the ack's shape: a DM that travelled under channel encryption still gets a proven ack when we hold the peer's key, because the secret comes from X25519 rather than from the channel. That is more coverage than the receipt needs and it costs one X25519 per ack generated - both worth stating rather than implying the opposite. test_proof_from_third_peer_fails_under_recipient_key pins the property the guard relies on: Carol's proof for a packet Alice sent to Bob verifies under Carol's key and fails under Bob's. The guard itself is not directly assertable while every branch of the verdict switch returns true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 * Make the sender check honor ACK_PROOF_ENFORCE The mismatch branch returned true unconditionally. Today every branch of this function returns true, so that reads as equivalent - but if enforcement is ever switched on it is a hole rather than a no-op. The sender field is not authenticated, so an attacker would simply address the ack from anyone other than the node we sent to, take the early return, and skip the proof requirement entirely. Hold only success acks to it. A nak legitimately arrives from an intermediate rather than from the destination - NO_CHANNEL, PKI_UNKNOWN_PUBKEY and MAX_RETRANSMIT all do - so naks keep today's behavior in either mode. Broadcast gets its own unconditional return rather than sharing the condition. orig->packet->to is NODENUM_BROADCAST there and can never equal any sender, so folding it into the sender check would, under enforcement, stop every reliable broadcast from ever being acked. This does not make enforcement sound on its own and the comment says so: ABSENT still permits, so spoofing the sender and omitting the proof gets through regardless, and ABSENT cannot be made to block for the reasons recorded at ACK_PROOF_ENFORCE. The narrower point is that a flag named "enforce" should not have a branch that silently ignores it. Raised by CodeRabbit on #11932. Its reading - that this is a live authorization bypass a third party can use to suppress retransmissions - does not hold: the sender field is unauthenticated either way, and perhapsGenerateImplicitAckForOwn Overheard already clears a pending retransmission on a replayed copy of our own ciphertext, with no ack and no key involved. The structural point stands on its own merits. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 * Avoid cppcheck's duplicateValueTernary in the enforce check `return isAck ? !ACK_PROOF_ENFORCE : true` has the same value in both arms while the flag is off, which is the whole point of the line - and is exactly what cppcheck reports: style: Same value in both branches of ternary operator. [duplicateValueTernary] That failed the `check` matrix on every platform. Write it as an if, which expresses the same thing and does not trip the rule, and say so in a comment so it does not get folded back into a ternary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Ben Meadors
|
8ef996f5b4 |
fix(radio): never read a RadioLib error code as a packet's time on air (#11940)
* fix(radio): never read a RadioLib error code as a packet's time on air
getTimeOnAir() reads the packet type back over SPI on every chip and answers RADIOLIB_ERR_WRONG_MODEM when the chip is no longer in the mode we left it in (a reset or brownout leaves it in FSK). RadioLib returns times as an unsigned microsecond count and its negative int16_t error codes through that same value, so computePacketTime() divided -20 by 1000 and handed 4294967ms up as the time one packet spent on the air.
That single sample is enough to take the node off the air until it is rebooted:
- channelUtilizationPercent() reads 7158% and utilizationTXPercent() 119%,
matching the 7000%+ ChUtil spike and the 120% AirUtil in #11935, so both TX
gates stay shut for the hour it takes that bucket to age out;
- receiveDetected() uses the same figure as its false-header timeout, so
activeReceiveStart is never cleared and canSendImmediately() sees a busy
channel for 72 minutes.
Classify the returned value instead of trusting it. A failed getTimeOnAir() now falls back to calculateTimeOnAir() on the modem parameters we configured - arithmetic with no readback that can fail - and the RX path, which already used calculateTimeOnAir(), is guarded the same way.
Fixes #11935
* style: trunk fmt RadioLibInterface.h
|
||
|
|
b1470cd719 |
fix(heltec): sleep the T1 and T096 panels on screen-off to stop image retention (#11894)
* fix(heltec-t1): sleep the panel on screen-off to stop image retention The T1 is excluded from the LovyanGFX sleep()/wakeup() calls because it uses TFT_eSPI, so DISPLAYOFF only dropped the backlight and the ST7735 kept driving the last frame unlit for the whole screen-off timeout. That constant static image is what burns ghost pixels into the panel. Add opt-in TFT_SLEEP_WHEN_OFF for the TFT_eSPI path: DISPOFF + SLPIN on screen-off, SLPOUT (120 ms) + DISPON on wake, issued as raw MIPI DCS since TFT_eSPI exposes no sleep API. Frame memory survives sleep-in, so the previous frame reappears and the dirty-window diff continues unchanged. The sleep flag keeps the double displayOn() in Screen::handleSetOn() from paying the delay twice. * fix(heltec-t096): sleep the panel on screen-off to stop image retention The T096 sits behind the same TFT_eSPI exclusion as the T1, so DISPLAYOFF only dropped its backlight while the ST7735S kept driving the last frame unlit for the whole screen-off timeout. Same panel, same bus, same burn-in. Opt in to TFT_SLEEP_WHEN_OFF; the TFTDisplay side of the fix is already generic. * fix(tft): harden the TFT_SLEEP_WHEN_OFF wake/sleep sequence Drive VTFT_CTRL LOW before SLPOUT, so the rail is up before the panel is addressed. Wait out the remainder of the 120 ms the controller needs after SLPIN before sending SLPOUT, so a wake landing as the screen timeout fires is not dropped. Guard DISPLAYOFF on panelAsleep to match DISPLAYON. Correct the T1 VTFT_CTRL comment: LOW enables the rail, not HIGH. * fix(tft): wait out only what is left of the sleep-in window before SLPOUT Throttle::isWithinTimespanMs() is true for the whole 120 ms after SLPIN, so the wake path paid a fresh 120 ms on top of however much had already elapsed. A wake 119 ms after the SLPIN waited ~120 ms rather than ~1 ms, up to 119 ms of avoidable latency on every quick off/on. Add Throttle::remainingMs(), which returns what is left of the interval and saturates at 0 instead of underflowing to a ~49 day wait. It reads the clock once, so a caller that tests and then waits cannot be preempted between the two and land on that underflow - which a separate isWithinTimespanMs() plus subtraction at the call site could. heltec-mesh-node-t1 and -t096 both build; neither is board_level = pr, so CI does not compile this path. Docker native suite green, 1515/1515. * trunk --------- Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com> Co-authored-by: Ben Meadors <benmmeadors@gmail.com> Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz> |
||
|
Tom
|
08cd97ea2d |
test(harness): make run-tests.sh drivable by a caller that cannot see the terminal (#11862)
* test(harness): make run-tests.sh drivable by a caller that cannot see the terminal A tool call or a fresh session reads a captured output file, gets interrupted mid-run, and starts cold. Two things in the wrapper tripped that caller: an interrupt left pio's build tree running with no record of it (the next invocation started a second build into the same .pio/build/, or pgrep'd and matched itself); and pio's own "[PASSED]" / "N succeeded" lines made a half-finished output file read as green. run-tests.sh: - Run record in .pio/runtests/current.tsv for the life of a run. A second invocation prints RESULT: BUSY and exits 4 without touching the build directory. Valid while the holder pid OR the recorded process group is alive, so a SIGKILLed wrapper with live scons children reads ORPHANED rather than clear. - --status / --wait / --abort. --abort kills the whole tree by pgid. - pio runs under setsid with its pgid recorded; INT/TERM/HUP kill the tree and record RESULT: ABORTED (exit 5), log kept. - One result() for every verdict: prints to the stdout the script started with (a signal can arrive inside a redirected pio call) and writes .pio/runtests/last-result.tsv with head, args, env, finish time, kept log and a tree fingerprint (HEAD + working-tree diff + untracked files). --status marks the last verdict STALE when the tree has changed since. - Banner naming the final RESULT: line as the only verdict. - Non-Linux host: RESULT: UNSUPPORTED, exit 6, instead of the AMBER code. - A failed build removes .pio/build/<env>/meshtasticd, which run bare would reprint the last good run. - FILTERED lists the not-run count, not 77 suite names. bin/run-tests.cmd forwards into WSL with the exit code passed through, so the same command line works from cmd.exe and PowerShell; no logic is duplicated. test/README.md, copilot-instructions.md and the mirrors document the new codes and the rule. * test(harness): one-suite warm-up, and name the build phase instead of freezing the counter Measured on a full native run: the warm-up, `pio test --without-testing` with no filter, builds AND links every suite - 78 links, 2320 s, 29.7 s each, 39 minutes before the first test ran - and prints a "[PASSED]" line for each program it merely linked. CI never did this; its warm-up is one `platformio run`. The shared src objects are the same whichever suite links them, so the warm-up now links one: the filtered suite when there is one, else test_utf8. The run itself still builds every suite, as it must. The heartbeat counted objects newer than its marker, which sits still through PlatformIO's single-threaded scons dependency scan and through each link - twelve minutes at "430/754 objs, ETA 17m" on that run, which reads as a hung build to a caller who cannot run ps. It now names the phase from the processes in the recorded group: [scons] / [compile] (with the ETA) / [link] / [test], and --status prints the same phase word. * test(harness): define phase_of_run before --status can call it * test(harness): bind --wait to the run it observed; serialize the run-record publish Review findings on #11862, all three valid: - --wait stored a state and then waited for any record to clear, so a run that finished and a second that started between polls would be followed to the second's verdict, and a run that turned ORPHANED mid-wait could print a stale last-result. Each run now has an id (pid-start) in current.tsv and last-result.tsv; --wait captures it and reports only a matching verdict, else ABORTED-without-verdict. - run_state() then the current.tsv write was a check-then-act pair: two invocations in the same instant could both see IDLE. The pair is now one critical section under a short-lived flock, and both records are published by rename so no reader can see a partial file. The record stays the ownership token; the lock only serializes the handoff (a SIGKILLed holder releases flock but not the record, which is why flock alone was rejected). - The FILTERED and AMBER examples in copilot-instructions.md carried a literal suite count, which the same document says never to do. Verified on a live run: --status RUNNING, a concurrent start refused BUSY, a --wait started before --abort reported the aborted run's own verdict with the matching id, exit 5; no build process survived. * fix(waypoints): do not create an empty store file on clear clearAllWaypoints() wrote a two-byte empty store unconditionally, so test_waypoint_expiry left Waypoints_default.wpts behind in every run and the suite has read AMBER (undeclared shared state) since it landed. An existing file - stale or unreadable included - is still rewritten as empty, so a reset after a failed load clears flash as before; a file that is not there is left not there. |
||
|
Tom
|
332c4d7c6f |
Narrow the ad-hoc NodeInfo greeting (#11897)
* feat(nodedb): greet only while the node store is under half full The ad-hoc greeting in MeshService::handleFromRadio() was gated on !isFull(), so a node kept sending unsolicited NodeInfo right up to the last free slot - on a dense mesh that is the regime where the store is already churning and the greeting is least likely to buy a lasting entry. Add NodeDB::isHalfEmpty(), true only when strictly more than half the slots are free, and gate the greeting on it instead. The comparison is written as 2 * numMeshNodes < cap so a half-full store reads false with no integer rounding, and MAX_NUM_NODES is read into a local because portduino resolves it through a runtime call. The helper keeps the MINIMUM_SAFE_FREE_HEAP term that !isFull() used to contribute: low heap disqualifies the store regardless of occupancy, so a sparse database on a memory-starved device still does not transmit. Admission is untouched - updateFrom() and getOrCreateMeshNode() still fill to capacity. Only greeting stops early. * fix(nodeinfo): raise the minimum greeting window to 30 minutes The !shorterTimeout branch of NodeInfoModule::allocReply() used a 10-minute base, so a node that had just greeted one neighbour could greet the next ten minutes later. Raise the base to 30 minutes. This is the floor, not the window: getConfiguredOrDefaultMsScaled() still multiplies by the congestion coefficient for the roles that scale, so a busy mesh stretches it further. ROUTER/ROUTER_LATE and the tracker/sensor roles bypass the scaling and get a flat 30 minutes. The interactive paths are unaffected - they pass shorterTimeout and keep their own 60-second gate. The periodic broadcast is unaffected too: default_node_info_broadcast_secs is 3 hours with a 1-hour minimum, both clear of the new floor, so the timer is not swallowed by the throttle. * fix(nodeinfo): a send restarts the routine broadcast countdown sendOurNodeInfo() left the OSThread schedule alone, so an ad-hoc send had no effect on the periodic broadcast: run() anchors the next run at runned() + interval, and nothing re-anchored it when the send came from a greeting, a PKI decrypt failure or a completed key verification. The routine copy could follow minutes behind an ad-hoc one, putting two NodeInfos on the air for no gain. Call setIntervalFromNow() with the configured broadcast interval once the packet is queued, so the next periodic copy is a full interval from the send rather than from the last tick. It sits on the return-true path only: a send vetoed by allocReply() - throttle, airtime ceiling, reply suppression - must not be able to silence the routine broadcast. Calling it from inside runOnce() is harmless, since run() then applies the same interval from a last_run of effectively now. * test(nodeinfo): cover the send window, the countdown reset and the greeting gate Three behaviours from this branch had no coverage: isHalfEmpty()'s exclusive boundary, the 30-minute send floor, and the countdown reset on a send. isHalfEmpty() goes to test_nodedb_blocked, which already owns the full-store cases and clears the hot store per test. Three tests sweep the cap over the sizes real deployments have - portduino resolves MAX_NUM_NODES from General.MaxNodes on every read, so a predicate that cached it would greet at the wrong occupancy - and pin the band where admission outlives greeting. That suite had no tearDown; it has one now, restoring the cap so an assertion firing mid-sweep cannot leak a 2-node cap into the tests after it. test_nodeinfo_send_window is new because nothing in the tree stands up NodeInfoModule's send path. Six tests: the floor at 30 minutes with 10 refused, the interactive 60-second gate staying separate, the countdown re-armed by a broadcast and by an ad-hoc unicast, left alone by a refused send, and a preset change consumed only by a send that goes out. The scaling above 40 online nodes is deliberately not retested here - getConfiguredOrDefaultMsScaled() is test_default's contract, per preset and per role. These tests pin the base and leave the multiplier alone. NodeInfoModule gains two PIO_UNIT_TESTING accessors for the countdown: concurrency::OSThread is a private base, so a test shim cannot reach it and only the class itself can. They compile out of a shipping build. The heap term in isHalfEmpty()/isFull() stays uncovered: memGet.getFreeHeap() returns UINT32_MAX on portduino, so a native test could only pin a stub. * chore(trunk): exempt test_nodedb_blocked from the trufflehog Lob detector test_removeNodeByNum_presentNodeOnFullDb is exactly 35 characters after the test_ prefix, which is the length of a Lob API key, and trufflehog's detector matches the bare identifier. The name is years old; it surfaces now only because this branch touches the file, and the pre-push gate reports a finding in a changed file as new. Added to the ignore block that already carries the same detector's hex-literal false positives, with the reason stated alongside them. Nothing in that file is a credential. * fix(nodeinfo): exempt a licensed station from the floor, delay only on a real send Two review findings on the 30-minute window. Ham mode sets node_info_broadcast_secs to 600 s for the FCC minimum call-sign announcement (AdminModule.cpp). The new floor refused every one of those sends until 30 minutes had passed, so a licensed station's call sign went out three times less often than the regulation asks - a regression the old 10-minute base did not have. A licensed station now keeps its own interval whenever that is shorter than the floor. The exemption is exactly the licensed case because nothing else can get under the floor: a set-config clamps the field to an hour, and the userprefs path clamps identically. sendOurNodeInfo() ignored what sendToMesh() returned, so a packet the router declined - no interface, queue full - still re-armed the routine broadcast and still reported success, which let runOnce() consume a pending channel change for a send that never reached the air. Only ERRNO_OK and ERRNO_SHOULD_RELEASE now count; sendToMesh() has already released the packet in both cases. Both are pinned by tests that fail without them, measured: the licensed case fails at "11 min is past it, and the floor must not override it", the declined send at "a declined send is not a send". The licensed test carries an unlicensed control on the same configuration, so deleting the floor outright would not satisfy it. * test(nodeinfo): assert the deadline the scheduler reads, from an aged last_run The countdown cases asserted Thread::interval, which is not what schedules the next run: shouldRun() keys off _cached_next_run, and the two ways of writing it differ. setIntervalFromNow() recomputes it from now; Thread::setInterval() recomputes it from last_run. Swap the call in sendOurNodeInfo() for the latter and the period still reads three hours while the deadline lands wherever the last tick was - firing the routine copy right behind an ad-hoc send, the exact thing the reset exists to prevent. Every test passed. Assert the deadline instead, from a fixture where the two answers are distinguishable: ageLastRunForTests() calls Thread::runned() with an hour-old timestamp, the state a periodic thread is genuinely in between runs, so a deadline off last_run lands an hour early against a five second tolerance. Measured: with setInterval() in place of setIntervalFromNow(), the new case fails by 3600004 ms and the eight others pass, including the one asserting the period - which is what says the old assertion could not see this. runned() and _cached_next_run are protected in Thread and OSThread is a private base, so the hooks live on NodeInfoModule, with the two already there. Raised by Copilot on #11897. * fix(nodeinfo): a declined send must not start the throttle window either allocReply() stamped TransmitHistory when it built the packet, before anything had been sent. The previous commit made sendOurNodeInfo() report a router rejection instead of swallowing it, but the stamp was already written by then, so a packet that never reached the air still started the window - and with the floor now at 30 minutes, that silences the node for half an hour over a send that failed. allocReply() has two callers and only one of them can see the outcome: the module framework sends its own reply through currentReply, with no post-send hook a module can reach (MeshModule::sendResponse is not virtual). So the stamp stays there for that path, and sendOurNodeInfo() defers it across its own allocReply() call and stamps once the router has accepted the packet. deferHistoryStamp mirrors the shorterTimeout member alongside it - same call-scoped signal, same lifetime. test_sendWindow_aRejectedSendDoesNotStartTheWindow asserts both halves: no stamp after the rejection, and the retry immediately after goes out. The existing rejected-send case checked the first failure and the countdown only, which is how this survived it. 248/248 across every suite that touches NodeInfoModule (admin_session_repro, admin_radio, nodeinfo_send_window, traffic_management, fuzz_packets) plus transmit_history, whose subject this is. Raised by CodeRabbit on #11897. |
||
|
TomandBen Meadors
|
20f7ab1be9 |
Relay a PKI unicast with a known party in LOCAL_ONLY and KNOWN_ONLY (#11898)
* fix(router): relay a PKI unicast with a known party in LOCAL_ONLY and KNOWN_ONLY A frame the relay cannot decrypt takes the OPAQUE_RELAY_ONLY path in Router::perhapsHandleReceived() and returns before handleReceived(), so it never reaches a module. The rebroadcast_mode rule for opaque traffic is therefore the IS_ONE_OF list in relayOpaquePacket(), and LOCAL_ONLY and KNOWN_ONLY were not on it: a node in either mode dropped every opaque frame, including a PKI unicast with a party it knows. The gate in RoutingModule::handleReceivedProtobuf() that used to allow exactly that is unreachable for these packets and no longer decides anything. Add both modes to the list, with the identity rule the unreachable gate carried: a PKI-shaped unicast (channel 0, not broadcast) with `from` or `to` known to us. An unreadable broadcast and a unicast between two strangers stay dropped in these modes, which is what the proto documents - they ignore foreign meshes. This is not only direct messages. Remote administration and key verification are PKI unicasts too, and a KNOWN_ONLY relay was black-holing those between two other nodes just the same. CORE_PORTNUMS_ONLY reached this list the same way in #11843; this is the remaining pair. * test(rebroadcast_mode): pin the relay decision per mode, in both directions What this node carries for other nodes, per DeviceConfig.rebroadcast_mode, for packets it can read and packets it cannot. The harness pushes a real PKI frame through Router::perhapsHandleReceived() and counts what reaches the radio. Both directions are load-bearing, and each is guarded by cases the other leaves green. Measured by rebuilding the firmware three ways: - as shipped: 9/9 pass. - with the two modes taken back out of relayOpaquePacket(): the known-party and remote-admin cases fail, the stranger and foreign-mesh cases still pass. - with the modes listed but the identity qualifier deleted: the stranger and foreign-mesh cases fail, the known-party cases still pass. So a revert and an over-broadening each fail their own tests, and neither can be satisfied by breaking the other. The suite and its harness come from the opaque-packet-handling branch and compile against develop unmodified. Two assertions were dropped because they pin behaviour this branch does not add: delivery of unreadable frames to the phone, and a queued copy of our own suppressing the originator's repeat, which needs relayOpaquePacket() to consult the TX queue. * fix(test): guard the harness include, drop a comment its test outlived Two review findings on the imported suite. The harness builds real PKI frames through CryptoEngine entry points a MESHTASTIC_EXCLUDE_PKI build does not declare, but it was included above the guard, so the empty-suite branch that exists for those builds could not compile. Move the include inside the guard and pull the base includes the stub branch needs above it. The phone-delivery case was dropped when the suite came across - this branch does not deliver unreadable frames to the phone - but its comment stayed behind and now described the test below it, which is about the signature policy. * test(rebroadcast_mode): make the from-known and channel-0 operands load-bearing The qualifier has three operands, and the suite only exercised one of them. Every existing case that a known party carries has a known DESTINATION: the remote-admin case marks both parties, the known-destination case marks the target. Rewriting the identity test to consult p->to alone passed all nine. And the only non-PKI-shaped frame in the suite was a broadcast, which !isBroadcast(p->to) rejects before p->channel is ever read, so deleting the channel gate passed all nine too. Add the two cases that close it: a known SOURCE with a destination we have never heard of, which must relay in both modes, and a known party on a channel hash we do not hold, which must not - that is someone else's channel traffic, addressed, not PKI. Measured both ways. With the from operand and the channel gate removed from relayOpaquePacket(), the two new cases fail and the other nine pass; with the shipping code, 11/11. Raised by CodeRabbit on #11898. --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Jonathan BennettandClaude Opus 5
|
d3b4b343e7 |
Send an ack over PKC when no channel can carry it (#11891)
* Send an ack over PKC when no channel can carry it PKI needs only the two keys, so a DM can reach us over a channel we do not carry. Its ack is a ROUTING packet, which wouldEncryptWithPKC() excludes, so today it is channel-encoded, fails at setActiveByIndex() with NO_CHANNEL, and is never sent. The sender sees nothing and retransmits to exhaustion for a message that was in fact delivered. Fall back to PKC for exactly that case. This is the one place an ack is deliberately made opaque to relays; normally that costs next-hop learning and intermediate retransmission cancel, which is why ROUTING is PKC-excluded in general, but here there is no readable alternative to lose, because without this the ack does not exist. The predicate is scoped as tightly as that argument reaches: a unicast ROUTING packet we originate, carrying a request_id, to a destination whose key we hold, under the same ham/sim/private-key preconditions PKC always has, and only when the channel index does not resolve. It tests channels.getHash() rather than setActiveByIndex() so it has no side effect; generateHash already returns -1 for an invalid key, so the two agree on which indexes are unusable. Four cases in test_packet_signing pin the corners: the fallback fires, it does not paper over an ack with no destination key, it does not catch a non-ack on the same unusable channel, and an ack on a channel that does resolve still goes out readable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 * Short-circuit the fallback so an out-of-range channel logs no error wouldEncryptWithPKC() reaches channels.getName(chIndex) before its portnum exclusion, and getByIndex() logs "Invalid channel index" on the way past. With the general predicate tested first, an ack on an out-of-range index printed that error and then went on to encode successfully. Test ackFallback first so the case that is about to succeed never asks. Also record why the range check leads inside the predicate: getHash() is a bare hashes[i] with no bounds test of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
Jonathan BennettandClaude Opus 5
|
ee02cc3426 |
Games joystick input (#11917)
* fix(games): correct the high-score announcement argument order GAMES_HIGH_SCORE_STRING is "New %s high score %lu by %s!" but the arguments were passed as (name, initials, score): the initials string was formatted through %lu and the score integer through %s. That is a format/argument mismatch, so the announcement printed garbage at best and dereferenced the score as a pointer at worst. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(input): report which physical gamepad button produced an event A joystick event only carried the action it was mapped to, so a consumer could not tell two buttons apart once they shared one action, and games were limited to the handful of actions the broker defines. Carry the originating evdev button code in InputEvent::kbchar, encoded into a reserved 0xC0..0xDF range that misses printable ASCII and every INPUT_BROKER_MSG_ value (SystemCommands switches on kbchar without looking at inputEvent, so a collision there would reboot the node rather than move a paddle). D-pad events are axes, not buttons, and keep leaving kbchar at 0 -- which is exactly what lets a consumer tell stick from button. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(portduino): let one joystick action bind several buttons Input.JoystickButtons took a single evdev code per action, so a pad's A and Y could not both select, and the shoulder buttons could not sit alongside the D-pad. Accept a list of codes as well as a bare scalar; the config writer inverts its code->action map back out, emitting a list only where an action has more than one button. ConfigCheck gains a real checker for the section (it was previously waved through as free-form) covering the three ways a mapping silently does nothing: an action name the driver does not know, an evdev name where the numeric code belongs, and one code claimed by two actions. Two fixtures and shell-test cases cover the clean list form and those three faults. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(games): use the gamepad's extra buttons, and return home when idle Games now receive the physical button alongside the action, so a pad with more than two usable buttons controls more than two things: - Snake: a shoulder button mapped to left/right turns relative to the snake's heading (L counter-clockwise, R clockwise) while the D-pad keeps steering absolutely. The two are told apart by kbchar, not by hardcoding one pad's codes. - Breakout: the ball now rides the paddle after each serve until the player fires it with B or A, so a life is not lost to a ball already in flight when the player looks up. The paddle also keeps its position between lives. A game can claim BACK for the duration (Game::wantsBackButton) so B serves instead of pausing, and releases it once the ball is live. - Start (BTN_BASE4 / BTN_START) is mapped to select like any other button, so it launches games and drives the menus; inside a running game GamesModule picks it out of kbchar and pauses instead. Separately, the games frame no longer holds a walked-away device hostage: after 15 s with no input it returns to the home frame, so the device still reads as a Meshtastic node. The timer is suspended while a picker or banner is up (e.g. high-score initials entry, which the input handler never sees) so it cannot yank the user out mid-entry. Screen::isInteractionBusy() generalises the old module-intercept check -- modal module, intercepting module, game, or an open interactive overlay -- and MessageRenderer uses it before popping an incoming-message banner. A transient banner REPLACES an active overlay, so an arriving message could otherwise discard a half-entered high score. The message is still stored, its thread still selected, and the unread indicator still set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(ui): compose freetext on the on-screen keyboard from a gamepad A gamepad can drive the on-screen keyboard but cannot type, so on a host with a joystick and no configured keyboard device the OSK is the only way to compose freetext. Set osk_found there, and gate the "Freetext" menu entries on whether the device can enter text at all (physical keyboard, OSK, or touchscreen virtual keyboard) rather than on kb_found alone -- those entries were hidden on exactly the devices that needed them. The OSK prompt that CannedMessageModule already had inline in the message selector becomes showOnScreenKeyboard(), so the menu path can reach it too. Menus call in from a banner callback and the banner is torn down as soon as that callback returns, which would take the keyboard down with it, so the menu path defers the launch to runOnce(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(games): address review on frame fallback and joystick input gating Breakout: the paddle suppression was far too broad. aLinuxJoystick is constructed on every Linux host whether or not a gamepad is configured (InputBroker.cpp), so `aLinuxJoystick && kbchar == 0` was true everywhere and swallowed LEFT/RIGHT from the keyboard, trackball and ExpressLRS -- on a host with no joystick attached at all. Gate on the stick actually driving the paddle instead: LinuxJoystick assigns heldX before it emits and only auto-repeats while heldX is set, so every axis LEFT/RIGHT arrives with a zone held and nothing else does. kbchar == 0 still distinguishes an axis from a shoulder button mapped to left/right, which must keep nudging the paddle. Screen: showHomeFrame() did nothing when the home frame was hidden, since setFrames() only assigns positions.home for !hiddenFrames.home. That stranded the games inactivity bounce on the frame it was trying to leave. Fall back to the messages frame, which setFrames() always adds. Test: rename test_ballWaitsOnPaddleUntilLaunched to test_ball_waitsOnPaddleUntilLaunched, matching the repo convention and its neighbours in the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(games): give games the whole InputEvent so Breakout can identify the source Follow-up to review on #11917. The previous narrowing still could not tell sources apart: kbchar == 0 is shared by the joystick's D-pad axis and by every other driver that sends a bare LEFT/RIGHT, so while the D-pad was held a keyboard or touchscreen press was still discarded. heldXZone() proves the axis is driving, not that this particular event came from it. Pass the event itself to Game::handleInput() rather than (ev, kbchar). Games that only care about the action read event->inputEvent; Snake keeps using kbchar for shoulder steering; Breakout now also checks event->source against LinuxJoystick's origin name, so only that driver's own axis repeats are suppressed. Chose the event over a third positional parameter so the signature does not have to grow again the next time a game needs something the event already carries. All three conditions in Breakout are load-bearing: source says it came from this gamepad, kbchar == 0 says it is the axis rather than a shoulder button mapped to left/right, and heldXZone() != 0 says the axis is what is driving right now so tick() already has it covered. LinuxJoystick::originName() exposes the name the driver stamps into InputEvent::source, alongside the existing heldXZone()/heldYZone() accessors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Thomas Göttgens
|
c29bd00971 |
Rewrite the MQTT region root topic only on the default broker (#11899)
* fix(mqtt): rewrite the region root topic only on the default broker A region change rewrote any root starting with "msh", on any broker. Custom roots such as msh/home were clobbered, and private brokers had their topics moved even though a regional broker is regional already. An empty root, which MQTT treats as the default, was never updated. Region changes now go through MQTT::applyRegionRootTopic(), which rewrites the root only on the default broker and only when the root is empty, the default, or a msh/<region> the firmware wrote itself. * fix(mqtt): parse the broker address before the default-server check Persist module config only when the root actually changed. Replace a stray NUL byte in the test with the \0 escape. * fix(mqtt): count the regional roots as the default root topic |
||
|
Benjamin Faershtein
|
3aac179397 | fix(phoneapi): wake clients after config sync (#11818) | ||
|
Thomas GöttgensandBen Meadors
|
0dafcc90fe |
fix(api): retain the unwritten tail on a short TCP API write (#11890)
* fix(api): retain the unwritten tail on a short TCP API write ServerAPI closed the session whenever stream->write() returned fewer bytes than requested. A short write is transmit-buffer backpressure, not a dead socket, and it is most likely during the back-to-back frames of the initial NodeDB dump, so a node at its node cap dropped clients on effectively every connect. Route TCP frames through StreamFrameWriter, the retained-tail path the USB CDC console already uses: the remainder is re-offered on the next pass and the session is closed only when the link itself is gone. Poll at 25ms while output is still undelivered, since nothing wakes the thread when the socket frees transmit space. Fixes #11822 * fix(api): block log re-encoding while a TCP frame is retained emitLogRecord() writes into txBufLog and StreamFrameWriter can now hold that buffer as a retained tail, so a second log record would overwrite bytes the transport has not sent yet. Gate encoding on the retained-frame state, matching SerialConsole. No caller reaches this today (emitLogRecord() is only used by SerialConsole), but retaining the buffer at all is new here. --------- Co-authored-by: Ben Meadors <benmmeadors@gmail.com> |
||
|
Ben Meadors
|
585ce17f59 |
fix(nrf52): stop concurrent flash writers corrupting LittleFS, and stop a failed save formatting it (#11872)
* fix(nrf52): serialise the warm-node ring against LittleFS on the shared flash cache On nRF52840 the warm-node store writes its 3-page record ring straight through flash_nrf5x_write/erase/flush, holding only spiLock. Every LittleFS writer instead holds Adafruit_LittleFS's own mutex, and two of them run on other tasks entirely: Bluefruit's bond saves on the callback task, and - since phone config writes moved into BLE context - a whole saveToDisk on the BLE task. Neither takes spiLock. Both writers share one 4 KB page cache, one SoftDevice flash semaphore and one result word. flash_cache_write repoints that cache when the requested page differs from the cached one, so a second writer arriving mid-write flushes the first writer's page and re-points the buffer; the first writer's remaining memcpy then lands in the wrong page's image. Ring records end up inside LittleFS metadata, or the reverse. The collision also exhausts the flash layer's 20 x 1 ms busy-retry budget against an 85 ms page erase, and flash_cache_flush discards the failure, so 32 LittleFS blocks vanish with no error reaching the filesystem. What the user sees is a torn directory pair on the next mount, a format, and critical error 13. Take the filesystem mutex in the five ring entry points that reach flash, after spiLock and never before - the order every existing path already uses. The ring touches no LittleFS call itself, so the non-recursive mutex is never re-entered. Longest new hold is a page rotation at roughly half a second, against a 2 s supervision timeout and a 90 s watchdog. Non-nRF52840 backends are untouched. * fix(nodedb): make saveProto report a failed readback or rename SafeFile::close() already verifies the .tmp by hash and renames it over the live file, and saveProto captured that result, logged it, and then returned the pb_encode status alone. A torn or half-programmed page therefore counted as a successful save: for the fullAtomic files the old contents silently survived, for nodes.proto (written in place) the file was simply gone, and saveToDisk's recovery path never fired for the one failure it exists for. * fix(nodedb): retry a failed save before formatting, and never format on a low rail saveToDisk answered any failed write with an immediate fsFormat(), which is where most "critical error 12/13" reports and the total config wipe behind them come from. A write that fails once is far more often a busy SoftDevice or a VDD dip mid-save than a corrupt filesystem, so: - retry twice, 150 ms apart, re-checking powerHAL_isPowerLevelSafe() before each attempt and before the format; on a low rail return false and leave the filesystem alone (the next save lands once the rail recovers, and boot already waits for a safe level) - check fsFormat()'s result instead of assuming it worked - after a successful format rewrite every segment, not only the ones this call asked for: the format took config.proto and the node identity with it, so a nodes-only save that ended in a format used to come back up as a new node - with encrypted storage a format also destroys the DEK; skip the resave rather than land the private key and PSKs on flash in plaintext RP2040 feeds its watchdog across the delays, as the neighbouring code does. * fix(nrf52): quiesce flash before every software reset and power-off The Adafruit flash layer keeps one 4 KB page image and one SoftDevice flash semaphore for the whole chip. Every reset path we own - Power::reboot(), enterDfuMode() (admin enter_dfu_mode_request, which arrives on the BLE task since #10967), cpuDeepSleep()'s reset and system-off arms, and the wio-t1000-s secure DFU handler - went straight to NVIC_SystemReset or sd_power_system_off while another task could be half-way through a page program or erase. A reset in that window leaves the page erased or partly programmed; LittleFS finds the torn metadata on the next mount and the corruption handler formats the filesystem. nrf52FlashQuiesce() takes spiLock and the LittleFS mutex, waits out whatever write is in flight, flushes the page cache, and keeps both locks because the caller resets next. The corruption-reboot handler and __assert_func are left alone: they run inside the filesystem call stack or a fault, where taking the mutex would deadlock. nRF54L is a second copy of these paths since #11867 and still defines ARCH_NRF52, so it gets the same function on the same core flash layer. * fix(nrf52): quiesce flash before the library BLE DFU handler jumps to the bootloader On every board except wio-t1000-s the Nordic DFU service is the framework's BLEDfu, whose START_DFU handler runs on the callback task and jumps to the bootloader with no regard for a flash write in progress on the loop task. That is the OTA path the Apple app and nRF Connect use (Android sends enter_dfu_mode_request instead, which the previous commit covers). QuiescingBLEDfu re-installs the control-point write callback after BLEDfu::begin() and wraps the library's: flush under both locks, then drop the LittleFS mutex before handing over, because the library reloads the bond keys through LittleFS on its way to the jump and the mutex is not recursive. spiLock stays held across the handler: every LittleFS writer on the BLE task takes it first, the loop task cannot preempt the callback task, and the handler never blocks after the flush, so nothing can dirty flash before bootloader_util_app_start(). If the handler returns, nothing jumped, and the lock is released. The library callback is a file-static, so it is read back out of the characteristic through a pointer-to-member obtained via a using-declaration; that is well-formed C++ and compiles under the pinned GCC 9.3 with LTO. * test(nodedb): pin the save-failure contract of saveProto and saveToDisk A failed rename must come back as false from saveProto, a one-off unsafe rail reading during a write must be retried and land, and a rail still unsafe at the retry gate must make saveToDisk return false with the filesystem untouched. The rail is scripted through a strong powerHAL_isPowerLevelSafe() over the weak native default; on Windows the default is strong, so only the rename case runs there. The format branch itself is unreachable natively (a FLASH_CORRUPTION critical error exits the portduino process), which is what the survival assertions pin. * fix(nodedb): only format when the filesystem itself is unreadable Making saveProto honest about write failures gave the recovery path a new way in: any persistent write failure now reached fsFormat(), which takes every file with it. A busy or lock-protected nRF52 flash fails every write for as long as it lasts, so two retries are not enough to tell that apart from a corrupt filesystem, and guessing wrong costs the node its config, keys and bonds. Reads settle it. They never touch the SoftDevice write path that a busy flash fails on, so if /prefs still walks and a stored proto still opens and reads, the metadata chain is intact and the write failure was transient - return false and let the caller try again later. Genuine corruption is not silently tolerated: lfs asserts on it, and the nRF52 handler reboots and formats on the way back up. Covered by a test that fails without this: a save whose rename cannot succeed, against an otherwise healthy filesystem, must leave devicestate untouched. * trunk: exempt Unity test entry points from trufflehog trufflehog's Lob detector matches "test_" followed by alphanumerics, which describes every Unity test function name. It fired on a new test in test_nodedb_save_retry and will fire again on the next suite added. Scoped to test/**/test_main.cpp, alongside the existing gitleaks exemption for the synthetic node-DB fixtures. * fix(nodedb): feed the RP2040 watchdog around the format and the resave saveToDisk() only feeds the watchdog at the top of each retry. The last retry, the readable probe, fsFormat() and the five-segment resave then share one 8 s budget (watchdog_enable in main-rp2xx0.cpp) with no loop left to feed it. A timeout during the resave leaves the filesystem empty and the node boots on defaults with a new identity - the exact outcome this PR exists to prevent, reached by a different road. Feed once before the probe and again before the resave. Both feeds sit outside any lock: filesystemStillReadable() takes spiLock itself, and the format has already released it. ARCH_RP2040 covers rp2040 and rp2350 alike, and the blocks compile out everywhere else, so no other platform and no native test changes. Raised by @caveman99 in review. * fix(nodedb): narrow the save-probe comment and name the full-filesystem case The comment on filesystemStillReadable() claimed "real corruption asserts in lfs and formats on reboot". That does hold on nRF52 - nrf52.ini builds with -DLFS_NO_ASSERT and force-includes cpp_overrides/lfs_util.h, whose LFS_NO_ASSERT arm routes LFS_ASSERT to the lfs_assert() in main-nrf52.cpp, which stamps NRF52_MAGIC_LFS_IS_CORRUPT and resets into the format - but NodeDB.cpp compiles for ESP32, RP2040 and portduino too, where nothing of the sort is wired up. It is also not true on nRF52 under POFWARN, where lfs_assert() deliberately skips the stamp. Drop the claim rather than qualify it three ways. The log line now names what a field log actually needs to tell apart: a filesystem that still reads but cannot be written is either busy or full. Raised by @caveman99 in review. * fix(sx128x): quiesce flash before the 2.4GHz region reset reinitChip() saves the region, waits 2 s and resets. On nRF52 that was the last software reset still going straight to NVIC_SystemReset with a page program possibly in flight, so "every software reset" in the earlier commit did not quite hold. The quiesce stays inside the ARCH_NRF52 arm on purpose. The #else arm logs and falls through to lora.setCRC() further down, which re-enters spiBeginTransaction(); a quiesce hoisted above the #if would take spiLock and never give it back, self-deadlocking portduino and stm32wl. Routing this through Power::reboot() is wrong for the same class of reason: setupModules() runs before initLoRa, so its notifyReboot observers and waypointStore.saveToFlash() are live and would add a flash write to an aborted radio init. Raised by @caveman99 in review. * fix(nrf52): only quiesce on the DFU control write that actually resets QuiescingBLEDfu wrapped every control-point write, so a write that was never going to reset still blocked the Bluefruit callback task on spiLock, forced an early page-cache commit and held back the GATT authorize reply. Only START_DFU resets; gate on that. Deliberately no "request->len &&" term. The library's own test is `request->data[0] == START_DFU` with no length check (BLEDfu.cpp:110 in both the nRF52 and nRF54 cores), and Bluefruit hands the callback a copy of a reused event buffer, so a zero-length write carrying a stale 0x01 still resets inside the library. A len term here would let exactly that reset run unquiesced, which is the case this wrapper exists for. Reading data[0] is always in bounds: ble_gatts_evt_write_t declares uint8_t data[1] and the copy covers it. Raised by @caveman99 in review. |
||
|
Jonathan BennettandClaude Opus 5
|
6f3f0bd7c2 |
Prove explicit acks with Routing.ack_proof (#11877)
* Prove explicit acks with Routing.ack_proof
Explicit acks are ROUTING_APP packets, and ROUTING_APP is excluded from PKC, so
an ack travels under channel encryption alone - and the default channel key is
public. Anyone in range can forge one, and the client grants its strongest
delivery claim on the strength of the ack's unauthenticated `from`.
Where the acknowledged packet was PKI encrypted the endpoints already share a
Curve25519 secret, so the recipient can prove receipt in ~10 encoded bytes:
ack_proof = HMAC-SHA256(shared_key,
"ack" | LE32(from) | LE32(to) | LE32(request_id)
| routing)[0..8)
where `routing` is the encoded Routing message without the ack_proof field,
taken as received with that byte range removed rather than re-encoded. Excising
keeps the value a function of the received bytes alone, so it does not depend on
two implementations' encoders agreeing and does not drop fields this build has
never heard of. For the same reason the sender appends the field rather than
setting it on a decoded struct and re-encoding.
What this does and does not buy. It buys an authenticated delivery receipt from
the actual recipient, which is the property a forged ack costs a user and which
matters where people act on a delivery confirmation. It does NOT protect the
retransmission loop, and must not be described as if it does:
perhapsGenerateImplicitAckForOwnOverheard clears a pending retransmission on any
overheard rebroadcast of our own (from, id), header-only and keyless, so
replaying the originator's own ciphertext stops their retries more cheaply than
forging an ack. No ack authentication of any kind closes that path.
Advisory only, and deliberately not a step toward enforcement. A rule requiring
a proof once a peer has sent one would make a missing proof destroy the only
delivery signal we have, on state the user cannot see: the proof needs the peer
to hold our key, and peer-side eviction, a downgrade or a factory reset are all
invisible to us. A valid proof marks the ack verified; anything else behaves
exactly as today. The client renders the difference.
Verification costs one X25519 and nothing caches the shared secret, so
ackProofPermitsAction gates on findPendingPacket first - otherwise a forged ack
naming any packet id, which is visible in the cleartext header, would force a DH.
Depends on meshtastic/protobufs#1094 for the generated field.
* test: correct a comment that predates ack_proof being generated
The wire-roundtrip test described ack_proof as an unknown field, which was true
while the prototype hand-encoded it. The field is generated now, so this build
understands it - but older firmware does not, which is the case the assertion
actually covers.
* Fix the EXCLUDE_PKI test build, and format
Two copies of test_proof_binds_error_reason and test_proof_binds_direction were
sitting in the #else branch of test_ack_proof, where Identity, makeIdentity,
makeAck, becomeNode, crypto and ACK_PROOF_SIZE do not exist. They were also
unregistered, so they were dead code that only served to break the build. The
native suite never compiles that branch, so a local run could not see it.
Also apply clang-format to a declaration that had been wrapped by hand.
* Exempt test_ack_proof from trufflehog's Lob false positive
Same detector and same shape as the three suites already listed here: it
stitches nearby hex literals into one candidate string, and this suite's node
numbers and request ids (0x0A0A0A0A, 0x0B0B0B0B, 0xABCD1234) happen to match a
Lob API key. The suite holds no literal key material - every key it uses comes
from crypto->generateKeyPair at runtime.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
Thomas Göttgens
|
f36a1ea821 |
nRF52: reclaim flash to bring rak4631 back under its size budget (#11873)
* build(nrf52): drop unused TinyUSB classes and assert function-name strings Only the CDC class is used on nRF52. Disable the MSC, HID, MIDI, vendor and video class drivers in the Adafruit TinyUSB config, and pass an empty __ASSERT_FUNC so assert() no longer embeds __PRETTY_FUNCTION__ strings. File and line are still reported. rak4631 estimate: ~7.4 KB flash, ~2.3 KB RAM. * fix(nrf52): link only the secp256r1 cc310 curve domain CRYS_ECPKI_GetEcDomain indexes ecDomainsFuncP, which references the parameter tables of all eleven cc310 curves. Bluefruit LESC pairing only requests secp256r1, so override the lookup to return that domain alone. rak4631 estimate: ~7.4 KB flash. * fix(airtime): replace powf in the channel-utilization EMA fold foldChannelUtil was the only powf caller on nRF52. The exponent is an integer step count, so raise the EMA factor by squaring instead; a multi-day sleep still folds in at most 32 multiplications. rak4631 estimate: ~1.9 KB flash. * fix(graphics): use double sin/cos in the compass renderers The compass renderers were the only sinf/cosf callers on nRF52 screen builds, pulling in the float trig kernels next to the double ones GeoCoord already links. Call the double variants instead. rak4631 estimate: ~3.2 KB flash. * fix(motion): use double atan2 for magnetometer heading fallbacks MMC5983MA, QMC6309 and the InkHUD map centre were the remaining application atan2f callers. The double atan2 is already linked, so the float variant only added atan2f, __ieee754_atan2f and atanf. The saving lands once meshtastic/Fusion#1 removes the library's atan2f as well. rak4631 estimate: ~0.8 KB flash with Fusion#1. * fix(hopscale): trim diagnostic logging to state changes and anomalies Drop the save/restore confirmations, the hourly histogram and trend dumps, the denominator step logs and the per-packet hop_limit log (printPacket already reports HopLim). Keep the save-failure and histogram-full warnings, the congestion on/off transition and a single periodic status line, and remove lastScaledPerHop, which only fed the logs. * fix(hopscale): silence cppcheck uselessAssignmentArg on restored count * perf(crypto): use full-schedule AES128/AES256 for AES-CCM aesSetKey used AESSmall128/AESSmall256, which re-derive round keys for every block. AES128/AES256 precompute the schedule, encrypt faster and are already linked by encryptAESCtr, so the AESSmall*/AESTiny* code drops out. No change on ESP32, where AESSmall* already aliases AES128/AES256. rak4631 estimate: ~3.9 KB flash; cipher object up to 184 bytes larger. * perf(nrf52): use the shared software CTR for AES-256 and remove tiny-aes CryptoCell only accelerates AES-128, which stays on hardware. AES-256 CTR now calls CryptoEngine::encryptAESCtr (rweather CTR<AES256>, already linked) instead of the in-tree tiny-aes copy, whose sources were removed in the previous commit. Output is identical. rak4631 estimate: ~0.8 KB flash. * perf(mesh): use std::map for pending retransmissions and API port timestamps NextHopRouter::pending and PhoneAPI::lastPortNumToRadio were the only unordered_map instances linked on nRF52. Switching them to std::map, which is already linked, drops the libstdc++ hashtable, rehash policy and prime table. GlobalPacketId gains operator<; the unused hash functor is removed. rak4631 estimate: ~2.1 KB flash. * perf: parse sensor decimals without strtod The WS85 serial parser (strtof) and DFRobotLarkSensor (String::toFloat) were the only callers of newlib's strtod. Add parseDecimalFloat to meshUtils for plain [+-]digits[.digits] fields and use it at both sites. Covered by test_type_conversions against strtof. rak4631 estimate: ~4.5 KB flash. * perf(gps): compute tan from sin/cos in UTM and OSGR conversion latLongToUTM and latLongToOSGR were the only tan callers. sin and cos are already linked, so deriving tan from them drops tan and __kernel_tan. rak4631 estimate: ~1.1 KB flash. * fix(graphics): only dispatch the theme menu when TFT coloring is enabled The Theme option is only offered with GRAPHICS_TFT_COLORING_ENABLED, but handleMenuSwitch dispatched ThemeMenu unconditionally, linking kThemes and the theme accessors into monochrome builds where the menu is unreachable. rak4631 estimate: ~1 KB flash. * fix(senxx): trim diagnostic logging to errors and user-visible actions Keep all errors and warnings and a single version line; shorten the admin action messages; drop progress chatter, state save/restore confirmations and the per-reading and VOC-state debug dumps. The nested VOC restore branch collapses to one condition with the same behaviour. rak4631 estimate: ~2 KB flash. * build(nrf52): define CRYPTO_AES_NO_DECRYPT CTR and CCM only encrypt, so the AES inverse tables and round helpers are dead code on nRF52. Takes effect once the Crypto dependency includes meshtastic/Crypto#5. rak4631 estimate: ~1.0 KB flash. |
||
|
|
ae8dee9582 |
Fix: MQTT topic not updated when LoRa region changes (#10565)
* Initial plan
* Fix: update MQTT topics when LoRa region changes
When the LoRa region is changed via AdminModule::handleSetConfig,
moduleConfig.mqtt.root is updated (e.g. from msh/US to msh/EU_868)
but the running MQTT instance kept using the stale topic strings
(cryptTopic / jsonTopic / mapTopic) that were set at construction time.
Introduce MQTT::reinitTopics() which:
- resets the topic strings to their base values and prepends the
current moduleConfig.mqtt.root, and
- disconnects from the broker so the next reconnect re-subscribes
under the new topic prefix.
Call reinitTopics() from MQTT's constructor (replacing the inline
block) so the logic lives in one place, and call it from
AdminModule::handleSetConfig right after moduleConfig.mqtt.root is
rewritten on a region change.
Add a unit test (test_reinitTopicsUpdatesOnRegionChange) that verifies
both the updated subscriptions and the updated publish topic after a
simulated region change.
* Fix: call mqtt->reinitTopics() on region change via menuhandler
* Format MQTT test with trunk style
* fix(mqtt): drop undeclared jsonTopic refs in reinitTopics()
reinitTopics() assigned to a jsonTopic member that does not exist on this
branch (the MQTT class only has cryptTopic and mapTopic), so MQTT.cpp failed
to compile ("'jsonTopic' was not declared in this scope") and broke every
build that compiles it. Remove the jsonTopic lines so reinitTopics() rebuilds
exactly the topics the original constructor did.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(mqtt): simplify reinitTopics and its call sites
* fix(mqtt): rebuild topics in runOnce when the root changes
Replaces the per-call-site reinitTopics() calls. Also keep the device state and node database segments when the EU clamp swaps the region.
* fix(mqtt): refresh topics in onSend when the root changed
Rename the region change test to underscore-separated segments.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Thomas Göttgens <tgoettgens@gmail.com>
|
||
|
Tom
|
ee76117835 |
fix(router): relay opaque packets in CORE_PORTNUMS_ONLY (#11844)
* fix(router): relay opaque packets per rebroadcast_mode, not only in ALL |
||
|
Tom
|
2d6dad9ee9 |
Portduino: Fix LR2021 switch tables, power ceilings and IRQ handling (#11382)
* fix(portduino): recognise the LR2021 power ceilings in --check loadConfig() has read Lora.LR2021_MAX_POWER and Lora.LR2021_MAX_POWER_HF since LR2021 support landed, but neither was listed in the config checker's schema. --check therefore reported both as "unknown key ... ignored by meshtasticd" -- false, and actively misleading: it tells the user to delete a key that is doing exactly what they wanted. This breaks the contract stated above schema(), that a key taught to loadConfig() is added there too. CI enforces that by running --check over bin/config.d/**, but no shipped config sets either key -- or mentions lr2021 at all -- so nothing ever tripped over the omission. It could only surface for someone hand-writing an LR2021 config. LR20x0 is the only module with two power ceilings, one per band, selected at runtime by region; every other family expresses the split as separate module names and needs a single key. That is the likely reason the pair was missed while every other *_MAX_POWER key was added. Also adds both to valueSpecs(), so a wrong-typed value is reported rather than silently replaced by the default. * feat(portduino): configurable IRQ DIO and a chip-neutral RF switch table for LR20x0 Two gaps found bringing an LR2021 up under meshtasticd on a Luckfox Lyra Zero W. Both sit in the LR20x0 support added in #11252, and they interact: the switch table has to be written slightly wrong to pass validation, and the interrupt lands on a pin that table is driving. Every symptom is silent, because begin() only exercises SPI and BUSY -- the radio reports init success and then receives nothing. IRQ DIO could not be set on Portduino ------------------------------------- LR20x0Interface picked the IRQ DIO purely at compile time, and neither LR2021_IRQ_DIO_NUM nor IRQ_DIO_NUM exists for a Portduino target, so meshtasticd always fell through to RadioLib's default of DIO5 -- which is also the first RF switch line on carriers using the DIO5-DIO8 table. A variant says this with a #define (the pro-micro DIY board uses DIO9); a carrier has only the YAML, and had no way to say it. Adds Lora.IRQ_DIO_NUM, and an ARCH_PORTDUINO branch after the two existing #define branches, so a variant that already sets one still wins. The switch table was parsed as LR11xx-only ------------------------------------------ Pin names resolved to RADIOLIB_LR11X0_DIOn whatever the radio, and the mode set was the LR11xx's, so MODE_RX_HF -- a mode the LR20x0 really has -- was rejected as an unknown key and had to be omitted. The two families are not interchangeable: an LR11xx has no DIO9, so its fifth switch slot is DIO10, while an LR20x0's fifth slot is DIO9 and DIO10 is its sixth. A table naming DIO10 was therefore driving the wrong pin on an LR20x0. The YAML layer now stores what was written -- a DIO number and a neutral mode id -- and each interface supplies its own DIO constants and OpMode_t map to a shared builder. Neither family's constants are assumed to coincide with the other's. This also fixes a round trip in the config writer, which decoded pins by comparing against RADIOLIB_LR11X0_* and always emitted five values per mode row: for a four-pin table it produced YAML that --check would reject for mismatched row lengths. --check ------- Findings are now judged against the resolved module rather than a fixed list, so a mode or pin the part does have can no longer be rejected, and one it does not have is named instead of silently accepted. The claim that the table "is only applied to LR11xx radios" was stale and is corrected, and the missing-table warning now covers both families. "auto" is excluded throughout: the module has not been probed yet, so absence cannot be judged. The IRQ/switch-pin collision is reported in both directions, including the harder case where no key is set and the radio default collides -- nothing in the file looks wrong. Note that listing a pin is what breaks it, not driving it: setRfSwitchTable() reassigns the DIO function for every pin in the list whatever the levels say, so an all-LOW column is still a collision. Seven fixtures cover these, including a false-positive guard: DIO5 as the interrupt is normal, and must stay silent when the table is elsewhere. * feat(portduino): let the YAML ask for a TCXO probe, across every family that has one A variant declares "a TCXO may or may not be fitted" at compile time with TCXO_OPTIONAL, because the board is known when the image is built. A Portduino carrier cannot: the same meshtasticd binary runs on hardware populated either way, so the statement has to arrive as YAML and be answered at runtime. Adds Lora.TCXO_OPTIONAL, and TCXO_OPTIONAL_ENABLED in RadioLibInterface.h to unify the two, so each driver asks the question once rather than growing a second, Portduino-shaped code path. On an embedded target it stays a compile-time constant, so `if (TCXO_OPTIONAL_ENABLED)` folds away exactly as the old `#if` did: the nrf52_promicro_diy_tcxo image, which defines TCXO_OPTIONAL and so exercises the converted branches, still ends at 0xDF1D0 -- the same address as before this change. It is defined there rather than in a header of its own because InterfacesTemplates.cpp includes all three interface .cpp files into one translation unit, where a per-file definition would collide. Covers every family that has a TCXO reference to probe for: SX126x (sx1262/sx1268/LLCC68), LR11xx and LR20x0. With no DIO3_TCXO_VOLTAGE given, the TCXO attempt uses RadioLib's own 1.6 V default rather than being skipped -- otherwise there is nothing to fall back FROM and the flag would silently do nothing. This is also the FIXME that sat on the Portduino branch in LR20x0Interface: an unset voltage now means "no TCXO" explicitly. Two things are deliberately left alone: Each family keeps its own probe order. LR11xx tries XTAL first, because a TCXO-first attempt hangs RadioLib's unbounded calibration wait on a module with no TCXO fitted, whereas XTAL fails fast and cleanly on a module that has one; LR20x0 and SX126x try the TCXO first. A carrier therefore behaves the same way in a Portduino build as in an embedded one, and changing an order stays a hardware-behaviour decision rather than a tidying-up one. The SX126x retry is Portduino-only. An embedded TCXO_OPTIONAL board already gets this from initLoRa(), which constructs a second SX126x interface with no Vref when the first fails; retrying inside init() as well would leave that ladder step unreachable and change how every existing t-echo-class board reports its oscillator. A Portduino build has no ladder to fall through, because the module is named in YAML rather than probed. --check learns the key, reports which Vref will actually be tried, and warns when it is set on a radio with no TCXO reference, where it is read, stored and inert. * docs(portduino): condense the comments on this branch The repo asks for one or two lines and no multi-paragraph blocks, on the grounds that the diff and the commit message carry the rationale while the code carries the behaviour. What landed here was well past that: 163 added comment lines, including a 26-line block above a single macro. Removes the rhetoric, the issue numbers and the before-and-after asides, and the notes on where a thing used to live. No added block is longer than three lines now. Two facts needed stating and are stated once each rather than repeated at every use: the slot/DIO divergence between the families, in PortduinoGlue.h, and the per-family TCXO probe order, in RadioLibInterface.h. The longest surviving explanation is why an all-LOW switch column still collides with the interrupt, which sits in the fixtures README because without it that pair of fixtures reads as contradictory. Comments only; no functional change. * address CodeRabbit review on #11382 - SX126xInterface: distinguish an explicit DIO3_TCXO_VOLTAGE from the TCXO_OPTIONAL probing default in the debug log instead of always claiming the config field was set. - ConfigCheck: modesFor() now reports an unresolved use_autoconf against the union of both radio families' modes, not the LR11xx subset - fixes a false "not a mode this part has" warning for valid LR2021-only modes (e.g. RFSW_RX_HF) before autodetection resolves the module. - PortduinoGlue loadConfig: build rfswitch_mode_high[m] as a fresh per-row bitmask instead of OR-accumulating onto a stale value, so a config re-parse can clear a slot back to LOW. - PortduinoGlue YAML serialization: gate rfswitch_table emission on has_rfswitch_table rather than rfswitch_dio_num[0] >= 0 (missed sparse pin lists), and track each emitted pin's original slot so row values line up correctly instead of shifting when a low slot is absent. - config-dist.yaml: document the per-family TCXO/XTAL probe order (SX126x/LR20x0 TCXO-first, LR11x0 XTAL-first). - Trim three overlong comments per the coding-guideline nitpicks. Left the SX126x XTAL-retry-on-oscillator-failure nitpick alone - RadioLib's begin() already does its own XOSC_START_ERR recovery internally, and narrowing our wrapper's retry condition on top of that needs hardware to verify it doesn't regress a real failure path. Verified: bin/test-config-check.sh GREEN 69/69 against an isolated native build; pio test -e native -f test_rtc PASSED. * fix CI: cppcheck duplicateValueTernary, harden kRfSwitchModes init LR11x0Interface::init(): work around cppcheck's duplicateValueTernary on `TCXO_OPTIONAL_ENABLED ? 0 : tcxoVoltage` (both branches fold to 0 on a board with no ARCH_PORTDUINO, no TCXO_OPTIONAL, and no explicit Vref, since tcxoVoltage already reduces to 0 via the same macro chain) by splitting it into a plain assignment + if, rather than suppressing the warning. The other TCXO_OPTIONAL_ENABLED ternaries in this PR (LR11x0Interface.cpp:75, SX126xInterface.cpp:76, LR20x0Interface.cpp: 86,240) pick between TCXO_OPTIONAL_DEFAULT_VOLTAGE (1.6f) and 0, which can never coincide, so they're unaffected and left as-is. ConfigCheck.cpp: kRfSwitchModes was a namespace-scope global with dynamic initialization (a lambda IIFE) reading kRfSwitchModeNames, which is defined in a different translation unit (PortduinoGlue.cpp). Currently safe only because kRfSwitchModeNames's initializer is constant-expression-only (string literals + enum constants), which the standard guarantees completes before any TU's dynamic initializers - but that safety is silent and would break if PortduinoGlue.cpp's array initializer ever stopped being a constant expression, with nothing to warn a future editor. Converted to a function-local static (Meyers' singleton), which is correct by construction regardless of the other TU's initializer, updating all 4 call sites (definition + 3 uses) from kRfSwitchModes to kRfSwitchModes(). Verified: pio test -e native -f test_radio PASSED; bin/test-config- check.sh GREEN 69/69 against an isolated native build. * refactor: simplify TCXO voltage handling across interfaces and improve comments * fix rfswitch_table cross-file merge; drop now-stale checker warning Three CodeRabbit findings on 09e0d390c, addressed together since #2 and #3 are the same root cause: 1. PortduinoGlue.cpp: require an exact "DIO<n>" match when parsing rfswitch_table.pins. sscanf's %d stops at the first non-digit, so "DIO5invalid" silently parsed as DIO5 at runtime even though ConfigCheck.cpp's static validator (exact match against kRfSwitchPins) already rejected it - checker and loader disagreed. 2. PortduinoGlue.cpp: reset all 5 pin slots and all 8 mode rows before applying a table, rather than only overwriting what the new table mentions. A later config.d file that omitted a mode a prior file had set (e.g. only redefining MODE_TX) let the earlier file's MODE_RX leak through, contradicting "last file wins" - the rule every other Lora: key already follows. 3. ConfigCheck.cpp: with #2 fixed, rfswitch_table behaves like any other cross-file key, so removed the special-cased ERROR in checkCrossFileOverlap ("These do NOT override each other... OR of every table") - it described the pre-fix OR-accumulation bug and is no longer accurate. Falls through to the generic "last file wins" INFO now. Renamed/repurposed the rfswitch-sticky fixture to rfswitch-last-wins and updated its assertion (was rc=1 asserting the old error text, now rc=0 asserting the generic info) and the fixtures README. Verified: bin/test-config-check.sh GREEN 69/69 against an isolated native build, including the renamed assertion. * Assert the effective rfswitch table, not just the overlap diagnostic The "last one wins" case checked that the cross-file info fires and that the result is clean. Neither observes the table the loader actually ended up with, so the merge bug ee9b5b81e fixed - a later table leaving an earlier file's pins and mode rows as carryover - would still have passed it. Raised by CodeRabbit. Asserting the value needs two things the existing case cannot supply. The winner has to be deterministic. Both files in rfswitch-last-wins/ sit in config.d/, which is walked with a bare directory_iterator and no sort, so which one lands last is up to the filesystem - the point configd-conflict/ exists to make, and the reason the checker warns rather than assuming alphabetical order. rfswitch-replace/ puts the losing table in config.yaml instead, which is always loaded before config.d/. The loser is the wider of the two, four pins and three all-HIGH mode rows against the winner's two pins and one all-LOW row, so carryover shows up as a surviving pin, a surviving mode row, or a HIGH that should be LOW. The table has to be observable. The check report says no more than "RF switch table : set", and check-yaml cannot help: it is --check --output-yaml, and --check wins and exits before the dump - which the case just below it asserts. emit_yaml() does serialise the effective table, so the assert helper grows a yaml mode that passes --output-yaml alone. Confirmed non-vacuous: with the reset loop in loadConfig() removed, the new assertion fails and the old one still passes. Config-check suite GREEN 70/70. Native suite GREEN 44/44, 968 cases. * Say nothing about rows the radio will never read Two of the RF-switch diagnostics judged a table against a family's mode list without first asking whether the module reads a table at all. modesFor() treated every module that was not an LR20x0 as LR11xx-like, so an sx1262 carrying a table was told which of its rows were "not a mode sx1262 has" and which modes it had omitted - alongside the correct warning that the whole table is inert. pinsFor() already returned an empty set for these parts and its caller already guarded on that; the mode path now matches. The missing-mode advice is dropped under "auto" as well. The module has not been probed, so the union of both families is all there is to compare against, and naming its absent modes would advise adding MODE_TX_HP, MODE_GNSS and MODE_WIFI rows to what may turn out to be an LR20x0. Fixtures for both silences, and an assert() needle prefixed with '!' to hold them: a line that is merely absent today is otherwise nobody's regression. Also corrects the kLr11x0SwitchDios/kLr20x0SwitchDios comments. They describe a slot mapping, but buildRfSwitchTable() searches them by value to find the parallel pin constant - and with 7 DIOs against 5 YAML pin slots, the LR20x0 array could not be positional. * Warn when the two TCXO keys ask for opposite things DIO3_TCXO_VOLTAGE written out as false or 0 asks for DIO3 to be left alone, and stores identically to the key being absent - so TCXO_OPTIONAL then probes DIO3 at the radio default anyway. Both keys behave exactly as documented; only together are they wrong, which is what makes the outcome surprising. loadConfig() now keeps the distinction that the store loses, and --check reports the contradiction and which key to drop. The flag is diagnostic only and is not serialized: an explicit false and an absent key both round-trip as absent, as they did before. Also fixes the Portduino TCXO log lines, which named the variant define SX126X_DIO3_TCXO_VOLTAGE on a path where the knob is the YAML key, and adds a TODO over the SX126x XTAL retry. RadioLib has autocorrected that case itself since 7.5.0 - SX126x::modSetup() retries config() on the XTAL when begin() fails with SPI_CMD_FAILED and XOSC_START_ERR - so the ordinary case never reaches our retry and what does is mostly invalid settings, logged as a TCXO fault. * feat(portduino): bound Lora.IRQ_DIO_NUM, accept the older spelling, document both An LR20x0 raises its interrupt on DIO5 through DIO11. Anything else was read straight out of the YAML and programmed into RadioLib, where it routes the IRQ nowhere: begin() touches only SPI and BUSY, so the radio reports init success and then never receives a packet - the same failure the switch-pin collision check already covers, reached by a typo instead. Refuse it in loadConfig() and again in the driver before it reaches RadioLib, warning both times. Because loadConfig() discards the value, the merged config cannot tell a rejected number from an absent key, so --check judges the range in its per-file pass where the offending line is still known. LR2021_IRQ_DIO_NUM, the spelling carried by the two earlier LR2021 branches, is read when IRQ_DIO_NUM is absent and reported as shadowed when it is not. Both keys, the DIO range and the collision that makes the setting matter are now described in config-dist.yaml. * fix(portduino): a rejected IRQ_DIO_NUM returns to the radio default loadConfig() runs once per file - the main config, then each file in config.d/ - and they all write the same portduino_config. An out-of-range Lora.IRQ_DIO_NUM warned that it was falling back to the radio default but left any valid value an earlier file had set, so LR20x0Interface went on programming that stale DIO. Reset it to -1, the unset sentinel every other reader already tests for. Pinned by a new fixture: DIO9 in the main config, out of range in config.d/. The main config is always read first, so the ordering is deterministic, unlike two files in config.d/ (see rfswitch-last-wins). The assertion requires the summary to name the radio default and not DIO9; with the reset removed and rebuilt, it is the only assertion that fails. |