Are analogReads() allowed in IntervalTimer (interrupt) routine?

Firmware Libraries
1

I’m going crazy…I believe that analogReads() are allowed in the SparkIntervalTimer ISR routines because I have seen some examples in topics. But it is not working for me…if I include an anlogRead in the ISR, my Photon goes into continual reset mode (blinking red light), restarts and then back to blinking red light.

I must be doing something really stupid…not unusual. This is my first attempt at interrupts and/or software timers. Thanks in advance for any help!

There is only one line of code in the Interrupt Service Routine. Here is my code that works (incrementer instead of analogRead inside the ISR):

#include "SparkIntervalTimer.h"
#define PUBLISH_TIME 1000

IntervalTimer myTimer;

unsigned long currentMillis, prior_publish_time;
uint16_t recent_sample;

void SampleISR(void);

void setup() {
  myTimer.begin(SampleISR, 100, hmSec);
  }

void loop() {
  currentMillis = millis();

// Print the variable from the Interrupt Service Routine every second
  if ((currentMillis - prior_publish_time) >= PUBLISH_TIME) {

    Serial.printlnf("recent_sample: %d", recent_sample);
    prior_publish_time = currentMillis;
  }

}

void SampleISR(void) {
    //recent_sample = analogRead(A0);   //??????
    recent_sample++;
}

But if I switch the recent_sample++ to the commented-out line that includes the analogRead…the device never gets out of reset.

SAMPLE OUTPUT for the incrementer version, successfully increments recent_sample about 20 times per second (every 50ms).

Opening serial monitor for com port: “COM6”
Serial monitor opened successfully:
recent_sample: 1
recent_sample: 20
recent_sample: 40
recent_sample: 60
recent_sample: 80
recent_sample: 100
recent_sample: 119
recent_sample: 139
recent_sample: 159
recent_sample: 179
recent_sample: 199
recent_sample: 218
recent_sample: 238

2

I have tested your code and see the same issue although this should not happen.
But we have seen plenty of instances where some some (alleged) safety improvements in the device OS have broken things that previously worked.
I think this is one such case again.

But as it seems it’s not an issue with analogRead() in ISRs of every kind but in particular timer interrupts. When using the same ISR with a pin interrupt (attachInterrupt()) the SOS+14 (Heap Error) does not happen.

This ugly workaround illustrates that

#include <SparkIntervalTimer.h>

#define WORKAROUND  // comment out this line and the crash comes back

const    uint32_t PUBLISH_TIME = 1000;
volatile uint16_t nSample      = 0;
volatile bool     newData      = false;
IntervalTimer     myTimer;

void SampleISR(void);

void setup() {
#if defined(WORKAROUND)
  pinMode(D1, OUTPUT);
  attachInterrupt(D1, isrSample, CHANGE);
  myTimer.begin(isrToggle, 100, hmSec);
#else
  myTimer.begin(isrSample, 100, hmSec);
#endif
}

void loop() {  
  static uint32_t msPreviousPub = 0;
  uint32_t        msNow         = millis();
  
  if (newData && ((msNow - msPreviousPub) >= PUBLISH_TIME)) {
    newData = false;
    Serial.printlnf("recent_sample: %d", nSample);
    msPreviousPub = msNow;
  }
}

void isrToggle() {
  digitalWriteFast(D1, !pinReadFast(D1));
}

void isrSample(void) {
  nSample = analogRead(A0);
  newData = true;
}

Maybe @peekay123 can chime in on this as the SparkIntervalTimer library is his generous contribution.
And @rickkas7 can maybe comment on the possible reason and how to solve that.

BTW, when using variables in an ISR you should always declare them as volatile.

3

Thanks Scruff…so I am not going crazy. My code extracted above is from a larger “sketch” that is sampling a voltage to determine the power consumed by a pool pump. That “sketch” will be incorporated into my larger “pool controller” project in which most Photon pins are currently used (including D0/D1 for the I2C interface).

Even though I have been working on this project sporadically over the last year and a half, I am still a newbie on just about everything, so bear with me.

I don’t understand the underlying problem (that’s ok) but your workaround seems to “enable/use” D1 as an interrupt/output pin. In our case, it seems that the timer being allocated to our IntervalTimerInterrupt should be Timer3 (the first available from the “pool of timers”)… and Timer3 is usually associated with pins D2/D3. Your workaround uses pin D1 (usually associated with Timer4)…that isn’t intuitive to me … is using that pin for the workaround a requirement or can I use a different pin (one that is currently NOT used in the code’s final destination: the “pool controller” firmware)?

