Skip to content

Fix LR11x0 and LR2021 packet loss when a stale SPI reply is read as the length #3261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fkallay1 wants to merge 1 commit into
base: dev
Choose a base branch
from
+89 −1
Open

Fix LR11x0 and LR2021 packet loss when a stale SPI reply is read as the length #3261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion src/helpers/radiolib/CustomLR1110.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 48 additions & 0 deletions src/helpers/radiolib/CustomLR2021.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down