Sleep modes can fail with no feedback to the user

[Eqqman]

Hello- The LowPower_stop() function relies on ST's HAL_PWR_EnterSTOPMode() function to enter sleep modes. Sleep mode is actually entered with the WFI ARM assembly instruction with has the following explanation from ARM: WFI (Wait For Interrupt) makes the processor suspend execution (Clock is stopped) until one of the following events take place:

• An IRQ interrupt
• An FIQ interrupt
• A Debug Entry request made to the processor

If one of these events is pending or happens to trigger at the exact time the processor executes WFI, program execution will continue in HAL_PWR_EnterSTOPMode() with no flags set that sleep mode wasn't actually entered. Most software I've seen written doesn't take into account the fact that entry into sleep mode could have actually failed, resulting in inconsistent behavior when using a debugger or lots of interrupts. To guarantee that the device does in fact sleep code should check the wake-up flag (defined as PWR_FLAG_WU in the ST libraries). ST's low-power functions recommend clearing this bit (it does start at 0 on POR) before invoking sleep modes, but I've yet to see any application code do so. A more robust implementation of this library's STM32LowPower::deepSleep() function would be as follows:

void STM32LowPower::deepSleep(uint32_t millis)
{
  if((millis > 0) || _rtc_wakeup) {
    programRtcWakeUp(millis, DEEP_SLEEP_MODE);
  }
  __HAL_PWR_CLEAR_FLAG(PWR_FLAG_WU);
   while ((PWR->CSR & PWR_FLAG_WU) == 0x00)  {
    LowPower_stop(_serial);
  }
}

For even more functionality a timeout value could be added so that the process could be aborted if the device is failing to sleep excessively. Then give a return value that the user can check to verify that the device woke up from sleep or sleep mode failed.

[fpistm]

Hi @Eqqman Thanks for this report. I guess, it would be better to add this in the core: https://github.com/stm32duino/Arduino_Core_STM32/blob/f5be50774e8236fb5a78c9a99d862cee2ab6df6f/libraries/SrcWrapper/src/stm32/low_power.c#L208

[ABOSTM]

Hi @Eqqman , I don't agree with your analysis: Calling deepSleep() is not a guaranty to go in stop mode, it will just try. If it fails then it means there is an event preventing Stop mode. It needs to be tackled (by application) before going to Stop mode. So there is no reason to wait in while loop until MCU really go to Stop. For example if you are able to wakeup with UART, and UART character is received at the same time you go to Stop mode, then MCU don't go to STOP. Instead the application can continue running and deal with the character received. Then application can decide to go to Stop mode if there is nothing else to do.

Also this Flag doesn't prevent MCU to go in Stop mode, it just inform that wakeup event occurs. So it may be useful to clear the flag, but in condition it is read by application to influence code execution. But there is no Arduino API to read the flag. So in this context, from my point of view, it is useless to clear the flag.

[Eqqman]

Hi @Eqqman , I don't agree with your analysis: Calling deepSleep() is not a guaranty to go in stop mode, it will just try.

Yes, this is correct. As I said, all example code I've reviewed seems to assume that just because this function was called, the device actually went to sleep. This means that this condition is not strictly a bug but will lead to bugs because the operation of the sleep function is non-intuitive and is likely to be misused. The changes I discussed will allow the function to guarantee sleep, but if this is not required then of course leave it as-is. But from a design point of view this seems like a poor decision since again, it leads to bad coding practices. Lots of folks using this function are likely to wonder why their batteries are draining out when they thought the device went to sleep. Personally, I don't think the application writer should have to go digging down into the library functions to see what is really happening, but I understand the 'buyer beware' viewpoint.

EDIT: another possibility is to add wrapper functions to the library so that users can call ones that either do or don't provide a sleep guarantee as their requirements determine.

EDIT: just seeing there is a different issue with the fix anyway- the Wakeup flag will only be set by the RTC alarm or an external pin operating in SystemWakeup mode but not EXTI mode, so as a general-purpose "did I really fall asleep" check it doesn't work by itself. As per ST's STM32L071 reference:

"Bit 0 WUF: Wakeup flag This bit is set by hardware and cleared by a system reset or by setting the CWUF bit in the PWR power control register (PWR_CR) 0: No wakeup event occurred 1: A wakeup event was received from the WKUP pin or from the RTC alarm (Alarm A or Alarm B), RTC Tamper event, RTC TimeStamp event or RTC Wakeup). Note: An additional wakeup event is detected if the WKUP pins are enabled"

[ABOSTM]

I really have a different point of view: If MCU didn't go to sleep, there is a good reason. And it would be a bad coding practice to just ignore this reason and force the sleep. On the contrary, the application should tackle this reason, in the same way it would tackle it if the wakeup occurs later.

Note: WKUP pins work through EXTI

[Eqqman]

Note: WKUP pins work through EXTI

Yes, if they are enabled as EXTI and the associated WKUP bit was not set.

EDIT: Might still work even with WKUP set, but depends if your external circuit will still work correctly with the WKUP internal pull-down enabled.

[Eqqman]

On the contrary, the application should tackle this reason, in the same way it would tackle it if the wakeup occurs later.

I would agree, which is partly why I was even fooling around with the Wakeup flag to begin with. I've noticed the Arduino libraries make fault tolerance difficult as even if the underlying low-level functions generate error messages, they don't get passed back up to the user. Most begin() functions return void so if things couldn't be started based on your settings, you wouldn't know.

[ABOSTM]

I consider the issue answered. Closed