NRF52 - power management improvements (#9211)

* minor NRF52 test cleanup

* detect USB power input on ProMicro boards

* prevent booting on power failure detection

* introduce PowerHAL layer

* powerHAL basic implementation for NRF52

* prevent data saves on low power

* remove comment

* Update src/platform/nrf52/main-nrf52.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/power/PowerHAL.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/main.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Merge missing voltage threshold comparison

* add missing variable

* add missing function declaration

* remove debug strings

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Ben Meadors <benmmeadors@gmail.com>

src/Power.cpp

@@ -1,11 +1,14 @@
 /**
  * @file Power.cpp
- * @brief This file contains the implementation of the Power class, which is responsible for managing power-related functionality
+ * @brief This file contains the implementation of the Power class, which is
- * of the device. It includes battery level sensing, power management unit (PMU) control, and power state machine management. The
+ * responsible for managing power-related functionality of the device. It
- * Power class is used by the main device class to manage power-related functionality.
+ * includes battery level sensing, power management unit (PMU) control, and
+ * power state machine management. The Power class is used by the main device
+ * class to manage power-related functionality.
  *
- * The file also includes implementations of various battery level sensors, such as the AnalogBatteryLevel class, which assumes
+ * The file also includes implementations of various battery level sensors, such
- * the battery voltage is attached via a voltage-divider to an analog input.
+ * as the AnalogBatteryLevel class, which assumes the battery voltage is
+ * attached via a voltage-divider to an analog input.
  *
  * This file is part of the Meshtastic project.
  * For more information, see: https://meshtastic.org/
@@ -19,6 +22,7 @@
 #include "configuration.h"
 #include "main.h"
 #include "meshUtils.h"
+#include "power/PowerHAL.h"
 #include "sleep.h"
 
 #if defined(ARCH_PORTDUINO)
@@ -171,22 +175,12 @@ Power *power;
 
 using namespace meshtastic;
 
-#ifndef AREF_VOLTAGE
+// NRF52 has AREF_VOLTAGE defined in architecture.h but
-#if defined(ARCH_NRF52)
+// make sure it's included. If something is wrong with NRF52
-/*
+// definition - compilation will fail on missing definition
- * Internal Reference is +/-0.6V, with an adjustable gain of 1/6, 1/5, 1/4,
+#if !defined(AREF_VOLTAGE) && !defined(ARCH_NRF52)
- * 1/3, 1/2 or 1, meaning 3.6, 3.0, 2.4, 1.8, 1.2 or 0.6V for the ADC levels.
- *
- * External Reference is VDD/4, with an adjustable gain of 1, 2 or 4, meaning
- * VDD/4, VDD/2 or VDD for the ADC levels.
- *
- * Default settings are internal reference with 1/6 gain (GND..3.6V ADC range)
- */
-#define AREF_VOLTAGE 3.6
-#else
 #define AREF_VOLTAGE 3.3
 #endif
-#endif
 
 /**
  * If this board has a battery level sensor, set this to a valid implementation
@@ -233,7 +227,8 @@ static void battery_adcDisable()
 #endif
 
 /**
- * A simple battery level sensor that assumes the battery voltage is attached via a voltage-divider to an analog input
+ * A simple battery level sensor that assumes the battery voltage is attached
+ * via a voltage-divider to an analog input
  */
 class AnalogBatteryLevel : public HasBatteryLevel
 {
@@ -311,7 +306,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
 
 #ifndef BATTERY_SENSE_SAMPLES
 #define BATTERY_SENSE_SAMPLES                                                                                                    \
-    15 // Set the number of samples, it has an effect of increasing sensitivity in complex electromagnetic environment.
+    15 // Set the number of samples, it has an effect of increasing sensitivity in
+       // complex electromagnetic environment.
 #endif
 
 #ifdef BATTERY_PIN
@@ -341,7 +337,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
             battery_adcDisable();
 
             if (!initial_read_done) {
-                // Flush the smoothing filter with an ADC reading, if the reading is plausibly correct
+                // Flush the smoothing filter with an ADC reading, if the reading is
+                // plausibly correct
                 if (scaled > last_read_value)
                     last_read_value = scaled;
                 initial_read_done = true;
@@ -350,8 +347,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
                 last_read_value += (scaled - last_read_value) * 0.5; // Virtual LPF
             }
 
-            // LOG_DEBUG("battery gpio %d raw val=%u scaled=%u filtered=%u", BATTERY_PIN, raw, (uint32_t)(scaled), (uint32_t)
+            // LOG_DEBUG("battery gpio %d raw val=%u scaled=%u filtered=%u",
-            // (last_read_value));
+            // BATTERY_PIN, raw, (uint32_t)(scaled), (uint32_t) (last_read_value));
         }
         return last_read_value;
 #endif // BATTERY_PIN
@@ -420,7 +417,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
     /**
      * return true if there is a battery installed in this unit
      */
-    // if we have a integrated device with a battery, we can assume that the battery is always connected
+    // if we have a integrated device with a battery, we can assume that the
+    // battery is always connected
 #ifdef BATTERY_IMMUTABLE
     virtual bool isBatteryConnect() override { return true; }
 #elif defined(ADC_V)
@@ -441,10 +439,10 @@ class AnalogBatteryLevel : public HasBatteryLevel
     virtual bool isBatteryConnect() override { return getBatteryPercent() != -1; }
 #endif
 
-    /// If we see a battery voltage higher than physics allows - assume charger is pumping
+    /// If we see a battery voltage higher than physics allows - assume charger is
-    /// in power
+    /// pumping in power On some boards we don't have the power management chip
-    /// On some boards we don't have the power management chip (like AXPxxxx)
+    /// (like AXPxxxx) so we use EXT_PWR_DETECT GPIO pin to detect external power
-    /// so we use EXT_PWR_DETECT GPIO pin to detect external power source
+    /// source
     virtual bool isVbusIn() override
     {
 #ifdef EXT_PWR_DETECT
@@ -461,8 +459,12 @@ class AnalogBatteryLevel : public HasBatteryLevel
         }
         // if it's not HIGH - check the battery
 #endif
-#elif defined(MUZI_BASE)
+
-        return NRF_POWER->USBREGSTATUS & POWER_USBREGSTATUS_VBUSDETECT_Msk;
+// technically speaking this should work for all(?) NRF52 boards
+// but needs testing across multiple devices. NRF52 USB would not even work if
+// VBUS was not properly connected and detected by the CPU
+#elif defined(MUZI_BASE) || defined(PROMICRO_DIY_TCXO)
+        return powerHAL_isVBUSConnected();
 #endif
         return getBattVoltage() > chargingVolt;
     }
