Skip to content

Enforce the IRQ mask, time CAD scans, and decode the rest of the command set - #2

Merged
A13xB0 merged 3 commits into
MeshBench:mainfrom
NomDeTom:irq-mask-enforced-and-cad-that-takes-time
Oct 13, 2026
Merged

A13xB0 merged 3 commits into
MeshBench:mainfrom
NomDeTom:irq-mask-enforced-and-cad-that-takes-time

Conversation

@NomDeTom

@NomDeTom NomDeTom commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

The model accepted several things a firmware programs and then ignored them, or answered them with a constant. Each one presents the same way: a node configured wrong that reports nothing wrong. This makes the chip act on what it is told, record what it cannot act on, and count what it refuses.

ABI 1.3 → 1.6, one minor per commit. Additions only: new fields are appended to the end of vsx_state and vsx_counters, and there are two new entry points. No host needs rebuilding to keep loading the library.

Commits

ace6864 enforce the IRQ mask, and make a scan cost what a scan costs (1.4)

  • IrqMask from SetDioIrqParams is now applied: every flag goes through raiseIrq(), and masked events are counted in irq_suppressed.
  • SetCadParams is decoded. A scan holds the chip for its programmed dwell (1–16 symbols), samples the carrier across the whole dwell, and ends in standby. detPeak/detMin are recorded, not applied. New counters: cad_runs, cad_detections.
  • SetRx's timeout is enforced; continuous, single-shot and deadline receive stay distinct.

5faabfe check what the modem was programmed to, and who the chip is listening to (1.5)

  • SetModulationParams validates SF, bandwidth code and coding rate. A refused field leaves the setting unchanged and is counted in params_rejected. The test harness had been sending bandwidth 0x1A, which is not a valid code; it now sends 0x03.
  • Low data rate optimisation comes from the programmed field. It falls back to the symbol-duration rule only if the field was never programmed. Example: at SF11/250 kHz a 50-byte frame was quoted at 723 ms; the part takes 641 ms.
  • Sync word: the chip comes up on 0x1424, reports it in vsx_state.sync_word, and refuses frames delivered with another word via the new vsx_deliver_frame_from(). The counter is sync_mismatches. vsx_deliver_frame() is unchanged and still delivers.
  • SetTx's timeout is enforced: if the host has not called vsx_transmit_finished() before a nonzero deadline, the chip raises Timeout and goes to standby.

8fb9623 decode the rest of the command set, and say which parts are only recorded (1.6)

  • New src/commands.cpp. Every command outside MeshCore's driver path is now decoded, and each is marked acted on, recorded, or placeholder.
  • Acted on: SetPacketType, StopTimerOnPreamble, SetLoRaSymbNumTimeout, SetFs, GetStats/ResetStats. GetStatus also, which used to be a constant 0x22.
  • Recorded only: SetPaConfig, SetRegulatorMode, SetDIO2AsRfSwitchCtrl, SetDIO3AsTcxoCtrl, CalibrateImage, test modes.
  • Placeholders: SetRxDutyCycle records its periods but the receiver stays on. SetRxTxFallbackMode treats FS as standby. vsx_deliver_frame_failed() is the only way to raise CrcErr/HeaderErr, and nothing calls it yet.
  • SetPacketParams reads all six fields. Header type, CRC and invert-IQ now feed vsx_est_airtime_ms().

Behaviour change for hosts

A firmware that never programs IrqMask now sees an empty IRQ status register, where it used to see every flag. Per CLAUDE.md, the pins in consumers.tsv (meshcore-native) and the MeshBench release pin should move only after an emulated board boots on this chip, forwards traffic, and shows irq_suppressed == 0. That boot has not been done yet.

Testing

STRICT=1 ./build.sh: all seven test binaries pass with 0 failures, and STRICT=1 ./build.sh sanitize passes them again under ASan/UBSan. STRICT=1 ./build.sh shared builds. The new suites are test_cad, test_modem and test_commands, and test_interrupts and test_c_abi are extended.

Two things the model accepted and then ignored, both on the path a firmware
uses to decide whether to transmit.

