search

arduino-libraries / ArduinoLowPower

Powersave features for SAMD boards
GNU Lesser General Public License v2.1
81 stars 57 forks source link

One attachInterruptWakeup causes all interrupts to wake from deep sleep #16

Open trogper opened 5 years ago

trogper commented 5 years ago

I have multiple standard arduino attachInterrupt calls and one LowPower.attachInterruptWakeup, however all interrupts cause the arduino to wake up, and none of them, if I have no LowPower.attachInterruptWakeup

facchinm commented 5 years ago

This line makes the interrupt wakeup capable https://github.com/arduino-libraries/ArduinoLowPower/blob/master/src/samd/ArduinoLowPower.cpp#L93 , but it's also called in the core itself (https://github.com/arduino/ArduinoCore-samd/blob/master/cores/arduino/WInterrupts.c#L80) so once EIC clock is not stopped during sleep every configured pin can wakeup the board. @cmaglie @sandeepmistry should we remove the wakeup setting from the core and leave it in the library only?

sandeepmistry commented 5 years ago

@facchinm I think that line was added for people not using the low power lib: https://github.com/arduino/ArduinoCore-samd/pull/90

I'm ok with removing it, if you think it's best.

trogper commented 5 years ago

What about breaking older projects/sketches, which "rely" on this bug and are not using LowPower.attachInterruptWakeup on all required pins?

sslupsky commented 5 years ago

@trogper Can we include a warning at compile time?

trogper commented 5 years ago

@sslupsky I personally have used this library only once, so I won't be affected by the change. How would you differentiate correct usage from incorrect? When would you show the warning? Always? Do Arduino users/programmers read warnings if the program compiles and works? (in other words: is warning appropriate?) Wouldn't warning bother people who use it correctly?

sslupsky commented 5 years ago

Good points. Maybe the object can be changed so that when the method is called you specify which external interrupt is attached. The "default" can be "all"?

mamama1 commented 2 years ago

whatever solution is chosen, proper documentation will fix all issues and questions, i think. another option would be to add an additional (optional) boolean parameter to attachInterrupt like standbyWakeupCapability or something like that. that way it stays in the core and is obvious to anyone who is attaching an interrupt.