Photon freezing during interrupt function (maybe publish and/or other reason)

For some reason, publishing during my code that runs on an interrupt causes the photon to FREEZE, not crash. The breathing cyan stops, and just stays at whatever state of brightness it was, indefinitely. The gist of the code looks as such:

attachInterrupt(highWaterPin, waterNotHighFunc, RISING);

void waterNotHighFunc()
{
    if (millis() - lastTripTime > bounceDelay) {
        lastTripTime = millis();
        Serial.println("water not high");
        waterHigh = false;
        Particle.publish("waterLow", "false");
        detachInterrupt(highWaterPin);
        attachInterrupt(highWaterPin, waterHighFunc, FALLING);
    }
}

The code works perfectly fine without the Particle.publish line. The same Particle.publish seems to work elsewhere in the code, during other parts of the loop (no other blocking code like delay() or anything). Using pins D2/D3 with interrups.

The only way I was able to get it to work was to put the publish in a function, and call it with a timer:

Timer lowTimer(1000, pubWaterLow);

attachInterrupt(lowWaterPin, waterLowFunc, RISING);

void waterLowFunc()
{
    if (millis() - lastTripTime > bounceDelay) {
        lastTripTime = millis();
        Serial.println("water low");
        waterLow = true;
        pinMode(D7, OUTPUT);
        digitalWrite(D7, HIGH);
        detachInterrupt(lowWaterPin);
        attachInterrupt(lowWaterPin, waterNotLowFunc, FALLING);
        lowTimer.start();
    }
}

void pubWaterLow()
{
    Particle.publish("waterLow", boolToText(waterLow));
    lowTimer.stop();
}

I just wanna know…what’s up with that?

waterLowFunc() is running at the interrupt level.
Any interrupt code must be very brief and NOT call libraries. Run time a few microseconds.
It must set flags, etc. for the non-interrupt part of the application to react and process.
Such flags need to be declared volatile.

(PS: Please don’t use pseudo vulgar language in this public place).

4 Likes

@wordsforthewise, I agree with @stevech regarding ISR code though not completely. You can call library functions ONLY if they are non-blocking or very brief. Particle.publish() is not one these. One thing I noticed is the lowTimer.start() call in your ISR. The documentation for Softwate Timers lists the startFromISR() function which is the correct function to call.

As @stevech pointed out, a different approach would be to set a (volatile) flag in your ISR which is read in loop(). When the flag is set, you publish and reset the flag. No need for a timer.

Another thing is the change of interrupt function in the ISR. You may want to consider using a single ISR using attachInterrupt in CHANGE mode. The ISR will be called on a rising or falling event. In the ISR, you can read lowWaterPin and branch your code accordingly - a RISING event will show the pin HIGH and a FALLING event will show the pin LOW.

Finally, again in your ISR, you have pinMode(D7, OUTPUT). Why not set this once in setup()! :smile:

2 Likes

@peekay123, I am observing same issue while just starting timer in ISR code. I found interrupt got fired multiple times (9-10) rapidly and photon got freeze most of the times and timer is not even expiring. Any idea?

Showing your code (or the gist of it) would help.
Also are you sure not to have a floatimg or bouncing pin?

Please post some code and we can try and help. @ScruffR snuck in there too :wink: You get two for one.

It is ok to start a timer in an interrupt, but be aware that the timer is reset each time .start() is called.

start()
Starts a stopped timer (a newly created timer is stopped). If start() is called for a running timer, it will be reset.

You are hopefully using the ISR version of the start() function? Just checking :slight_smile:

So, if your interrupt fires frequently before the timer expires, the timer code will never fire. If you want it to fire regardless of how frequently the interrupt fires, you will need to use a timer flag (or guard). Two issues arise here. The isActive() function does not say it is ISR safe. If we use an external variable in an ISR, it needs to be a special type as well.

The other situation is the ISR might be firing too many times that you could be overflowing the stack also causing a crash. The code within the ISR needs to be compact and fast should this be the case.

Pseudo code:

// Init variables and timers, I am going to make this a one shot timer,
// only to be fired when it is started.
Timer myTimer(10000, myTimer_dostuff, true);
volatile boolean myTimerFlag;

