fix(wifi): one power-save mapping for CLI, startup and reconnect

`set wifi.powersave min` stored 0 and applied WIFI_PS_MIN_MODEM, but the
bridge's post-reconnect mapping read the same 0 as WIFI_PS_NONE. A node
explicitly configured for minimum modem sleep therefore ran with power save off
after its first reconnect, while `get wifi.powersave` still reported min. The
stored default is already 1 (`none`), so nothing here changes the product
default — it stops an operator's explicit choice from being reinterpreted.

Move the value/name/mode table into WifiPowerSavePolicy (pure, host-tested) and
use it from the CLI setter and getter, the web config snapshot and the bridge.
The bridge now also applies the mode when it finds the STA already associated at
start — that path has no connect transition to carry the setting, so a bridge
restart used to leave whatever mode was set before.
This commit is contained in:
agessaman
2026-09-09 14:44:14 -07:00
parent d8ffae3230
commit dfdcc25b50
6 changed files with 163 additions and 34 deletions
+8 -22
View File
@@ -15,6 +15,7 @@
#include "TxtDataHelpers.h"
#include "AlertReporter.h" // for alertReporterBannedChannelMatch[Hex]()
#include "MQTTObserverValidation.h" // pure input validators (host-testable)
#include "WifiPowerSavePolicy.h" // one powersave value->mode/name mapping
#include <Utils.h>
#include <new>
#ifdef ESP_PLATFORM
@@ -438,41 +439,28 @@ bool CommonCLI::handleObserverSetCmd(uint32_t sender_timestamp, const char* conf
if (!persistObserverPrefs(reply)) return true;
strcpy(reply, "OK");
} else if (memcmp(config, "wifi.powersave ", 15) == 0) {
const char* value = &config[15];
uint8_t ps_value;
bool valid = false;
if (memcmp(value, "min", 3) == 0 && (value[3] == 0 || value[3] == ' ')) {
ps_value = 0;
valid = true;
} else if (memcmp(value, "none", 4) == 0 && (value[4] == 0 || value[4] == ' ')) {
ps_value = 1;
valid = true;
} else if (memcmp(value, "max", 3) == 0 && (value[3] == 0 || value[3] == ' ')) {
ps_value = 2;
valid = true;
}
if (!valid) {
if (!WifiPowerSavePolicy::parseName(&config[15], &ps_value)) {
strcpy(reply, "Error: must be none, min, or max");
} else {
_mqtt_prefs.wifi_power_save = ps_value;
if (!persistObserverPrefs(reply)) return true;
const char* ps_name = WifiPowerSavePolicy::nameFor(ps_value);
#ifdef ESP_PLATFORM
if (WiFi.status() == WL_CONNECTED) {
wifi_ps_type_t ps_mode = (ps_value == 1) ? WIFI_PS_NONE :
(ps_value == 2) ? WIFI_PS_MAX_MODEM : WIFI_PS_MIN_MODEM;
esp_err_t ps_result = esp_wifi_set_ps(ps_mode);
// Same mapping the bridge applies on every association, so this cannot
// drift back apart (see WifiPowerSavePolicy).
esp_err_t ps_result =
esp_wifi_set_ps((wifi_ps_type_t)WifiPowerSavePolicy::modeFor(ps_value));
if (ps_result == ESP_OK) {
const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
sprintf(reply, "OK - power save set to %s", ps_name);
} else {
sprintf(reply, "OK - saved, but failed to apply: %d", ps_result);
}
} else {
const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
sprintf(reply, "OK - saved as %s (will apply on next WiFi connection)", ps_name);
}
#else
const char* ps_name = (ps_value == 1) ? "none" : (ps_value == 2) ? "max" : "min";
sprintf(reply, "OK - saved as %s", ps_name);
#endif
}
@@ -1090,9 +1078,7 @@ bool CommonCLI::handleObserverGetCmd(uint32_t sender_timestamp, const char* conf
#endif
}
} else if (memcmp(config, "wifi.powersave", 14) == 0) {
uint8_t ps = _mqtt_prefs.wifi_power_save;
const char* ps_name = (ps == 1) ? "none" : (ps == 2) ? "max" : "min";
sprintf(reply, "> %s", ps_name);
sprintf(reply, "> %s", WifiPowerSavePolicy::nameFor(_mqtt_prefs.wifi_power_save));
} else if (memcmp(config, "timezone.offset", 15) == 0) {
// Must precede the "timezone" (8-byte) check below — that prefix-matches
// "timezone.offset" too, so the more-specific key has to come first or
+73
View File
@@ -0,0 +1,73 @@
#pragma once
#include <stdint.h>
#include <string.h>
// The single mapping between the stored `wifi.powersave` preference, its CLI
// name, and the IDF power-save mode.
//
// Pure lookup so host tests can hold startup, reconnect, CLI and web config to
// one table: they used to map the same stored value differently, so a node set
// to `min` silently ran with power save off after its first reconnect while
// `get wifi.powersave` still said min.
//
// Stored values are fleet state — never renumber them. The product default is
// `none`, which is a *default* (MQTTDefaults.h), not a reinterpretation of an
// operator's explicit `min`.
namespace WifiPowerSavePolicy {
enum StoredValue : uint8_t {
kMin = 0, // WIFI_PS_MIN_MODEM
kNone = 1, // WIFI_PS_NONE (default)
kMax = 2, // WIFI_PS_MAX_MODEM
};
// Mirrors wifi_ps_type_t. MQTTBridge.cpp static_asserts these against the SDK.
enum Mode : uint8_t {
kModeNone = 0,
kModeMinModem = 1,
kModeMaxModem = 2,
};
// Anything outside the known range reads as the default rather than as the
// lowest-numbered mode, so a corrupt byte cannot silently enable modem sleep.
static inline Mode modeFor(uint8_t stored) {
switch (stored) {
case kMin: return kModeMinModem;
case kMax: return kModeMaxModem;
case kNone: return kModeNone;
default: return kModeNone;
}
}
static inline const char* nameFor(uint8_t stored) {
switch (stored) {
case kMin: return "min";
case kMax: return "max";
case kNone: return "none";
default: return "none";
}
}
// Parses a CLI argument, which may be followed by trailing text (the observer
// setters take the rest of the command line). Returns false and leaves *out
// untouched for anything else.
static inline bool parseName(const char* value, uint8_t* out) {
if (value == nullptr || out == nullptr) return false;
static const struct { const char* name; uint8_t stored; } kNames[] = {
{ "min", kMin },
{ "none", kNone },
{ "max", kMax },
};
for (unsigned i = 0; i < sizeof(kNames) / sizeof(kNames[0]); i++) {
const size_t len = strlen(kNames[i].name);
if (strncmp(value, kNames[i].name, len) == 0 &&
(value[len] == '\0' || value[len] == ' ')) {
*out = kNames[i].stored;
return true;
}
}
return false;
}
} // namespace WifiPowerSavePolicy
+21 -10
View File
@@ -1,4 +1,5 @@
#include "MQTTBridge.h"
#include "../WifiPowerSavePolicy.h"
#include "../MQTTConnectionPolicy.h"
#include "../MQTTMessageBuilder.h"
#include "../MQTTPacketQueuePolicy.h"
@@ -427,6 +428,19 @@ int MQTTBridge::getMaxActiveSlots() {
#endif
}
// One mapping for startup, reconnect and CLI (see WifiPowerSavePolicy). The
// stored default is `none`; `min` means MIN_MODEM here exactly as the CLI says
// it does.
void MQTTBridge::applyWifiPowerSave() {
#ifdef ESP_PLATFORM
static_assert((int)WifiPowerSavePolicy::kModeNone == (int)WIFI_PS_NONE, "wifi_ps_type_t drift");
static_assert((int)WifiPowerSavePolicy::kModeMinModem == (int)WIFI_PS_MIN_MODEM, "wifi_ps_type_t drift");
static_assert((int)WifiPowerSavePolicy::kModeMaxModem == (int)WIFI_PS_MAX_MODEM, "wifi_ps_type_t drift");
if (!_obs) return;
esp_wifi_set_ps((wifi_ps_type_t)WifiPowerSavePolicy::modeFor(_obs->wifi_power_save));
#endif
}
uint8_t MQTTBridge::getLastWifiDisconnectReason() { return s_wifi_disconnect_reason; }
unsigned long MQTTBridge::getLastWifiDisconnectTime() { return s_wifi_disconnect_time; }
@@ -2804,6 +2818,12 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) {
_wifi_status_initialized = true;
setWifiOutage(AlertFaultPolicy::applyWifiStatus(
(uint32_t)now, current_wifi_status == WL_CONNECTED, wifiOutage(), false));
#ifdef ESP_PLATFORM
// Already associated at bridge start (end()/begin() leaves STA up): there is
// no connect transition below to carry the setting, so apply it here or the
// node runs on whatever the previous mode was.
if (current_wifi_status == WL_CONNECTED) applyWifiPowerSave();
#endif
}
if (now - _last_wifi_check <= 10000) {
// Events own the snapshot between 10 s polls. If STA is associated again
@@ -2827,16 +2847,7 @@ bool MQTTBridge::handleWiFiConnection(unsigned long now) {
s_wifi_connected_at = now;
_wifi_reconnect_backoff_attempt = 0;
#ifdef ESP_PLATFORM
wifi_ps_type_t ps_mode;
uint8_t ps_pref = _obs->wifi_power_save;
if (ps_pref == 1) {
ps_mode = WIFI_PS_NONE;
} else if (ps_pref == 2) {
ps_mode = WIFI_PS_MAX_MODEM;
} else {
ps_mode = WIFI_PS_NONE; // default: no power save; eliminates DTIM wake latency on mains-powered bridges
}
esp_wifi_set_ps(ps_mode);
applyWifiPowerSave();
#ifdef MQTT_WIFI_TX_POWER
WiFi.setTxPower(MQTT_WIFI_TX_POWER);
#else
+1
View File
@@ -517,6 +517,7 @@ private:
void getClientVersion(char* buffer, size_t buffer_size) const;
void logMemoryStatus();
void refreshOriginFromPrefs();
void applyWifiPowerSave(); // one mapping, applied on every association
// begin()/end()-scoped PSRAM buffers. Each allocation is independent so a
// transient heap shortage degrades to the existing stack fallback instead
// of making the bridge unusable.
+2 -2
View File
@@ -14,6 +14,7 @@
#include <helpers/MQTTPacketFilter.h>
#include <helpers/MQTTPresets.h>
#include <helpers/WebConfigKeys.h>
#include <helpers/WifiPowerSavePolicy.h>
#include <helpers/bridges/MQTTBridge.h>
#include "WebConfigHtml.h"
@@ -722,8 +723,7 @@ void WebConfigServer::handleConfigGet(AsyncWebServerRequest* req) {
JsonObject wifi = doc.createNestedObject("wifi");
wifi["ssid"] = (const char*)_obs->wifi_ssid;
wifi["pwd"] = _obs->wifi_password[0] ? SECRET_SENTINEL : "";
wifi["powersave"] = _obs->wifi_power_save == 0 ? "min"
: _obs->wifi_power_save == 2 ? "max" : "none";
wifi["powersave"] = WifiPowerSavePolicy::nameFor(_obs->wifi_power_save);
JsonObject mqtt = doc.createNestedObject("mqtt");
mqtt["origin"] = (const char*)_obs->mqtt_origin;
@@ -0,0 +1,58 @@
#include "helpers/WifiPowerSavePolicy.h"
#include <gtest/gtest.h>
using namespace WifiPowerSavePolicy;
// F11: the CLI applied stored 0 as MIN_MODEM while the reconnect path applied it
// as NONE, so a node configured for `min` changed behaviour after every
// reconnect and `get wifi.powersave` still reported min. One table, one answer.
TEST(WifiPowerSavePolicy, StoredValuesMapToOneModeEach) {
EXPECT_EQ(kModeMinModem, modeFor(kMin));
EXPECT_EQ(kModeNone, modeFor(kNone));
EXPECT_EQ(kModeMaxModem, modeFor(kMax));
}
TEST(WifiPowerSavePolicy, NamesRoundTripWithStoredValues) {
for (uint8_t stored = 0; stored <= 2; stored++) {
uint8_t parsed = 0xFF;
ASSERT_TRUE(parseName(nameFor(stored), &parsed)) << "stored " << (int)stored;
EXPECT_EQ(stored, parsed);
EXPECT_EQ(modeFor(stored), modeFor(parsed));
}
}
// A byte outside the stored range must read as the product default, not as
// whatever mode happens to sit at that index.
TEST(WifiPowerSavePolicy, OutOfRangeStoredValueReadsAsDefault) {
for (int stored = 3; stored <= 255; stored++) {
EXPECT_EQ(kModeNone, modeFor((uint8_t)stored)) << "stored " << stored;
EXPECT_STREQ("none", nameFor((uint8_t)stored));
}
}
// The setters take the remainder of the command line, so a trailing argument
// must still parse — and a longer word starting with a valid name must not.
TEST(WifiPowerSavePolicy, ParsesCliArgumentsExactly) {
uint8_t stored = 0xFF;
EXPECT_TRUE(parseName("min", &stored));
EXPECT_EQ(kMin, stored);
EXPECT_TRUE(parseName("none extra", &stored));
EXPECT_EQ(kNone, stored);
EXPECT_TRUE(parseName("max ", &stored));
EXPECT_EQ(kMax, stored);
stored = 0xFF;
EXPECT_FALSE(parseName("minimum", &stored));
EXPECT_FALSE(parseName("", &stored));
EXPECT_FALSE(parseName("off", &stored));
EXPECT_FALSE(parseName("MIN", &stored));
EXPECT_FALSE(parseName(nullptr, &stored));
EXPECT_EQ(0xFF, stored); // rejected input leaves the caller's value alone
}
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}