@@ -485,8 +487,9 @@ class AnalogBatteryLevel : public HasBatteryLevel
 #else
 #if HAS_TELEMETRY && !MESHTASTIC_EXCLUDE_ENVIRONMENTAL_SENSOR && !defined(DISABLE_INA_CHARGING_DETECTION)
         if (hasINA()) {
-            // get current flow from INA sensor - negative value means power flowing into the battery
+            // get current flow from INA sensor - negative value means power flowing
-            // default assuming  BATTERY+  <--> INA_VIN+ <--> SHUNT RESISTOR <--> INA_VIN- <--> LOAD
+            // into the battery default assuming  BATTERY+  <--> INA_VIN+ <--> SHUNT
+            // RESISTOR <--> INA_VIN- <--> LOAD
             LOG_DEBUG("Using INA on I2C addr 0x%x for charging detection", config.power.device_battery_ina_address);
 #if defined(INA_CHARGING_DETECTION_INVERT)
             return getINACurrent() > 0;
@@ -502,8 +505,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
     }
 
   private:
-    /// If we see a battery voltage higher than physics allows - assume charger is pumping
+    /// If we see a battery voltage higher than physics allows - assume charger is
-    /// in power
+    /// pumping in power
 
     /// For heltecs with no battery connected, the measured voltage is 2204, so
     // need to be higher than that, in this case is 2500mV (3000-500)
@@ -512,7 +515,8 @@ class AnalogBatteryLevel : public HasBatteryLevel
     const float noBatVolt = (OCV[NUM_OCV_POINTS - 1] - 500) * NUM_CELLS;
     // Start value from minimum voltage for the filter to not start from 0
     // that could trigger some events.
-    // This value is over-written by the first ADC reading, it the voltage seems reasonable.
+    // This value is over-written by the first ADC reading, it the voltage seems
+    // reasonable.
     bool initial_read_done = false;
     float last_read_value = (OCV[NUM_OCV_POINTS - 1] * NUM_CELLS);
     uint32_t last_read_time_ms = 0;
@@ -654,7 +658,8 @@ bool Power::analogInit()
 #ifdef CONFIG_IDF_TARGET_ESP32S3
     // ESP32S3
     else if (val_type == ESP_ADC_CAL_VAL_EFUSE_TP_FIT) {
-        LOG_INFO("ADC config based on Two Point values and fitting curve coefficients stored in eFuse");
+        LOG_INFO("ADC config based on Two Point values and fitting curve "
+                 "coefficients stored in eFuse");
     }
 #endif
     else {
@@ -662,13 +667,7 @@ bool Power::analogInit()
     }
 #endif // ARCH_ESP32
 
-#ifdef ARCH_NRF52
+    // NRF52 ADC init moved to powerHAL_init in nrf52 platform
-#ifdef VBAT_AR_INTERNAL
-    analogReference(VBAT_AR_INTERNAL);
-#else
-    analogReference(AR_INTERNAL); // 3.6V
-#endif
-#endif // ARCH_NRF52
 
 #ifndef ARCH_ESP32
     analogReadResolution(BATTERY_SENSE_RESOLUTION_BITS);
@@ -779,7 +778,8 @@ void Power::reboot()
     HAL_NVIC_SystemReset();
 #else
     rebootAtMsec = -1;
-    LOG_WARN("FIXME implement reboot for this platform. Note that some settings require a restart to be applied");
+    LOG_WARN("FIXME implement reboot for this platform. Note that some settings "
+             "require a restart to be applied");
 #endif
 }
 