I haven’t tested this: specifying a particular timer for my IntervalTimer…would that make a difference?
EDIT: Doesn’t help…I specified TIMER6 in the myTimer.begin(…TIMER6) and it still does the same reset thing. TIMER6 is not associated with any pin function.

Finally…I don’t really want this thread to get sidetracked off the main issue, but I do have a question about volatility. If an interrupt thread is the only one that “writes” a BYTE variable, and the loop thread only “reads” that BYTE variable…I can’t see that there would ever be an issue with access…or would there? And then this extension: I wouldn’t expect a 2-BYTE (16-bit) variable to ever be written/read as single bytes, so I wouldn’t expect to ever see a problem with access there either (larger variables probably YES). Are these thoughts incorrect?

Thanks, John

4

I do have a question about volatility

The reason for using volatile in front of the type is to tell the compiler to always read the variable from RAM rather than potentially using a cached version of the variable which would not work with an interrupt.

2 Likes
5

It could be any interrupt enabled pin irrespective of the timer used for the timing and also the timer tied to the interrupt pin.
That "workaround" should mainly illustrate that neither the speed of the ISR call nor the actual analog reading is causing the SOS+14 panic - otherwise it should also happen when the timer triggers the pin interrupt which in turn executes the analogRead() instruction.

BTW, @rickkas7 has once provided a project that uses the HW timers to perform analog sampling for audio recording purposes via DMA. This way you can probably circumvent the issue caused by the combination of SparkIntervalTimer and analogRead() (which does more than a single ADC reading)

6

I measure AC current by reading the voltage output by a ACS712 using analogRead() but I do it using a micros() timer compare reading the values into a buffer and all within a no interrupt block - to minimise the time with interrupts ignored - this is necessary. Then all the RMS calculation is done outside the no interrupt block. This is a less troublesome way than ISRs IMHO.

7

Armor…thanks for the input. In general, i have tried to write all my code to be non-blocking, very low latency. These days it’s probably not necessary (processor speed), but I learned what little I know many years ago. I am starting much of of this project with a very low knowledge base, especially the analog “stuff”.

I have a Photon sketch (using an SCT-013 current transformer) that gives me +/-1% power readings for the cases I have checked (which is a caveat). It is sooooo low overhead that I want to pursue it a bit further. For 50/60Hz powered devices, it requires one “fairly” precisely-timed adc sample every 49ms (20 per second) in order to get a (seemingly accurate for my purposes) instantaneous power reading every second.

I have posted this sketch over on the Emon site and rightfully getting some pushback from their experts, most of which is based on accuracy concerns. My testing so far has shown it to be accurate for resistive loads and until I test it on my pool pump, I am unsure about inductive loads. In the end, the main question, for me, is it accurate enough? In my case I am using these power calculations for monitoring and characterization purposes.

BUT…that sketch only works because it is simple, and the samples are taken very close to their expected window using simple loop() timers. There is no way it would work within the framework of my larger pool controller firmware.

I am hoping that interrupts (possibly timers) will get me the accuracy I need to hit those timing windows.

8

How about rectifying and smoothing the signal - this would rid you of the need for precise timing.
Alternatively you could use Rick’s audio sampling approach for continuous measurements at a suitable multiple of your grid frequency and find the peak in the recorded set of samples.

9

Now you are getting into AC stuff…which I understand very little. But…I can see that could be an ideal solution to smooth those pool pump power variations that I expect to see when I “characterize” the power of my pool pump during its In-Floor-Cleaning-Sytem operation.

I would have very little idea how to start that project, especially without an oscilloscope.

In any case, I will to pursue the coding method a bit further using your workaround (on a different pin) and/or the audio example you posted. I move at slow motion these days so I’ll report back on my success.

Thanks for the help…

Hopefully the “real solution” to this problem “analogRead during SparkIntervalTimer ISR routine” will be tracked down as well.

1 Like
10

Hey Scruff...I have looked at this example...at first glance it seems to use exactly the same method we have tried...but I am not a coder so I will start stripping away things to see if I find something I missed at first glance.

EDIT : stripping away stuff to get to bare minimum...do operations with these lines (either here or later variations in the code) accomplish something similar to your workaround?

System.on(button_click, buttonHandler);
pinMode(D7, OUTPUT);

ANSWER: YES TO THE 2nd LINE: pinMode(D7, OUTPUT);

