-
Notifications
You must be signed in to change notification settings - Fork 394
To support PowerSaving for all ESP32-based repeaters and NRF52-based repeaters. #1353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
To support PowerSaving for all ESP32-based repeaters and NRF52-based repeaters. #1353
Conversation
…es DIO0 to wakeup MCU instead of DIO1
fschrempf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IoTThinks Thanks for your work! As I have already posted a solution for the sleep implementation for NRF52 in #1238 before, would you mind rebasing your work onto the latest version of this PR?
| uint8_t sd_enabled; | ||
| sd_softdevice_is_enabled(&sd_enabled); // To set sd_enabled to 1 if the BLE stack is active. | ||
|
|
||
| if (!sd_enabled) { // BLE is off ~ No active OTA, safe to go to sleep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of checking for the softdevice, I would rather implement a generic mechanism to prevent the sleep which can be used in other cases too. See fe17894.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to have consistent approach for ESP32 and NRF52.
In ESP32, we don't have "enterSleep" and "preventSleep".
I think we need to keep thing simple and effective.
This code works not only for OTA, but also for repeaters with WiFi or BLE active due to any reasons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You seem to miss the point. The mentioned commit from my PR implements an easy and straight-forward way in the BaseBoard base class, that can be used by all board types. Your implementation is specific to ESP32 and can only be used from inside the board class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have same "sleep" methods for ESP32 and NRF52.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #1238 I floated the idea to use safeToSleep. This could easily be implemented in a generic manner by adding it to MeshCore.h and overriding it in ESP32 as well as NRF52 (or even specific variants if necessary ). Pseudo patch:
diff --git a/src/MeshCore.h b/src/MeshCore.h
index b52ac21..a0ea11b 100644
--- a/src/MeshCore.h
+++ b/src/MeshCore.h
@@ -59,6 +59,11 @@ public:
virtual void enterSleep(uint32_t secs) {
if (!prevent_sleep) sleep(secs);
}
+ virtual bool safeToSleep() { return true; }
virtual void preventSleep() { prevent_sleep = true; }
virtual uint32_t getGpio() { return 0; }
virtual void setGpio(uint32_t values) {}With your changes moving the DIO1 pin configuration from the header files into platform.io the safeToSleep method should probably only require implementation in the NRF52Board.h and ESP32Board.h. Pseudo patch:
diff --git a/src/helpers/NRF52Board.h b/src/helpers/NRF52Board.h
index f187242..fac7fc3 100644
--- a/src/helpers/NRF52Board.h
+++ b/src/helpers/NRF52Board.h
@@ -16,6 +16,11 @@ public:
virtual void reboot() override { NVIC_SystemReset(); }
virtual void sleep(uint32_t secs) override;
virtual void onRXInterrupt() override;
virtual bool safeToSleep() override { return digitalRead(P_LORA_DIO_1) == LOW; }
};|
To fix the memory leaking for SoftwareTimer, to perform internal testing and to let the community to do beta test for NRF52-based repeaters. |
I've already done that work in #1238. There is no reason to reimplement my suggestions. Please, just use my branch and add your commits on top. |
|
Let me and some friends in busy-traffic areas test to use a simpler but more robust implementation without SuspendLoop, ResumeLoop and not touching setFlag for NRF52 repeaters. |
|
I have pushed the change to implement power saving for NRF52 to use System-Idle On instead. |
fschrempf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you are somehow seeking to provide an alternative solution for #1238, can you at least mention in the description how your solution works and what benefits it has over the one in my PR? Thanks!
Also, can you please try to separate your changes logically into multiple commits and not chronologically stack each improvement as new commit on top of the previous one? As I mentioned before this is what git rebase --interactive and git push --force are made for.
I know that a lot of people do it like this, but this is not how git is intended to be used and it makes it impossible to have useful history in the end. And it leads you to copy code from my branch discarding the authorship (which can be seen as somewhat disrespectful among developers) instead of just doing git cherry-pick from my branch.
This is also something I wish the maintainers would take more into account when reviewing/merging PRs.
7b0546a to
c8e7727
Compare
c8e7727 to
67a4398
Compare
|
I have put in changes to implement light sleep for ESP32 without changing setFlag at all. |
To put back missing code board->onAfterTransmit();
9879bd4 to
f988eb3
Compare
fschrempf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you did a force-push, but the commits are still not separated properly. If you need help doing it, feel free to ask.
One more thing: You always begin your comments end commit subjects with "To ...". This doesn't really makes sense. Just leave out the "to" and state your changes in an imperative way.
| -D WIFI_DEBUG_LOGGING=1 | ||
| -D WIFI_SSID='"myssid"' | ||
| -D WIFI_PWD='"mypwd"' | ||
| -D OFFLINE_QUEUE_SIZE=256 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be removed, right?
| -D WIFI_SSID='"ssid"' | ||
| -D WIFI_PWD='"password"' | ||
| -D WIFI_DEBUG_LOGGING=1 | ||
| -D OFFLINE_QUEUE_SIZE=256 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be removed, right?
| } | ||
| // To configure GPIO wakeup | ||
| esp_sleep_enable_gpio_wakeup(); | ||
| gpio_wakeup_enable((gpio_num_t) wakeupPin, GPIO_INTR_HIGH_LEVEL); // To wake up when receiving a LoRa packet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the gpio_num_t cast. It's not needed.
| #if defined(ESP32) && defined(RADIO_SX1276) && defined(P_LORA_DIO_0) // SX1276 | ||
| gpio_set_intr_type((gpio_num_t)P_LORA_DIO_0, GPIO_INTR_POSEDGE); | ||
| #elif defined(ESP32) && defined(P_LORA_DIO_1) // SX1262 | ||
| gpio_set_intr_type((gpio_num_t)P_LORA_DIO_1, GPIO_INTR_POSEDGE); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole block can be replaced with a single line
| #if defined(ESP32) && defined(RADIO_SX1276) && defined(P_LORA_DIO_0) // SX1276 | |
| gpio_set_intr_type((gpio_num_t)P_LORA_DIO_0, GPIO_INTR_POSEDGE); | |
| #elif defined(ESP32) && defined(P_LORA_DIO_1) // SX1262 | |
| gpio_set_intr_type((gpio_num_t)P_LORA_DIO_1, GPIO_INTR_POSEDGE); | |
| #endif | |
| gpio_set_intr_type(wakeupPin, GPIO_INTR_POSEDGE); |
| gpio_set_intr_type((gpio_num_t)P_LORA_DIO_1, GPIO_INTR_POSEDGE); | ||
| #endif | ||
|
|
||
| // To disable CPU interrupt servicing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // To disable CPU interrupt servicing | |
| // To enable CPU interrupt servicing |
| #if defined(NRF52_PLATFORM) | ||
| #include "NRF52Board.h" | ||
|
|
||
| #include "nrf_sdm.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this, do we?
| uint8_t sd_enabled; | ||
| sd_softdevice_is_enabled(&sd_enabled); // To set sd_enabled to 1 if the BLE stack is active. | ||
|
|
||
| if (!sd_enabled) { // BLE is off ~ No active OTA, safe to go to sleep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
| uint32_t startTime = millis(); | ||
|
|
||
| // To wake up when a LoRa packet comes or sleep timeout. Safe to 49-day overflow | ||
| while (digitalRead(P_LORA_DIO_1) == LOW && (millis() - startTime < secs*1000)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the boards with DIO0 interrupt instead of DIO1? The digitalRead(P_LORA_DIO_1) would be incorrect in this case, right?
As mentioned above we could use a getIrqPin() function to retrieve the correct pin.
I did force-push to fix some simple missing codes. And clean PRs are usually approved faster. :D
Well, if you don't like then I will not include the "To..." |
I know, thanks. If FOSS developers work on the same topic and take over work from each other they usually credit by taking the commits with the original authorship.
Ok, good idea. I think we can do it like this. |
Ok, maybe. To me it sounds awkward and I've never seen this style elsewhere. But maybe that's just me... |
Ok, do as you like. |
|
@fschrempf Thanks a lot for your comprehensive reviews. Since, the code for lightsleep for esp32 and NRF52 have changed. Then I will make a final clean push. |
|
@IoTThinks , thanks for the work here :) I'll test this out on my RAK repeater. I was thinking if it would be good to show how much the repeater has slept on the repeater stats to get some insight. Could very well be added in some other PR of course. |
Hi @4np, No idea if we can change the repeater stats on our own. Or we need help from MeshCore app developer. CLI is surely possible. |
@fschrempf: Oh, I see. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a look at the changes and provided some -minor- feedback.
| uint8_t sd_enabled; | ||
| sd_softdevice_is_enabled(&sd_enabled); // To set sd_enabled to 1 if the BLE stack is active. | ||
|
|
||
| if (!sd_enabled) { // BLE is off ~ No active OTA, safe to go to sleep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In #1238 I floated the idea to use safeToSleep. This could easily be implemented in a generic manner by adding it to MeshCore.h and overriding it in ESP32 as well as NRF52 (or even specific variants if necessary ). Pseudo patch:
diff --git a/src/MeshCore.h b/src/MeshCore.h
index b52ac21..a0ea11b 100644
--- a/src/MeshCore.h
+++ b/src/MeshCore.h
@@ -59,6 +59,11 @@ public:
virtual void enterSleep(uint32_t secs) {
if (!prevent_sleep) sleep(secs);
}
+ virtual bool safeToSleep() { return true; }
virtual void preventSleep() { prevent_sleep = true; }
virtual uint32_t getGpio() { return 0; }
virtual void setGpio(uint32_t values) {}With your changes moving the DIO1 pin configuration from the header files into platform.io the safeToSleep method should probably only require implementation in the NRF52Board.h and ESP32Board.h. Pseudo patch:
diff --git a/src/helpers/NRF52Board.h b/src/helpers/NRF52Board.h
index f187242..fac7fc3 100644
--- a/src/helpers/NRF52Board.h
+++ b/src/helpers/NRF52Board.h
@@ -16,6 +16,11 @@ public:
virtual void reboot() override { NVIC_SystemReset(); }
virtual void sleep(uint32_t secs) override;
virtual void onRXInterrupt() override;
virtual bool safeToSleep() override { return digitalRead(P_LORA_DIO_1) == LOW; }
};|
|
||
| // MCU to enter light sleep | ||
| esp_light_sleep_start(); | ||
| Serial.println(esp_sleep_get_wakeup_cause()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it's better to use the MESH_DEBUG_PRINTLN function:
| Serial.println(esp_sleep_get_wakeup_cause()); | |
| MESH_DEBUG_PRINTLN(esp_sleep_get_wakeup_cause()); |
| virtual float getMCUTemperature() override; | ||
| virtual void reboot() override { NVIC_SystemReset(); } | ||
| virtual void enterLightSleep(uint32_t secs); | ||
| virtual void sleep(uint32_t secs) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need 2 functions, it looks like all can be implemented inside sleep()? In case we do want 2 functions, I don't think enterLightSleep() should have public visibility.
|
@4np Thanks a lot for review. I will fix for some feedbacks and will check how much changes if using safeToSleep. |
|
I just profiled this with my rusty old Nordic Power Profiler II. Looks good so far! Makes repeaters finally competitive to our Mesthastic colleagues. It reduced the idle consumption from ~26mA to most times sleeping with 7.5-8mA. Hardware: HT-CT62 (ESP32C3 + SX1262) <- 3.0V LDO <- TI BQ25185 <- Nordic Power Profiler II (3.7V) |
|
When implementing please keep in mind hat we also have the RAK3172 in the 'variants'. It's a ST STM32WLE55C based device. This one is still missing a power saving implementation - it's on my list of looking deeper into it but my schedule is a bit too tight right now. |
|
@h0lad Is RAK3172 repeater compilable now? Last time, it is not compilable when I try. Thanks for testing. |
|
@h0lad Are you able to compile for RAK3172?
|
|
@IoTThinks , Do not know how to make File target `mergebin'. Stop.$ FIRMWARE_VERSION="adhoc-$(date +"%Y%m%d%H%M")" sh build.sh build-firmware rak3x72_repeater
...
Building .pio/build/rak3x72_repeater/firmware.bin
========================================= [SUCCESS] Took 10.47 seconds =========================================
Environment Status Duration
---------------- -------- ------------
rak3x72_repeater SUCCESS 00:00:10.472
========================================== 1 succeeded in 00:00:10.472 ==========================================
Processing rak3x72_repeater (board: rak3172; platform: platformio/[email protected]; framework: arduino)
-----------------------------------------------------------------------------------------------------------------
Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/ststm32/rak3172.html
PLATFORM: ST STM32 (19.1.0) > BB-STM32WL
HARDWARE: STM32WLE5CCU 48MHz, 64KB RAM, 224KB Flash
DEBUG: Current (stlink) External (jlink, stlink)
PACKAGES:
- framework-arduinoststm32 @ 0.0.0
- framework-cmsis @ 2.50900.0 (5.9.0)
- toolchain-gccarmnoneeabi @ 1.120301.0 (12.3.1)
LDF: Library Dependency Finder -> https://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 25 compatible libraries
Scanning dependencies...
Dependency Graph
|-- SPI @ 1.1.0
|-- Wire @ 1.0.0
|-- RadioLib @ 7.5.0
|-- Crypto @ 0.4.0
|-- RTClib @ 2.1.4
|-- Melopero RV3028 @ 1.2.0
|-- CayenneLPP @ 1.6.1
|-- Adafruit Little File System Libraries @ 0.11.0
|-- Adafruit BusIO @ 1.17.2
|-- ed25519
|-- nrf52
Building in release mode
*** Do not know how to make File target `mergebin' (~/Developer/MeshCore/mergebin). Stop.
========================================== [FAILED] Took 1.13 seconds ==========================================
Environment Status Duration
---------------- -------- ------------
rak3x72_repeater FAILED 00:00:01.130
===================================== 1 failed, 0 succeeded in 00:00:01.130 =====================================Something else that came up in talks with other MeshCore users is that some have a GPS inside their repeaters and use the GPS for location and clock sync. In the PRs we may have overlooked this use case.
Or should power saving be unsupported altogether in the case a GPS is present? |
|
@4np Let me check the code to understand the behaviour of a GPS module during the current sleep. My guess now is MCU will sleep and GPS may not sleep. For repeaters, GPS may be mainly for time sync as the position is quite stationary. Btw, I still think we should remove time in repeaters. |
|
@4np This is RAK4631 repeater with GPS module from RAK. Although we may optimize code for GPS module to ... sleep more, I still think GPS module for repeaters is not neccessary as we actually do not need "time" for repeaters. However, let me check what is the behavior now.
|
|
I can compile the RAK3172 here (Opensuse Thumbleweed) without problems. Even build a energy harvester PCB for it - runs fine! Only the power consumption is a bit problematic: the STM32WLE55C sucks around 14mA while hanging in Idle without power saving. |
Love to see RAK3172 working. By right, RAK3172 can sleep very well as it is STM32. |
|
Ok, let me try the RAK3172. |





Hi all,
This PR is to add PowerSaving for all ESP32-based repeaters and NRF52-based repeaters including 4 boards with old SX1276.
Credits:
(12 Jan 2026) In latest approach, I use hardware and software events instead of SuspendLoop and ResumeLoop to avoid deadlock issue.
SuspendLoopandResumeLoopfrom Mr. Fschrempf and the check of HIGH-level DIO1 to prevent deadlock from Mr. 4np at NRF52 Repeater Powersaving. Thanks a lot.Known issue:
Testing:
I have tested the change with main branch and achieve 10mA for ESP32-based repeaters and 6mA for NRF52-based repeaters.
Some kind friends have tested boards with SX1276 working.
After the merge to dev, I have compiled 29 boards and confirm no compilation error. I have tested the merged code and it worked for Heltec v3, v4 and RAK4631.
Heltec v3 and RAK 4631 in PowerSaving mode

This is a RAK4631 repeater with PowerSaving in action: Sleep at 6mA, wakeup when a LoRa packet comes, process it, wake up for 5s and go to sleep again.
Power.Saving.for.RAK4631.in.action.mp4