Repository navigation
HeaderValid waits for the preamble, the SFD and the header block - #3
Merged
Merged
Conversation
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.
HeaderValid was raised a fixed twelve symbols after a carrier appeared. A LoRa header ends after the whole preamble, the two sync symbols, the 2.25-symbol SFD and the eight-symbol header block, so it is now raised at the programmed preamble length + 4.25 + 8 symbols. MeshCore programs a 32-symbol preamble at SF8 and below and 16 above; at SF8 the flag previously came 32 symbols early. The receiver's own programmed preamble length stands in for the transmitter's, since the chip is not told who is sending. A receiver in implicit-header mode raises no HeaderValid. Two new cases in test_reception.cpp: HeaderValid is not set 100 ms into a carrier at SF8/62.5 kHz with a 16-symbol preamble and is set by 120 ms (the first assertion fails on the previous code), and an implicit-header receiver never sets it.
Contributor
|
BTW props, loving this so far. Looks good as you are removing the leniency and making it stricter! Thanks for the PRs so far. |
NomDeTom
marked this pull request as ready for review
October 13, 2026 02:47
This was referenced Oct 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #2: this branch is #2's head (
8fb9623) plus one commit,df0fe02(reception: HeaderValid waits for the preamble, the SFD and the header). It is a draft until #2 merges.What this changes
HeaderValid(andSyncWordValid) was raised a fixed twelve symbols after a carrier appeared (kHeaderSymbols = 12.0). It is now raised at the programmed preamble length + 4.25 + 8 symbols: the preamble, the two sync symbols and the 2.25-symbol SFD, then the eight-symbol header block.SetPacketParams) stands in for the transmitter's, since the chip is not told who is sending.HeaderValid.kHeaderSymbolsis replaced bykSyncAndSfdSymbols(4.25) andkHeaderBlockSymbols(8.0).PreambleDetectedtiming (kPreambleSymbols, 4) is unchanged.MeshCore programs a 32-symbol preamble at SF8 and below and 16 above (
preambleLengthForSFinRadioLibWrappers.h, 1.16.0 and 1.17.0). At SF8 and 62.5 kHz the header now ends 44.25 symbols (about 181 ms) into a frame, against 12 symbols (about 49 ms) before.No ABI change.
Verification
STRICT=1 ./build.sh: 7 test binaries, 0 failures../build.sh sanitize: 7, 0 failures, no sanitizer reports.STRICT=1 ./build.sh shared: builds.clang-format18.1.8 (theCLANG_FORMAT_VERSIONpin),--dry-run --Werror include/*.h src/*.h src/*.cpp test/*: clean. That includes Enforce the IRQ mask, time CAD scans, and decode the rest of the command set #2's files, which the review on Enforce the IRQ mask, time CAD scans, and decode the rest of the command set #2 could not check against the pinned version.test_reception.cpp:HeaderValid waits for the preamble, the SFD and the header block: at SF8, 62.5 kHz and the default 16-symbol preamble,HeaderValidis not set 100 ms into a carrier and is set by 120 ms. Against8fb9623the first assertion fails.an implicit-header receiver never raises HeaderValid.c66dc12(this commit before formatting; it differs only in comments and line breaks), run headless in MeshBench (scotland-strict, five seeds). Total time withPreambleDetectedorHeaderValidset, summed over nodes: 1.16 442 s, 1.17 493 s, against 483 s and 493 s at8fb9623.