// Timer function
void myTimer_dostuff() {

  // This should be the last thing this function should do as the
  // ISR could fire while this is running.
  myTimerFlag = false;
}

// ISR routine
void myISRcode() {

  // This mistake might be to just call myTimer.startFromISR(); each time
  // which may not let the timer expire.

  // Check our flag to see if we need to start it, otherwise, let it be
  if (!myTimerFlag) {
    myTimerFlag = true;
    myTimer.startFromISR();
  }
}

void setup() {
  // Assume timer is off / stopped at this point
  myTimerFlag = false;
  // Attach our interrupt
  attachInterrupt(D2, myISRcode, CHANGE);
}

void loop() {
   ....
}
1 Like

Thanks all for the reply.

What I am trying here is, simple motion detection as a first step of my project.

Motion detection @ D5 (detect at 3v3)
LED @ D1 (active low)

Before connecting motion detection module I thought of simulating photon app by just giving 3v3 at D5 (momentarily) and turn LED at D1 on for certain duration.

Now, if I could sense motion once I wanted to turn LED on for 10sec and if I detect motion again and before 10sec expiry time I would like to keep LED to remain on for another 10sec. Hence, code suggested by @cermak may not work for me as it ignores interrupts that are firing after first trigger.

Reference Code:

    #include "Particle.h"
    
    #define LED_PIN D1
    #define MOTION_SENSOR_PIN D5
    
    #define dbg Serial.printf
    
    /* Delay before switching off device */
    #define TURN_OFF_DELAY 10000
    
    static void motion_sensor_detection_handler(void);
    static void timer_callback_handler(void);
    
    static Timer device_running_timer(TURN_OFF_DELAY, timer_callback_handler, true);
    
    static void timer_callback_handler() {
      dbg("timer expired\n");
      
      /* turn off LED after specified delay */
      digitalWrite(LED_PIN, HIGH);
    }
    
    static void motion_sensor_detection_handler () {
      dbg("motion detected, restarting timer\n");
    
      /* start or reset the time for specific duration */
      device_running_timer.startFromISR();
    
      /* turn LED on */
      digitalWrite(LED_PIN, LOW);
    }
    
    void setup() {
      
      /* turn off led */
      pinMode(LED_PIN, OUTPUT);
      digitalWrite(LED_PIN, HIGH);
    
      /* attach ISR to sensor pin */
      pinMode(MOTION_SENSOR_PIN, INPUT_PULLDOWN);
      attachInterrupt(MOTION_SENSOR_PIN, motion_sensor_detection_handler, RISING);
    
      Serial.begin();
    }

I thought timer is resetting itself upon calling startFromISR() but looks like, as @cermak mentioned, stack overflow is taking place. But why it should be? starting same timer again and again is part of the same thread I guess.

Maybe I am missing something while dealing with interrupt here or at initialization of interrupt, please suggest.

Thanks.

That code should work. What firmware version do you have on the Photon?

// You will need the Photon is serial mode.
$ particle serial identify

Your device id is 42....
Your system firmware version is 0.6.2

I have a spare Photon to wire up with a push button to see if I can replicate the problem.

1 Like

Would a forum moderator please split this topic. This is a photon freeze, but does not have anything do with the original title (Publish). The original problem was Particle.publish() within an ISR. Thanks!


The sample code above I wired up with a switch to simulate the signal and I did get the Photon to freeze. Is this what you encountered @mehulhirpara?

$ particle compile photon

Compiling code for photon

Including:
    src/stackTest.ino
    project.properties
attempting to compile firmware 
downloading binary from: /v1/binaries/597fc0f670d8122e7ac1268e
saving to: photon_firmware_1501544689961.bin
Memory use: 
   text	   data	    bss	    dec	    hex	filename
   6076	      8	   1472	   7556	   1d84	/workspace/target/workspace.elf

