sx127x LoRa radio driver race condition

[tve]

I believe that the sx127x LoRa radio driver has some race conditions WRT RX/TX switching. I haven't used the driver but have written an FSK driver for that chip, which reminded me of the issue and I looked at the LoRa driver source code to see how it handled things. So perhaps I'm interpreting the code incorrectly and all this is a non-issue :^)

Suppose the app performs the following on a periodic timer:

values = read_sensors()
packet = format_packet(values)
radio.write(packet)

Assume enableReceiver = true in the radio config, for example, the app expects to receive commands via packets.

Now suppose a packet is received by the radio during read_sensors() or format_packet(). The RX interrupt will fire and the interrupt pin's onReadable callback will be queued. The packet is sitting in the FIFO. But the app continues and calls radio.write() which switches the radio to TX mode, overwrites the FIFO, and transmits.

As soon as the app returns from the periodic timer the pin onReadable callback is called. The radio driver thinks TX is done since the radio is in TX mode and ends the TX and switches to RX mode. The result is that the received packet is lost and the transmitted packet is likely aborted before it is fully transmitted (exact outcome depends on timing).

The above situation is not that likely to occur because most uses of the chip are sensors that TX only and that RX very rarely, if ever, and have specific time windows for that where they can suppress TX.

Fixes to consider:

• just document the issue and leave it at that
• before starting TX, check whether RX has completed and raise an error
• before starting TX, check whether RX is in progress and raise an error
• add a writeIf method that performs above checks and has a return value to indicate whether TX happened or not, as opposed to throwing an error in write
• if writeIf returns that TX is not possible (or write throws), guarantee that a fresh onWritable callback is made when TX is possible again

[phoddie]

@cmidgley?

[cmidgley]

I was a bit concerned about the interrupt handling in this as well (I simply ported this from a C++ module with very few structural changes). Since LoRa is not a reliable transport, all apps using it should already be implemented to understand that packets might be lost. Given that, unless this causes some other interesting fault (exception? crashed radio? problems waking from sleep?), or it happens frequently, I'm not sure I'd spend much time worrying about it.

If it is a frequent problem, a better answer might be to detect that the RX is pending and queue the TX radio transition to occur upon the RX completion. This sounds like work, especially considering the amount of testing that would be needed to verify its reliability.

If not a common problem, yet the failure mode causes behavioral problems on following RX/TX packets (meaning, not just the current lost RX and/or TX packet), then perhaps the fix can be simplified to just focus on solving that behavior and not trying to recover the packet.

In general, I'd not be in favor of throwing exceptions given that packets are commonly lost anyway and therefore any LoRa app requires handling of that condition already.

[phoddie]

Thanks, @cmidgley. Viewing this condition like packet-loss seems like a reasonable perspective, as long as it is infrequent.

Given that this radio is essentially a free-running external peripheral, these sorts of conditions feel inevitable. That being the case, I'd look to the manufacturer for guidance. This seems like the kind of thing that an application note would provide guidance for handling. The manufacturer has more knowledge than us, and so should be able to offer the best advice. @tve, have you seen anything like that?

[tve]

My 2 cents... I regard it as definitely uncool to corrupt two packets by not putting a check into the driver to see whether a packet has been received or is in the process of being received. There is an RxDone flag in RegIrqFlags to check the first condition, one could also read the interrupt pin (faster). There is a RX on-going flag (and other flags about RX status) in the RegModemStat. This covers the race condition where RX is happening or happened and will get corrupted by a TX. (Semtech docs usually state when a flag is set and often omit to specify when it is cleared, so one always has to experiment a bit...)

If the radio is not in RX mode then the above checks don't apply unless you implement what's often called Listen Before Talk. This is something LoRaWAN doesn't require and so Semtech doesn't talk about it. There are features in the radio that allow for this but it is time and power consuming so not something to have by default. This is not something I suggested although it is something commonly used/required in packet radio protocols.

BTW, I often see very high success rates with LoRa transmissions, it all depends on the conditions, and one could also argue that the lower the success rates the more precious every packet is ;-)

I am also sensitive to the fact that this is a ported driver, hence my first bullet point suggestion... Writing a good driver for these radios that supports multiple use-cases is actually quite hard and a lot of work. It pretty much requires reading the datasheet multiple times from top-to bottom with a fine-toothed comb and then trying out the combinations that are left out...

[phoddie]

@tve, I don't believe @cmidgley or I is disagreeing with you.

Writing a good driver for these radios that supports multiple use-cases is actually quite hard and a lot of work

Exactly. That's why I was asking whether the manufacturer provided any guidance. They have the knowledge and experience well beyond "reading the datasheet multiple times from top-to bottom with a fine-toothed comb."

[tve]

manufacturer provided any guidance.

Not that I've ever seen for any of the issues at hand. They provide app notes around physical board layout, antenna matching, and lots of stuff around the LoRa modulation choices.

Something that occurred to me is that in the original C++ driver the RXDone interrupt handler most likely was an actual interrupt handler that took care of the received packet in some form and thus the worst of the race conditions (packet received and then clobbered by TX) probably didn't occur in the C++ version.

[phoddie]

Not that I've ever seen for any of the issues at hand

Too bad. Manufacturers are incredibly uneven in the information they provide, alas. Thanks for looking.

[cmidgley]

In my use case, it will almost never happen. Given the legal requirements (FCC, etc) of the LoRa frequencies, LoRa devices generally use very little time transmitting. I see how in a highly congested area this could be a meaningful problem, but for me, it's a time/value equation - given my current understanding of the problem and my use case, I'm not seeing enough value to put in the time to try to fix it.

However ... I totally agree that it would be best to fix this race condition. Might you consider taking on the project? Perhaps start by creating a test that can (somewhat) consistently create the race condition, so the fix can be verified that it doesn't introduce any new failure modes (as that's what worries me most - having special code that attaches to a race condition thereby increasing the risk for edge-case stability issues).