adafruit / Adafruit_CircuitPython_MagTag

Helper library for the Adafruit MagTag
MIT License
29 stars 24 forks source link

Improve docs for using the Alarms when using MagTag library #64

Closed vanhine closed 1 year ago

vanhine commented 3 years ago

Setting a PinAlarm does not work when using this MagTag library.

I am using circuitpython v6

This is due to peripherals.py initializing all of the button pins to be used by the *_pressed() methods.

When trying to set a PinAlarm I receive the following error:

Traceback (most recent call last):
  File "code.py", line 265, in <module>
ValueError: BUTTON_A in use
makermelissa commented 3 years ago

We should add PinAlarm to the library itself. It wasn't quite functional at the time I made the library and we never go around to adding it in. We should probably add it to FunHouse too afterwards, but this library is more important for low power.

ThomasAtBBTF commented 3 years ago

I am working on a project where 12 processor pins are used on a nRF52840 and I am running into the same / similar problem. It would be good if already initialized / in use pins could be added to alarm.light_sleep_until_alarms. I fear, deinitializing all 12 pins (which I use to detect user input) before "going to sleep" and reinitializing them after "wake up" takes to long and will result in loosing user input events.

Or, can I initialize the PinAlarm pin-instances globally and use the values of these pins "while not sleeping" in my regular code?

PS: A hardware alternative would be to use a M74HC133 to couple all the pins and connect the output to another "wakeup-pin". But I hope that this can be avoided by software.

ThomasAtBBTF commented 3 years ago

Maybe I better open a new Feature Request in CircuitPython

patja commented 2 years ago

Hello. I am wondering if there are workarounds to this issue.

Is there a prior version of the library that will work? I am trying to get a pin alarm to wake the MagTag from deep sleep, as demonstrated by this project: https://learn.adafruit.com/magtag-dishwasher-status/wake-on-button and I am guessing it must have worked at the time it was written.

Any advice on a hack to free up the pin for use in the alarm, or other suggestions, are appreciated.

joshuapinter commented 2 years ago

I'm now running up against this limitation as well.

@makermelissa Is this something that we can fork and submit a PR for? I don't have a time of time and I'm new to CircuitPython, but let me know what you'd like to see and I'll see what I can do. Thanks.

joshuapinter commented 2 years ago

Just to jump on this a little more, can somebody explain a bit for me.

# Buttons
self.buttons = []
for pin in (board.BUTTON_A, board.BUTTON_B, board.BUTTON_C, board.BUTTON_D):
  switch = DigitalInOut(pin)
  switch.direction = Direction.INPUT
  switch.pull = Pull.UP
  self.buttons.append(switch)

Is the accelerometer input pin (I believe it is IO9 based on https://learn.adafruit.com/assets/98548) shared with the buttons?

Or can someone explain a bit of the electronics and what needs to happen to allow us to easily set the accelerometer pin alarm, then I can take a look at modifying the library to make it work.

Thanks! (software eng here but total noob when it comes to python and microcontrollers :)

patja commented 2 years ago

I was able to work around this issue by calling deinit() on the button pin prior to setting it up for a pin alarm. You won't be able to use it for anything except an alarm until it wakes up again, but that worked for me. I just moved the deinit() and pin alarm setup to the end of my script, right before going into deep sleep.

Then I ran afoul of bug 5343 in the main CircuitPython repo which was just fixed a few days ago. You need to get the nightly build from S3 to get this fix. Without this bug fix, you will have no way to know it was a pin alarm that woke the device.

joshuapinter commented 2 years ago

@patja Awesome, thanks for the response!

For deinit(), do you need to call it with any params? Or just magtag.deinit() (assuming I initialized it with magtag = MagTag())?

ladyada commented 2 years ago

@makermelissa is out for holiday but will take a look when she returns :)

patja commented 2 years ago
magtag.peripherals.buttons[0].deinit()
magtag.peripherals.buttons[1].deinit()
a_alarm = alarm.pin.PinAlarm(pin=board.BUTTON_A, value=False, pull=True) #note pull
b_alarm = alarm.pin.PinAlarm(pin=board.BUTTON_B, value=False, pull=True) #note pull
time_alarm = alarm.time.TimeAlarm(monotonic_time=time.monotonic() + 600) # 600 seconds = 5 minutes
alarm.exit_and_deep_sleep_until_alarms(time_alarm,a_alarm,b_alarm)`

Wakes when whichever comes first: time alarm or pin alarm

joshuapinter commented 2 years ago

@patja Friggin' awesome! Thanks for posting the code. I'll be using an alarm/interrupt from the accelerometer so I'll see if anything special needs to be done with that as well. I'll post back here with any interesting results.

I hope your Christmas Gifts turned out great and were appreciated by the recipients! :)

makermelissa commented 2 years ago

I'm now running up against this limitation as well.

@makermelissa Is this something that we can fork and submit a PR for? I don't have a time of time and I'm new to CircuitPython, but let me know what you'd like to see and I'll see what I can do. Thanks.

Yes, please do submit a PR if you would like. I think the code for this actually lives inside the Superclass library Adafruit_CircuitPython_PortalBase. Otherwise, I will take a look soon.

@patja thanks for posting a solution that works. It will probably help with debugging the real issue.

patja commented 2 years ago

I've thought about this issue and I'm not totally convinced a code change is merited. Might just need documentation updates.

Perhaps my imagination is lacking, but what is the scenario in which a pin must be both used for interaction as a button AND as an alarm pin simultaneously?

It seems fine to me to just deinit the pin from the Magtag object to free it up for alarm use just before going to sleep.

I guess some abstraction of this could be helpful so the library just does this for you...ask to setup an alarm pin and it will automatically deinit it first as part of the operation? That might cause other problems if someone is expecting to use the pin for button interaction after setting it as an alarm pin. Maybe better to be explicit in setting the expectation that at one point in time a pin can be either an alarm pin or an interactive button pin, but never both at the same time.

joshuapinter commented 2 years ago

@patja Yup, I agree. After playing with your sample code and setting up an alarm for both buttons and accelerometer pins, everything seemed to work well and make sense. I think with better docs, using deinit seems like the most clear and transparent thing to do. A little less magic behind the scenes is sometimes (often?) the right way to go.

I'll shelve the PR and any code changes for now.

Thanks again for all your help and code samples. It helped tremendously and will hopefully help others coming across this as well.

tekktrik commented 2 years ago

Opening this up for Hacktoberfest!

Documentation is needed for letting readers know that buttons must call deinit() or the peripherals attribute of the MagTag object must call deinit() in order for them to be used as a PinAlarm before the MagTag enters light/deep sleep.