IrqMask was parsed by SetDioIrqParams and never read again, so the status
register answered every event whatever the firmware had enabled. That is
forgiving in the direction that hides a fault: a driver waiting on an interrupt
it forgot to enable waits for ever on silicon and works perfectly here. Every
flag now goes through raiseIrq(), and what the mask keeps out is counted in
irq_suppressed, because a masked event is silence and silence looks exactly
like a quiet mesh.

SetCadParams was not decoded at all, so a scan ran with the reset defaults
however it had been configured, answered inside the SPI command, and always
came back to standby. A firmware could therefore scan the channel and transmit
in the same millisecond, which is listen before talk with the listening taken
out: the dwell is the entire price of the technique, 1 to 16 symbols, most of a
second at SF12. The scan now holds the chip for that dwell, samples the carrier
across it rather than at its ends, and ends in standby.

detPeak and detMin are recorded and not applied. They threshold a correlator
peak and this model is told whether a carrier is present, not how strong it is.
Thresholding a boolean would be the chip inventing a signal level and deciding
reception from it, which belongs to the simulator.

SetRx's timeout came with it, because enforcing a mask over a bit that can
never be set is half a job. Continuous receive and single shot stay distinct
from a real deadline.

vsx_state and vsx_counters gain fields at the end only; VSX_ABI_MINOR 3 -> 4.
No host needs rebuilding to keep working, but a host that never programmed
IrqMask now sees an empty register, so the pins want moving with a board booted
behind them rather than on green tests here.
Four things, all of which present the same way: a node configured wrong that
reports nothing wrong.

SetModulationParams took every field on trust. A spreading factor, bandwidth
code or coding rate the part does not define was adopted or quietly ignored,
and the modem stayed on whatever it had. This repository's own harness had been
sending bandwidth 0x1A, which is not a code at all, so every timing assertion
in it ran at the default 250 kHz while its comment said 62.5 and nothing
failed. Each field is now checked, a refused one leaves the setting alone and
is counted in params_rejected, and the harness sends 0x03.

Low data rate optimisation is the field the firmware programs, not something to
infer from the spreading factor. The threshold is a symbol duration, so SF11 is
over it at 125 kHz and under it at 250, and inferring it from SF alone was
wrong at every bandwidth but one. It was wrong in the figure the firmware asks
this chip for to time its own CSMA: at SF11 and 250 kHz a 50 byte frame was
quoted at 723 ms against the 641 ms the part would take. The programmed field
now wins, and a firmware that never sent one falls back to the symbol duration,
which is the rule the engine's own airtime already used.

The sync word is written through two registers rather than a command, so it
landed in the register array and nothing read it back. Two meshes sharing a
frequency under different words therefore heard each other perfectly, which is
the opposite of the property the word exists to provide. The chip now comes up
on the private word the part resets to, reports what it holds, and refuses a
frame sent under another one. vsx_deliver_frame_from carries the transmitter's
word; vsx_deliver_frame is unchanged and still delivers, because a host that
makes no claim about the transmitter has given no grounds to refuse and every
host that has not been taught about the word must go on working.

SetTx's timeout, which was discarded like SetRx's. There is no continuous case
for a transmission, so any nonzero value is a deadline, and reaching it means
the waveform never finished: Timeout, and back to standby. TxDone disarms it,
or a chip that transmitted perfectly would report a fault a hundred
milliseconds later.

vsx_state and vsx_counters gain fields at the end only; VSX_ABI_MINOR 4 -> 5.
…rded

Every opcode outside MeshCore's driver used to fall through to "acknowledged
and ignored": the firmware issued it, got a status byte, and carried on with
the chip in a state it had not asked for. They are decoded now, in commands.cpp,
and each one says in its comment which of three things it is - acted on,
recorded, or a placeholder - because a model that quietly does nothing is worse
than one that does nothing loudly.

