Documentation can be improved on Interrupts

The Archive Community Resources
1

I was just about to post the question below under Firmware when I discovered the actual problem…

DO NOT call delay() from within an interrupt service routine. It will crash the system. This should be added to the documentation for attachInterrupt(), et. al. IMHO.

======================

I’ve got some code which is supposed to count infrared pulses from two electric meters. I’m using TSL267 sensors from Texas Instruments wired to pins D3, D4 with 100k pull-up resistors from 3v3 to the sensor outputs.

The code compiles without issue and flashes just fine. It will run as expected until I flash at the sensors with my TiVO remote (handy flashing IR test source). As soon as I do that, the Photon flashes the SOS T (… — … -) which I’m led to believe means “Hard Fault”.

All of the HTTP stuff has been omitted because it would only confuse things. I get the same behavior even with the HTTP code omitted.

// This #include statement was automatically added by the Particle IDE.
#include "HttpClient/HttpClient.h"

#define SERVER "www.delong.com"
#define PORT 80
#define CGI_PATH "/cgi-bin/post_kwh.cgi"

#define LED D7
#define METER_1 D3
#define METER_2 D4

uint32_t meter_1;
uint32_t meter_2;

char MY_ID[40] = "Invalid_Photon";

#define MY_VER "0.1"
#define NAME_1 "PGE"
#define NAME_2 "SunRun"

#define LOGGING

HttpClient http;
http_header_t headers[] = {
    { "Accept", "*/*" },
    { NULL, NULL },
};

http_request_t request = {0};
http_response_t response;

void setup() {
  pinMode(LED, OUTPUT);
  pinMode(METER_1, INPUT);
  pinMode(METER_2, INPUT);
  (System.deviceID()).toCharArray(MY_ID, 40);
  attachInterrupt(METER_1, m1_ISR, RISING);
  attachInterrupt(METER_2, m2_ISR, RISING);
}

void m1_ISR() {
    meter_1++;
    digitalWrite(LED, HIGH);
    delay(50);
    digitalWrite(LED, LOW);
}

void m2_ISR() {
    meter_2++;
    digitalWrite(LED, HIGH);
    delay(50);
    digitalWrite(LED, LOW);
}

void loop() {
    int result;
    
    meter_1=0;
    meter_2=0;

    delay(60*1000); // Collect 60 seconds worth of pulses
    
    post_pulsecounts(meter_1, meter_2);
    
    
}

void post_pulsecounts(uint32_t m1, uint32_t m2)
{
    // Redacted and not relevant to the problem
}
2

As a general rule, interrupt service routines should be as short as possible, as fast as possible and use as few resources as possible. Never call a library routine from within an ISR unless you are absolutely certain that it is fully reentrant and has no side effects.

3

Excellent advice, but I’ll stand by my statement that having this in the documentation for the interrupt related commands would be helpful.

4

Variables should be declared as volatile if changed within an interrupt.

Also there is loads of info on interrupts in the docs including use of delay() and delaymicroseconds(this works!). millis() also doesnt work within an ISR.

for what you are trying to do, increment the value and set a flag in the ISR - nothing else. in the loop check the flag and flash the LED then set the flag to 0.

I always try and avoid the use of delay, instead use something that checks how much time has passed. there are plenty of examples on the forum of how to do this.

5

Maybe there is, but it’s not on or linked from the page in the Reference docs where one goes looking up things like attachInterrupt() and I didn’t find it in my other searches, either.

As to how to handle it, I actually just updated the ISR to toggle the LED each time it got called and didn’t worry about a delay. For my particular purposes that’s adequate. In the long run, the outward indication (LED blink) is not particularly important. The important thing is the count for each 60 second period getting sent to the web site.

The next steps (now that I have confirmed that the hardware works as intended with a random IR light source) is to do the more elaborate prep of the sensors for outdoor mounting and get them on the meters to confirm functionality in the real world.

Beyond that, I’ll need to eventually shift from random 60 second intervals to clock-synchronized 60-second intervals (IOW, post at the top of each minute rather than post after each 60 seconds). Also, I should create secondary variables for the posting and quickly clear the counters to avoid conflicts between the post and the ISR.

(Generally, the ISR will get called approximately 1-5 times per hour in the real world, but I should clean that up anyway, just for correctness).

I will probably switch to using an interval timer for posting with a loop that doesn’t actually do anything but call delay.

Thanks for the tip about volatile. While I don’t think it would matter much to the current code, it reminded me that I need to copy and clear the counters before posting rather than post them raw.

6

There is info in the docs, its directly above the detach.interrupt() so you need to scroll down a bit from the attach interrupt() section (its still in the attach.interrupt section, just a long way down because of the amount of info).

Have a look at my thread here, i have set up a water usage monitor, it uses interrupts to count pulses from my water meter and publishes them to the cloud. The sleep part doesnt work well with the current firmware, hope they squash the wake up bug soon. but otherwise it works really well. i set the current meter value using a function and it keeps it up to date. next step will be creating a webhook to do something with the data