address review comments

4 files changed, 58 insertions, 67 deletions

diff --git a/shared-bindings/alarm/__init__.c b/shared-bindings/alarm/__init__.c
index 195ec6374..c983130a1 100644
--- a/shared-bindings/alarm/__init__.c
+++ b/shared-bindings/alarm/__init__.c
@@ -43,19 +43,21 @@
 //|
 //| Provides alarms that trigger based on time intervals or on external events, such as pin
 //| changes.
-//| The program can simply wait for these alarms, or go into a sleep state and
-//| and be awoken when they trigger.
+//| The program can simply wait for these alarms, or go to sleep and be awoken when they trigger.
 //|
 //| There are two supported levels of sleep: light sleep and deep sleep.
 //|
-//| Light sleep leaves the CPU and RAM powered so the program can resume after sleeping.
-//|
-//| *However, note that on some platforms, light sleep will shut down some communications, including
-//| WiFi and/or Bluetooth.*
+//| Light sleep keeps sufficient state so the program can resume after sleeping.
+//| It does not shut down WiFi, BLE, or other communications, or ongoing activities such
+//| as audio playback. It reduces power consumption to the extent possible that leaves
+//| these continuing activities running. In some cases there may be no decrease in power consumption.
 //|
 //| Deep sleep shuts down power to nearly all of the microcontroller including the CPU and RAM. This can save
 //| a more significant amount of power, but CircuitPython must restart ``code.py`` from the beginning when
 //| awakened.
+//|
+//| For both light sleep and deep sleep, if CircuitPython is connected to a host computer,
+//| maintaining the connection takes priority and power consumption may not be reduced.
 //| """
 
 //|
@@ -75,45 +77,39 @@ void validate_objs_are_alarms(size_t n_args, const mp_obj_t *objs) {
     }
 }
 
-//| def wait_until_alarms(*alarms: Alarm) -> Alarm:
-//|     """Wait for one of the alarms to trigger. The triggering alarm is returned.
-//|     is returned, and is also available as `alarm.wake_alarm`. Nothing is shut down
-//|     or interrupted. Power consumption will be reduced if possible.
-//|
-//|     If no alarms are specified, return immediately.
-//|     """
-//|     ...
-//|
-STATIC mp_obj_t alarm_wait_until_alarms(size_t n_args, const mp_obj_t *args) {
-    validate_objs_are_alarms(n_args, args);
-    common_hal_alarm_wait_until_alarms(n_args, args);
-    return mp_const_none;
-}
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_wait_until_alarms_obj, 1, MP_OBJ_FUN_ARGS_MAX, alarm_wait_until_alarms);
-
-//| def sleep_until_alarms(*alarms: Alarm) -> Alarm:
+//| def light_sleep_until_alarms(*alarms: Alarm) -> Alarm:
 //|     """Go into a light sleep until awakened one of the alarms. The alarm causing the wake-up
 //|     is returned, and is also available as `alarm.wake_alarm`.
 //|
-//|     Some functionality may be shut down during sleep. On ESP32-S2, WiFi is turned off,
-//|     and existing connections are broken.
-//|
 //|     If no alarms are specified, return immediately.
 //|
-//|     **If CircuitPython is connected to a host computer,** `alarm.sleep_until_alarms()`
-//|     **does not go into light sleep.**
-//|     Instead, light sleep is simulated by doing `alarm.wait_until_alarms()`,
+//|     **If CircuitPython is connected to a host computer, the connection will be maintained,
+//|     and the microcontroller may not actually go into a light sleep.**
 //|     This allows the user to interrupt an existing program with ctrl-C,
-//|     and to edit the files in CIRCUITPY, which would not be possible in true light sleep
+//|     and to edit the files in CIRCUITPY, which would not be possible in true light sleep.
+//|     Thus, to use light sleep and save significant power,
+//      it may be necessary to disconnect from the host.
 //|     """
 //|     ...
 //|