Compile succeeded.
Saved firmware to: /Users/cermak/Particle/projects/stackTest/photon_firmware_1501544689961.bin
RobPro:stackTest cermak$ particle flash --serial photon_firmware_1501544689961.bin ; particle serial monitor --follow
! PROTIP: Hold the SETUP button on your device until it blinks blue!
? Press ENTER when your device is blinking BLUE 
sending file: photon_firmware_1501544689961.bin

Flash success!
Polling for available serial device...
Opening serial monitor for com port: "/dev/cu.usbmodem1421"
Serial monitor opened successfully:
Restarting system to apply firmware update...
Serial connection closed.  Attempting to reconnect...
Serial monitor opened successfully:
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
timer expired
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
motion detected, restarting timer
<freeze> nothing further although I am continuing to press the button

At some point, the Photon stopped responding to the button and the LED went from breathing cyan (normal) to solid cyan. I rewired the D1 to the onboard D7 light. It is off.

@cermak, instead of splitting it off I rather changed the topic title :wink:

Serial is a shared resource, using it in two seperate threads (application & SW timers) and in an ISR is calling for trouble.

You could alter the dbg() function to only fill a string that gets printed from loop() to check the theory.

Also look at these
https://docs.particle.io/reference/firmware/photon/#synchronizing-access-to-shared-system-resources
https://docs.particle.io/reference/firmware/photon/#single_threaded_block-
https://docs.particle.io/reference/firmware/photon/#atomic_block-

2 Likes

@cermak, yes I encountered with same issue.

@ScruffR, that’s correct and I had a same doubt that I should not print anything in ISR. I didn’t try this option as yesterday photon was not moving into flash mode quickly whether I manually do it or from CLI with flash command. It was taking nearly 15-20mins of time between two trails :angry:.

Anyway, let me give it a try tonight and update you all.

1 Like

That could be the freezing issue. I’ll add some guards on the Serial and/or do something more creative. Thanks for the references.

There is a secondary issue. Using the same code, I hit the button once, I can see it went through the ISR, but the timer is not activated. If I hit it a second+ time, then I get the expected timer expired response. I was able to reproduce this twice. I will take some video later.

I will take a look around on the forum to see if anyone has seen something similar.

2 Likes

@cermak, not sure why all the static functions though that won’t hurt anything. I would fix the Serial.print() issue before testing anything else. As @ScruffR suggested, setting a flag in the timer or ISR which is polled/reset in loop() to print an appropriate message is a good approach. With a button generating the interrupts, you may want to implement some debounce code. Speaking of loop(), it may be worth posting that code as well.

1 Like

An interesting issue but my question would be why use an interrupt for this at all?

@peekay123 - Thanks, the original code is below. Will work on getting things moved around. I was kind of curious about the use of static too.

I added the SYSTEM_THREAD(ENABLED); to see if it made a difference and it does not at present, but the Serial is still present. One odd side effect of SYSTEM_THREAD(ENABLED); I have noticed. Upon RESET and the led is in the green flashing state establishing wifi, if I try to particle flash --serial the Photon will go into serial mode briefly and then kick right back out to flashing green and the serial flash will fail. If I let the Photon completely come up into the breathing cyan, then I can do a serial flash.

I am not too worried about the debouce code at this point. One press of the button can generate up to 8 hits to the ISR - I’ve got a noisy button!. This is ok, that would be like a anemometer recording (pulses) counts in a stiff wind. The ISR should be resilient to anything you throw at it?

EDIT: @Viscacha - no, you don’t really need an ISR for what @mehulhirpara is doing, but if you put it in the loop(), then you are retesting the signal every time. Since this is a “motion” detection, the ISR is probably chosen to allow for as quick a detection as possible.

I am planning to deploy a weather station which includes an anemometer, so having this freeze up in an ISR is of some concern.

/*
 * Project stackTest
 * Description:
 * Author:
 * Date:
 */

#include "Particle.h"
SYSTEM_THREAD(ENABLED);
SerialLogHandler logHandler;

#define LED_PIN D7
#define MOTION_SENSOR_PIN D5

#define dbg Serial.printf

/* Delay before switching off device */
#define TURN_OFF_DELAY 10000

static void motion_sensor_detection_handler(void);
static void timer_callback_handler(void);

