Weird delay problem.

[Pommie]

I've got a project that is currently using a ½ second delay to check timing and other things.
The delay uses a ms counter provided by a timer. The delay code is,

void delay(uint32_t ms){
    uint32_t time;
    time=tickMs+ms;
    while(tickMs<time);
}

tickMs is the interrupt variable.
It works fine most of the time but occasionally the delay is short.
I've got the code lighting an LED and clicking a solenoid.
I find I can hear irregularities much easier than see them.
From startup the short delay is the 5th on cycle which is a count around 9500.
If I set the variable to start at 1000 then the short tick is on the 3rd cycle..
If I set the variable to start at 4096 then the short tick is in the same place (5th cycle).
If I change from delay(500); to __delay_ms(500); then the problem goes away.

I've switched from timer 6 to timer 2.
I've ruled out WDT resets by putting a delay at startup.
I've rewritten the delay code from,

void delay(uint32_t ms){
    uint32_t  previous;
    previous=tickMs;
    while((tickMs-previous)<ms);
}

I'm completely stumped.
Anyone got any idea what may cause this or how to debug it?

Mike.

 

[danadak]

You set the timer variable, and any other used in ISR, as "volatile" ?

volatile c language at DuckDuckGo

DuckDuckGo. Privacy, Simplified.

duckduckgo.com

Regards, Dana.

 

[Pommie]

You set the timer variable, and any other used in ISR, as "volatile" ?

Only the tickMs variable needed to be volatile but unfortunately, that didn't fix it.

Mike.

 

[rjenkinsgb]

Wraparound? eg. If the timer was less that the increment from an overflow, the "timer + delay" comparison value is instantly less than the timer.

eg. Timer 60,000, delay 9000 means timer + delay comparison value is 3463, so less than the timer.

Use signed values & subtract the timer from the comparison, then wait for that to be >= 0?

 

[Pommie]

Wraparound? eg. If the timer was less that the increment from an overflow, the "timer + delay" comparison value is instantly less than the timer.

eg. Timer 60,000, delay 9000 means timer + delay comparison value is 3463, so less than the timer.

Use signed values & subtract the timer from the comparison, then wait for that to be >= 0?

I thought it must be something like that but the relevant variables are 32 bit so it shouldn't wrap around for ~50 days. I'll just change it to != instead of less than.

I striped the code down to it's bare minimum and that fixed it. Must be a timing error but it's still got me stumped.

I just realised what it was. An ISR occurs as the 32 bit variable is being read in the comparison. I'll have to make the read mS atomic. I'll try this tomorrow.

Mike.

 

[rjenkinsgb]

OK.
When I need timers in the non-interrupt part of a program, I include them in the interrupt routine so don't see that type of effect.

eg. Global variables, then in the 1mS part of the interrupt - or whatever stage for the count speed you need; I sometimes use 1mS, 10mS and 100mS ones.

bool         tmr_1_flag;
uint16_t     tmr_1_count;

if (tmr_1_flag) {
    if(tmr_1_count > 0) {
        tmr_1_count--;
    }
    else {
        tmr_1_flag = false
    }
}

Set the counter & set the flag; when the flag is reset, the timer has expired.

 

[Pommie]

I used a simple IRQ flag to indicate that the variable had been updated, ended up with,

uint32_t getMs(void){
    uint32_t retval;
    flagISR=0;
    retval=tickMs;
    if(flagISR)
        retval=tickMs;
    return(retval);
}

And in delay,

void delay(uint16_t ms){
    uint32_t time;
    time=getMs()+ms;
    while(getMs()<time);
}

All good now. Will put the other code back tomorrow (beer time now) and I expect the problem will be gone.

Mike.

 

[danadak]

You also get un-needed stack push calling getMs() inside ISR,
why not use a pointer to the value.

Is "time" a reserved compiler word ?

Not expert here but still "time" not "volatile".....maybe because its local does not matter....

And looping in delay kind of a waste of MIPS.....


Regards, Dana.

 

[Pommie]

getMS() is never called from the ISR. The ISR simply has,

    if(TMR6IF){
        flagISR=1;
        tickMs++;
        TMR6IF=0;
    }

(Yes, I changed back to timer 6)
AFAIA, time is not a reserved word and doesn't need to be volatile as not accessed in the ISR.

Mike.

 

[danadak]

What processor chip being used ?

Regards, Dana.

 

[Pommie]

It's a Pic18F25K22. Probably bigger than I need but can always cut back.

Mike.

 

[danadak]

Were these errata fixed in part you are using, see timer issue :

https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/Errata/80000498K.pdf

Regards, Dana.

 

[tumbleweed]

That's the latest errata list for that part, so all those issues still apply... AFAIK that's the latest silicon rev.

Luckily, I don't think any of those apply to the way Mike's using the timer.

 

[Pommie]

Luckily, only odd numbered timers (I'm using timer 6) are affected.

Mike.