-STATIC mp_obj_t alarm_sleep_until_alarms(size_t n_args, const mp_obj_t *args) {
+STATIC mp_obj_t alarm_light_sleep_until_alarms(size_t n_args, const mp_obj_t *args) {
     validate_objs_are_alarms(n_args, args);
-    common_hal_alarm_sleep_until_alarms(n_args, args);
+
+    // See if we are connected to a host.
+    // Make sure we have been awake long enough for USB to connect (enumeration delay).
+    int64_t connecting_delay_msec = CIRCUITPY_USB_CONNECTED_SLEEP_DELAY * 1024 - supervisor_ticks_ms64();
+    if (connecting_delay_msec > 0) {
+        common_hal_time_delay_ms(connecting_delay_msec * 1000 / 1024);
+    }
+
+    if (supervisor_workflow_active()) {
+        common_hal_alarm_wait_until_alarms(n_args, args);
+    } else {
+        common_hal_alarm_light_sleep_until_alarms(n_args, args);
+    }
     return mp_const_none;
 }
-MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_sleep_until_alarms_obj, 1, MP_OBJ_FUN_ARGS_MAX, alarm_sleep_until_alarms);
+MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_light_sleep_until_alarms_obj, 1, MP_OBJ_FUN_ARGS_MAX, alarm_light_sleep_until_alarms);
 
 //| def exit_and_deep_sleep_until_alarms(*alarms: Alarm) -> None:
 //|     """Exit the program and go into a deep sleep, until awakened by one of the alarms.
@@ -130,11 +126,10 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_sleep_until_alarms_obj, 1, MP_OBJ_FUN_
 //|     If no alarms are specified, the microcontroller will deep sleep until reset.
 //|
 //|     **If CircuitPython is connected to a host computer, `alarm.exit_and_deep_sleep_until_alarms()`
-//|     does not go into deep sleep.**
-//|     Instead, deep sleep is simulated by first doing `alarm.wait_until_alarms()`,
-//|     and then, when an alarm triggers, by restarting CircuitPython.
+//|     then the connection will be maintained, and the system will not go into deep sleep.**
 //|     This allows the user to interrupt an existing program with ctrl-C,
 //|     and to edit the files in CIRCUITPY, which would not be possible in true deep sleep.
+//|     Thus, to use deep sleep and save significant power, you will need to disconnect from the host.
 //|
 //|     Here is skeletal example that deep-sleeps and restarts every 60 seconds:
 //|
