r112 compares 2.8.1, develop and this branch, and neither of the first two can
carry MESHTASTIC_BENCH_INSTRUMENTATION. It turns on the radio-edge lines, the TX
timeline, the slow-thread log and the millisecond log stamps, and r63 found that
log volume can change the loss a board shows - so having it on one side of the
comparison would be putting a known confound into the measurement.
The flag defaults to 0 and changes no behaviour by design, but no build of this
branch has had it off before, so this is the configuration to compile first.
-dmshell-next keeps it for diagnostic runs. RADIOLIB_GODMODE stays in the r112
envs: the LR11x0 and LR2021 quanta #error without it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5WNHGFh2kKwGGH79UPx44
The r112 round compares 2.8.1, develop, this branch and this branch without
slot parity, with board and server built from the same commit in every arm.
This is the branch end: <board>-r112-a is the whole stack, flags identical to
the -dmshell-next envs; <board>-r112-anp is the same build with
DMSHELL_TEST_SLOT_PARITY taken off, so the slot gate and the measured lead have
no parity to key on.
native-r112-a serves both: the server's parity is the runtime DMSHELL_SLOT_PARITY,
even for -a and unset for -anp.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5WNHGFh2kKwGGH79UPx44
develop's launchTransmit() override empties the TX FIFO before handing the
packet to RadioLib. LR2021_TX_PRESTAGE defines its own override, whose staging
writes the FIFO itself, so with the quantum on the two definitions clash. Guard
the upstream one out where the prestage replaces it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W5WNHGFh2kKwGGH79UPx44
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. LR11x0Interface::stageTxEarly()
likewise wrote the TX payload with WriteBuffer8 unlocked, and onNotify() calls it
before the sequence that wraps the scan and the transmit.
Either command can land in the middle of another task's sequence - the RX readout's,
in practice - leaving the chip mid-command with BUSY asserted. Both tasks then spin
in RadioLib's BUSY wait until its 1000 ms spiConfig.timeout fires, and because the
radio holds exactly one frame, whatever arrives during that second overwrites what is
still sitting in the chip.
Measured on a T1000-E (LR1110) over 300 s sessions, two arms interleaved, 3 runs each:
unlocked both locks
BUSY waits >=20 ms 5-13 0
longest BUSY wait 999-1000 ms 1 ms
1 s radio stalls 5, 11, 13 0, 0, 0
frames decoded by
the chip but never
fetched 7, 16, 18 0, 0, 0
total packet loss 0.59/1.23/1.30% 0.00/0.00/0.07%
Disjoint on all four, each at the exact 3v3 permutation floor (p=0.100); 1.00% mean
loss over eight unlocked runs against 0.02%. No re-inits, recoveries or reboots in
any run. Locking only the IRQ-status read was not enough (p=0.500): it clears one
collision family and leaves the payload write's.
The sequence mutex is recursive, so a caller already holding it re-enters, and
lockRadioSequence() is a no-op off FreeRTOS, so Portduino behaviour is unchanged.
SX126xInterface::stageTxEarly() has the same shape but is gated behind
irqPolledOverUsb(), true only for the single-threaded Portduino CH341 server, so it
cannot collide and is left alone.
Ported from 263aeef66 on claude/dmshell-r100-busy, with the comments trimmed.
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
MESHTASTIC_BENCH_INSTRUMENTATION now also:
- times each readout task frame: "RX read out by task: wake to readout
<n> ms (SPI <n> us), readout to handler <n> ms", and with
MESHTASTIC_RX_RETRY_WRONG_MODEM logs each WRONG_MODEM read again
("RX readout read again after ...").
- sets MESHTASTIC_BENCH_RX_COUNTERS: RadioLibInterface::rxCounters(),
which the DMShell test client logs for each session ("radio rx_good=",
"radio retry_-20=").
- sets MESHTASTIC_RX_FAIL_PROBE: at a failed readout, the chip's packet
type, mode and IRQ flags (" rx fail probe ..."), read on the LR11x0 and
LR2021 with RADIOLIB_GODMODE.
-DSX126X_STATE_SAMPLER_MS=<n> (and SX126X_STATE_SAMPLER_TASK) logs each
change in an SX126x's mode and IRQ flags. The master flag does not set it:
its SPI looks change the radio timing the other lines measure.
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
MESHTASTIC_BENCH_INSTRUMENTATION now also sets:
- MESHTASTIC_LOG_RADIO_EDGES: radio edge lines at DEBUG instead of TRACE,
and each window the radio cannot hear (scan, TX, readout restart),
logged as "Radio back in RX after <what>, deaf <n> ms" when it ends;
a running RX picked back up, or one re-armed at TX_DONE, closes it
without a deaf window. The task re-arm at TX_DONE logs its SPI time.
- MESHTASTIC_TX_TIMELINE: each TX timed from its channel scan to RX being
back ("TX timeline us:"), and, for a packet with a slot parity, where
it landed on the grid it drew from ("TX slot:"), with the radio lock
wait under the radio task, the slot gate under
MESHTASTIC_TX_SLOT_GATE_MS ("TX slot gate:") and the lead under
MESHTASTIC_TX_SLOT_LEAD ("TX lead:").
BenchClock.h times these on the nRF52 cycle counter, as Adafruit's
micros() steps in FreeRTOS ticks, and on micros() elsewhere.
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
-DMESHTASTIC_BENCH_INSTRUMENTATION=1 (default 0) turns on the loss bench's
diagnostic logging, set up in src/BenchInstrumentation.h. It changes no
behaviour, only adds lines and their cost. This part sets:
- MESHTASTIC_LOG_RECORD_MILLIS: API log records carry "millis=<uptime> "
(a radio task line's own logging time), and console lines print ms.
- MESHTASTIC_SLOW_THREAD_MS (10): "Thread <name> ran <n> ms".
- MESHTASTIC_LOG_USB_STATS: console bytes and log records the port takes
and refuses, every 10 s; USB stalls of 1 s or more, with the HWCDC
registers on ESP32-S3 and the TX latch kicks kickLatchedTx() makes;
the loop's radio upkeep timing.
With the radio task it also logs the task's unused stack at each upkeep.
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
RadioLibInterface::readChipRxStats() reads the chip's receive counters:
GetStats on the SX126x, getStats() on the LR11x0 (needs RADIOLIB_GODMODE)
and getLoRaRxStats() on the LR2021. The DMShell test client reads them at
the start and end of each session, under the radio lock, and logs the
difference:
DMShellTest stats session=0x... chip rx_done=N crc_err=N header_err=N false_sync=N
or "chip none" where the chip or RadioLib keeps no counters. The SX126x
read also logs its raw bytes ("chip stats raw ...").
MESHTASTIC_RADIO_CHIP_STATS gates all of it. It is on with RADIO_DEBUG, and
with MESHTASTIC_BENCH_INSTRUMENTATION without the rest of RADIO_DEBUG's lines.
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
Three compile-time switches, all off by default:
- MESHTASTIC_RADIO_TASK (ESP32, nRF52): runs the RadioLib radio thread
from its own FreeRTOS task, one priority above the loop and below the
RX readout task. The task holds a recursive radio lock while the thread
runs; loop-side entry points into the radio take it with
RADIO_TASK_LOCK(). The static packet pool claims slots in a critical
section. The task's log lines are queued and printed by the loop
(MESHTASTIC_RADIO_TASK_LOG_LINES sets the depth;
MESHTASTIC_RADIO_TASK_LOG_DIRECT writes them from the task instead),
and no log line sits between the TX timer and SET_TX.
- MESHTASTIC_TX_SLOT_GATE_MS=<ms>: a TX with a slot parity scans only in
the first <ms> of a slot of its parity on the frame-end grid. The timer
fires MESHTASTIC_TX_SLOT_GATE_EARLY_SLOTS (default 2) slots early and the
radio thread waits out the rest; a timer further ahead is set again
(MESHTASTIC_TX_SLOT_GATE_REARM=0 scans at once instead).
- MESHTASTIC_TX_SLOT_LEAD (needs the gate): aims SET_TX
MESHTASTIC_TX_SLOT_SET_TX_AT_MS (default 1) into the slot, starting the
scan the measured scan-to-SET_TX time before it and firing the timer the
measured wake latency plus MESHTASTIC_TX_SLOT_LEAD_GUARD_MS (default 1)
before the scan. Works with the early TX stage on a CH341 host.
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
LR11X0_TX_STAGE_EARLY (default off) writes the next packet's payload
into the TX buffer while RX runs, through wantsEarlyTxStage() and
stageTxEarly(), instead of in the scan's standby. At the scan, a
buffer that still holds scanForTx's payload becomes the prestage, and
a busy verdict keeps it for the rescan. The RX buffer is separate, so
a received frame neither is aborted by the write nor overwrites it.
Needs LR11X0_TX_PRESTAGE.
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
Both default off. The RX re-arm at TX_DONE these sat beside is #11983's
now, unconditional, so it is no longer a switch here.
LR11X0_CAD_EXIT_LBT scans with CAD exit mode 0x10 when a payload is
prestaged: a clear CAD keys up from it and launchTransmit() only
releases the CAD flags; a busy CAD leaves the chip in standby and
rearmReceive() restarts RX. The chip mode read after the scan decides
which, through #11983's readChipMode(). Needs LR11X0_TX_PRESTAGE.
LR11X0_CAD_SLIM starts the CAD without RadioLib's packet-type reads and
second standby, and resends the CAD parameters only when they change.
Needs RADIOLIB_GODMODE.
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_STANDBY_XOSC (default 1) gates the two setStandbyXOSC(true)
calls: the TCXO-board one in reinitChip() and the ISR re-arm one.
-DSX126X_STANDBY_XOSC=0 leaves the SX126x on STDBY_RC.
LR11X0_STANDBY_XOSC (default off, needs RADIOLIB_GODMODE) puts the
LR11x0's standby and TX/RX fallback on STBY_XOSC, reapplied after
every begin().
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
-DLR2021_CAD_EXIT_LBT (default off) arms the whole TX before the scan,
with the payload LR2021_TX_PRESTAGE staged, and scans with
SetLoraCadParams exit mode 0x10: a clear CAD keys the chip up by itself,
so launchTransmit() only releases the latched CAD flags. A busy verdict
leaves the chip in standby, reported busy without a CAD>RX handoff, and
a refused scan is reported busy rather than sent without a CAD. The
outcome is read back from the chip's mode. The chip keys up without the
MCU, so the RF switch must be the chip's DIOs or none. Needs
LR2021_TX_PRESTAGE and -DRADIOLIB_GODMODE=1.
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
-DLR2021_TX_PRESTAGE (default off) writes the payload into the TX FIFO
in the standby before the CAD, so a clear verdict sends only the packet
params, IRQ setup and SET_TX. The FIFO appends, so bytes a busy verdict
left unsent are cleared before the next write. Needs
-DRADIOLIB_GODMODE=1.
-DLR2021_PRESTAGE_UPSTREAM (with LR2021_TX_PRESTAGE, default off) stages
through RadioLib's prestageTransmit() (jgromes/RadioLib#1883) instead,
whose stageMode(TX) skips the FIFO write for the staged payload. It needs
a RadioLib with #1883 but no GODMODE.
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
-DLR2021_STANDBY_XOSC (default off) uses STBY_XOSC for the driver's
standby and sets the chip's RX/TX fallback to STBY_XOSC after every
begin(), so the TCXO is not restarted before each CAD, RX and TX. The
channel scan then starts the CAD from that standby itself instead of
through scanChannel(), whose standby() is STBY_RC. Needs
-DRADIOLIB_GODMODE=1.
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
-DLR2021_LOAD_PRAM (default off) writes Semtech's LR2021 PRAM (Lora-net
usp v1.2.1 lr20xx_driver, Clear BSD) after every begin(), in init() and
fullBegin(), activates it, checks it loaded, and re-applies begin()'s
modem settings. Semtech lists the DC-DC regulator's sub-GHz LoRa
sensitivity loss among what it fixes. Needs -DRADIOLIB_GODMODE=1 and
RadioLib 7.8, whose WriteRegMem32 no longer appends a stray byte.
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
RadioLib 7.8 replaced the LR2021's setRegMode(simo, rampTimes) with
setRegMode(simo) and a public setRegulatorDCDC(), and applies Semtech's
DCDC ramp workaround itself. On 7.8 or later, init() selects the DC-DC
regulator through setRegulatorDCDC(); older RadioLib keeps the godmode
setRegMode() call. -DLR2021_REGULATOR_LDO (7.8 only, default off) keeps
the LDO instead.
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
An RX_DONE that wakes the readout task never reaches isrLevel0Common(), so
its frame end was never stamped and the anchored backoff grid only saw TX
ends and polled frames. rxDoneFromIsr() now stamps the tick, the task
turns it into the frame's air end when it reads the frame, and delivery
notes it before handling, as the ISR_RX path does.
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
Router::runOnce() handles every queued reception (decrypt, modules, the
phone; ~20 ms on nRF52) before the radio thread's TX timer can run, so a
TX due in that time leaves late by as much, out of the slot its parity
drew. On Jonathan's DMShell bench (round 57, T1000-E client, 3 x 300 s
per arm) holding the reception took drawn fire-to-air with RX handling
in the window from p90 26 ms to 7 ms, collided bursts from 6.0 to 0.0
per run (SDR agreeing), and total loss from 1.59% to 1.17%.
- RadioLibInterface arms the TX timer through one helper,
scheduleTransmitDelayCompleted(), which notes when it falls due if the
front packet has a slot parity (0 otherwise), and clears the note when
the timer runs. getTxDueMs() reports it; other radios report 0.
- Router::runOnce() holds the next reception when such a TX is due
within 30 ms, or is just past due and the radio thread has not run
yet, and returns until 2 ms after the due time. Another arrival does
not cut the hold short while the TX is pending. Each packet is held
at most once, for at most 32 ms.
- Packets without a slot parity never set a due time, so ordinary mesh
traffic is unaffected.
- Router.cpp now includes Throttle.h on every platform, not only
portduino.
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
lastFrameEndWhat is written by the same writer and read by the same reader as
lastFrameEndMs, so leaving it a plain pointer kept the data race the previous
commit removed, just narrowed to the trace line. Same treatment: std::atomic
with relaxed access, free on every target here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
The radio worker writes lastFrameEndMs from onNotify() while a draw for a
packet being queued reads it on whatever thread called send(), so the access
was an unsynchronized read of a plain uint32_t. std::atomic with relaxed
ordering is free on every target here and matches MeshService's fromNum.
Relaxed is the right ordering: the anchor publishes nothing but itself, both
writers are the radio worker so the compare-then-store races with no other
write, and losing the race against a reader only anchors that one draw to the
frame before last.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
A parity belongs to the packet being queued, the way priority and tx_after do,
so put it there: MeshPacket.slot_parity, set by whoever queues the packet and
read by the driver when it draws that packet's backoff.
That removes the machinery the hook needed. RadioTxHook::slotParity() and its
fan-out are gone, and so is DMShellTxHook with its id-keyed claim ring, which
only existed because a hook is asked about a packet whose payload is already
encrypted and so has to recognise its own frames by id. DMShell now sets
packet->slot_parity in sendFrameToPeer(), beside the priority it already sets.
Needs meshtastic/protobufs#1123 and a submodule bump once that merges; the
generated header here was produced with nanopb 0.4.9.1 from the pinned
submodule commit plus that enum and field.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
A slot parity is something a module asks for on a packet it is queueing, not a
property of a build. Remove MESHTASTIC_TX_SLOT_PARITY and its validation guard;
getTxDelayMsec() now takes the parity from the TX hooks alone, and a node whose
packets no module speaks for keeps the ordinary random draw.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
checkRxDoneIrqFlag() and checkTxDoneIrqFlag() notify the dispatch path for an
edge the interrupt never delivered, so the ISR timestamp frameEndFromIsr()
reads still belongs to an earlier frame and the anchored slot grid lines up
with the wrong instant. Stamp now before notifying: it is the closest honest
answer once the interrupt's own time is gone.
Also correct the test header, which still claimed opposite parities rule out
adjacent slots. They rule out the same slot.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
Trunk Check runs clang-format 20.1.0 over every file a PR modifies, whole file,
so the RDEF macro in RadioInterface.cpp and two `){};` constructor bodies in
RadioLibInterface.h are reported against this branch although it did not write
them. This is `trunk fmt` output, nothing hand-edited.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
Two nodes exchanging a stream are each other's main collision partner: both redraw
their backoff after the same frame, and land on the same slot far more often than
chance, because the draw is timed from each node's own handling of that frame rather
than from the frame itself.
A module can now claim a packet for one parity of the slot grid through
RadioTxHook::slotParity(), returning the new meshtastic.SlotParity from mesh.proto.
A claimed packet's plain CSMA draw counts its slots from the air end of the last
frame this node sent or heard - the one instant both ends share - and takes only
slots of that parity, so two nodes on opposite parities are always at least a whole
slot apart however differently they handled the frame. The weighted rebroadcast draw
is untouched, and with no parity claimed the draw is exactly the code that was there.
The driver keeps the anchor up to date whether or not anything asks for it: the air
end is stamped in the TX_DONE / RX_DONE interrupt itself, since everything after that
is handling delay a peer does not share.
DMShell claims its own frames when DMSHELL_SLOT_PARITY=even|odd is set, by packet id
rather than port number, since the packet is encrypted by the time the driver asks.
Unset, no hook is registered at all. -DMESHTASTIC_TX_SLOT_PARITY=<0|1> gives a whole
node a default parity, for the far end of a session that runs no module of its own.
The mesh.proto enum needs its own protobufs PR and a submodule bump; the generated
files here were regenerated from it with nanopb 0.4.9.1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkrqYBzhAxyybmoezb1TJV
Everything between a clear CAD verdict and SET_TX is time the channel
goes unwatched. RadioLib's TX staging writes the whole payload in that
window. The scan already puts the chip in standby, so write the payload
there instead: a clear verdict then sends only the packet params, the
IRQ setup and SET_TX.
The packet params keep RX's maximum length until the verdict, so a
detection's RX still takes a full frame. A detection's RX may overwrite
the staged bytes, but a busy verdict drops the stage and no TX follows.
The write happens after RX has been stopped, so it cannot land on a
frame being received.
Setting RadioLib's staged mode needs RADIOLIB_GODMODE, so the prestage
is on wherever that is set (LR11X0_TX_PRESTAGE=0 opts out), and the
T1000-E now sets it.
Measured on a T1000-E (LR1110) against a CH341 host, SHORT_TURBO:
total loss 9.40% -> 7.82% (p=0.029).
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
(cherry picked from commit cd55f137de)
(cherry picked from commit 8e02fd6084)
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
computePacketTime() asks the chip for its packet type (getTimeOnAir()) on a
transmit, and for the header info on a receive the readout task did not
capture. Its callers - finishSentPacket() after every send, ReliableRouter's
retransmit timing, DMShell - hold no radio sequence, so a readout could land
inside the read. On the LR11x0/LR20x0 a read is two SPI transfers, so a readout
between them takes the other's reply.
r113's SPI probe saw radio SPI outside any sequence from the main loop task on
every board, 2 transactions per received frame on LR chips and 1 on SX126x,
which is one chip read per exchange on each. These are the per-send chip reads
on that path that were still unlocked (inferred; the next probe run confirms).
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
pollMissedIrqs() runs from the main loop about once a second. Its
checkRxDoneIrqFlag() and checkTxDoneIrqFlag() read the chip's IRQ flags
without the radio-sequence lock, so the RX readout task could preempt one
mid-read.
On LR11x0 and LR20x0 that read is not self-contained. RadioLib's
LRxxxx::getIrqStatus() sets the Module-wide spiConfig.widths[STATUS] to 0
for its transfer and restores it after. A readout that lands in between
frames its own commands with that width, so its status bytes come back as
data. On an LR2021, GET_RX_PKT_LENGTH then returns the two status bytes as
the length (always 0x100 or more), and the frame is dropped as a bad length.
On an LR1110 the status byte is read as the packet type instead.
The bench caught one "RX readout task: ... 1 bad length" on an LR2021 running
develop, with the ring not full. Only a value that isn't a length can exceed
256 there.
Take the lock for the whole poll. It is recursive, and checkCadHandoffTimeout()
and checkStaleRxFlags() already take it inside.
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
* 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>
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>
* 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 d2fb16a closed for
checkCadHandoffTimeout(), checkStaleRxFlags() and resetAGC(), and the one
canSendImmediately() was wrapped for. The readout task runs at a higher
priority and can land between any two of them, splitting a command and
wedging BUSY for RadioLib's whole timeout. The three now sit in one
RadioSequence block, with the RSSI validation left outside it.
The throttle check also moves ahead of the lock. It was behind the chip
reads, so every main-loop iteration paid for them to reach a check that
discards the sample; now the skipped iterations cost nothing and do not
contend for the sequence lock.
readOutFromTask() returned without clearing anything when a DIO1 edge
carried no RX_DONE. A CAD handoff's RX arms with CRC_ERR and HEADER_ERR in
its DIO1 mask on SX126x, LR11x0 and LR2021, and a damaged header raises
HEADER_ERR with no RX_DONE behind it: the task woke, found no RX_DONE,
cleared nothing and left the flag latched, so on an edge-triggered line the
next frame's RX_DONE raised no edge at all. checkStaleRxFlags() steps aside
while cadHandoffRxStart is set, so nothing noticed until
checkCadHandoffTimeout() fired at twice the maximum packet airtime. The
error flags are now retired there, inside the handoff window only - plain RX
does not route them to DIO1, and checkStaleRxFlags() needs HEADER_ERR left
alone to spot a wedged SX1280 RX.
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
* Don't strand a TX timer, and don't adopt an RX a reconfigure voided
Four fixes from a re-review of the readout task.
onNotify()'s ISR_RX case had two early breaks in its readout branch, both of
which skipped the trailing setTransmitDelay(). There is one notification slot,
and readOutFromTask() notifies with overwrite set whenever no TX is in flight,
so reaching onNotify() at all can mean a pending TRANSMIT_DELAY_COMPLETED was
discarded. Nothing else re-arms that timer: setTransmitDelay() runs from send()
and from the tail of these two cases, so a queued packet stranded this way
waited for an unrelated send() or radio interrupt. The breaks become a
rearmWanted flag, leaving the re-arm conditional and setTransmitDelay()
unconditional. Scheduling it on those two paths is safe - the
TRANSMIT_DELAY_COMPLETED handler re-delays when the radio turns out to be busy.
adoptReceiveArmedFromIsr() trusted rearmState, which the TX_DONE interrupt sets
before the radio thread runs. But onNotify()'s ISR_TX sequence calls
RadioTxHooks::beforeTransmit() first, and the beacon hook's
reconfigureForBeaconTX() reaches reconfigure(), which standbys the chip and
reprograms the modem. The task's arm is gone, rearmState still says ARMED, so
adopt reported true and onNotify() skipped its startReceive(). Worse, adopt's
own RadioLibInterface::startReceive() then cleared rxOffline and
chipRecoveryFailures for a chip that never took an RX - and rxOffline is what
periodicRadioMaintenance() gates its retry on, so a chip left deaf by a failed
reconfigure, or by a post-reconfigure arm that errored, had its recovery path
switched off. The three drivers that own rearmState now clear it in
reconfigure(). A reconfigure that succeeds ends in its own startReceive(), so
the only cost is one extra arm on the beacon-TX path, where correctness is
worth more than the standby.
sleep() made trySetStandby() and the chip's sleep command as an unguarded pair
in all five drivers, so a readout could land between them and talk to a chip on
its way down. Each now takes the sequence lock. The comment claiming interrupts
are already disabled there was wrong - disable() and NodeDB's config-reload
park both reach sleep() with the ISR attached - and now says what actually
holds the readout task off.
LR11x0Interface and LR20x0Interface's rearmReceiveFromIsr() overrides lacked
INTERRUPT_ATTR although isrTxLevel0() calls them, unlike the SX126x override
and the base-class ISR helpers. Added, so they are placed in RAM on the
architectures that care.
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
* Read a captured frame's LoRa header info with the frame, not after it
The radio thread accounts for a frame the readout task took by asking the
chip for the LoRa header's coding rate and CRC flag, from
deliverCapturedFrames(), outside the radio-sequence lock. By then the chip
is back in RX - with the continuous-RX resume it never left it - so
GetLoRaPacketStatus describes whatever it is receiving now rather than the
frame being accounted for, and that read can also land inside a sequence
the readout task is running.
On an LR2021 that shows up as "Bogus RX coding rate 9 from radio" on about
a third of received frames once the resume is on, and on none without it.
Nothing is lost - computePacketTime() falls back to the configured rate,
which is the right one - but the airtime estimate is taken from a reading
that does not belong to the frame.
Read it in readOutFromTask() instead, with the frame's SNR and RSSI and
under the same lock, carry it in CapturedRxInfo, and have
computePacketTime() use the captured value for a captured frame. The
thread readout path still reads the chip, where the chip still holds that
frame's status.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014NeGVaLzYEgfHdj2hcXiSr
(cherry picked from commit acdb14a9b5e5886115c9e8215573d288a8c1ed0d)
* Reach the RF95 driver through its pointer when reading header info
RF95Interface holds its driver in a std::unique_ptr, so the header-info
override has to go through lora-> like the rest of the file does. The
value-held form took out every build.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVTg7Na1pVvCcwZoSv9Hnd
* SX126x: drop the duty-cycled RX arm, which is unreachable
startRxCommand() chose between a continuous RX and
startReceiveDutyCycleAuto(), and continuousRxWanted() answered that choice
partly from the preamble: RadioLib's duty cycle stays awake for
2 x minSymbols and sleeps through whatever preamble is left, so a preamble
no longer than that window leaves nothing to sleep through.
preambleLength is only ever preambleLengthDefault (16) or, above 2 GHz,
wideLoraPreambleLengthDefault (12), set in applyModemConfig() and written
nowhere else. Both are inside the 16-symbol wake window, so the predicate
was true in every region and every preset and the duty-cycled branch could
not be reached. The comment on it, about saving power by letting the radio
sit in standby mostly, described something that never happened.
Send the continuous RX outright and set rxArmedContinuous unconditionally.
A static_assert on the two preamble constants fails the build if a preamble
ever outgrows the wake window, which is when the duty-cycled arm and a flag
recording which command was sent would have to come back.
No behaviour change: the command on the wire is the one the chip was
already getting.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ckvvn9GeSGZht3RAiMWDax
* LR20x0: empty the TX FIFO of leftovers before each transmit
The LR2021's TX FIFO appends, and RadioLib's TX staging writes the payload
without clearing it first. A TX that ends without sending everything it
staged leaves those bytes behind, and the next SET_TX sends them ahead of
the new payload, at the new payload's length. The remainder stays behind
in turn, so every later packet goes out shifted.
On the X1 bench (r108) that is what the uplink collapse looked like: a
45-byte packet went out at the next packet's 59 bytes, carrying the first
14 bytes of that next packet's header, and the server could not decrypt
it. Seen only with the continuous-RX resume on, which is what cut the TX
short is not yet known.
Before each transmit, read the TX FIFO level, warn if it is not empty, and
clear it then (or whenever the read fails). One cut-short TX now costs
that packet only, and the warning marks where it happened. RadioLib keeps
GetTxFifoLevel and ClearTxFifo private, so both go out as raw SPI.
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 start a delayed transmit from inside a standby
Every driver's trySetStandby() calls checkNotification() first, to handle
any pending radio interrupt before forcing standby. But notifyLater()
stores its notification at once and only delays the wake-up, so while a
packet waits out its transmit delay, TRANSMIT_DELAY_COMPLETED is already
sitting in the slot. A standby reached from outside the radio thread's own
notification handling consumes it: the scan and transmit run right there,
ahead of the delay, and the standby that follows cuts the transmit off a
few ms into its airtime while it reports as sent.
pollMissedIrqs() is such a path, from the main loop: its CAD-handoff
timeout and its stale-header re-arm both call startReceive(). On an X1
(LR2021) running #11983 the bench caught it directly: CAD>RX timeout, CAD
arm, CAD free, Started Tx, then a standby 3 ms into a 25 ms frame. RadioLib
7.7.1 never clears the LR2021's TX FIFO, so the unsent payload went out
ahead of every later packet and the node's uplink collapsed.
The standby now handles every pending notification except
TRANSMIT_DELAY_COMPLETED, which stays pending for its own wake-up.
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
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* 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>
RadioLib waits up to spiConfig.timeout (1000 ms) on the chip's BUSY line before failing
a command, and the firmware never set it. A chip left mid-command holds BUSY until that
wait gives up, so each collision cost a whole second of a deaf radio and the frames that
arrived in it.
MESHTASTIC_RADIOLIB_SPI_TIMEOUT_MS (default 50; 0 keeps RadioLib's 1000 ms) is applied
after begin() in the SX126x, SX128x, LR11x0 and LR20x0 init, so the boot calibration
keeps RadioLib's default. Recovery re-inits run under the bound.
Measured on a T1000-E (round 100, before the locking fix): the 50 ms arm removed every
one-second block (21 to 0) with no re-inits, recoveries or reboots. Waits were bimodal,
one 32 ms boot calibration per run and the rest at the 1000 ms ceiling, so no wait that
would have completed falls between the bound and the old timeout.
Claude-Session: https://claude.ai/code/session_01REkPVFh6kvG4AZJ5A8AtM9
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* DMShell: an on-board test client for the remote shell
A DMShell session is normally driven by bin/dmshell_client.py over the API,
which puts the test's own traffic on the serial port it is logging. This
builds the client into the firmware instead: -DDMSHELL_TEST_PEER=0x<node>
gives the board a client that opens a session to that node and types a
workload itself, so a run needs no host and the serial port stays in
logging mode for its whole length.
It speaks the same protocol as the Python client, on the sequence and
replay bookkeeping already in DMShellRecovery.h, and keeps the same
per-session counters so the two can be compared directly.
Unset, nothing is built and nothing is linked: the flag guards both source
files and the one hook in Modules.cpp. DMSHELL_TEST_* set the workload (the
command, keystroke interval, session length and count, how keystrokes are
batched into frames, the input window). DMSHELL_TEST_SLOT_PARITY, when set,
stamps slot_parity on every packet it sends.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VUcRFYYQhAcSSKRC2h7mMn
* DMShellTest: require PKI on the peer's frames
The test client accepted any decoded packet on the remote shell port whose
`from` matched its peer. A channel-encrypted packet can claim any `from`, so
a forged CLOSED with the live session id would end a session and a forged
`ack_seq` would move the send window. The server already refuses a frame
without PKI; the client now does the same.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VUcRFYYQhAcSSKRC2h7mMn
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Since the packet authenticity gate (d6b12ea3f), a PKI DM between two other
nodes decodes as DECODE_OPAQUE, so perhapsHandleReceived() relays it and
returns before dispatchReceived(). That was the only place such a DM was
marked pki_encrypted and published on the PKI topic, so a gateway no longer
uplinked DMs (or the PKI-encrypted ACKs answering them) from its mesh, and
DMs between mesh and MQTT stopped completing.
Publish those DMs from the opaque path instead, under the same conditions as
before (MQTT and its encryption enabled, channel 0, unicast, not to or from
us, not via MQTT). A small (from, id) ring keeps the copy heard from each
relay from being published again, since opaque frames stay out of
PacketHistory. The now-unreachable DECODE_OPAQUE branch in
dispatchReceived() is removed.
Claude-Session: https://claude.ai/code/session_01ECUkcuNgzRpL3MdDjZih2h
Co-authored-by: Claude <noreply@anthropic.com>
RadioLib's TX staging ends in fixSensitivity(), which programs the
sensitivity register from the configured bandwidth (SX1262/SX1268
datasheet 15.1); setBandwidth() updates bandwidthKhz and the modulation
params but does not redo it, which is why RadioLib applies it per
transmit. The CH341 launch path skips the register fixes on the grounds
that an earlier staging set them and the chip keeps them, and that holds
until the bandwidth itself changes: a preset switch into or out of
500 kHz left the register at the old bandwidth's value for every later
prestaged TX. reconfigure() now drops txStagedByRadioLib and both staged
payloads, so the next TX goes through RadioLib's full staging. The
recovery path already did this via reinitChip().
Claude-Session: https://claude.ai/code/session_01YUvVvYndkocdQiZ1EXGarW
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
PlatformIO's upload-port autodetection enumerates boards from every installed
platform, not just the one being built. platform-raspberrypi's
_add_default_debug_tools() acts on any of cmsis-dap, raspberrypi-swd and
picoprobe that a board lists in upload.protocols, and asserts that
debug.openocd_target is set for each of them.
t-impulse-plus lists cmsis-dap but has no openocd_target, so that assert fired
during port detection and aborted `-t upload` for every environment in the
repo. Flashing an ESP32-S3 board failed on an nRF52840 board's manifest:
*** [upload] AssertionError : Missing target configuration for t-impulse-plus
The manifest has been missing the key since #10497; it only started biting when
the platform pin moved in #11814. Use nrf52840-mdk-rs, as the other thirty
nRF52840 boards in boards/ already do. It is the only board in the tree that
trips this.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
