Updated examples for callback receive mode

[DzikuVx]

Current examples for callback receive suggests that it is OK to to heavy processing inside ISR.
This is not true and can lead to multiple problems, including SPI locking, race conditions and other hard to debug side effects.
Proposed examples sets flag in ISR but actual reading from SPI and processing is done inside mal loop

[florian-guillemard]

Hi,

I tried the DzikuVx's version and it appear I need to add a "pinMode(LORA_IRQ, INPUT);" for the interrupt to work..

Even with that, the interrupt is called with the good number of characters to read but when I read those, the data seems to be the previous (sometimes) or a shifted version of the previous..

Also, DzikuVx, why do you put 'recipient' in a 'int' instead of a 'byte' ?

Regards,

[DzikuVx]

@NK36 I did not changed the logic, only moved code from left to right. But you are correct, byte can be used over there. I did not had to define DIO1 pin as input to make it work.

[florian-guillemard]

Maybe because I have not the same board as you..

I have this one :
https://www.aliexpress.com/item/TTGO-LORA-SX1278-ESP32-0-96-OLED-16-Mt-bytes-128-Mt-bit-433Mhz-for-arduino/32825380086.html?shortkey=FrQZ3YFJ&addresstype=600

With this pinout :
https://raw.githubusercontent.com/darthm0e/esp32-101/master/Heltec_WIFI-LoRa-32_DiagramPinout.jpg

Don't you think ?

But I can't see why it behave different :/

[florian-guillemard]

If I want to use the interrupt, I need to read the data into the interrupt..
I can't read correct data outside the interrupt!

[szotsaki]

In pull request #50 there is a reworked interrupt handling with changes separated into atomic commits.

[florian-guillemard]

Hi,

Thanks szotsaki, I spent a lot of time on my problem and find out that it was due to PlatformIO which don't use the very last version of Arduino for ESP32... I was willing to use SPIFFS and it was not in my directories.. so I have to trick PlatformIO and then I retried to use LoRa with interrupt. Then it was successful!

Still, I had to put the IRQ pin to INPUT in my setup()...

Thank you both of you for your help ;)

[sandeepmistry]

Hi @DzikuVx,

Thanks for submitting this!

This is not true and can lead to multiple problems, including SPI locking, race conditions and other hard to debug side effects.

Interesting, maybe a better solution is to change the library. Do you think moving the clearing of the IRQ flags line ( writeRegister(REG_IRQ_FLAGS, irqFlags);) after the callback can fix this?

https://github.com/sandeepmistry/arduino-LoRa/blob/master/src/LoRa.cpp#L471

[DzikuVx]

Hi @sandeepmistry I do not think this really solves the problem. It only masks it for some cases.
For example my application requires both read callback and timing processing on other pin via pin interrupt. Two ISRs fighting for priority, niah...
Also doing processing in ISR callback requires all variables to be volatile if they are about to be read outside ISR (that leads to other problems). It's just so much simpler and safer for not advanced users to avoid doing this in ISR

[sandeepmistry]

For example my application requires both read callback and timing processing on other pin via pin interrupt. Two ISRs fighting for priority, niah...

Right, but that's different from the example right?

Also doing processing in ISR callback requires all variables to be volatile if they are about to be read outside ISR (that leads to other problems). It's just so much simpler and safer for not advanced users to avoid doing this in ISR

This was based on the Wire libraries API's, which also calls the user callback in an interrupt context. https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/libraries/Wire/examples/slave_receiver/slave_receiver.ino

[DzikuVx]

@sandeepmistry I've lost track where this conversation is going. This is your project, your repo, you decide if current examples are fine or if they could be improved to lower possibility of future problems for new users.

[morganrallen]

This has come up a couple times now and I agree the examples are problematic. There is only a minor conflict to be resolved so accepting this shouldn't be a big deal. I'll see about testing this and getting some version of it implemented.