add wifi.radio.power_management et al

Author: dhalbert

ports/espressif/common-hal/wifi/Radio.c

@@ -140,7 +140,7 @@ void common_hal_wifi_radio_set_tx_power(wifi_radio_obj_t *self, const mp_float_t
 }
 
 wifi_power_management_t common_hal_wifi_radio_get_power_management(wifi_radio_obj_t *self) {
-    wifi_ps_type ps;
+    wifi_ps_type_t ps;
     esp_err_t ret = esp_wifi_get_ps(&ps);
     if (ret == ESP_OK) {
         switch (ps) {
@@ -156,30 +156,29 @@ wifi_power_management_t common_hal_wifi_radio_get_power_management(wifi_radio_ob
 }
 
 
-void common_hal_wifi_radio_set_power_management(wifi_radio_obj_t *self, wifi_power_management_t power_management, mp_uint_t listen_interval) {
+void common_hal_wifi_radio_set_power_management(wifi_radio_obj_t *self, wifi_power_management_t power_management) {
     switch (power_management) {
         case POWER_MANAGEMENT_MIN:
             esp_wifi_set_ps(WIFI_PS_MIN_MODEM);
             break;
         case POWER_MANAGEMENT_MAX: {
             // listen_interval is only used in this case.
             wifi_config_t *config = &self->sta_config;
-            config->sta.listen_interval = listen_interval;
+            // This is a typical value seen in various examples.
+            config->sta.listen_interval = 3;
             esp_wifi_set_ps(WIFI_PS_MAX_MODEM);
             esp_wifi_set_config(ESP_IF_WIFI_STA, config);
         }
         break;
         case POWER_MANAGEMENT_NONE:
             esp_wifi_set_ps(WIFI_PS_NONE);
             break;
+        case POWER_MANAGEMENT_UNKNOWN:
+            // This should be prevented in shared-bindings.
+            break;
     }
 }
 
-mp_int_t common_hal_wifi_radio_get_listen_interval(wifi_radio_obj_t *self) {
-    wifi_config_t *config = &self->sta_config;
-    return config->sta.listen_interval;
-}
-
 mp_obj_t common_hal_wifi_radio_get_mac_address_ap(wifi_radio_obj_t *self) {
     uint8_t mac[MAC_ADDRESS_LENGTH];
     esp_wifi_get_mac(ESP_IF_WIFI_AP, mac);

ports/espressif/mpconfigport.mk

@@ -40,9 +40,6 @@ CIRCUITPY_FULL_BUILD ?= 1
 # If SSL is enabled, it's mbedtls
 CIRCUITPY_SSL_MBEDTLS = 1
 
-# Wifi Power Save
-CIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT = 1
-
 # Never use our copy of MBEDTLS
 CIRCUITPY_HASHLIB_MBEDTLS_ONLY = 0
 

ports/raspberrypi/bindings/cyw43/__init__.c

@@ -16,7 +16,7 @@
 
 #include "lib/cyw43-driver/src/cyw43.h"
 
-static int power_management_value = PM_DISABLED;
+static uint32_t power_management_value = CONST_CYW43_DEFAULT_PM;
 
 void cyw43_enter_deep_sleep(void) {
 #define WL_REG_ON 23
@@ -43,14 +43,23 @@ MP_DEFINE_CONST_OBJ_TYPE(
     print, shared_bindings_microcontroller_pin_print
     );
 
+uint32_t cyw43_get_power_management_value() {
+    return power_management_value;
+}
+
+void cyw43_set_power_management_value(uint32_t value) {
+    power_management_value = value;
+    bindings_cyw43_wifi_enforce_pm();
+}
+
 //| PM_STANDARD: int
-//| """The standard power management mode"""
+//| """The default power management mode; same as PM_PERFORMANCE"""
 //| PM_AGGRESSIVE: int
 //| """Aggressive power management mode for optimal power usage at the cost of performance"""
 //| PM_PERFORMANCE: int
 //| """Performance power management mode where more power is used to increase performance"""
 //| PM_DISABLED: int
-//| """Disable power management and always use highest power mode. CircuitPython sets this value at reset time, because it provides the best connectivity reliability."""
+//| """Disable power management and always use highest power mode."""
 //|
 //|
 //| def set_power_management(value: int) -> None:
@@ -85,8 +94,7 @@ MP_DEFINE_CONST_OBJ_TYPE(
 //|
 static mp_obj_t cyw43_set_power_management(const mp_obj_t value_in) {
     mp_int_t value = mp_obj_get_int(value_in);
-    power_management_value = value;
-    bindings_cyw43_wifi_enforce_pm();
+    cyw43_set_power_management_value(value);
     return mp_const_none;
 }
 static MP_DEFINE_CONST_FUN_OBJ_1(cyw43_set_power_management_obj, cyw43_set_power_management);
@@ -126,10 +134,10 @@ static const mp_rom_map_elem_t cyw43_module_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR_CywPin), MP_ROM_PTR(&cyw43_pin_type) },
     { MP_ROM_QSTR(MP_QSTR_set_power_management), &cyw43_set_power_management_obj },
     { MP_ROM_QSTR(MP_QSTR_get_power_management), &cyw43_get_power_management_obj },
-    { MP_ROM_QSTR(MP_QSTR_PM_STANDARD), MP_ROM_INT(PM_STANDARD) },
-    { MP_ROM_QSTR(MP_QSTR_PM_AGGRESSIVE), MP_ROM_INT(PM_AGGRESSIVE) },
-    { MP_ROM_QSTR(MP_QSTR_PM_PERFORMANCE), MP_ROM_INT(PM_PERFORMANCE) },
-    { MP_ROM_QSTR(MP_QSTR_PM_DISABLED), MP_ROM_INT(PM_DISABLED) },
+    { MP_ROM_QSTR(MP_QSTR_PM_STANDARD), MP_ROM_INT(CONST_CYW43_DEFAULT_PM) },
+    { MP_ROM_QSTR(MP_QSTR_PM_AGGRESSIVE), MP_ROM_INT(CONST_CYW43_AGGRESSIVE_PM) },
+    { MP_ROM_QSTR(MP_QSTR_PM_PERFORMANCE), MP_ROM_INT(CONST_CYW43_PERFORMANCE_PM) },
+    { MP_ROM_QSTR(MP_QSTR_PM_DISABLED), MP_ROM_INT(CONST_CYW43_NONE_PM) },
 };
 
 static MP_DEFINE_CONST_DICT(cyw43_module_globals, cyw43_module_globals_table);

ports/raspberrypi/bindings/cyw43/__init__.h

@@ -16,21 +16,25 @@ const mcu_pin_obj_t *validate_obj_is_free_pin_including_cyw43(mp_obj_t obj, qstr
 const mcu_pin_obj_t *validate_obj_is_free_pin_or_gpio29(mp_obj_t obj, qstr arg_name);
 const mcu_pin_obj_t *validate_obj_is_pin_including_cyw43(mp_obj_t obj, qstr arg_name);
 
-#define CONSTANT_CYW43_PM_VALUE(pm_mode, pm2_sleep_ret_ms, li_beacon_period, li_dtim_period, li_assoc) \
+// This is equivalent to the code in cyw43.h, except that the values are computed at compile time.
+// A `CONST_` prefix has been added to the computation function (expressed as a macro) and the values.
+
+#define CONST_cyw43_pm_value(pm_mode, pm2_sleep_ret_ms, li_beacon_period, li_dtim_period, li_assoc) \
     (li_assoc << 20 | /* listen interval sent to ap */ \
         li_dtim_period << 16 | \
         li_beacon_period << 12 | \
         (pm2_sleep_ret_ms / 10) << 4 | /* cyw43_ll_wifi_pm multiplies this by 10 */ \
         pm_mode /* CYW43_PM2_POWERSAVE_MODE etc */)
 
-// CYW43_DEFAULT_PM (except a compile-time constant)
-#define PM_STANDARD CONSTANT_CYW43_PM_VALUE(CYW43_PM2_POWERSAVE_MODE, 200, 1, 1, 10)
-// CYW43_AGGRESSIVE_PM (except a compile-time constant)
-#define PM_AGGRESSIVE CONSTANT_CYW43_PM_VALUE(CYW43_PM2_POWERSAVE_MODE, 2000, 1, 1, 10)
-// CYW43_PERFORMANCE_PM (except a compile-time constant)
-#define PM_PERFORMANCE CONSTANT_CYW43_PM_VALUE(CYW43_PM2_POWERSAVE_MODE, 20, 1, 1, 1)
-// The 0xa11140 magic value
-#define PM_DISABLED CONSTANT_CYW43_PM_VALUE(CYW43_NO_POWERSAVE_MODE, 200, 1, 1, 10)
+#define CONST_CYW43_DEFAULT_PM (CONST_CYW43_PERFORMANCE_PM)
+
+#define CONST_CYW43_NONE_PM (CONST_cyw43_pm_value(CYW43_NO_POWERSAVE_MODE, 10, 0, 0, 0))
+
+#define CONST_CYW43_AGGRESSIVE_PM (CONST_cyw43_pm_value(CYW43_PM1_POWERSAVE_MODE, 10, 0, 0, 0))
+
+#define CONST_CYW43_PERFORMANCE_PM (CONST_cyw43_pm_value(CYW43_PM2_POWERSAVE_MODE, 200, 1, 1, 10))
 
+extern uint32_t cyw43_get_power_management_value(void);
+extern void cyw43_set_power_management_value(uint32_t value);
 extern void bindings_cyw43_wifi_enforce_pm(void);
 void cyw43_enter_deep_sleep(void);

ports/raspberrypi/common-hal/wifi/Radio.c

@@ -5,6 +5,7 @@
 // SPDX-License-Identifier: MIT
 
 #include "supervisor/port.h"
+#include "shared-bindings/wifi/PowerManagement.h"
 #include "shared-bindings/wifi/Radio.h"
 #include "shared-bindings/wifi/Network.h"
 
@@ -107,6 +108,41 @@ void common_hal_wifi_radio_set_tx_power(wifi_radio_obj_t *self, const mp_float_t
     cyw43_ioctl(&cyw43_state, CYW43_IOCTL_SET_VAR, 9 + 4, buf, CYW43_ITF_AP);
 }
 
+wifi_power_management_t common_hal_wifi_radio_get_power_management(wifi_radio_obj_t *self) {
+    uint32_t pm_value = cyw43_get_power_management_value();
+
+    switch (pm_value) {
+        case CONST_CYW43_PERFORMANCE_PM:
+            return POWER_MANAGEMENT_MIN;
+        case CONST_CYW43_AGGRESSIVE_PM:
+            return POWER_MANAGEMENT_MAX;
+        case CONST_CYW43_NONE_PM:
+            return POWER_MANAGEMENT_NONE;
+        default:
+            return POWER_MANAGEMENT_UNKNOWN;
+    }
+}
+
+
+void common_hal_wifi_radio_set_power_management(wifi_radio_obj_t *self, wifi_power_management_t power_management) {
+    uint32_t pm_setting = CONST_CYW43_DEFAULT_PM;
+    switch (power_management) {
+        case POWER_MANAGEMENT_MIN:
+            pm_setting = CONST_CYW43_PERFORMANCE_PM;
+            break;
+        case POWER_MANAGEMENT_MAX:
+            pm_setting = CONST_CYW43_AGGRESSIVE_PM;
+            break;
+        case POWER_MANAGEMENT_NONE:
+            pm_setting = CONST_CYW43_NONE_PM;
+            break;
+        default:
+            // Should not get here.
+            break;
+    }
+    cyw43_set_power_management_value(pm_setting);
+}
+
 mp_obj_t common_hal_wifi_radio_get_mac_address_ap(wifi_radio_obj_t *self) {
     return common_hal_wifi_radio_get_mac_address(self);
 }

py/circuitpy_defns.mk

@@ -617,6 +617,7 @@ $(filter $(SRC_PATTERNS), \
 	supervisor/StatusBar.c \
 	wifi/AuthMode.c \
 	wifi/Packet.c \
+	wifi/PowerManagement.c \
 )
 
 ifeq ($(CIRCUITPY_BLEIO_HCI),1)

py/circuitpy_mpconfig.mk

@@ -705,9 +705,6 @@ CFLAGS += -DCIRCUITPY_WEB_WORKFLOW=$(CIRCUITPY_WEB_WORKFLOW)
 CIRCUITPY_WIFI_RADIO_SETTABLE_MAC_ADDRESS?= 1
 CFLAGS += -DCIRCUITPY_WIFI_RADIO_SETTABLE_MAC_ADDRESS=$(CIRCUITPY_WIFI_RADIO_SETTABLE_MAC_ADDRESS)
 
-CIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT?= 0
-CFLAGS += -DCIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT=$(CIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT)
-
 # tinyusb port tailored configuration
 CIRCUITPY_TUSB_MEM_ALIGN ?= 4
 CFLAGS += -DCIRCUITPY_TUSB_MEM_ALIGN=$(CIRCUITPY_TUSB_MEM_ALIGN)

shared-bindings/wifi/PowerManagement.c

@@ -9,25 +9,38 @@
 #include "shared-bindings/wifi/PowerManagement.h"
 
 //| class PowerManagement:
-//|     """Power-saving options for wifi"""
+//|     """Power-saving options for wifi
+//|
+//|     .. note:: On boards using the CYW43 radio module, the choices below correspond to the
+//|       power management values defined in the `cyw43` module:
+//|       `PowerManagement.MIN` is the same as `cyw43.PM_PERFORMANCE`, `PowerManagement.MAX`
+//|       is the same as `cyw43.PM_AGGRESSIVE`, and `PowerManagement.NONE` is the same as
+//|       `cyw43.PM_DISABLED`. If a custom value was set with `cyw43.set_power_management()`
+//|       not corresponding to one of these three values, then `PowerManagement.UNKNOWN` will be returned.
+//|       """
 //|
 //|     MIN: PowerManagement
 //|     """Minimum power management (default). The WiFi station wakes up to receive a beacon every DTIM period.
 //|        The DTIM period is set by the access point."""
 //|     MAX: PowerManagement
-//|     """Maximum power management, at the expense of some performance. The WiFi station wakes up every 100 ms."""
+//|     """Maximum power management, at the expense of some performance. The WiFi station wakes up less often than `MIN`."""
 //|     NONE: PowerManagement
 //|     """No power management: the WiFi station does not sleep."""
+//|     UNKNOWN: PowerManagement
+//|     """Power management setting cannot be determined."""
+//|
 
 // In order of the enum type.
 MAKE_ENUM_VALUE(wifi_power_management_type, power_management, NONE, POWER_MANAGEMENT_NONE);
 MAKE_ENUM_VALUE(wifi_power_management_type, power_management, MIN, POWER_MANAGEMENT_MIN);
 MAKE_ENUM_VALUE(wifi_power_management_type, power_management, MAX, POWER_MANAGEMENT_MAX);
+MAKE_ENUM_VALUE(wifi_power_management_type, power_management, UNKNOWN, POWER_MANAGEMENT_UNKNOWN);
 
 MAKE_ENUM_MAP(wifi_power_management) {
     MAKE_ENUM_MAP_ENTRY(power_management, NONE),
     MAKE_ENUM_MAP_ENTRY(power_management, MIN),
     MAKE_ENUM_MAP_ENTRY(power_management, MAX),
+    MAKE_ENUM_MAP_ENTRY(power_management, UNKNOWN),
 };
 
 static MP_DEFINE_CONST_DICT(wifi_power_management_locals_dict, wifi_power_management_locals_table);

shared-bindings/wifi/PowerManagement.h

@@ -9,9 +9,11 @@
 #include "py/enum.h"
 
 typedef enum {
-    POWER_MANAGMENT_NONE = 0,
+    POWER_MANAGEMENT_NONE = 0,
     POWER_MANAGEMENT_MIN = 1,
     POWER_MANAGEMENT_MAX = 2,
+    // Value can't be determined.
+    POWER_MANAGEMENT_UNKNOWN = 3,
 } wifi_power_management_t;
 
 extern const mp_obj_type_t wifi_power_management_type;

shared-bindings/wifi/Radio.c

@@ -6,6 +6,7 @@
 
 #include "shared-bindings/wifi/__init__.h"
 #include "shared-bindings/wifi/AuthMode.h"
+#include "shared-bindings/wifi/PowerManagement.h"
 
 #include <string.h>
 
@@ -189,33 +190,29 @@ MP_PROPERTY_GETSET(wifi_radio_tx_power_obj,
     (mp_obj_t)&wifi_radio_set_tx_power_obj);
 
 //|     power_management: PowerManagement
-//|     """Wifi power management setting. The default is `wifi.PowerManagement.MIN.
+//|     """Wifi power management setting. See `wifi.PowerManagement`. The default is `wifi.PowerManagement.MIN`.
 //|     """
 static mp_obj_t wifi_radio_get_power_management(mp_obj_t self_in) {
-    #if CIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT
     wifi_radio_obj_t *self = MP_OBJ_TO_PTR(self_in);
     return cp_enum_find(&wifi_power_management_type, common_hal_wifi_radio_get_power_management(self));
-    #else
-    return MP_OBJ_FROM_PTR(&power_management_MIN);
-    #endif
 }
-MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_get_listen_interval_obj, wifi_radio_get_listen_interval);
+MP_DEFINE_CONST_FUN_OBJ_1(wifi_radio_get_power_management_obj, wifi_radio_get_power_management);
 
 static mp_obj_t wifi_radio_set_power_management(mp_obj_t self_in, mp_obj_t power_management_in) {
-    #if CIRCUITPY_WIFI_RADIO_SETTABLE_POWER_MANAGEMENT
     wifi_radio_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    wifi_power_management_t power_management = cp_enum_value(&wifi_power_management_type, power_management_in, MP_QSTR_power_management);
+    wifi_power_management_t power_management =
+        cp_enum_value(&wifi_power_management_type, power_management_in, MP_QSTR_power_management);
+    if (power_management == POWER_MANAGEMENT_UNKNOWN) {
+        mp_arg_error_invalid(MP_QSTR_power_management);
+    }
     common_hal_wifi_radio_set_power_management(self, power_management);
-    #else
-    mp_raise_NotImplementedError(NULL);
-    #endif
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_2(wifi_radio_set_listen_interval_obj, wifi_radio_set_listen_interval);
+MP_DEFINE_CONST_FUN_OBJ_2(wifi_radio_set_power_management_obj, wifi_radio_set_power_management);
 
-MP_PROPERTY_GETSET(wifi_radio_listen_interval_obj,
-    (mp_obj_t)&wifi_radio_get_listen_interval_obj,
-    (mp_obj_t)&wifi_radio_set_listen_interval_obj);
+MP_PROPERTY_GETSET(wifi_radio_power_management_obj,
+    (mp_obj_t)&wifi_radio_get_power_management_obj,
+    (mp_obj_t)&wifi_radio_set_power_management_obj);
 
 //|     mac_address_ap: ReadableBuffer
 //|     """MAC address for the AP. When the address is altered after interface is started
@@ -870,7 +867,7 @@ static const mp_rom_map_elem_t wifi_radio_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_ipv4_subnet_ap),    MP_ROM_PTR(&wifi_radio_ipv4_subnet_ap_obj) },
     { MP_ROM_QSTR(MP_QSTR_ipv4_address),    MP_ROM_PTR(&wifi_radio_ipv4_address_obj) },
     { MP_ROM_QSTR(MP_QSTR_ipv4_address_ap),    MP_ROM_PTR(&wifi_radio_ipv4_address_ap_obj) },
-    { MP_ROM_QSTR(MP_QSTR_listen_interval),    MP_ROM_PTR(&wifi_radio_listen_interval_obj) },
+    { MP_ROM_QSTR(MP_QSTR_power_management),    MP_ROM_PTR(&wifi_radio_power_management_obj) },
 
     { MP_ROM_QSTR(MP_QSTR_set_ipv4_address),    MP_ROM_PTR(&wifi_radio_set_ipv4_address_obj) },
     { MP_ROM_QSTR(MP_QSTR_set_ipv4_address_ap),    MP_ROM_PTR(&wifi_radio_set_ipv4_address_ap_obj) },