From 152c6d5d2a5ff949f43db419ea831659850f4ed3 Mon Sep 17 00:00:00 2001 From: Fedor Kallay Date: Thu, 20 Aug 2026 18:47:08 +0200 Subject: [PATCH] Recover from a stale SPI reply read as the received packet length On both LR11x0 and LR2021 a "get" command is two SPI transactions: send the opcode, then read the reply. SPItransferStream() waits 1 us after the first transaction before it starts polling BUSY, so when BUSY has not risen yet the wait is skipped and the reply is read before the chip has prepared it. The chip answers with its default [stat 2B][irq 4B] stream instead, and the length is parsed straight out of that. On LR2021 the length is built from two bytes, so the result is irq[31:16] - exactly 4 with RX_DONE set. A 50 or 133 byte frame was reported as len=4, readData() read 4 bytes, its clearRxFifo() discarded the rest and the frame was rejected as corrupt. It is not counted as a receive error either, because readData() returned success. On LR11x0 the length is a single byte, irq[31:24], which reads 0, and recvRaw() then skips the packet on its 'len > 0' test. The chip does say whether a reply is on its way: the command status field of stat1 is CMD_DAT ("successfully processed, data is being transmitted") for the read half of a get, and CMD_OK ("nothing to collect") when the status stream comes back instead. RadioLib consumes that byte internally and only rejects CMD_FAIL and CMD_PERR, so the distinction is lost. Reading the length here directly keeps the status byte visible, which makes the check possible - the same trick LRxxxx::getIrqStatus uses to read the IRQ word. The Rx buffer is still intact at that point, so re-reading recovers the frame. Measured on both families in a live mesh at SF7, with the BUSY wait skipped on purpose on every fourth length read: - LR2021 (XIAO nRF52840 with a NiceRF LoRa2021F33-2G4): 11 of 11 sabotaged reads returned 4 and reported CMD_OK; the retry returned the true length and the node received the same traffic as a second receiver on the same channel. - LR11x0 (Seeed T1000-E): 4 of 4 came back as 0 with CMD_OK, and the retry recovered 84, 20 and 196 byte frames. The status is also what keeps the check honest, and this is where the first version of this change was wrong. On LR2021, judging by the value alone looks attractive - compare it against irq[31:16] - but 68 is both a common frame length and what those bytes read when RX_DONE and CRC_ERROR are set together, so it fires on genuine frames. On LR11x0 the same idea was used with 'len == 0 && RX_DONE', on the assumption that a zero length cannot be genuine there. Hardware says otherwise: on the T1000-E that state occurs 25 times in three minutes, roughly one per two and a half received frames, with the IRQ word at 0x38, 0x78 (header error) or 0xB8 (CRC error). A probe doing three extra reads on each of them recovered nothing - 0 out of 25 - so the zero is usually honest and only the status can tell it apart from a stale reply. Both chips therefore use the same check now. The existing 'len == 0 && HEADER_ERR' handling in CustomLR1110::getPacketLength is left untouched. Note this only covers the path through recvRaw(). LR11x0::readData() reads the length and buffer offset again through the two-argument getPacketLength(bool, uint8_t*), which is not virtual and therefore cannot be guarded from a subclass. If the race is lost there, the payload comes out empty or shifted by the bogus offset. That part can only be fixed in the driver, and has been reported to RadioLib as an issue with the reproduction. Build tested: t1000e_repeater, MeshTracker_X1_repeater. --- src/helpers/radiolib/CustomLR1110.h | 42 ++++++++++++++++++++++++- src/helpers/radiolib/CustomLR2021.h | 48 +++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/helpers/radiolib/CustomLR1110.h b/src/helpers/radiolib/CustomLR1110.h index 75674a1dfd..3e21390b88 100644 --- a/src/helpers/radiolib/CustomLR1110.h +++ b/src/helpers/radiolib/CustomLR1110.h @@ -13,8 +13,48 @@ class CustomLR1110 : public LR1110 { public: CustomLR1110(Module *mod) : LR1110(mod) { } + // Two SPI transactions with the status word kept - mirror of the LR2021 helper. + int16_t readRxPktLenWithStatus(bool wait, uint8_t* stat, uint16_t* val, uint8_t* off = NULL) { + int16_t st = mod->SPIwriteStream(RADIOLIB_LR11X0_CMD_GET_RX_BUFFER_STATUS, NULL, 0, wait, false); + Module::BitWidth_t sw = mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS]; + Module::BitWidth_t cw = mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD]; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = Module::BITS_0; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD] = Module::BITS_0; + uint8_t buff[4] = { 0 }; + st = mod->SPIreadStream(RADIOLIB_LRXXXX_CMD_NOP, buff, sizeof(buff), wait, false); + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = sw; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD] = cw; + if (stat) *stat = buff[0]; + if (val) *val = buff[2]; + if (off) *off = buff[3]; + return st; + } + + // Guard against a stale SPI reply being parsed as the received length. A "get" is + // two transactions (LRxxxx::SPIcommand): send the opcode, then read the reply. + // SPItransferStream() waits 1 us before polling BUSY, so if BUSY has not risen yet + // the reply is read too early and the chip answers with its default [stat 2B][irq 4B] + // stream. getRxBufferStatus() then takes the length from irq[31:24] and the buffer + // offset from irq[23:16] - and that offset is what shifts a payload. + // + // The value cannot decide this on its own here: a length of 0 while RX_DONE is set is + // a state the chip reaches routinely. Measured on a T1000-E, 25 times in three + // minutes - roughly one per two and a half received frames - with the IRQ word + // reading 0x38, 0x78 (header error) or 0xB8 (CRC error). Re-reading recovers nothing + // there, 0 out of 25, so treating every zero as suspect only burns SPI reads. The + // status separates the two cleanly: those reads report CMD_DAT, while a reply read + // too early reports CMD_OK and re-reading then returns the true length - 4 of 4 with + // the BUSY wait skipped on purpose, recovering 84, 20 and 196 byte frames. size_t getPacketLength(bool update) override { - size_t len = LR1110::getPacketLength(update); + uint8_t stat = 0; + uint16_t val = 0; + size_t len = 0; + for (int i = 0; i < 3; i++) { + readRxPktLenWithStatus(true, &stat, &val); + len = val; + if ((stat & 0x0E) == RADIOLIB_LRXXXX_STAT_1_CMD_DAT) break; + if (i == 2) len = LR1110::getPacketLength(update); // never worse than the plain read + } if (len == 0 && getIrqStatus() & RADIOLIB_LR11X0_IRQ_HEADER_ERR) { // we've just received a corrupted packet // this may have triggered a bug causing subsequent packets to be shifted diff --git a/src/helpers/radiolib/CustomLR2021.h b/src/helpers/radiolib/CustomLR2021.h index a89ae94330..60b930b69b 100644 --- a/src/helpers/radiolib/CustomLR2021.h +++ b/src/helpers/radiolib/CustomLR2021.h @@ -75,6 +75,54 @@ class CustomLR2021 : public LR2021 { return LR2021::startReceive(RADIOLIB_LR2021_RX_TIMEOUT_INF, RADIOLIB_IRQ_RX_DEFAULT_FLAGS | (1UL << RADIOLIB_LR2021_IRQ_PREAMBLE_DETECTED), RADIOLIB_IRQ_RX_DEFAULT_MASK, 0); } + // Read the received length ourselves, keeping the status word. + // A "get" on this family is two SPI transactions (LRxxxx::SPIcommand): send the + // opcode, then read the reply. SPItransferStream() waits 1 us before polling + // BUSY, so when BUSY has not risen yet the reply is read too early and the chip + // answers with its default [stat 2B][irq 4B] stream instead. RadioLib strips the + // status and hands the rest back as data, so getRxPktLength() returns irq[31:16] + // - exactly 4 with RX_DONE set. Setting the status width to 0 keeps both status + // bytes in our own buffer, the same trick LRxxxx::getIrqStatus uses. + int16_t readRxPktLenWithStatus(bool wait, uint8_t* stat, uint16_t* val) { + int16_t st = mod->SPIwriteStream(RADIOLIB_LR2021_CMD_GET_RX_PKT_LENGTH, NULL, 0, wait, false); + Module::BitWidth_t sw = mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS]; + Module::BitWidth_t cw = mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD]; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = Module::BITS_0; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD] = Module::BITS_0; + uint8_t buff[4] = { 0 }; + st = mod->SPIreadStream(RADIOLIB_LRXXXX_CMD_NOP, buff, sizeof(buff), wait, false); + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_STATUS] = sw; + mod->spiConfig.widths[RADIOLIB_MODULE_SPI_WIDTH_CMD] = cw; + if (stat) *stat = buff[0]; + if (val) *val = ((uint16_t)buff[2] << 8) | (uint16_t)buff[3]; + return st; + } + + // Trust the length only when the chip says the reply is ours. The command status + // field of stat1 has four values and CMD_DAT means "successfully processed, data + // is being transmitted" - the only correct one for the read half of a get. A + // reply produced by the race reports CMD_OK instead, i.e. "nothing to collect", + // which is exactly the case where the status stream comes back. The Rx FIFO is + // still intact at this point (readData() runs later), so re-reading recovers the + // frame instead of losing it. + // Measured on hardware: with the BUSY wait skipped on purpose in the live RX + // path, 11 of 11 bogus reads reported CMD_OK and every frame was recovered; on + // genuine frames whose length happened to equal irq[31:16] the status correctly + // reported CMD_DAT. Judging by that value alone - as an earlier version did - + // therefore misfires on real frames, which is why the status decides here. + size_t getPacketLength(bool update = true) override { + uint8_t stat = 0; + uint16_t val = 0; + for (int i = 0; i < 3; i++) { + readRxPktLenWithStatus(true, &stat, &val); + if ((stat & 0x0E) == RADIOLIB_LRXXXX_STAT_1_CMD_DAT) return val; + } + // never end up worse than the plain library read + return LR2021::getPacketLength(update); + } + + + bool isReceiving() { uint32_t irq = getIrqStatus(); bool preamble = irq & RADIOLIB_LR2021_IRQ_PREAMBLE_DETECTED; // bit 5