sandeepmistry / arduino-LoRa

An Arduino library for sending and receiving data using LoRa radios.
MIT License
1.65k stars 630 forks source link

High rate transmission and reception race conditions? #477

Open dantard opened 3 years ago

dantard commented 3 years ago

Hello, Maybe I'm missing something but I noticed two problems when two MKRWAN 1300 are used to exchange frames at high rate (50-80 packets per second). The first one is that the base address of the TX queue and RX queue seem to be the same:

writeRegister(REG_FIFO_TX_BASE_ADDR, 0);
writeRegister(REG_FIFO_RX_BASE_ADDR, 0);

Under certain circumstances (I can't reproduce them consistently since it must be happen during a race condition), the read() call returns the frame just sent.

To avoid that, I set the REG_FIFO_RX_BASE_ADDR register to other address but I'm not sure what that address should/could be exactly.

A similar issue happens with:

writeRegister(REG_IRQ_FLAGS, irqFlags);

which is always called in the parsePacket() call. Due to (I guess) a similar race condition, the call returns sometimes a positive value when a packet has been already read (through the read() call) provoking a duplicate packet at user side. I believe that this code should instead be only called when irqFlags & IRQ_RX_DONE_MASK like this:

  if (irqFlags & IRQ_RX_DONE_MASK){
     writeRegister(REG_IRQ_FLAGS, irqFlags);
  }

Thanks for your attention.

IoTThinks commented 3 years ago

Would the finding be applicable to other boards (Arduino, ESP32...) too?

dantard commented 3 years ago

I would say it affects all the boards compatible with this code that is those based on Semtech SX1276/77/78/79 chipset.

IoTThinks commented 3 years ago

@dantard Would you mind creating a pull request for this? It is easier to compare the code changes in the pull request.

Thanks a lot.

engeen-nl commented 3 years ago

I can confirm I've seen the same behaviour for the MKR 1310. I measure 9 ms between the readings of the duplicate message. Sometimes a message just sent comes back as a received message.

Good find @dantard about the REG_FIFO_TX_BASE_ADDR. Page 35 of the Semtech SX1276 datasheet:

By default, the device is configured at power up so that half of the available memory is dedicated to Rx (RegFifoRxBaseAddr initialized at address 0x00) and the other half is dedicated for Tx (RegFifoTxBaseAddr initialized at address 0x80).

That would mean128 bytes for receiving and 128 bytes for sending. Maybe @sandeepmistry wanted to support larger packages?

I can confirm changing RegFifoTxBaseAddr to 0x80 and only resetting the irqFlags when done solved these issue completely on my side. Thank you dantard!

@IoTThinks we're not all git experts, please consider applying these changes. It regards lines 135, 136 and 233 of LoRa.cpp.