Here is the new code I stripped from rickkas and then changed to make it look like ours. It works as long as I include

pinMode(D7,OUTPUT);

Well...that was unexpected....haha.

#include "Particle.h"
#include "SparkIntervalTimer.h"

IntervalTimer timer;
void timerISR(); // forward declaration

const    uint32_t PUBLISH_TIME = 1000;
volatile uint16_t nSample      = 0;
volatile uint16_t counter      = 0;
volatile bool timer_started = false;

void setup() {
	Serial.begin(9600);
	pinMode(D7, OUTPUT);  // THIS IS THE LINE REQUIRED TO MAKE THIS WORK????
}

void loop() {
	static uint32_t msPreviousPub = 0;
    uint32_t        msNow         = millis();

	if (!timer_started) timer.begin(timerISR, 100, hmSec);

  	if ((msNow - msPreviousPub) >= PUBLISH_TIME) {
    	Serial.printlnf("recent_sample: %d, counter %d", nSample, counter);
    	msPreviousPub = msNow;
	  }
}


void timerISR() {
	nSample = analogRead(A0);
	timer_started = true;
	counter++;
}

SAMPLE OUTPUT
Serial connection closed. Attempting to reconnect...
Serial monitor opened successfully:
recent_sample: 2022, counter 1
recent_sample: 2034, counter 20
recent_sample: 2024, counter 40
recent_sample: 2031, counter 60
recent_sample: 2028, counter 80
recent_sample: 2025, counter 100
recent_sample: 2032, counter 119
recent_sample: 2023, counter 139
recent_sample: 2030, counter 159
recent_sample: 2026, counter 179
recent_sample: 2025, counter 199
recent_sample: 2032, counter 218

11

Sorry, this may have been the wrong repo.
Try this (there are three slightly different approaches)

12

Hey…adding in that one line makes my existing code work, so I am happy. I do have a couple outputs already in my pool controller firmware, so hopefully when I port this sketch to that code, it will still work.

I’m ok with the five samples per analog conversion, especially since I don’t take that many samples.

But, still a bit disconcerting that a seemingly unrelated output port setting makes the interrupts work.

13

That is a very interesting finding and I’d consider this another indication that there actually is some kind of bug going on.

@marekparticle, since Rick doesn’t seem to get the pings, can you take this on please?

14

Odd. SparkIntervalTimer uses the “deepest” (as near to the original interrupt vector as possible) level of interrupt attachment allowed by DeviceOS. It may be that the overhead of the latest analogRead() is causing issues or the under-the-hood work done by the undocumented attachSystemInterrupt() used by SparkIntervalTimer is part of the problem.

One thing I noticed in the docs is that analogRead() takes an average of 5 samples (actually 10 using two ADC channels it seems). This leads to a conversion time of about 80 microseconds. Though the conversion is done using DMA, the surrounding code is not atomic so the actual read could take longer if interrupted by a higher level interrupt. What I find perplexing is that your addition of a single line of code in the ISR “fixes” the problem. I agree with @ScruffR that this shouldn’t happen and suggests a timing issue where the extra line creates a small delay or causes the compiler to change its optimization method for the ISR.

1 Like
15

Peekay…actually the single line of code is in the setup() section…not in the interruption ISR. So yes…even stranger.

16

@peekay123, yup! You only need to add pinMode(D7, OUTPUT) in setup() and the timer interrupt does not cause an SOS+14 anymore.
Very strange indeed.

17

@Jonpcar, that line wasn’t there in your original code. I am not sure why adding pinMode(D7, OUTPUT) would fix the issue. However, you could try forcing the use of specific timer in SparkIntervalTimer, one that’s not tied to any pin functions. That would be Timer7. You would do that using:

timer.begin(timerISR, 100, hmSec, TIMER7);

Try that with and without pinMode() statement.

18

I did try that yesterday with Timer 7, it didn’t work. I think it will work with the “miracle” line of code but i can’t try it now.

The working example started from the rikkas code and I stripped everything out, but decided to leave those two lines of code in that I posted earlier since they were similar to the Scruff workaround. The pinmode() just happened to be it. Actually I had no clue what the buttonhandler was…

19

@ScruffR, @Jonpcar, that is VERY weird! Something isn’t right in DeviceOS land IMO.

1 Like
20

@avtolstoy - would you mind taking a look here? This is unexpected afaik.

@Jonpcar, if you don’t hear back from us soon on this thread, would you mind opening up a support ticket so we can go through the standard bug reporting process?

1 Like