Add pin alarming

[tannewt]

This mostly works but I have a crash happening when exiting deep sleep off of USB and reading the wake up object. The object has an incorrect type even though I'm setting it. If I log where I set it, the problem goes away. The last commit is the debug logs for it.

I also noticed that GPIO is disabled during time-based light sleep. I wonder if other peripherals are also disabled. The docs only discuss the RTC domains. @igrr can you explain what peripherals are preserved during light sleep?

[igrr]

@tannewt All peripherals are clock-gated during light sleep. RTC_IO, RTC_CNTL, Touch, and the ULP are the only ones which can keep working.

[tannewt]

Here is the test code I'm using:

import alarm
import alarm.pin
import alarm.time
import time
import neopixel
import board
import digitalio

if alarm.wake_alarm:
    print("awake", alarm.wake_alarm, alarm.wake_alarm.pin)
else:
    print("no wake up alarm")

enable = digitalio.DigitalInOut(board.NEOPIXEL_POWER)
enable.switch_to_output(False)

# Sleep for 5 so USB can start.
time.sleep(5)

pixels = neopixel.NeoPixel(board.NEOPIXEL, 4)
pixels.fill(0x0f0000)

time_alarm = alarm.time.TimeAlarm(monotonic_time=time.monotonic()+10)
print("light time sleep")
a = alarm.light_sleep_until_alarms(time_alarm)
print(a)

pixels.fill(0x000f00)
pin_alarm = alarm.pin.PinAlarm(pin=board.D11, value=False, pull=True)

print("light pin sleep")
a = alarm.light_sleep_until_alarms(pin_alarm)
while not a:
    pass
print(dir(a))
print(a, a.pin)

time.sleep(1)

print("sleeping")

# We don't turn off the neopixels explicitly.
# We expect code after the VM to do it.

alarm.exit_and_deep_sleep_until_alarms(pin_alarm)

[jfurcean]

I was able to reproduce the error. Uncommenting ESP_LOGI(TAG, "type"); on line 151 of PinAlarm.c prevented the crash. Interestingly, if I had the serial monitor of the mu-editor open it would still crash at the same point even with the line uncommented.

[dhalbert]

I spent a while looking at the assembly code of both the good and bad versions. I don't see an error in the assembly code. It's a bit tricky to read because xtensa uses register windows, which make the registers used for in/out arg passing vary based on the caller or callee, and also based on whether it's a call8, call4, etc. See http://wiki.linux-xtensa.org/index.php/ABI_Interface.

We are debugging

circuitpython/ports/esp32s2/common-hal/alarm/pin/PinAlarm.c

Line 107 in 3923351

