lora: Fix IRQ bit mask for sx126x

[maholli]

Fixes #1112 which causes corrupt packets to silently be passed as valid by the driver.

[projectgus]

Hi @maholli, sorry it took me a little while to get back to this. Thanks for very clear description of the problem and the fix.

I have a request about the code comment surrounding this change, and also please edit micropython/lora/lora-sx126x/manifest.py and bump the bugfix version of this package so mip will update it.

Thanks again!

[projectgus]

These comments are for the old (broken) code, so please change it to say something "just the IRQs that the driver is interrupted by or needs to read back via a status command" and remove the second "Note:" comment entirely.

[projectgus]

@maholli I went to apply the suggestions above so I could quickly merge this, but I realised that there's another bug here - if dio1 is not set (i.e Python code will poll for interrupt status) then the mask bits are set to 0 and probably a number of things don't work!

I've pushed a fix here: projectgus@44b6fca

Any chance you're in a position to test this? If it looks good to you then you can cherry-pick it here, or I can open a new PR.

[maholli]

@maholli I went to apply the suggestions above so I could quickly merge this, but I realised that there's another bug here - if dio1 is not set (i.e Python code will poll for interrupt status) then the mask bits are set to 0 and probably a number of things don't work!

I've pushed a fix here: projectgus@44b6fca

Any chance you're in a position to test this? If it looks good to you then you can cherry-pick it here, or I can open a new PR.

@projectgus I think this would cause unwanted (or at the very least, different) behavior compared to the current lib.

I agree that including _IRQ_DRIVER_RX_MASK makes sense based on the name, but since the driver hasn't been doing that, by adding it now it will introduce _IRQ_HEADER_ERR to the actual IRQ mask which I think might have strange behavior. Based on my testing, _IRQ_HEADER_ERR is firing all the time but unlike the other bits it doesn't trigger an "RxDone" event. That means by the time a real packet comes along and RxDone triggers and you read the IRQ byte, you have a high likelihood of a stale _IRQ_HEADER_ERR bit hanging out in there which can cause the valid packet to be tossed.

So I agree that the driver could use other tweaks, but I think it'll take dedicated time to test and understand the implications of the changes - deserving more time and attention than I have to spare :)

Thoughts?