@@ -156,6 +151,10 @@ MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_sleep_until_alarms_obj, 1, MP_OBJ_FUN_
 STATIC mp_obj_t alarm_exit_and_deep_sleep_until_alarms(size_t n_args, const mp_obj_t *args) {
     validate_objs_are_alarms(n_args, args);
 
+    // Shut down WiFi, etc.
+    common_hal_alarm_prepare_for_deep_sleep();
+
+    // See if we are connected to a host.
     // Make sure we have been awake long enough for USB to connect (enumeration delay).
     int64_t connecting_delay_msec = CIRCUITPY_USB_CONNECTED_SLEEP_DELAY * 1024 - supervisor_ticks_ms64();
     if (connecting_delay_msec > 0) {
@@ -163,6 +162,7 @@ STATIC mp_obj_t alarm_exit_and_deep_sleep_until_alarms(size_t n_args, const mp_o
     }
 
     if (supervisor_workflow_active()) {
+        // Simulate deep sleep by waiting for an alarm and then restarting when done.
         common_hal_alarm_wait_until_alarms(n_args, args);
         reload_requested = true;
         supervisor_set_run_reason(RUN_REASON_STARTUP);
@@ -175,9 +175,6 @@ STATIC mp_obj_t alarm_exit_and_deep_sleep_until_alarms(size_t n_args, const mp_o
 }
 MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(alarm_exit_and_deep_sleep_until_alarms_obj, 1, MP_OBJ_FUN_ARGS_MAX, alarm_exit_and_deep_sleep_until_alarms);
 
-//| """The `alarm.pin` module contains alarm attributes and classes related to pins.
-//| """
-//|
 STATIC const mp_map_elem_t alarm_pin_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_pin) },
 
@@ -191,9 +188,6 @@ STATIC const mp_obj_module_t alarm_pin_module = {
     .globals = (mp_obj_dict_t*)&alarm_pin_globals,
 };
 
-//| """The `alarm.time` module contains alarm attributes and classes related to time-keeping.
-//| """
-//|
 STATIC const mp_map_elem_t alarm_time_globals_table[] = {
     { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_time) },
 
@@ -213,7 +207,7 @@ STATIC mp_map_elem_t alarm_module_globals_table[] = {
     // wake_alarm is a mutable attribute.
     { MP_ROM_QSTR(MP_QSTR_wake_alarm), mp_const_none },
 
-    { MP_ROM_QSTR(MP_QSTR_sleep_until_alarms), MP_OBJ_FROM_PTR(&alarm_sleep_until_alarms_obj) },
+    { MP_ROM_QSTR(MP_QSTR_light_sleep_until_alarms), MP_OBJ_FROM_PTR(&alarm_light_sleep_until_alarms_obj) },
     { MP_ROM_QSTR(MP_QSTR_exit_and_deep_sleep_until_alarms),
                                                MP_OBJ_FROM_PTR(&alarm_exit_and_deep_sleep_until_alarms_obj) },
 
diff --git a/shared-bindings/alarm/__init__.h b/shared-bindings/alarm/__init__.h
index 26dbb2897..380c65ea8 100644
--- a/shared-bindings/alarm/__init__.h
+++ b/shared-bindings/alarm/__init__.h
@@ -32,8 +32,10 @@
 #include "common-hal/alarm/__init__.h"
 
 extern mp_obj_t common_hal_alarm_wait_until_alarms(size_t n_alarms, const mp_obj_t *alarms);
-extern mp_obj_t common_hal_alarm_sleep_until_alarms(size_t n_alarms, const mp_obj_t *alarms);
+extern mp_obj_t common_hal_alarm_light_sleep_until_alarms(size_t n_alarms, const mp_obj_t *alarms);
 extern void common_hal_alarm_exit_and_deep_sleep_until_alarms(size_t n_alarms, const mp_obj_t *alarms);
+extern void common_hal_alarm_prepare_for_deep_sleep(void);
+extern NORETURN void common_hal_alarm_enter_deep_sleep(void);
 
 // Used by wake-up code.
 extern void common_hal_alarm_set_wake_alarm(mp_obj_t alarm);
diff --git a/shared-bindings/alarm/pin/PinAlarm.c b/shared-bindings/alarm/pin/PinAlarm.c
index a6497d4cd..a435407ac 100644
--- a/shared-bindings/alarm/pin/PinAlarm.c
+++ b/shared-bindings/alarm/pin/PinAlarm.c
@@ -38,18 +38,16 @@
 //| class PinAlarm:
 //|     """Trigger an alarm when a pin changes state."""
 //|
-//|     def __init__(self, *pins: microcontroller.Pin, value: bool, all_same_value: bool = False, edge: bool = False, pull: bool = False) -> None:
+//|     def __init__(self, pin: microcontroller.Pin, value: bool, edge: bool = False, pull: bool = False) -> None:
 //|         """Create an alarm triggered by a `microcontroller.Pin` level. The alarm is not active
 //|         until it is passed to an `alarm`-enabling function, such as `alarm.sleep_until_alarms()` or
 //|         `alarm.exit_and_deep_sleep_until_alarms()`.
 //|
-//|         :param microcontroller.Pin \*pins: The pins to monitor. On some ports, the choice of pins
+//|         :param microcontroller.Pin pin: The pin to monitor. On some ports, the choice of pin
 //|           may be limited due to hardware restrictions, particularly for deep-sleep alarms.
 //|         :param bool value: When active, trigger when the pin value is high (``True``) or low (``False``).
 //|           On some ports, multiple `PinAlarm` objects may need to have coordinated values
 //|           for deep-sleep alarms.
-//|         :param bool all_same_value: If ``True``, all pins listed must be at ``value`` to trigger the alarm.
-//|           If ``False``, any one of the pins going to ``value`` will trigger the alarm.
 //|         :param bool edge: If ``True``, trigger only when there is a transition to the specified
 //|           value of ``value``. If ``True``, if the alarm becomes active when the pin value already
 //|           matches ``value``, the alarm is not triggered: the pin must transition from ``not value``
@@ -65,47 +63,44 @@
 STATIC mp_obj_t alarm_pin_pin_alarm_make_new(const mp_obj_type_t *type, mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     alarm_pin_pin_alarm_obj_t *self = m_new_obj(alarm_pin_pin_alarm_obj_t);
     self->base.type = &alarm_pin_pin_alarm_type;
-    enum { ARG_value, ARG_all_same_value, ARG_edge, ARG_pull };
+    enum { ARG_pin, ARG_value, ARG_edge, ARG_pull };
     static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_pin, MP_ARG_REQUIRED | MP_ARG_OBJ },
         { MP_QSTR_value, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_BOOL },
-        { MP_QSTR_all_same_value, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
         { MP_QSTR_edge, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
         { MP_QSTR_pull, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
     };
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(0, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    for (size_t i = 0; i < n_args; i ++) {
-        validate_obj_is_free_pin(pos_args[i]);
-    }
+    mcu_pin_obj_t *pin = validate_obj_is_free_pin(args[ARG_pin].u_obj);
 
-    common_hal_alarm_pin_pin_alarm_construct(
-        self, pos_args, n_args,
+    common_hal_alarm_pin_pin_alarm_construct(self,
+        pin,
         args[ARG_value].u_bool,
-        args[ARG_all_same_value].u_bool,
         args[ARG_edge].u_bool,
         args[ARG_pull].u_bool);
 
     return MP_OBJ_FROM_PTR(self);
 }
 
-//|     pins: Tuple[microcontroller.Pin]
-//|     """The trigger pins."""
+//|     pin: microcontroller.Pin
+//|     """The trigger pin."""
 //|
-STATIC mp_obj_t alarm_pin_pin_alarm_obj_get_pins(mp_obj_t self_in) {
+STATIC mp_obj_t alarm_pin_pin_alarm_obj_get_pin(mp_obj_t self_in) {
     alarm_pin_pin_alarm_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    return common_hal_alarm_pin_pin_alarm_get_pins(self);
+    return common_hal_alarm_pin_pin_alarm_get_pin(self);
 }
-MP_DEFINE_CONST_FUN_OBJ_1(alarm_pin_pin_alarm_get_pins_obj, alarm_pin_pin_alarm_obj_get_pins);
+MP_DEFINE_CONST_FUN_OBJ_1(alarm_pin_pin_alarm_get_pin_obj, alarm_pin_pin_alarm_obj_get_pin);
 
-const mp_obj_property_t alarm_pin_pin_alarm_pins_obj = {
+const mp_obj_property_t alarm_pin_pin_alarm_pin_obj = {
     .base.type = &mp_type_property,
-    .proxy = {(mp_obj_t)&alarm_pin_pin_alarm_get_pins_obj,
+    .proxy = {(mp_obj_t)&alarm_pin_pin_alarm_get_pin_obj,
               (mp_obj_t)&mp_const_none_obj,
               (mp_obj_t)&mp_const_none_obj},
 };
 
-//|     value: Tuple[microcontroller.Pin]
+//|     value: bool
 //|     """The value on which to trigger."""
 //|
 STATIC mp_obj_t alarm_pin_pin_alarm_obj_get_value(mp_obj_t self_in) {
@@ -122,7 +117,7 @@ const mp_obj_property_t alarm_pin_pin_alarm_value_obj = {
 };
 
 STATIC const mp_rom_map_elem_t alarm_pin_pin_alarm_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_pins), MP_ROM_PTR(&alarm_pin_pin_alarm_pins_obj) },
+    { MP_ROM_QSTR(MP_QSTR_pin), MP_ROM_PTR(&alarm_pin_pin_alarm_pin_obj) },
     { MP_ROM_QSTR(MP_QSTR_value), MP_ROM_PTR(&alarm_pin_pin_alarm_value_obj) },
 };
 
diff --git a/shared-bindings/alarm/pin/PinAlarm.h b/shared-bindings/alarm/pin/PinAlarm.h
index cb6946812..49ba71089 100644
--- a/shared-bindings/alarm/pin/PinAlarm.h
+++ b/shared-bindings/alarm/pin/PinAlarm.h
@@ -34,8 +34,8 @@
 
 extern const mp_obj_type_t alarm_pin_pin_alarm_type;
 
-void common_hal_alarm_pin_pin_alarm_construct(alarm_pin_pin_alarm_obj_t *self, const mp_obj_t pins[], size_t num_pins, bool value, bool all_same_value, bool edge, bool pull);
-extern mp_obj_tuple_t *common_hal_alarm_pin_pin_alarm_get_pins(alarm_pin_pin_alarm_obj_t *self);
+void common_hal_alarm_pin_pin_alarm_construct(alarm_pin_pin_alarm_obj_t *self, mcu_pin_obj_t *pin, bool value, bool edge, bool pull);
+extern mcu_pin_obj_t *common_hal_alarm_pin_pin_alarm_get_pin(alarm_pin_pin_alarm_obj_t *self);
 extern bool common_hal_alarm_pin_pin_alarm_get_value(alarm_pin_pin_alarm_obj_t *self);
 extern bool common_hal_alarm_pin_pin_alarm_get_edge(alarm_pin_pin_alarm_obj_t *self);
 extern bool common_hal_alarm_pin_pin_alarm_get_pull(alarm_pin_pin_alarm_obj_t *self);