mp_obj_t alarm_pin_pin_alarm_get_wakeup_alarm(size_t n_alarms, const mp_obj_t *alarms) {

I got the assembly code with:

$ xtensa-esp32s2-elf-objdump -D build-adafruit_magtag_2.9_grayscale/firmware.elf

I won't paste the full excerpts here, because this looks like a dead end. But here are few notes. In both versions a4 gets set to the address of a copy of &alarm_pin_pin_alarm_type quite early, because that constant is used early in the routine in:

circuitpython/ports/esp32s2/common-hal/alarm/pin/PinAlarm.c

Lines 111 to 113 in 3923351

	if (!MP_OBJ_IS_TYPE(alarms[i], &alarm_pin_pin_alarm_type)) {
	continue;
	}

a4 remains undisturbed in both versions. It is used to set the base type of the alarm object in the C code
https://github.com/adafruit/circuitpython/blob/39233510392107e9087e90757dd96da88b78b0f7/ports/esp32s2/common-hal/alarm/pin/PinAlarm.c#L138-140

The corresponding assembly code is looks like this in both versions (with changes to actual hex values and some register changes, but the registers are still fine and don't get smashed):

400b2a4d:	d0dfe5        	call8	4008384c <m_malloc>    ; m_new_obj()
400b2a50:	3ff751        	l32r	a5, 40082a2c <_stext+0x2a0c>   ; get address for use in following for loop
                                                                       ; unrelated to code immediately following
400b2a53:	030c      	movi.n	a3, 0
400b2a55:	1578      	l32i.n	a7, a5, 4              ; unrelated to code immediately following
400b2a57:	0a49      	s32i.n	a4, a10, 0             ; alarm->base.type = &alarm_pin_pin_alarm_type;
400b2a59:	1a39      	s32i.n	a3, a10, 4             ; alarm->pin = NULL;

So if the assembly code looks OK, what's wrong? I think that the bug may not be in this routine, but instead is further along. Adding the ESP_LOGI() moves things around (e.g. the <_stext+nnnnn> addresses are different in the bad and good versions), so the object may be getting smashed later in the program flow. If we could set a watchpoint...

[igrr]

If we could set a watchpoint...

@dhalbert You can set a watchpoint from within the program using cpu_hal_set_watchpoint.

https://github.com/espressif/esp-idf/blob/b0150615dff529662772a60dcb57d5b559f480e2/components/hal/include/hal/cpu_hal.h#L101

https://github.com/espressif/esp-idf/blob/b0150615dff529662772a60dcb57d5b559f480e2/components/hal/esp32s2/include/hal/cpu_ll.h#L103

Do I understand right the gist of the issue: you are saving a micropython object in RTC slow memory before sleep, and (at least) one object field has incorrect value after wakeup?

[dhalbert]

@dhalbert You can set a watchpoint from within the program using cpu_hal_set_watchpoint.

Great to know, thanks!

Do I understand right the gist of the issue: you are saving a micropython object in RTC slow memory before sleep, and (at least) one object field has incorrect value after wakeup?

No, it's not that tricky. the object is created after CircuitPython restarts, when the VM and our object heap should be in good shape. We check what kind of event woke us up and then create the object.

[microdev1]

the object is created after CircuitPython restarts, when the VM and our object heap should be in good shape.

I ran some tests and found out that delaying the object creation does seem to work.
I am calling alarm_save_wake_alarm() from user code :- 16a203f

import alarm
alarm.save()
print(alarm.wake_alarm)

Above delays setting wake_alarm by 90 ms and avoids the crash.
It looks like wake_alarm is set while the heap is still not configured which corrupts the object.

[dhalbert]

I think that the problem may be that the .wake_alarm is not being protected against garbage collection. I am pursuing this theory.

[dhalbert]

I was working in a clone of tannewt/circuitpython, instead of as a remote, so I don't seem to be able to push to the PR. However, here is a patch file with fixes that prevent the crash.

0001-mark-alarm.wake_alarm-during-gc-sweep.patch.zip

The test program above is acting oddly, so I wrote a simpler program. Tested on battery power on and USB (simulated deep sleep).

import alarm
import alarm.pin
import time
import neopixel
import board
import digitalio


if alarm.wake_alarm:
    print("awake", alarm.wake_alarm, alarm.wake_alarm.pin)
    alarm.sleep_memory[0] += 1
else:
    print("no wake up alarm")
    alarm.sleep_memory[0] = 0

print("count:", alarm.sleep_memory[0])

enable = digitalio.DigitalInOut(board.NEOPIXEL_POWER)
enable.switch_to_output(False)
pixels = neopixel.NeoPixel(board.NEOPIXEL, 4)

pixels.fill(0x000f00)
pin_alarm = alarm.pin.PinAlarm(pin=board.D11, value=False, pull=True)

board.DISPLAY.refresh()

print("about to deep_sleep")

# We don't turn off the neopixels explicitly.
# We expect code after the VM to do it.

alarm.exit_and_deep_sleep_until_alarms(pin_alarm)

[dhalbert]

Looks good to me, with some minor naming changes. But what do we want to do about light sleep? Is the code here fine for idle-based light sleep?

[tannewt]

Looks good to me, with some minor naming changes. But what do we want to do about light sleep? Is the code here fine for idle-based light sleep?

I've removed ESP light sleep and now just idle. I also tweaked the messaging in main to add "Code stopped by auto-reload."

Please take another look!

[dhalbert]

Looks good to me! I did not run further tests.

I'll have to revise the sleep guide about light sleep power consumption (doc and graphs), and see if we see any reduction during the sleep.

[tannewt]

Looks good to me! I did not run further tests.

I'll have to revise the sleep guide about light sleep power consumption (doc and graphs), and see if we see any reduction during the sleep.

I've seen about 30mA. The PinAlarm also takes ~1.4mA while deep sleeping because it has the RTC peripherals enabled.

[dhalbert]

Re-approving in anticipation of a clean build.

[dhalbert]

Thanks for the last bit. I was doing my pushes via the web. I'll let you merge if you're satisfied.