@@ -789,9 +789,12 @@ void Power::shutdown()
 #if HAS_SCREEN
     if (screen) {
 #ifdef T_DECK_PRO
-        screen->showSimpleBanner("Device is powered off.\nConnect USB to start!", 0); // T-Deck Pro has no power button
+        screen->showSimpleBanner("Device is powered off.\nConnect USB to start!",
+                                 0); // T-Deck Pro has no power button
 #elif defined(USE_EINK)
-        screen->showSimpleBanner("Shutting Down...", 2250); // dismiss after 3 seconds to avoid the banner on the sleep screen
+        screen->showSimpleBanner("Shutting Down...",
+                                 2250); // dismiss after 3 seconds to avoid the
+                                        // banner on the sleep screen
 #else
         screen->showSimpleBanner("Shutting Down...", 0); // stays on screen
 #endif
@@ -830,7 +833,8 @@ void Power::readPowerStatus()
     int32_t batteryVoltageMv = -1; // Assume unknown
     int8_t batteryChargePercent = -1;
     OptionalBool usbPowered = OptUnknown;
-    OptionalBool hasBattery = OptUnknown; // These must be static because NRF_APM code doesn't run every time
+    OptionalBool hasBattery = OptUnknown; // These must be static because NRF_APM
+                                          // code doesn't run every time
     OptionalBool isChargingNow = OptUnknown;
 
     if (batteryLevel) {
@@ -843,9 +847,10 @@ void Power::readPowerStatus()
             if (batteryLevel->getBatteryPercent() >= 0) {
                 batteryChargePercent = batteryLevel->getBatteryPercent();
             } else {
-                // If the AXP192 returns a percentage less than 0, the feature is either not supported or there is an error
+                // If the AXP192 returns a percentage less than 0, the feature is either
-                // In that case, we compute an estimate of the charge percent based on open circuit voltage table defined
+                // not supported or there is an error In that case, we compute an
-                // in power.h
+                // estimate of the charge percent based on open circuit voltage table
+                // defined in power.h
                 batteryChargePercent = clamp((int)(((batteryVoltageMv - (OCV[NUM_OCV_POINTS - 1] * NUM_CELLS)) * 1e2) /
                                                    ((OCV[0] * NUM_CELLS) - (OCV[NUM_OCV_POINTS - 1] * NUM_CELLS))),
                                              0, 100);
@@ -853,12 +858,12 @@ void Power::readPowerStatus()
         }
     }
 
-// FIXME: IMO we shouldn't be littering our code with all these ifdefs.  Way better instead to make a Nrf52IsUsbPowered subclass
+// FIXME: IMO we shouldn't be littering our code with all these ifdefs.  Way
-// (which shares a superclass with the BatteryLevel stuff)
+// better instead to make a Nrf52IsUsbPowered subclass (which shares a
-// that just provides a few methods.  But in the interest of fixing this bug I'm going to follow current
+// superclass with the BatteryLevel stuff) that just provides a few methods. But
-// practice.
+// in the interest of fixing this bug I'm going to follow current practice.
-#ifdef NRF_APM // Section of code detects USB power on the RAK4631 and updates the power states.  Takes 20 seconds or so to detect
+#ifdef NRF_APM // Section of code detects USB power on the RAK4631 and updates
-               // changes.
+               // the power states.  Takes 20 seconds or so to detect changes.
 
     nrfx_power_usb_state_t nrf_usb_state = nrfx_power_usbstatus_get();
     // LOG_DEBUG("NRF Power %d", nrf_usb_state);
@@ -932,8 +937,9 @@ void Power::readPowerStatus()
 
 #endif
 
-    // If we have a battery at all and it is less than 0%, force deep sleep if we have more than 10 low readings in
+    // If we have a battery at all and it is less than 0%, force deep sleep if we
-    // a row. NOTE: min LiIon/LiPo voltage is 2.0 to 2.5V, current OCV min is set to 3100 that is large enough.
+    // have more than 10 low readings in a row. NOTE: min LiIon/LiPo voltage
+    // is 2.0 to 2.5V, current OCV min is set to 3100 that is large enough.
     //
 
     if (batteryLevel && powerStatus2.getHasBattery() && !powerStatus2.getHasUSB()) {
@@ -955,8 +961,8 @@ int32_t Power::runOnce()
     readPowerStatus();
 
 #ifdef HAS_PMU
-    // WE no longer use the IRQ line to wake the CPU (due to false wakes from sleep), but we do poll
+    // WE no longer use the IRQ line to wake the CPU (due to false wakes from
-    // the IRQ status by reading the registers over I2C
+    // sleep), but we do poll the IRQ status by reading the registers over I2C
     if (PMU) {
 
         PMU->getIrqStatus();
@@ -998,7 +1004,8 @@ int32_t Power::runOnce()
         PMU->clearIrqStatus();
     }
 #endif
-    // Only read once every 20 seconds once the power status for the app has been initialized
+    // Only read once every 20 seconds once the power status for the app has been
+    // initialized
     return (statusHandler && statusHandler->isInitialized()) ? (1000 * 20) : RUN_SAME;
 }
 
@@ -1006,10 +1013,12 @@ int32_t Power::runOnce()
  * Init the power manager chip
  *
  * axp192 power
-    DCDC1 0.7-3.5V @ 1200mA max -> OLED // If you turn this off you'll lose comms to the axp192 because the OLED and the
+    DCDC1 0.7-3.5V @ 1200mA max -> OLED // If you turn this off you'll lose
- axp192 share the same i2c bus, instead use ssd1306 sleep mode DCDC2 -> unused DCDC3 0.7-3.5V @ 700mA max -> ESP32 (keep this
+ comms to the axp192 because the OLED and the axp192 share the same i2c bus,
- on!) LDO1 30mA -> charges GPS backup battery // charges the tiny J13 battery by the GPS to power the GPS ram (for a couple of
+ instead use ssd1306 sleep mode DCDC2 -> unused DCDC3 0.7-3.5V @ 700mA max ->
- days), can not be turned off LDO2 200mA -> LORA LDO3 200mA -> GPS
+ ESP32 (keep this on!) LDO1 30mA -> charges GPS backup battery // charges the
+ tiny J13 battery by the GPS to power the GPS ram (for a couple of days), can
+ not be turned off LDO2 200mA -> LORA LDO3 200mA -> GPS
  *
  */
 bool Power::axpChipInit()
