Bizarre behavior between ISR and loop()

I am using the following code in a photon:

volatile unsigned long h = 0;
volatile unsigned long t = 0;
volatile unsigned long t_last = 0;
volatile unsigned long hist[32];

void ISR (void)
{
    unsigned long now = micros();
    hist[h] =  now - t_last;
    t_last = now;
    h = (h + 1) & 31;
}

void loop (void)
{
    unsigned long hh;
    unsigned long ht;
    unsigned long now;

    noInterrupts();
    hh = h;
    ht = t_last;
    now = micros();
    interrupts();

    if (now - ht >= 0x80000000UL) {
        Serial.println("Error!  Time has moved backwards!");
    }
}

Every now and then, I see the error message.

The ISR is setup to trigger on the falling edge of a PWM signal with a frequency of about 27.5Hz (or a time interval of 36.36… msec). Certainly t_last cannot be changing in between noInterrupts() and interrupts().

So how is it possible ever that t_last could be greater than now?

I’m not showing the entire picture, such as what’s being done with the hist[] samples, since it’s irrelevant to the error message.

Background: I’m not at all new to embedded systems.

I’m thinking it’s because micros() does not wrap at an unsigned long value like millis() does. It wraps at 59,652,323 microseconds, so you need to explicitly take care of wrapping with micros.
https://docs.particle.io/reference/firmware/photon/#micros-

Note that the wrapping only applies to 0.4.3 and earlier. Later versions, micros() will increment through the full 32-bit value space.

noInterrupts() only disables registered interrupts for pins - system interrupts are still enabled. If you want to disable all interrupts, use an ATOMIC_BLOCK()

I discovered that original issue and posted it here.
I’m aware that recent versions don’t have that issue.
Thanks.

The interrupt is off of a GPIO pin (D2), so that really should do the job.

I added ATOMIC_BLOCK() { } around all the sections where I had noInterrupts(); … interrupts();

I still see the same behavior.

Anyone have any ideas?

Also, something interesting, in hex the value is one of three: 0xffffffc2a, 0xfffffc27 or oxfffffc31.

I find that odd.

@mgssnr, can you post your setup() code?

I think this one deals with the same

1 Like

Thanks for the response.

The relevant bits of the setup() code would be that I’m setting the ISR to trigger on the falling edge of D2.

I’ll post it this evening.

Yes, I saw this, and it looks similar.
I didn’t see that the other post had disabled interrupts, and I thought perhaps it was somehow different.

For what it’s worth, I think the implementation of micros() is the problem.

It would be better to have a timer tick away at the proper rate and micros() simply return the value of the timer, and have the timer automatically reset.

Depending on variables to be updated can be part of the problem.

When I’ve implemented such things on my own in the past, I set a timer up to run at the desired rate, and then read the timer, instead of having to rely on software. I may have to look into doing the same thing here. To that end, is there a list of the available (unused) timers in the Photon software?

Yup, this was how I thought micros() worked by just using system ticks to calculate a decadic fraction of a second.
And I hoped @mdma would come back to the other thread …

In 0.6.0 millis() and micros() will have a thread-safe block to prevent inconsistent computations, and millis() will tolerate having the SysTick interrupt stalled.

1 Like

Thanks, mdma.

When will 0.6.0 be released?

And, out of curiosity, why not just set up timers to tick off at microseconds and milliseconds? Don’t have the timers available?

We’ll have a pre-reelase for 0.6.0 available within a couple of weeks, the final release will depend upon feedback and how many iterations are needed to reach a stable release candidate.

SysTick does tick off at a millisecond, but I believe this is set at lower priority so that it doesn’t displace urgent application interrupts which may need low latency.

Thanks for the info on 0.6.0.

So SysTick is a timer register you read, not a counter that gets incremented every millisecond?

Why not do the same with micros()?

Thanks.

Because of the overhead - calling an ISR every microsecond to increment a counter would be wasteful of resources.

The mcu is already keeping track of a counter for each cpu cycle, so it’s more efficient to use that and compute the elapsed micros on demand.

I wasn’t suggesting a software tick based on a 1MHz timer. That’s nuts.

I am suggesting putting a free running timer in a mode such that it increments or decrements at a rate of 1MHz. That would give a 1MHz counter value that would simply count up or down, and it could be read at any time.

The simple answer is that we can do this without tying up a hardware timer resource, so that’s the approach we take.