static Timer device_running_timer(TURN_OFF_DELAY, timer_callback_handler, true);

static void timer_callback_handler() {
  dbg("timer expired\n");
  
  /* turn off LED after specified delay */
  digitalWrite(LED_PIN, HIGH);
}

static void motion_sensor_detection_handler () {
  dbg("motion detected, restarting timer\n");

  /* start or reset the time for specific duration */
  device_running_timer.startFromISR();

  /* turn LED on */
  digitalWrite(LED_PIN, LOW);
}

void setup() {
  
  /* turn off led */
  pinMode(LED_PIN, OUTPUT);
  digitalWrite(LED_PIN, HIGH);

  /* attach ISR to sensor pin */
  pinMode(MOTION_SENSOR_PIN, INPUT_PULLDOWN);
  attachInterrupt(MOTION_SENSOR_PIN, motion_sensor_detection_handler, RISING);

  Serial.begin();

  Log.info("System version: %s", (const char*)System.version());
}

void loop () {
}

@cermak, I prefer to use DFU mode for programming with CLI using particle flash --usb as it is more reliable IMO.

Exactly and thus the reason for debouncing. An ISR is a special event which affects many aspects of the system and thus needs careful design. An anemometer ISR would count pulses whereas a motion detect ISR would detect a state change. A button is not meant to provide a chain of interrupts but many do. If ad-hoc button event detection is necessary, then an ISR with debouncing is good. Otherwise, polling the button in loop(), again with debouncing, is also good. The use and design of the ISR is based on the application.

The reason for NOT using a shared hardware resource in an ISR or Software Timer is due to the interrupting nature of these mechanisms. Imagine doing a Serial.print() in loop and having the ISR interrupt that code and attempt a Serial.print() of its own! Not only is the output interrupted but the print code may not be re-entrant, causing corruption of the output stream. Add to that the fact that while the ISR is running, all same and lower level priority interrupts are blocked until the ISR returns. So when we suggest NOT putting Serial.print() in your code that way, there is a good reason.

If you want robust code, it needs to be designed robustly! :wink:

2 Likes

No update from me so far as I didn’t get any chance to work further.

@peekay123, agree that static declaration is not required as of now, but I used it as I would move motion detection functionality to little lower layer (as a background service) but after resolving current issue.

Also agree with the fact that I should not use shared resources like serial port from ISRs as FreeRTOS might have acquired the lock. Ideally, correct way would be to send a message to main task from ISR handler that interrupt is received and from main task or loop to print whatever is required and also to handle timers in main task. Looks like I am in hurry :runner:, though I am new to arduino programming and FreeRTOS, and didn’t pay attention on correct implementation, my bad. Does ardunio programming provides any signalling or messaging feature. I guess FreeRTOS does but have never worked on FreeRTOS.

Agree with @cermak, I used interrupt to detect motion as quick as possible and with interrupt approach main task could become free to take up some other work rather than polling for an interrupt.

I’ve done some quick testing (using a button to create the interrupts), and the Photon freezes only when there are several very quick calls to the ISR (due to button bounce). This is the code I used in the ISR to test (note that the Serial print is commented out),

static void motion_sensor_detection_handler () {
    char str[20];
    sprintf(str, "%d, ", micros() - lastM);
    lastM = micros();
    strcat(data, str);
  //dbg("MD-TS ");
  device_running_timer.startFromISR();
  digitalWrite(LED_PIN, LOW);
}

Here are a couple of typical strings (data) that result after pushing the button a few times,

11884618, 202885, 3273953, 243618, 23, 520, 21, 20, 19
50119734, 50120186, 50120343, 9241424, 66, 221238, 67, 66, 65

So far, after 6 tests, I see this freeze up only when there are 3 of these short intervals in succession between calls to the ISR (with the intervals being in the 20-70 microsecond range). I have to reset the Photon to look at data since the Photon is frozen, and doesn’t respond to a Particle.variable get.

2 Likes

@Ric, it shouldn’t be required, but could you test this with the ISR code wrapped in an ATOMIC_BLOCK {...}?

1 Like

I tested with the ATOMIC_BLOCK, and I still get the freeze.