Proposal: Introduce shutdownPeripherals in NRF52Board to prepare for shutdown

[fdlamotte]

Following #2895 which solved poweroff issues from repeater commandline on some variants (T-Echo is one of them) I wondered about the construct where poweroff in the variant was called before shutting down the specific peripherals of the boards

Obviously this specific shutdown code was never reached and I thing this was placed after the call to shutdown because it's mainly peripheral gating in there and it cuts power from some peripherals that are shut down in the poweroff function. In this case, the gates should be closed after the peripherals are correctly down

My idea is to add a shutdownPeripherals function, ideally the variants should overload this one instead of poweroff which calls it. This way one can let generic shutdown happen and then do specific shutdown to a board before CPU shutdown.

@ripplebiz @IoTThinks what do you think ?

shutdownPeripherals could even be promoted to MainBoard I think but I just wanted to illustrate the idea and keep the PR simple

[IoTThinks]

@fdlamotte It is fine for me as long as powerOff() will call shutdownPeripherals().
The tricky part is the order of the powering off of EN pin, MCUs and components.

In some boards, we have to shutdown the GPS, LoRa PA by sleep code before we set the EN to LOW (or something like that).
For example, for T-echo and T-echo Lite, we need to put NRF52Board::powerOff(); before the PIN_PWR_EN.
Else, the GPS may not sleep at uA as it can be powered on lightly by TX, RX pin.

  void powerOff() override {
    NRF52Board::powerOff();
    ...
    #ifdef PIN_PWR_EN
    digitalWrite(PIN_PWR_EN, LOW);
    #endif
  }

With shutdownPeripherals, it will become

  void powerOff() override {
    NRF52Board::powerOff(); => Already turn off major / common LoRa, OLED and GPS
    ...
   shutdownPeripherals(); => This may be for LEDs, Buzzers, sensor powers.
  }

Yes, I think shutdownPeripherals() is very helpful if some friends may want to off all LEDs, GPS... but still keep the board powered on to save power.
On T-echo Lite, I personally have to turn off the often-on Blue LED to ease my eyes.

[fdlamotte]

That's the idea @IoTThinks, but the way you made things didn't work because once NRF52Board::powerOff is called there is no instruction executed after as the MCU is stopped

So with what you did (and yes it was necessary to shut down peripherals correctly before closing the gates) the gates were not closed because the instructions to close them were not executed.

That's why I've introduced shutdownPeripherals, to permit the variant to do their own stuff (as closing gates such as PIN_PWR_EN on techo) before the mcu is stopped

[fdlamotte]

With shutdownPeripherals, it will become

  void powerOff() override {
    NRF52Board::powerOff(); => Already turn off major / common LoRa, OLED and GPS
    ...
   shutdownPeripherals(); => This may be for LEDs, Buzzers, sensor powers.
  }

you can't call powerOff before shutdownPeripherals ... that's why NRF52Board::shutdownPeripherals now holds the shutdown part from NRF52Board::powerOff, and powerOff now calls shutdownPeripherals before shutting down the MCU

The idea is to let the variant write their own shutdownPeripherals (and call NRF52Board::shutdownPeripherals when they want, and let the actual shutdown of the MCU take place at the end

[IoTThinks]

I see the bug now.
No wonder T-echo Lite shutdown at 1mA.

yes, then shutdownPeripherals() is a good idea.
You want to separate shutdownPeripherals() and powerOff()?

I still like the idea to off everything just by 1 command _board.powerOff().
How about this?
So we don't need to care about common components (OLED, GPS, LoRa) on new variants.
We only care about minor components (LEDs, buzzer...) on new variants.

void powerOff() {
turnOffDisplay...
turnOffGPS...
turnOffLoRa....

shutdownPeripherals() <==== This can be overriden by variants to put less code on new variants.
turnOffMCU...
}

[fdlamotte]

in this PR, shutdownPeripheral is called by poweroff, so from the application point of view you only have to do powerOff

shutdownPeripherals is just a stub to put the shutdown code apart of powering off the cpu

[IoTThinks]

This PR is great for me.
We may have another round to gradually put stuffs into shutdownPeripherial.
May be to fix T-echo and T-echo lite first.