glitches when using time overflow interrupt

Question

I have some trouble with, I guess, the overflow interrupt (used to increase resolution on 8-bit timer from 16µs/step to 1µs/step) in my CODE. It seems like the overflow interrupt triggers while the program is in the if-statements in my main loop and thereby screws thigns up!

    if(pInt == 1)           //PCNINT-if-statment
    {           
        pulse16 = (tot_overflow << 8) | TCNT1;          //adds tot_overflow and TCNT1 to be able to set if-statements in PCINT-while-loop with µs 

        if(PINB & (1 << PINB3))         //if PB3 is HIGH
        {
            TCNT1 = 0;      //resets Timer/Counter1
            tot_overflow = 0;       //resets tot_overflow variable   
        }

        else        
        { 
            if (pulse16 >1555)          //when stick 1 travels from 1555 µs towards 2006 µs  
            {
                PORTB &= ~(1 << relayPin);        //relay pole switch, + & - on motor 
                PORTB |= (1 << greenLED);        //LED green indicates forward motion
                PORTB &= ~(1 << redLED);        //turn off red LED
            }

                else if (pulse16 <1490)         //when stick 1 travels from 1490 ms towards 920 µs  
                {
                    PORTB |= (1 << relayPin);        //relay pole switch, - & + on motor 
                    PORTB &= ~(1 << greenLED);        //turn off green LED
                    PORTB |= (1 << redLED);         //LED red indicates backward motion
                }   

            else        //if µs is 1490> or <1555 - dead-span to prevent gliteches on relay when stick is in centre position 
            { 

            }   

        }

        pInt = 0;       //resets pInt to exit PCNINT-if-statment   
    }

The pInt is a "flag-variable" that indicates PCINT is triggered. The tot_overflow variable is increment every time the overflow interrupt is triggered.

I use an ATtiny85 as a RC-switch, it should go LOW on PB2-pin when µs from receiver is above 1555 and go HIGH when µs goes below 1490.
What happens is the following: when checking if µs is above 1555 or below 1490 and using overflow interrupt it sometimes turn the PB2-pin HIGH/LOW in the "dead-span" of 1490-1555 when it shouldn't, and sometimes outside the "dead-span"! Here's a VIDEO on the glitch. Notice that the green LED is the redLED, and the yellow LED is greenLED in my code.

I'm quite new at this and I'm not sure why this is happening and I don't understand why the code won't work. I have looked at the CTC function but I can't see that it would help since I still have to use the timer overflow interrupt to get my wanted reolution of 1µs/step.

---

EDIT

I have tried a couple of variations (of @yann-vernier's suggestions) and still don't get it to work properly, this is what gets closest to a working code:

while(1)        //leave and/or put your own code here
{
    static uint8_t tot_overflow;        //variable to count the number of Timer/Counter1 overflows  

    if (TIFR & (1 << TOV1) )
    {
        TIFR |= (1 << TOV1);        // clear timer-overflow-flag
        tot_overflow ++;
    }

    if(GIFR & (1 << PCIF) )         //PCINT-flag idicates PCINT
    {
        uint16_t pulse;         //variable to make a 16 bit integer from tot_overflow and TCNT1  

//          PORTB |= (1 << debugPin);       //pin is HIGH on when interrupt is intialized

        pulse = (tot_overflow << 8) | TCNT1;            //adds tot_overflow and TCNT1 to be able to set if-statements in PCINT-while-loop with µs 

this part I don't get to work:

        if ( ((TIFR & (1 << TOV1)) && ((pulse & 0xff))) < 0x80)
        {
            pulse += 0x100;   // Overflow had not been counted
        }

Im not sure I get what is happening above, the only thing I know is that I probably do it the wrong way! When I comment the above part it works the same as mu old code!

        else
        {
            if(PINB & (1 << PINB3))         //if PB3 is HIGH
            {
                TCNT1 = 0;      //resets Timer/Counter1
                tot_overflow = 0;       //resets tot_overflow variable   
            }

            else        
            {       
                if (pulse > 1555)           //when stick 1 travels from 1555 µs towards 2006 µs  
                {
                PORTB &= ~(1 << relayPin);        //relay pole switch, + & - on motor 
                    PORTB |= (1 << greenLED);        //LED green indicates forward motion
                    PORTB &= ~(1 << redLED);        //turn off red LED
                }

                    else if (pulse < 1490)          //when stick 1 travels from 1490 ms towards 920 µs  
                    {
                        PORTB |= (1 << relayPin);        //relay pole switch, - & + on motor 
                        PORTB &= ~(1 << greenLED);        //turn off green LED
                        PORTB |= (1 << redLED);         //LED red indicates backward motion
                    }   

                else        //if µs is 1490> or <1555 - dead-span to prevent gliteches on relay when stick is in centre position 
                { 
//                  PORTB |= (1 << greenLED);           //for debug to indicate dead-span   
//                  PORTB |= (1 << redLED);         //for debug to indicate dead-span   
                }   

            }

        }

        GIFR |= (1 << PCIF);    //clear PCINT-flag  
    }

    else
    {

    }

}

}