Acted on. SetPacketType, so GetPacketType stops answering a hardcoded LoRa;
only LoRa has a data path, and a firmware that switches is reported switched so
a host can refuse to believe the run. StopTimerOnPreamble, which is the setting
that stops a bounded receive cutting off a frame it is in the middle of.
SetLoRaSymbNumTimeout, the same deadline counted in symbols. SetFs. GetStats
and ResetStats. And GetStatus, which was the constant 0x22: it answered standby
whatever the part was doing, so a firmware polling it to find out whether its
receiver was still armed was told no by a chip that was receiving.

Recorded. SetPaConfig, which is half of what sets output power and the board
owns the other half. SetRegulatorMode, SetDIO2AsRfSwitchCtrl,
SetDIO3AsTcxoCtrl, CalibrateImage, and the two test modes. Recording is not a
lesser answer: the fault this model exists to catch is a node configured
differently from how its operator believes, and a value nobody can read back is
a value nobody can check.

Placeholders, each named in the code. SetRxDutyCycle records its periods and
leaves the receiver on, because a receiver deaf for a scheduled fraction of the
time changes which frames arrive and the engine already decides that.
SetRxTxFallbackMode is recorded and FS collapses into standby, because the
difference between the three fallbacks is start-up latency and current draw and
this model has neither. vsx_deliver_frame_failed is the entry point CrcErr and
HeaderErr need to be raisable at all, and nothing calls it: the chip cannot
know a CRC failed, and no host is wired to tell it.

SetPacketParams reads all six fields. Header type, CRC and invert-IQ were
discarded and estAirtimeMs assumed two of them, which is the same shape as the
low data rate optimisation bug and in the same figure the firmware times its
CSMA on: an implicit header is twenty symbols this model charged for and the
part did not.

deliverFrame() is now the one path a frame reaches the chip by. inbox stays
public because hosts push into it and making it private breaks them, which
leaves the hole open: a host that pushes walks around the sync-word check and
will walk around the next one too.

vsx_state and vsx_counters gain fields at the end only; VSX_ABI_MINOR 5 -> 6.
@NomDeTom
NomDeTom force-pushed the irq-mask-enforced-and-cad-that-takes-time branch from 97f53ca to 8fb9623 Compare September 26, 2026 11:41
@A13xB0

A13xB0 commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Reviewed the full diff, built it locally (STRICT=1 ./build.sh and ./build.sh sanitize: 7/7 binaries, 0 failures, no warnings), and checked the arithmetic and datasheet facts against sources.

Verified as correct

  • Airtime: recomputed Semtech's formula for SF11/250 kHz, 50 B, CR 4/5, 16-symbol preamble, explicit header, CRC on, and got 641.0 ms with LDRO off and 722.9 ms on. Matches the commit messages and the test bounds exactly, and the old sf_ >= 11 inference was overcharging about 13% at 250 kHz.
  • LDRO 16 ms threshold: SF11/125 kHz is 16.384 ms (on), SF11/250 kHz is 8.192 ms (off), and no symbol duration lands in the 16.0 to 16.38 ms gap, so 16.0 is unambiguous.
  • Sync-word registers 0x0740/0x0741 and values 0x1424/0x3444, the 15.625 us (1/64 ms) timeout step, cadSymbolNum as 2^n with max n=4, the whole bandwidth table, SF 5 to 12, CR 1 to 4, and every new opcode all check out against the datasheet.
  • Byte/buffer parity test passes, sanitizers clean, no em-dashes.

The honesty discipline is good throughout: the acted-on / recorded / placeholder split, and the note that the pins must not move until a board has booted and forwarded behind this chip.

One thing I could not check: formatting against the pinned clang-format 18.1.8 (only 23.1.1 was available here, which disagrees on correct lines). Worth confirming with the pinned wheel before merge.

