mcci-catena / arduino-lmic

LoraWAN-MAC-in-C library, adapted to run under the Arduino environment
https://forum.mcci.io/c/device-software/arduino-lmic/
MIT License
629 stars 207 forks source link

Arduini Pro Mini stuck on os_init #523

Closed aparcar closed 4 years ago

aparcar commented 4 years ago

Similar to https://github.com/mcci-catena/arduino-lmic/issues/521 my Arduino Pro Mini hangs on os_init. While Catena offers a working fork for the Feather M0 (Catena 4410) I can't find something similar for the Pro Mini.

Is it possible that the same GPIO changes corrupted the os_init() here as well?

JackGruber commented 4 years ago

Hi aparcar,

@terrillmoore for me it looks like the commit 306330fcb5be10c9897f96d2885af530d168a5d3 is the problem. I have today no time to investigate deeper, but with git bisect i figured out, that with this commit the pro Mini stuck in the os_init function.

With the a4044c0 i can join the network and send data.

Have a nice weekend Alex

terrillmoore commented 4 years ago

@JackGruber and @aparcar -- thanks for looking into this.

This is because "interrupts disabled" are stopping the clock from advancing properly. hal_waitUntil() is hanging because time is not advancing.

Various BSPs handle time differently when interrupts are disabled, and start up with different interrupt enabled/disabled state. The MCCI BSPs all have the property that time advances even when interrupts are disabled. But many BSPs do not.

The LMIC needs to be reworked to remove all the interrupt enable/disable logic that's presently there, as I am certain that there's no need for extended interrupt disables. (I think this was an artifact of the original adaptation for Arduino. I'll raise a linked bug.

aparcar commented 4 years ago

How to proceed here? Should I just use the outdated version or is there a way to change the behavior in the current release?

terrillmoore commented 4 years ago

Unluckily, I have a pressing commitment on another project, and can't look at this until that's done. Probalby another week. If you use git to clone the repo, you can then use git to backout the single change https://github.com/mcci-catena/arduino-lmic/commit/306330fcb5be10c9897f96d2885af530d168a5d3. The command is something like:

git revert 306330fcb5be10c9897f96d2885af530d168a5d3

The problem is that os_radio() disables interrupts. delay() and delay_microseconds() work (but may be inaccurate) on many platforms, even interrupts are disabled.

There is no good reason for os_radio() to disable interrupts other than to prevent re-entry into the radio driver. There are standard techniques for doing this without disabling interrupts. My first hurried attempt to fix this didn't work; I need a few hours when I'm not on deadline to correct this. Another approach would be to use #if, but on any platform disabling interrupts for so long is not a good thing. I'd prefer to fix the root cause. (Disabling interrupts like that causes other problems for timing inside the radio driver.)

bertrik commented 4 years ago

Aww, ran into this exact problem, traced it down to hal_waitUntil(). Good to see the issue acknowledged.

sweetpants commented 4 years ago

I reverted back to 3.0.99.9 (commit: e59c1234af435c5b97adab052f8cd56e9d9522eb) and works again.