@@ -1054,9 +1063,10 @@ bool Power::axpChipInit()
 
     if (!PMU) {
         /*
-         * In XPowersLib, if the XPowersAXPxxx object is released, Wire.end() will be called at the same time.
+         * In XPowersLib, if the XPowersAXPxxx object is released, Wire.end() will
-         * In order not to affect other devices, if the initialization of the PMU fails, Wire needs to be re-initialized once,
+         * be called at the same time. In order not to affect other devices, if the
-         * if there are multiple devices sharing the bus.
+         * initialization of the PMU fails, Wire needs to be re-initialized once, if
+         * there are multiple devices sharing the bus.
          * * */
 #ifndef PMU_USE_WIRE1
         w->begin(I2C_SDA, I2C_SCL);
@@ -1073,8 +1083,8 @@ bool Power::axpChipInit()
         PMU->enablePowerOutput(XPOWERS_LDO2);
 
         // oled module power channel,
-        // disable it will cause abnormal communication between boot and AXP power supply,
+        // disable it will cause abnormal communication between boot and AXP power
-        // do not turn it off
+        // supply, do not turn it off
         PMU->setPowerChannelVoltage(XPOWERS_DCDC1, 3300);
         // enable oled power
         PMU->enablePowerOutput(XPOWERS_DCDC1);
@@ -1101,7 +1111,8 @@ bool Power::axpChipInit()
         PMU->setChargeTargetVoltage(XPOWERS_AXP192_CHG_VOL_4V2);
     } else if (PMU->getChipModel() == XPOWERS_AXP2101) {
 
-        /*The alternative version of T-Beam 1.1 differs from T-Beam V1.1 in that it uses an AXP2101 power chip*/
+        /*The alternative version of T-Beam 1.1 differs from T-Beam V1.1 in that it
+         * uses an AXP2101 power chip*/
         if (HW_VENDOR == meshtastic_HardwareModel_TBEAM) {
             // Unuse power channel
             PMU->disablePowerOutput(XPOWERS_DCDC2);
@@ -1136,8 +1147,8 @@ bool Power::axpChipInit()
             // t-beam s3 core
             /**
              * gnss module power channel
-             * The default ALDO4 is off, you need to turn on the GNSS power first, otherwise it will be invalid during
+             * The default ALDO4 is off, you need to turn on the GNSS power first,
-             * initialization
+             * otherwise it will be invalid during initialization
              */
             PMU->setPowerChannelVoltage(XPOWERS_ALDO4, 3300);
             PMU->enablePowerOutput(XPOWERS_ALDO4);
@@ -1187,7 +1198,8 @@ bool Power::axpChipInit()
         // disable all axp chip interrupt
         PMU->disableIRQ(XPOWERS_AXP2101_ALL_IRQ);
 
-        // Set the constant current charging current of AXP2101, temporarily use 500mA by default
+        // Set the constant current charging current of AXP2101, temporarily use
+        // 500mA by default
         PMU->setChargerConstantCurr(XPOWERS_AXP2101_CHG_CUR_500MA);
 
         // Set up the charging voltage
@@ -1253,11 +1265,12 @@ bool Power::axpChipInit()
                   PMU->getPowerChannelVoltage(XPOWERS_BLDO2));
     }
 
-// We can safely ignore this approach for most (or all) boards because MCU turned off
+// We can safely ignore this approach for most (or all) boards because MCU
-// earlier than battery discharged to 2.6V.
+// turned off earlier than battery discharged to 2.6V.
 //
-// Unfortanly for now we can't use this killswitch for RAK4630-based boards because they have a bug with
+// Unfortunately for now we can't use this killswitch for RAK4630-based boards
-// battery voltage measurement. Probably it sometimes drops to low values.
+// because they have a bug with battery voltage measurement. Probably it
+// sometimes drops to low values.
 #ifndef RAK4630
     // Set PMU shutdown voltage at 2.6V to maximize battery utilization
     PMU->setSysPowerDownVoltage(2600);