Changes before merge

  1. src/VirtualSX1262.h is 512 lines, over the 500 hard limit, so the file length limits CI job fails. The maintainer is allowing this as a one-off override given the header is almost entirely accessor and field documentation, so no change is needed from you here. The CI job will still report it, which is expected and will be handled on the repo side.

  2. Defer vsx_deliver_frame_failed. It is a new ABI entry point that, by its own documentation, nothing can call under any RF model that exists today: calculated RF delivers frames whole or not at all, and the waveform demodulator that could fail a CRC "is not wired to say so". That is exactly the case the Limits table defers ("Speculative ABI: none; add an entry point at the second host that needs it"), and keeping it does not make CrcErr or HeaderErr reachable in any real run, it only pre-stages plumbing. Please drop it for now:

    • the vsx_deliver_frame_failed ABI symbol and its null-handle line in test_c_abi.c
    • the internal deliverFrameFailed and the kReceiveCrcError / kReceiveHeaderError constants
    • the "a frame that failed its CRC can be delivered" case in test_commands.cpp

    Re-add it in the PR that brings the waveform-RF CRC path, since that PR is the second host that needs it and will want to design the injection shape against a real caller. vsx_deliver_frame_from stays: the engine already knows every node's sync word, so it has a callable-today consumer and is the entry point the next pin-move uses.

    With _failed gone, stat_crc_errors and stat_header_errors read an honest zero under calculated RF. I would lean toward dropping those two fields as well so the struct carries nothing that cannot move, but that is marginal and your call. Keep the GetStats decode and stat_rx_packets.

  3. Delete the commented-out CAD exit-mode constants in registers.h (the // Untested. block with kCadExitOnly / kCadExitRx). Dead code, and git remembers it. Worth one line near cad_exit_mode saying CAD_RX is not honored (the chip always ends in standby), since the exit mode is a behavioural property rather than a signal-level one, so the "recorded only" reasoning that covers the thresholds does not quite cover it.

Minor

  1. registers.h has a comment "See the note on kModeFs in VirtualSX1262.h", but there is no kModeFs and no such note. Stale reference, please fix or drop.

  2. statusByte() reports kStatusCmdDataAvailable whenever rxLen_ > 0, and rxLen_ is never cleared after a ReadBuffer or on re-arm, so GetStatus reports data-available indefinitely once any frame has been received, even back in standby. Harmless to MeshCore since it is IRQ-driven, but it is a new artifact of making the status byte dynamic and is not what the part does. Also, using 0x6 (command TX done) as the generic "completed" status for non-TX commands is a slight stretch; it is fine for RadioLib, which only treats 0x3/0x4/0x5 as errors, but the comment calling it "completed" glosses over that.

  3. applyPaConfig stores paLut_, but it has no accessor and is not in vsx_state, unlike the other three PA fields. Minor inconsistency with the recorded-so-it-can-be-checked principle, even though paLut is spec-fixed at 0x01.

  4. Two test_interrupts.cpp cases still send the invalid bandwidth code 0x1A inline, now silently rejected and falling back to the 250 kHz default. Harmless because those cases do not assert timing, but given that killing 0x1A was a headline of this PR, it reads oddly to leave it in new-adjacent tests. Consider 0x03 for consistency.

@A13xB0
A13xB0 merged commit f2604d1 into MeshBench:main Oct 13, 2026
6 of 8 checks passed
A13xB0 added a commit to MeshBench/meshcore-native that referenced this pull request Oct 14, 2026
* bridge: report what the chip's IrqMask kept out

The radio stats record grows from 37 bytes to 47. Bytes 37-42 take the
emulator's tail so the two writers keep one shape: the DIO1 mask, then a
console rate of zero, since a native console is a socket rather than a UART.
Bytes 43-46 carry irqSuppressed(), the number of events the firmware's
IrqMask kept out of the chip's status register.

MeshBench reads the record by length, so a host that predates the field
keeps working with either size.

irqSuppressed() comes from MeshBench/virtual-sx1262#2; this does not build
against the chip at the current pin (2861950).

* vendor: virtual-sx1262 at f2604d1, which has irqSuppressed()

* vendor: virtual-sx1262 at v1.3.3

v1.3.3 adds, on top of f2604d1: HeaderValid waits for the preamble, the
SFD and the header block (#3), and a LoRa header raises HeaderValid
alone (#5), so a firmware whose mask correctly leaves SyncWordValid out
no longer counts every header as suppressed. No ABI change.

---------

Co-authored-by: R0ck <develop@hectospark.co.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants