[SX1262] Support deep sleep by allowing ::begin(...) without reset of module.

[cleishm]

Currently, invoking SX1262::begin(...) always resets the module during RadioLib initialization. This adds an additional argument, resetModule, which causes SX1262::begin(...) to avoid reseting the module during initialization, allowing the library to be configured while leaving the SX1262 in a known state, e.g. with a received packet in the packet buffer.

This enables support for ESP32 deep sleep without losing the packet buffer on wakeup. An example sketch for deep sleep is also provided.

NOTE: I do not like adding the additional resetModule=true to the already long list of arguments to ::begin(...). I have proposed #1607 to make this much cleaner. Making this a draft PR while #1607 is considered.

[cleishm]

Responding to @StevenCellist's related #1607 (comment):

But maybe it could be an idea to look at the LoRaWAN implementation: have a fully working stack that expects to live in RAM, no ESP32-deepsleep. But provide an additional interface for such a usecase that leaves the rest of the library intact. An idea could be to have a sort of export() function that creates such a struct as you propose from all cached variables or read from registers. And then a sort of import() function that takes a struct and re-instates all variables without reconfiguring the radio etc. Up to the user to ensure that the radio is not modified in between. With such an implementation, begin() can be left as is, keeping it easy for the newbies around here.

Would love to dig into this line of thinking.

I originally expected to have to export/import state to keep RadioLib functioning correctly over deep sleep, or find a way to keep it all in RTC memory (like @DarkZeros did here).

But, I discovered that it is possible to reinitialize the entire RadioLib stack, and even the module itself, without losing the packet buffer - just by making sure the SX1262 isn't fully reset. So it really is just a fresh "begin" of the library, just without resetting the module. And reading the packet that triggered the wakeup (i.e. keeping the packet buffer intact) is all people seem to be missing wrt deep sleep.

Regarding the extra resetModule parameter to ::begin: it does make the already long signature to ::begin even longer, and has the problem that it's the last parameter yet one people doing deep sleep would need to set. Making them have to set all the prior ones rather than using defaults. Hence the named arguments PR (#1607) to avoid all of that.

However, an alternative, similar to your suggestion, would be to add a method that has the same signature as ::begin but doesn't reset. Call it ::reinit. Or ::restart maybe. It would make invocation a little less clean (the user would need an if/else block), but would avoid the issues with adding a resetModule parameter that I just described, without having to add named arguments.

[cleishm]

BTW - Section 7.4 of the SX1262 datasheet (v1.2) describes the data buffer behavior, and it does seem that the only operation that will clear it is putting the module into sleep mode (note for the wary: that's the SX1262, not the MCU). So it would seem that, as long as you avoid resetting, sleeping the module, or changing the buffer's RX pointer (SetBufferBaseAddresses SPI command), one can do anything else on the module.

Given that, I suspect that even if the user called ::begin with resetModule=false and used entirely different configuration, reading the packet that caused the wakeup would still succeed. I don't know why they would want to change parameters in reality, but it does mean that it's not critical that parameters remain the same each time RadioLib is re-initialized after a deep sleep wakeup. Which would lend support to the idea that there is very little difference between a regular initialization of RadioLib (and the module) and a "woke up from deep sleep" initialization, and thus using the same pathway with just a "don't reset" flag seems sensible.

I haven't tested this on hardware yet (I'm traveling this weekend) but I can do so on Monday.

[DarkZeros]

It is worth noting that there are 2 different problems and use cases.

• Initialize without reset
• Not lose state in the module during deep sleep

The PR is implementing case 1, not case 2. After a deep sleep the old setup done to the module will still be completely lost. Typically this is "known" by the developer, but might be good to have.

In order not to lose the state, it should be stored on RTC_DATA. I had a working implementation wrapping all the classes in std::optional to avoid recontructing the objects after deep sleep. Still, it was needed to reset the GPIO states.

You can see a working example here https://github.com/DarkZeros/LightMyInk/blob/devel/main/radio.cpp
I could provide a minimum example if needed.

The PR on itself is fine, i am just adding some context, that it might be nice to have some support to wrap module speficics into a struct that survives deep sleeps using RTC_DATA atrributes. Wrapping all the classes is ok, but it uses too much memory, storing strings and other helper things.

[cleishm]

• Not lose state in the module during deep sleep

Curious. In your use cases, what state do you need to keep other than the packet buffer and why? I have deep sleep working well with just this change and can't think of what other state I'd ever need to have preserved.

[Edit to add] I just looked through that code, and I don't see anything that needs to be preserved in RTC memory. You're programmatically setting the power level, but always revert it back to the known value kMinPower after the operation. Every other value appears to be fixed and doesn't change over restarts, so you can just reinit (without the reset). Here's a very rough, uncompiled patch with notes: https://gist.github.com/cleishm/49dc80d8685026fad551d74644305d8a

[DarkZeros]

• Not lose state in the module during deep sleep

Curious. In your use cases, what state do you need to keep other than the packet buffer and why? I have deep sleep working well with just this change and can't think of what other state I'd ever need to have preserved.

[Edit to add] I just looked through that code, and I don't see anything that needs to be preserved in RTC memory. You're programmatically setting the power level, but always revert it back to the known value kMinPower after the operation. Every other value appears to be fixed and doesn't change over restarts, so you can just reinit (without the reset). Here's a very rough, uncompiled patch with notes: https://gist.github.com/cleishm/49dc80d8685026fad551d74644305d8a

Eaxctly right. This is what i mean that in most cases the developer can simply reset the known state after deep sleep. Therefore the PR is valid for most cases, still doesnt make the library deep sleep safe, just resilient.
A rtc state of the module is a safer, but also involves more changes and work, that might not be worth the effort.

In my case i did it this way since I wanted to use the upstream version of the lib. If this PR is merged i will switch to the new method.

[cleishm]

This is what i mean that in most cases the developer can simply reset the known state after deep sleep. Therefore the PR is valid for most cases, still doesnt make the library deep sleep safe, just resilient.

I don't want to push the point too hard, but I'm curious what state you're concerned with? As far as I can determine from the data sheet, this approach isn't just resilient, it's perfectly "safe" for deep sleep wake up.

[DarkZeros]

This is what i mean that in most cases the developer can simply reset the known state after deep sleep. Therefore the PR is valid for most cases, still doesnt make the library deep sleep safe, just resilient.

I don't want to push the point too hard, but I'm curious what state you're concerned with? As far as I can determine from the data sheet, this approach isn't just resilient, it's perfectly "safe" for deep sleep wake up.

If I'm not mistaken (i may have comoletely missunderstood the code) things like getPacketLen() and getDataRate() return the cached value inside the module, using the values calculated in begin(). Which might not match the actual packet len since it may not match the real module setup.

Again, it may not be an issue, and if it is, a very minor edge case.

[cleishm]

If I'm not mistaken (i may have comoletely missunderstood the code) things like getPacketLen() and getDataRate() return the cached value inside the module, using the values calculated in begin(). Which might not match the actual packet len since it may not match the real module setup.

Ah. So I think both of those are ok, for different reasons:

• getPacketLength(): From my own testing, I know retrieving this value after a deep sleep works correctly without storing anything in RTC memory. And looking at the SX126x code for this function, I can see that the value is obtained directly from the module, using the SPI command RADIOLIB_SX126X_CMD_GET_RX_BUFFER_STATUS (0x13). The datasheet indicates that only a command to change the buffer addresses (or a reset) would upset this, so it should be preserved over MCU restarts without needing anything in RTC memory.

• getDataRate(): This is certainly is calculated. It represents the effective data rate of the last packet transmitted. However, there would be no reason to deep sleep the MCU during packet transmission, as the timeframe for transmission could never be long enough to cover the deep sleep overhead. Thus there would no reason to be obtain this value after waking from deep sleep - you'd always obtain it immediately after packet transmission, and before going back into receive mode and sleeping the MCU.

[cleishm]

I haven't tested this on hardware yet (I'm traveling this weekend) but I can do so on Monday.

Tested and confirmed that I can call ::begin on wake-from-deep-sleep with entirely different parameters than were used at power-on and, as long as resetModule=false, the packet that caused the wakeup can still be read from the buffer correctly. To be clear, I cannot think of a valid use-case to do this normally. It just confirms that it's not critical to keep parameters the same each time RadioLib is re-initialized after a deep sleep wakeup.