@@ -1276,10 +1289,12 @@ bool Power::axpChipInit()
     attachInterrupt(
         PMU_IRQ, [] { pmu_irq = true; }, FALLING);
 
-    // we do not look for AXPXXX_CHARGING_FINISHED_IRQ & AXPXXX_CHARGING_IRQ because it occurs repeatedly while there is
+    // we do not look for AXPXXX_CHARGING_FINISHED_IRQ & AXPXXX_CHARGING_IRQ
-    // no battery also it could cause inadvertent waking from light sleep just because the battery filled
+    // because it occurs repeatedly while there is no battery also it could cause
-    // we don't look for AXPXXX_BATT_REMOVED_IRQ because it occurs repeatedly while no battery installed
+    // inadvertent waking from light sleep just because the battery filled we
-    // we don't look at AXPXXX_VBUS_REMOVED_IRQ because we don't have anything hooked to vbus
+    // don't look for AXPXXX_BATT_REMOVED_IRQ because it occurs repeatedly while
+    // no battery installed we don't look at AXPXXX_VBUS_REMOVED_IRQ because we
+    // don't have anything hooked to vbus
     PMU->enableIRQ(pmuIrqMask);
 
     PMU->clearIrqStatus();
@@ -1395,8 +1410,8 @@ class LipoCharger : public HasBatteryLevel
             bool result = PPM->init(Wire, I2C_SDA, I2C_SCL, BQ25896_ADDR);
             if (result) {
                 LOG_INFO("PPM BQ25896 init succeeded");
-                // Set the minimum operating voltage. Below this voltage, the PPM will protect
+                // Set the minimum operating voltage. Below this voltage, the PPM will
-                // PPM->setSysPowerDownVoltage(3100);
+                // protect PPM->setSysPowerDownVoltage(3100);
 
                 // Set input current limit, default is 500mA
                 // PPM->setInputCurrentLimit(800);
@@ -1419,7 +1434,8 @@ class LipoCharger : public HasBatteryLevel
                 PPM->enableMeasure();
 
                 // Turn on charging function
-                // If there is no battery connected, do not turn on the charging function
+                // If there is no battery connected, do not turn on the charging
+                // function
                 PPM->enableCharge();
             } else {
                 LOG_WARN("PPM BQ25896 init failed");
@@ -1454,7 +1470,8 @@ class LipoCharger : public HasBatteryLevel
     virtual int getBatteryPercent() override
     {
         return -1;
-        // return bq->getChargePercent(); // don't use BQ27220 for battery percent, it is not calibrated
+        // return bq->getChargePercent(); // don't use BQ27220 for battery percent,
+        // it is not calibrated
     }
 
     /**
@@ -1576,7 +1593,8 @@ bool Power::meshSolarInit()
 
 #else
 /**
- * The meshSolar battery level sensor is unavailable - default to AnalogBatteryLevel
+ * The meshSolar battery level sensor is unavailable - default to
+ * AnalogBatteryLevel
  */
 bool Power::meshSolarInit()
 {

src/main.cpp

@@ -10,6 +10,7 @@
 #include "ReliableRouter.h"
 #include "airtime.h"
 #include "buzz.h"
+#include "power/PowerHAL.h"
 
 #include "FSCommon.h"
 #include "Led.h"
@@ -332,6 +333,43 @@ __attribute__((weak, noinline)) bool loopCanSleep()
 void lateInitVariant() __attribute__((weak));
 void lateInitVariant() {}
 
+// NRF52 (and probably other platforms) can report when system is in power failure mode
+// (eg. too low battery voltage) and operating it is unsafe (data corruption, bootloops, etc).
+// For example NRF52 will prevent any flash writes in that case automatically
+// (but it causes issues we need to handle).
+// This detection is independent from whatever ADC or dividers used in Meshtastic
+// boards and is internal to chip.
+
+// we use powerHAL layer to get this info and delay booting until power level is safe
+
+// wait until power level is safe to continue booting (to avoid bootloops)
+// blink user led in 3 flashes sequence to indicate what is happening
+void waitUntilPowerLevelSafe()
+{
+
+#ifdef LED_PIN
+    pinMode(LED_PIN, OUTPUT);
+#endif
+
+    while (powerHAL_isPowerLevelSafe() == false) {
+
+#ifdef LED_PIN
+
+        // 3x: blink for 300 ms, pause for 300 ms
+
+        for (int i = 0; i < 3; i++) {
+            digitalWrite(LED_PIN, LED_STATE_ON);
+            delay(300);
+            digitalWrite(LED_PIN, LED_STATE_OFF);
+            delay(300);
+        }
+#endif
+
+        // sleep for 2s
+        delay(2000);
+    }
+}
+
 /**
  * Print info as a structured log message (for automated log processing)
  */
@@ -342,6 +380,14 @@ void printInfo()
 #ifndef PIO_UNIT_TESTING
 void setup()
 {
+
+    // initialize power HAL layer as early as possible
+    powerHAL_init();
+
+    // prevent booting if device is in power failure mode
+    // boot sequence will follow when battery level raises to safe mode
+    waitUntilPowerLevelSafe();
+
 #if defined(R1_NEO)
     pinMode(DCDC_EN_HOLD, OUTPUT);
     digitalWrite(DCDC_EN_HOLD, HIGH);

src/mesh/NodeDB.cpp

@@ -26,6 +26,7 @@
 #include <algorithm>
 #include <pb_decode.h>
 #include <pb_encode.h>
+#include <power/PowerHAL.h>
 #include <vector>
 
 #ifdef ARCH_ESP32
@@ -1418,6 +1419,14 @@ void NodeDB::loadFromDisk()
 bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_t *fields, const void *dest_struct,
                        bool fullAtomic)
 {
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway. Device should be sleeping at this point anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveProto() on unsafe device power level.");
+        return false;
+    }
+
     bool okay = false;
 #ifdef FSCom
     auto f = SafeFile(filename, fullAtomic);
@@ -1444,6 +1453,14 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_
 
 bool NodeDB::saveChannelsToDisk()
 {
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveChannelsToDisk() on unsafe device power level.");
+        return false;
+    }
+
 #ifdef FSCom
     spiLock->lock();
     FSCom.mkdir("/prefs");
@@ -1454,6 +1471,14 @@ bool NodeDB::saveChannelsToDisk()
 
 bool NodeDB::saveDeviceStateToDisk()
 {
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway. Device should be sleeping at this point anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveDeviceStateToDisk() on unsafe device power level.");
+        return false;
+    }
+
 #ifdef FSCom
     spiLock->lock();
     FSCom.mkdir("/prefs");
@@ -1466,6 +1491,14 @@ bool NodeDB::saveDeviceStateToDisk()
 
 bool NodeDB::saveNodeDatabaseToDisk()
 {
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway. Device should be sleeping at this point anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveNodeDatabaseToDisk() on unsafe device power level.");
+        return false;
+    }
+
 #ifdef FSCom
     spiLock->lock();
     FSCom.mkdir("/prefs");
@@ -1478,6 +1511,14 @@ bool NodeDB::saveNodeDatabaseToDisk()
 
 bool NodeDB::saveToDiskNoRetry(int saveWhat)
 {
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway. Device should be sleeping at this point anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveToDiskNoRetry() on unsafe device power level.");
+        return false;
+    }
+
     bool success = true;
 #ifdef FSCom
     spiLock->lock();
@@ -1533,6 +1574,14 @@ bool NodeDB::saveToDiskNoRetry(int saveWhat)
 bool NodeDB::saveToDisk(int saveWhat)
 {
     LOG_DEBUG("Save to disk %d", saveWhat);
+
+    // do not try to save anything if power level is not safe. In many cases flash will be lock-protected
+    // and all writes will fail anyway. Device should be sleeping at this point anyway.
+    if (!powerHAL_isPowerLevelSafe()) {
+        LOG_ERROR("Error: trying to saveToDisk() on unsafe device power level.");
+        return false;
+    }
+
     bool success = saveToDiskNoRetry(saveWhat);
 
     if (!success) {

src/platform/nrf52/architecture.h

@@ -5,6 +5,25 @@
 //
 // defaults for NRF52 architecture
 //
+
+/*
+ * Internal Reference is +/-0.6V, with an adjustable gain of 1/6, 1/5, 1/4,
+ * 1/3, 1/2 or 1, meaning 3.6, 3.0, 2.4, 1.8, 1.2 or 0.6V for the ADC levels.
+ *
+ * External Reference is VDD/4, with an adjustable gain of 1, 2 or 4, meaning
+ * VDD/4, VDD/2 or VDD for the ADC levels.
+ *
+ * Default settings are internal reference with 1/6 gain (GND..3.6V ADC range)
+ * Some variants overwrite it.
+ */
+#ifndef AREF_VOLTAGE
+#define AREF_VOLTAGE 3.6
+#endif
+
+#ifndef BATTERY_SENSE_RESOLUTION_BITS
+#define BATTERY_SENSE_RESOLUTION_BITS 10
+#endif
+
 #ifndef HAS_BLUETOOTH
 #define HAS_BLUETOOTH 1
 #endif

src/platform/nrf52/main-nrf52.cpp

@@ -9,12 +9,12 @@
 #define NRFX_WDT_ENABLED 1
 #define NRFX_WDT0_ENABLED 1
 #define NRFX_WDT_CONFIG_NO_IRQ 1
-#include <nrfx_wdt.c>
+#include "nrfx_power.h"
-#include <nrfx_wdt.h>
-
 #include <assert.h>
 #include <ble_gap.h>
 #include <memory.h>
+#include <nrfx_wdt.c>
+#include <nrfx_wdt.h>
 #include <stdio.h>
 // #include <Adafruit_USBD_Device.h>
 #include "NodeDB.h"
@@ -23,6 +23,7 @@
 #include "main.h"
 #include "meshUtils.h"
 #include "power.h"
+#include <power/PowerHAL.h>
 
 #include <hal/nrf_lpcomp.h>
 
@@ -30,6 +31,21 @@
 #include "BQ25713.h"
 #endif
 
+// WARNING! THRESHOLD + HYSTERESIS should be less than regulated VDD voltage - which depends on board
+// and is 3.0 or 3.3V. Also VDD likes to read values like 2.9999 so make sure you account for that
+// otherwise board will not boot at all. Before you modify this part - please triple read NRF52840 power design
+// section in datasheet and you understand how REG0 and REG1 regulators work together.
+#ifndef SAFE_VDD_VOLTAGE_THRESHOLD
+#define SAFE_VDD_VOLTAGE_THRESHOLD 2.7
+#endif
+
+// hysteresis value
+#ifndef SAFE_VDD_VOLTAGE_THRESHOLD_HYST
+#define SAFE_VDD_VOLTAGE_THRESHOLD_HYST 0.2
+#endif
+
+uint16_t getVDDVoltage();
+
 // Weak empty variant initialization function.
 // May be redefined by variant files.
 void variant_shutdown() __attribute__((weak));
@@ -38,12 +54,95 @@ void variant_shutdown() {}
 static nrfx_wdt_t nrfx_wdt = NRFX_WDT_INSTANCE(0);
 static nrfx_wdt_channel_id nrfx_wdt_channel_id_nrf52_main;
 
+// This is a public global so that the debugger can set it to false automatically from our gdbinit
+// @phaseloop comment: most part of codebase, including filesystem flash driver depend on softdevice
+// methods so disabling it may actually crash thing. Proceed with caution.
+
+bool useSoftDevice = true; // Set to false for easier debugging
+
 static inline void debugger_break(void)
 {
     __asm volatile("bkpt #0x01\n\t"
                    "mov pc, lr\n\t");
 }
 
+// PowerHAL NRF52 specific function implementations
+bool powerHAL_isVBUSConnected()
+{
+    return NRF_POWER->USBREGSTATUS & POWER_USBREGSTATUS_VBUSDETECT_Msk;
+}
+
+bool powerHAL_isPowerLevelSafe()
+{
+
+    static bool powerLevelSafe = true;
+
+    uint16_t threshold = SAFE_VDD_VOLTAGE_THRESHOLD * 1000; // convert V to mV
+    uint16_t hysteresis = SAFE_VDD_VOLTAGE_THRESHOLD_HYST * 1000;
+
+    if (powerLevelSafe) {
+        if (getVDDVoltage() < threshold) {
+            powerLevelSafe = false;
+        }
+    } else {
+        // power level is only safe again when it raises above threshold + hysteresis
+        if (getVDDVoltage() >= (threshold + hysteresis)) {
+            powerLevelSafe = true;
+        }
+    }
+
+    return powerLevelSafe;
+}
+
+void powerHAL_platformInit()
+{
+
+    // Enable POF power failure comparator. It will prevent writing to NVMC flash when supply voltage is too low.
+    // Set to some low value as last resort - powerHAL_isPowerLevelSafe uses different method and should manage proper node
+    // behaviour on its own.
+
+    // POFWARN is pretty useless for node power management because it triggers only once and clearing this event will not
+    // re-trigger it again until voltage rises to safe level and drops again. So we will use SAADC routed to VDD to read safely
+    // voltage.
+
+    // @phaseloop: I disable POFCON for now because it seems to be unreliable or buggy. Even when set at 2.0V it
+    // triggers below 2.8V and corrupts data when pairing bluetooth - because it prevents filesystem writes and
+    // adafruit BLE library triggers lfs_assert which reboots node and formats filesystem.
+    // I did experiments with bench power supply and no matter what is set to POFCON, it always triggers right below
+    // 2.8V. I compared raw registry values with datasheet.
+
+    NRF_POWER->POFCON =
+        ((POWER_POFCON_THRESHOLD_V22 << POWER_POFCON_THRESHOLD_Pos) | (POWER_POFCON_POF_Enabled << POWER_POFCON_POF_Pos));
+
+    // remember to always match VBAT_AR_INTERNAL with AREF_VALUE in variant definition file
+#ifdef VBAT_AR_INTERNAL
+    analogReference(VBAT_AR_INTERNAL);
+#else
+    analogReference(AR_INTERNAL); // 3.6V
+#endif
+}
+
+// get VDD voltage (in millivolts)
+uint16_t getVDDVoltage()
+{
+    // we use the same values as regular battery read so there is no conflict on SAADC
+    analogReadResolution(BATTERY_SENSE_RESOLUTION_BITS);
+
+    // VDD range on NRF52840 is 1.8-3.3V so we need to remap analog reference to 3.6V
+    // let's hope battery reading runs in same task and we don't have race condition
+    analogReference(AR_INTERNAL);
+
+    uint16_t vddADCRead = analogReadVDD();
+    float voltage = ((1000 * 3.6) / pow(2, BATTERY_SENSE_RESOLUTION_BITS)) * vddADCRead;
+
+// restore default battery reading reference
+#ifdef VBAT_AR_INTERNAL
+    analogReference(VBAT_AR_INTERNAL);
+#endif
+
+    return voltage;
+}
+
 bool loopCanSleep()
 {
     // turn off sleep only while connected via USB
@@ -72,22 +171,6 @@ void getMacAddr(uint8_t *dmac)
     dmac[0] = src[5] | 0xc0; // MSB high two bits get set elsewhere in the bluetooth stack
 }
 
-static void initBrownout()
-{
-    auto vccthresh = POWER_POFCON_THRESHOLD_V24;
-
-    auto err_code = sd_power_pof_enable(POWER_POFCON_POF_Enabled);
-    assert(err_code == NRF_SUCCESS);
-
-    err_code = sd_power_pof_threshold_set(vccthresh);
-    assert(err_code == NRF_SUCCESS);
-
-    // We don't bother with setting up brownout if soft device is disabled - because during production we always use softdevice
-}
-
-// This is a public global so that the debugger can set it to false automatically from our gdbinit
-bool useSoftDevice = true; // Set to false for easier debugging
-
 #if !MESHTASTIC_EXCLUDE_BLUETOOTH
 void setBluetoothEnable(bool enable)
 {
@@ -106,7 +189,6 @@ void setBluetoothEnable(bool enable)
         if (!initialized) {
             nrf52Bluetooth = new NRF52Bluetooth();
             nrf52Bluetooth->startDisabled();
-            initBrownout();
             initialized = true;
         }
         return;
@@ -120,9 +202,6 @@ void setBluetoothEnable(bool enable)
             LOG_DEBUG("Init NRF52 Bluetooth");
             nrf52Bluetooth = new NRF52Bluetooth();
             nrf52Bluetooth->setup();
-
-            // We delay brownout init until after BLE because BLE starts soft device
-            initBrownout();
         }
         // Already setup, apparently
         else
@@ -192,9 +271,24 @@ extern "C" void lfs_assert(const char *reason)
     delay(500); // Give the serial port a bit of time to output that last message.
     // Try setting GPREGRET with the SoftDevice first. If that fails (perhaps because the SD hasn't been initialize yet) then set
     // NRF_POWER->GPREGRET directly.
-    if (!(sd_power_gpregret_clr(0, 0xFF) == NRF_SUCCESS && sd_power_gpregret_set(0, NRF52_MAGIC_LFS_IS_CORRUPT) == NRF_SUCCESS)) {
+
-        NRF_POWER->GPREGRET = NRF52_MAGIC_LFS_IS_CORRUPT;
+    // TODO: this will/can crash CPU if bluetooth stack is not compiled in or bluetooth is not initialized
+    // (regardless if enabled or disabled) - as there is no live SoftDevice stack
+    // implement "safe" functions detecting softdevice stack state and using proper method to set registers
+
+    // do not set GPREGRET if POFWARN is triggered because it means lfs_assert reports flash undervoltage protection
+    // and not data corruption. Reboot is fine as boot procedure will wait until power level is safe again
+
+    if (!NRF_POWER->EVENTS_POFWARN) {
+        if (!(sd_power_gpregret_clr(0, 0xFF) == NRF_SUCCESS &&
+              sd_power_gpregret_set(0, NRF52_MAGIC_LFS_IS_CORRUPT) == NRF_SUCCESS)) {
+            NRF_POWER->GPREGRET = NRF52_MAGIC_LFS_IS_CORRUPT;
+        }
     }
+
+    // TODO: this should not be done when SoftDevice is enabled as device will not boot back on soft reset
+    // as some data is retained in RAM which will prevent re-enabling bluetooth stack
+    // Google what Nordic has to say about NVIC_* + SoftDevice
     NVIC_SystemReset();
 }
 

src/power/PowerHAL.cpp

@@ -0,0 +1,19 @@
+
+#include "PowerHAL.h"
+
+void powerHAL_init()
+{
+    return powerHAL_platformInit();
+}
+
+__attribute__((weak, noinline)) void powerHAL_platformInit() {}
+
+__attribute__((weak, noinline)) bool powerHAL_isPowerLevelSafe()
+{
+    return true;
+}
+
+__attribute__((weak, noinline)) bool powerHAL_isVBUSConnected()
+{
+    return false;
+}

src/power/PowerHAL.h

@@ -0,0 +1,26 @@
+
+/*
+
+Power Hardware Abstraction Layer. Set of API calls to offload power management, measurements, reboots, etc
+to the platform and variant code to avoid #ifdef spaghetti hell and limitless device-based edge cases
+in the main firmware code
+
+Functions declared here (with exception of powerHAL_init) should be defined in platform specific codebase.
+Default function body does usually nothing.
+
+*/
+
+// Initialize HAL layer. Call it as early as possible during device boot
+// do not overwrite it as it's not declared with "weak" attribute.
+void powerHAL_init();
+
+// platform specific init code if needed to be run early on boot
+void powerHAL_platformInit();
+
+// Return true if current battery level is safe for device operation (for example flash writes).
+// This should be reported by power failure comparator (NRF52) or similar circuits on other platforms.
+// Do not use battery ADC as improper ADC configuration may prevent device from booting.
+bool powerHAL_isPowerLevelSafe();
+
+// return if USB voltage is connected
+bool powerHAL_isVBUSConnected();