Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Photon]: System.sleep(_pin, _edge) never wakes #655

Closed
ScruffR opened this issue Oct 2, 2015 · 17 comments

Comments

@ScruffR
Copy link
Contributor

commented Oct 2, 2015

There is a similar issue for Core ( #640 ).
There is some confusion, since the docs state this should work, but it doesn't.
I've tested all pins with all FALLING/RISING/CHANGE and apart from some CHANGE combinations none of the other combinations did wake the Photon - not even WKP.

I tested with this code

void setup() 
{
  pinMode(D7, OUTPUT);
  Particle.function("GoToSleep", go2sleep);
}

void loop() 
{
    digitalWrite(D7, (millis() >> 2) & 0x88);
}

int go2sleep(String cmd)
{
    int i;
    if (i = cmd.toInt())
    {
        int pin = abs(i) % 20;  // 20 denotes pin D0
        PinMode input = INPUT;
        InterruptMode edge = CHANGE;

        if (abs(i) < 100) // >= 100 CHANGE
        {
            input = (i > 0) ? INPUT_PULLDOWN : INPUT_PULLUP;
            edge = (i > 0) ? RISING : FALLING;
        }
        pinMode(pin, input);
        System.sleep(pin, edge);
    }
    return i;
}

I also realized, that after the few CHANGE wakes that worked, the code continued from within the Particle.function() which does not fit the docs

When the specific interrupt arrives, the device awakens from stop mode, it will behave 
as if the device is reset and run all user code from the beginning with no values being 
maintained in memory from before the stop mode.

Although the note states

(Note: The new Particle Photon firmware will not reset before going into stop mode so 
all the application variables are preserved after waking up from this mode. The voltage 
regulator is put in low-power mode. This mode achieves the lowest power consumption 
while retaining the contents of SRAM and registers.)

What is meant with "new firmware"? A version number should be stated.
And also that the code carries on from where it left off (preserved registers doesn't mean too much to average users - as instruction and stack pointer would not do either)

@m-mcgowan m-mcgowan added this to the 0.4.7 milestone Oct 2, 2015

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Oct 2, 2015

The code above doesn't look right - System.sleep() takes 3 arguments - pin, mode and duration.

I expect this is due to the loose typing used on these old APIs.

@ScruffR

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2015

Nope, the docs do state
https://docs.particle.io/reference/firmware/core/#system-sleep-

System.sleep(uint16_t wakeUpPin, uint16_t edgeTriggerMode) can be used to put 
the entire device into a stop mode with wakeup on interrupt.

I can try the other overload too, but there is long seconds=0 anyhow.

This should also be doced and the docs should rather state the strongly typed prototype than uint16_t

  static void sleep(uint16_t wakeUpPin, InterruptMode edgeTriggerMode, long seconds=0);
  // for the docs write this
  //  static void sleep(uint16_t wakeUpPin, InterruptMode edgeTriggerMode [, long seconds] ); 
@technobly

This comment has been minimized.

Copy link
Member

commented Oct 2, 2015

Confirmed that this is broken:
System.sleep(pin, edge) and System.sleep(pin, edge, seconds)

Will be looking into what regressed here...

@bosluc

This comment has been minimized.

Copy link

commented Dec 13, 2015

Hi,

See also: http://community.particle.io/t/system-sleep-sleep-mode-deep-10-not-working/17239
This is also related:
Using System.sleep(SLEEP_MODE_DEEP, wakeOnTime) works fine if the Photon has no connected pins, however when put on a SparkFun Photon Battery Shield it will never wake up.

@m-mcgowan m-mcgowan modified the milestones: 0.4.9, 0.4.8 Dec 14, 2015

@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2015

Hello,

Just to update the status of this bug.

  1. Current wake code is pretty much the same as the STM example code and PWR, CSB registered were set correctly before entering stop mode. So the firmware was able to enter sleep mode without any issue.
  2. Based on the errata from STM, there are 3 issues that are related to sleep mode, or stop mode in STM language. Two of the errata affects the debugging in stop mode, one errata affects when writing to watchdog right before entering stop mode. We can reasonably rule out these 3 errata as the root cause of this bug.
  3. The issues seems to be related to attach interrupts before going into sleep. If the interrupt was not attached correctly, the interrupt will not be generated correctly to wake up the MCU.

Debug is still on going.

@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2015

Hello,

To my pleasant surprise, the system sleep and wakeup function seems to work perfectly with the latest develop branch. I have tested interrupt mode CHANGE, RISING and FALLING on A1A6, D0D7, and Wake pin.

Hi @ScruffR , @bosluc , can you help me to check if the latest develop branch has fixed the sleep issue for you?

Below is my test code for your reference.

#include "application.h"

// Cloud Function example usage.
// Invoke go2sleep cloud function from Particle Dev IDE>Particle>Show Code Functions
// go2sleep(A2,C) // Setup wake up pin to A2 and change interrupt mode
// go2sleep(D0,F) // Setup wake up pin to D0 and falling interrupt mode
// go2sleep(D1,R) // Setup wake up pin to D1 and rising interrupt mode
int go2sleep(String cmd)
{

// Select Pin
int pinNumber = cmd.charAt(1) - '0';
if (pinNumber< 0 || pinNumber >7) return -1;
if (cmd.startsWith("A")){
    pinNumber += 10;
}
if (pinNumber>=10){
    Serial.printlnf("Pin A%d selected", pinNumber-10);
} else {
    Serial.printlnf("Pin D%d selected", pinNumber);
}

// Select Mode
InterruptMode interruptMOde = CHANGE;
switch (cmd.charAt(3)){
    case 'C':
        interruptMOde = CHANGE;
        Serial.println("Mode is CHANGE");
        break;
    case 'F':
        interruptMOde = FALLING;
        Serial.println("Mode is FALLING");
        break;
    case 'R':
        interruptMOde = RISING;
        Serial.println("Mode is RISING");
        break;
    default:
        Serial.println("Illegal mode, use upper case letter!");
        break;
}
Serial.println("Going to sleep");
delay(1000);
System.sleep(pinNumber, interruptMOde, 0);
return 0;

}

void setup()
{
Particle.function("GoToSleep", go2sleep);
Serial.begin(9600);
}

void loop()
{
Serial.println("I am alive");
delay(1000);
}

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2015

Since we haven't taken any active steps to fix this in the develop branch, this makes me wonder if we have another case of uninitialized data, making this functionality work sometimes and not other times. A possible plan of attack is to check out a release that exhibits the bug, and look for uninitialized data surrounding the interrupt registers.

@ScruffR

This comment has been minimized.

Copy link
Contributor Author

commented Dec 20, 2015

Yup, that sounds like a possible reason, since even when I opened the issue I could not get a consistent case where things actually worked, but I had "best" (but inconsistent) results with CHANGE triggers.
RISING/FALLING worked almost never.

But as it seems at least WKP is currently working reliably again, which wasn't when I tested back then.

@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2015

Hi @ScruffR and @m-mcgowan

There is definitely something quirky here. I have switched to release/0.4.7 branch and I have tested out both my code and @ScruffR 's code. I expect the two codes to be either both working or both not working. The result is surprising, my test code worked while @ScruffR's didn't.

It looks like depend on how the application program was written, the system.sleep has different behavior. It might be an evidence that points to uninitialized data as the root cause. With different application program, the uninitialized portion of the struct might got different value and affects the behavior.

I will continue with the debug tomorrow with an eye for uninitialized structs.

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2015

Looks like HAL_Interrupts_Detach() doesn't fully initialize the EXTI_InitTypeDef struct, and HAL_Pin_Mode() doesn't initialize GPIO_InitStructure.

@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2015

Hi @m-mcgowan

I will initialize both structs in HAL_Interrupts_Detach() and HAL_Pin_Mode().

HAL_Interrupts_Detach() seems to be less of an issue, since EXTI_Init ignore other parameters when EXTI_LineCmd = DISABLE. HAL_Pin_Mode() seems to be more likely, since I recall some erratic behavior with GPIO pull-up and pull-down.

Thanks,
Peter

@technobly

This comment has been minimized.

Copy link
Member

commented Dec 20, 2015

👍

peterqu added a commit to peterqu/firmware that referenced this issue Dec 21, 2015
Fixed system wake up issue particle-iot#655
Initialized all structs in HAL_Interrupts and PinMode method.
Uninitialized GPIO_InitTypeDef struct in pinMode() can cause wake up
pin to be set as  Alternative Function pin.  This will lead to failure
in setting up the interrupt on the wake up pin.
m-mcgowan added a commit that referenced this issue Dec 22, 2015
@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2015

For future reference, there are still some timing-related corner cases when system.sleep() is called within a cloud function. If you ever found that the wake from pin does not work, consider add a simple delay before calling system.sleep as shown below.

delay(1000);
System.sleep(D0, RISING);

Based on my testing, the wake up from pin worked consistently across all pins for both photon and core.

@ScruffR

This comment has been minimized.

Copy link
Contributor Author

commented Dec 23, 2015

What does delay do, since sleep doesn't know it will be called?
Or would Particle.process() do the same thing?

@m-mcgowan

This comment has been minimized.

Copy link
Contributor

commented Dec 23, 2015

Yes, that's an interesting point ScruffR - the delay(1000) will run the background loop at least once. Clearly it is doing something, but it's not clear what. The cloud does set the time, but that shouldn't come into the picture here since no alarm is set. Peter, when you have time, please inspect the state of the STM32 pin / sleep configuration both with and without the delay to see if there is any difference. Also try changing the delay to HAL_Delay_Milliseconds(1000), which is a far simpler kind of delay.

@peterqu

This comment has been minimized.

Copy link
Contributor

commented Dec 26, 2015

Hi @m-mcgowan

So far the testing has shown that it is only an issue when system.sleep() is called from a cloud function. When called from a cloud function, a delay(1000), HAL_Delay_Milliseconds() or dummy Serial.print() are necessary in order for waking up from pin to work. I was wondering if it is related to this one (http://community.particle.io/t/flush-messages-before-going-to-sleep/8902).

I have examined the pin/sleep configuration. One possible issue is that there are multiple pin interrupts being configured by the system. I have set the wake-up pin to D0, which map to GPIO_B7. The SYSCFG\EXTI7 register should be set to 0x1 to select GPIO_B as the source. From time to time, SYSCFG\EXTI7 register have been set to 0x2 to select GPIO_C as the source, so a rising edge on pin D0 (GPIO_B7) will not generate and interrupt and not wake up the system.

Stepping through the code sometimes run into Handle_Mode_Button_EXTI_irq(), which apparently sets GPIO_C7 as the EXTI. There might be other interrupts being set by the systems, such as EXTI0 (GPIO_B0), EXTI7(GPIO_C7) and EXTI17, which have their EXTI\MR bit set to 1. GPIO_B0 often have pending interrupt (EXTI\PR0=1) right before entering the sleep mode.

It seems that there might be some complex interaction with interrupts and possible the cloud functionality that is causing the issue. As a way to debug, I was wondering if there is a clean way to turn off interrupts and cloud functionality before entering STOP mode.

@ScruffR

This comment has been minimized.

Copy link
Contributor Author

commented Dec 26, 2015

@peterqu, could you also try a Particle.process() instead of delay() or Serial.print()?

avtolstoy added a commit to avtolstoy/firmware that referenced this issue Jan 15, 2016
Fixed system wake up issue particle-iot#655
Initialized all structs in HAL_Interrupts and PinMode method.
Uninitialized GPIO_InitTypeDef struct in pinMode() can cause wake up
pin to be set as  Alternative Function pin.  This will lead to failure
in setting up the interrupt on the wake up pin.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.