ISR(TIMER1_OVF_vect)            //when Counter/Timer1 overflows  
{

}

ISR(PCINT0_vect)        //when pin-level changes on PB3
{ 

}

Is it close or am I still out in the blue?

Answer 1

@YannVernier Thanks for pushing me in the right direction, or giving me the rigth way to do it! ;-) I think I finally nailed it, with a litle extra help from a friend that is!
Here is the final CODE I didn't first get that I had to remove the TIMSK enable ande sei() plus the ISR routines, also the else-statement that was accidently put after:

    if ( ((TIFR & (1 << TOV1)) && ((pulse & 0xff))) < 0x80)
    {
        pulse += 0x100;   // Overflow had not been counted
    }

Answer 2

Yes, the overflow interrupt could happen at any time (although it does happen regularly). So could the pin change interrupt. Each of them in this case only touch a single 8-bit volatile variable (tot_overflow and pInt respectively), which in turn are polled by your main loop. This construction doesn't really help you unless the main loop has other work to do which may take longer than one full timer period (256*8=2048 cycles in this case).

So instead of enabling interrupts, you could check GIFR bit PCIF (instead of pInt) and TIFR bit TOV1 in your main loop, which would get you more easily understood behaviour. Resetting them is a special case of writing a 1 to that bit and only that bit. This would save you time for the pin change interrupt, as the main loop check could happen more frequently and would not need the latency of jumping to the interrupt service routine.

It would not, however, fix your problem. You are trying to measure a pulse width, which on slightly larger AVRs is easily done using the timer input capture feature. The timers in the ATtiny85 don't have this feature. On AVRs that do, the input capture and timer overflow interrupts are arranged in such a way that the input capture interrupt can safely read the software driven overflow counter.

When you get a timer overflow, you increment tot_overflow, and when you've detected a pin change, you read TCNT1 to combine the values. These two counters, while one feeds the other, are not read at the same time. Your threshold values are 0x5d2 and 0x613. If the rollover occurs after reading tot_overflow but before reading TCNT1, you may well get a time like 0x501 at a time which should be 0x601. Similarly if you stop tot_overflow from updating. If you read TCNT1 before tot_overflow, you might get 0x6fe when you should have read 0x5fe. Simply put, there is no safe order - but there is a relation. What we need to know is if the overflow count value was up to date or not at the time the counter value was read.

static uint8_t ovf;
if (TIFR & 1<<TOV1) {
   TIFR = 1<<TOV1;   // Clear overflow flag
   ovf++;
}
if (GIFR & 1<<PCIF) {
   GIFR = 1<<PCIF;   // clear pin change flag
   uint16_t timestamp = ovf<<8 | TCNT1;
   if (TIFR&1<<TOV1 && (timestamp&0xff)<0x80)
       timestamp += 0x100;   // Overflow had not been counted
   // Do what we like with the timestamp here
}

The key is that we check for an overflow after we have already loaded the timer value. If the overflow occurred before we read the value, the value read must be low; otherwise we want the old count. This particular sample actually relies on the overflow not being handled in an interrupt, but you can use the same method by enclosing the block in an interrupt masking. We need the two reads of TOV1 and ovf to match, and to read TOV1 after TCNT1. What we don't want is to have an interrupt process and thus clear TOV1 so that we can't infer the order of ovf and TCNT1. Note that using a pin change interrupt to do this logic grants us that automatically, since interrupt handlers run with interrupts disabled.

You won't get higher precision on your pulse widths than the latency variance for responding to the pin change, and in the code you've shown, the timer reset (which should also reset the prescaler, bit PSR1 in GTCCR). This usually means you do want to read the timer in the interrupt handler itself. We can also observe that you could choose a timer pace that makes your thresholds fit in the 8 bit value; for instance with timer1 running at 8MHz/64, your thresholds would be at 186 and 194, with an offset of 16-24µs. One might even do tricks like setting one threshold precisely at an overflow since the timer doesn't have to start at 0.