ATTiny84A - TIMER 0 flash question

Go To Last Post
23 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

High all:

 

I'm diving into AVR Timers and Interrupts with a little project.  To learn a bit more about the various timers, settings, and interrupts I used an ATTiny84A running 5 VDC on the Internal 8 MHz oscillator...CLOCK/8 fuse set.  So the device is running 1 MHz.  Fuses are:

 

EX: 0xFF   HIGH: 0xDF   LOW: 0x62

 

Here is the code I'm using for this test:

 

#define F_CPU 1000000UL

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>

volatile uint16_t intPulseCount;
int intFlash;

ISR(TIM0_COMPA_vect)
{
	intPulseCount++;
}

int main(void)
{
	DDRB  |= (1<<PORTB0)|(1<<PORTB1)|(1<<PORTB2);	//- Set B0, B1, and B2 as OUTPUT	

	//- Set up TIMER0 - 8-bit - Output Flash Timer
	TCCR0A = (0<<WGM02)|(1<<WGM01)|(0<<WGM00)|		//- Timer 0: CTC Mode (Clear Timer on Compare Match)
	         (0<<COM0A0)|(0<<COM0A1);				//- Output compare disable
	TCNT0 = 0;										//- Initial timer value
	OCR0A  = 250;									//- Timer 0: 0.002 second period
	TIMSK0 = (0<<OCIE0B)|(1<<OCIE0A)|(0<<TOIE0);	//- Timer 0: Output Compare Match A Interrupt Enabled
	TCCR0B = (0<<CS02)|(1<<CS01)|(0<<CS00);			//- Timer 0: Clock with 8 prescaler & start clock

	intPulseCount = 0;
	intFlash = 0;

	sei();											//- Enable Global Interrupts

	PORTB |= (1<<PORTB2);							//- Turn on power LED (3)
	PORTB |= (1<<PORTB1);							//- Turn on LED 2
	PORTB &= ~(1<<PORTB0);							//- Turn off LED 1

    while(1)
    {
		if (intPulseCount >= 5000)
		{
			ATOMIC_BLOCK(ATOMIC_FORCEON)
			{
				intPulseCount = 0;
			}

			if (intFlash == 1)
			{
				intFlash = 0;
				PORTB &= ~(1<<PORTB0);
			}
			else
			{
				intFlash = 1;
				PORTB |= (1<<PORTB0);
			}
		}
    }
}

 

It's almost there if I could figure out one issue: the LED flash isn't what I expect.  I expected it to be on for half a second, then off for half a second.  What I seeing is on for a quarter second, off for three quarters of a second.  While that isn't bad, I want to understand why it's different from what I expect to see.

 

Here is an example Video on YouTube of what I am seeing.

 

I've moved the ATOMIC_BLOCK to encompass only the "intPulseCount = 0" inside the main loop.  When I try to change the prescaler and OCR0A to another value that would provide the same timing, the flash patter will change a bit.  The overall cycle is 1 second, but sometimes I get a flash for half a second, then a quick "blip" flash...and that repeats.

 

I'm thinking it has to do with the interrupt being thrown faster than the main loop is doing its thing, but I'm not sure.  250 clock pulses goes by pretty fast.

 

Any nudge in the right direction would be appreciated.

 

Thanks.

 

-Joe

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Check your braces { } again, to see if they really line up where you think they should.

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

theusch wrote:

Check your braces { } again, to see if they really line up where you think they should.

 

Hmmmm....I don't see a problem with them.  I know the formatting was a little screwy when I Copy-n-Pasted the code over.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It may not make any difference but I can't see a power bypass cap anywhere (100nF), maybe it's under the board.

 

Without bypass cap the processor could reset randomly.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If the micro was resetting randomly the other LEDs would turn off, I believe.  But it was a good find so I put one on and tried it again.  No joy...

 

Something else really strange...

 

If I put the code like this:

 

.
.
.
DDRB |= (1<<PORTB0)|(1<<PORTB1)|(1<<PORTB2);	//- Set B0, B1, and B2 as OUTPUT

//- Set up TIMER0 - 8-bit - Output Flash Timer
TCCR0A = (1<<WGM01);				//- Timer 0: CTC Mode (Clear Timer on Compare Match)
TCNT0 = 0;					//- Initial timer value
OCR0A  = 250;					//- Timer 0: 0.002 second period
TIMSK0 = (1<<OCIE0A);				//- Timer 0: Output Compare Match A Interrupt Enabled
TCCR0B = (1<<CS01);				//- Timer 0: Clock with 8 prescaler & start clock
sei();					        //- Enable Global Interrupts

intPulseCount = 0;
intFlash = 0;	

PORTB |= (1<<PORTB2);				//- Turn on power LED (3)
PORTB |= (1<<PORTB1);				//- Turn on LED 2
PORTB &= ~(1<<PORTB0);				//- Turn off LED 1
.
.
.

...the LED still does the 25%-on/75%-off flash, but it is flashing.  If I change the code to this...

 

.
.
.
DDRB |= (1<<PORTB0)|(1<<PORTB1)|(1<<PORTB2);	//- Set B0, B1, and B2 as OUTPUT	

PORTB |= (1<<PORTB2);				//- Turn on power LED (3)
PORTB |= (1<<PORTB1);				//- Turn on LED 2
PORTB &= ~(1<<PORTB0);				//- Turn off LED 1

//- Set up TIMER0 - 8-bit - Output Flash Timer
TCCR0A = (1<<WGM01);				//- Timer 0: CTC Mode (Clear Timer on Compare Match)
TCNT0 = 0;					//- Initial timer value
OCR0A  = 250;					//- Timer 0: 0.002 second period
TIMSK0 = (1<<OCIE0A);				//- Timer 0: Output Compare Match A Interrupt Enabled
TCCR0B = (1<<CS01);				//- Timer 0: Clock with 8 prescaler & start clock
sei();						//- Enable Global Interrupts

intPulseCount = 0;
intFlash = 0;	
.
.
.

...the green LED comes on and stays on.  ?!?  What am I missing here?  I wouldn't think the order of where the output settings are would make the difference between it work and not working.  I pulled the code file into Notepad++ to make sure I don't have some sort of hidden control character in there or something.  Notepad++ also seems to show that the braces are lined up as I think they should be.  I'm not sure what I'm missing here...

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If the micro was resetting randomly the other LEDs would turn off, I believe.

But it would only be for microseconds so you would not see it as they would get turned on again at the beginning of the code. Sorry I tried laugh

 

Of course if you had a debugger you could put a breakpoint at the start of the code to see if a reset did happen.

 

Also I don't know why you use an int (16bit signed) for

int intFlash

when a uint8_t would do.

 

Unfortunately I don't have a T84 to test you code.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dataweasel wrote:

DDRB  |= (1<<PORTB0)|(1<<PORTB1)|(1<<PORTB2);  //- Set B0, B1, and B2 as OUTPUT
//           ^^^^^^      ^^^^^^      ^^^^^^
// should be DDB0        DDB1        DDB2

TCCR0A = (0<<WGM02)|(1<<WGM01)|(0<<WGM00)
//           ^^^^^
//           WGM02 resides in TCCR0B

OCR0A  = 250;  //- Timer 0: 0.002 second period
//       ^^^
//       249 (subtract one from desired count)

TCCR0B = (0<<CS02)|(1<<CS01)|(0<<CS00);  //- Timer 0: Clock with 8 prescaler & start clock

// with a 1MHz clock, divide-by-8 prescaler, and a count of 250, your
// interrupt should fire 500 times per second

while(1)
{
    if (intPulseCount >= 5000)
    {
        // the code in here should run only once every 10 seconds!
    }
}

 

Since the LED should be toggling only once every ten seconds something else

is going on, not a problem that can be solved with different code.

 

--Mike

 

EDIT: #9 caught something I missed which could explain things - accessing a

16-bit variable without disabling interrupts first

Last Edited: Tue. Oct 9, 2018 - 05:44 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I modified the code to run on a Mega88PA (same timer type) and changed the flashing led to PB1 as that's where the led is on my board.

 

The code runs OK however your expectations of " be on for half a second, then off for half a second " is not correct, it is more like 10 seconds as the post above suggests.

 

I had to change the clock not to be divided by 8 and changed the OCRA to 100, it now flashes at about 500ms.

 

/*
 * dataweasel.c
 *
 * Created: 9/10/2018 2:18:05 PM
 * Author : John Samperi
 */ 

#define F_CPU 1000000UL

#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>

volatile uint16_t intPulseCount;
int intFlash;

//ISR(TIM0_COMPA_vect)

ISR (TIMER0_COMPA_vect)
{
	intPulseCount++;
}

int main(void)
{
	DDRB  |= (1<<PORTB0)|(1<<PORTB1)|(1<<PORTB2);	//- Set B0, B1, and B2 as OUTPUT	

	//- Set up TIMER0 - 8-bit - Output Flash Timer
	TCCR0A = (0<<WGM02)|(1<<WGM01)|(0<<WGM00)|		//- Timer 0: CTC Mode (Clear Timer on Compare Match)
	         (0<<COM0A0)|(0<<COM0A1);				//- Output compare disable
	TCNT0 = 0;
										//- Initial timer value
//	OCR0A  = 250;									//- Timer 0: 0.002 second period
	OCR0A  = 100;

	TIMSK0 = (0<<OCIE0B)|(1<<OCIE0A)|(0<<TOIE0);	//- Timer 0: Output Compare Match A Interrupt Enabled
//	TCCR0B = (0<<CS02)|(1<<CS01)|(0<<CS00);			//- Timer 0: Clock with 8 prescaler & start clock
	TCCR0B = (1<<CS00);								//- Timer 0: Clock start clock

	intPulseCount = 0;
	intFlash = 0;

	sei();											//- Enable Global Interrupts

	PORTB |= (1<<PORTB2);							//- Turn on power LED (3)
	PORTB |= (1<<PORTB1);							//- Turn on LED 2
	PORTB &= ~(1<<PORTB0);							//- Turn off LED 1

    while(1)
    {
		if (intPulseCount >= 5000)
		{
			ATOMIC_BLOCK(ATOMIC_FORCEON)
			{
				intPulseCount = 0;
			}

			if (intFlash == 1)
			{
				intFlash = 0;
//				PORTB &= ~(1<<PORTB0);
				PORTB &= ~(1<<PORTB1);
			}
			else
			{
				intFlash = 1;
//				PORTB |= (1<<PORTB0);
				PORTB |= (1<<PORTB1);
			}
		}
    }
}

 

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't know if this is causing the problem you're seeing, but the following is wrong:

 

		if (intPulseCount >= 5000)

It is wrong because you are not protecting the 16-bit variable access from being corrupted by ISR modifications.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

But he is using

ATOMIC_BLOCK(ATOMIC_FORCEON)

and I have verified it working correctly but with a different chip.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

js wrote:

But he is using

ATOMIC_BLOCK(ATOMIC_FORCEON)

and I have verified it working correctly but with a different chip.

He's not using it when he reads the value in order to compare with 5000.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's the example given in the online user manual, that correctly reads the shared variable:

 

#include <inttypes.h>
#include <avr/interrupt.h>
#include <avr/io.h>
#include <util/atomic.h>
volatile uint16_t ctr;
ISR(TIMER1_OVF_vect)
{
  ctr--;
}
...
int
main(void)
{
   ...
   ctr = 0x200;
   start_timer();
   sei();
   uint16_t ctr_copy;
   do
   {
     ATOMIC_BLOCK(ATOMIC_FORCEON)
     {
       ctr_copy = ctr;
     }
   }
   while (ctr_copy != 0);
   ...
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Even so the variable can only be a few counts above, if anything, and that would be a valid comparison on >=5000.

 

So I guess he can move the line one up.

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

js wrote:

Even so the variable can only be a few counts above, if anything, and that would be a valid comparison on >=5000.

 

So I guess he can move the line one up.

I agree I don't see how it could make a huge difference.  I would expect at most an error of 0x100 which is only about 5% of 5000.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:

Here's the example given in the online user manual, that correctly reads the shared variable:

 

   do
   {
     ATOMIC_BLOCK(ATOMIC_FORCEON)
     {
       ctr_copy = ctr;
     }
   }
   while (ctr_copy != 0);
   ...
}

 

 

Okay...so use the ATOMIC_BLOCK to protect a quick copy of the value, then do the comparison with the copy?  This is similar to one place I read that has the ISR calling a function with a static variable or something like that.  Does anything change if I were to change the 16-bit variable to an 8-bit?  I know it takes more clock cycles to manipulate a 16-bit value.

 

-Joe

Last Edited: Tue. Oct 9, 2018 - 01:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not relevant to >>this<< problem, because you're setting it to '0' here, but WGM02 is not in TCCR0A.  It's in TCCR0B.

 

Also, as @avr-mike points out, the timing you're expecting (half-second on, half-second off) doesn't match the code.  At 1 MHz, with a prescaler of /8 and a period of 251 (note it is not 250, since in CTC mode the top value  must be 1 less than the period you desire), the timer frequency should be 1000000 / (8 * 251) = 498.007968127 Hz.  If you're counting 5000 overflows, then the LED should toggle every 10.039999962 seconds, not every half second.  How did you arrive at that figure?  Show your math.

 

I agree, something else is going on.  Try this sanity check:

#define F_CPU 1000000UL

#include <avr/io.h>
#include <util/delay.h>

int main(void) {

  DDRB |= (1 << PORTB0) | (1 << PORTB1) | (1 << PORTB2);

  PORTB |=  (1 << PORTB2);
  PORTB |=  (1 << PORTB1);
  PORTB &= ~(1 << PORTB0);

  while (1) {
    _delay_ms(500);
    PORTB &= ~(1 << PORTB0);
    _delay_ms(500);
    PORTB |= (1 << PORTB0);
  }

}

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"Read a lot.  Write a lot."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dataweasel wrote:

Okay...so use the ATOMIC_BLOCK to protect a quick copy of the value, then do the comparison with the copy?  This is similar to one place I read that has the ISR calling a function with a static variable or something like that.  Does anything change if I were to change the 16-bit variable to an 8-bit?  I know it takes more clock cycles to manipulate a 16-bit value.

Yes, grabbing a copy (using an atomic access) is the standard procedure.  If you use an 8-bit variable the problem goes away, because neither the variable read nor the setting of the variable to 0 (or any value) is now interruptible.  To summarize (for AVR or any other 8-bit device that I know of):

 

Read a variable:

8 bit is not interruptible

>8 bit is interruptible

 

Set a variable:

8 bit is not interruptible

>8 bit is interruptible

 

RMW (read-modify-write, e.g. counter++ or counter += 10):

Any size (8 bit or larger) is interruptible

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:

 

Read a variable:

8 bit is not interruptible

>8 bit is interruptible

 

Set a variable:

8 bit is not interruptible

>8 bit is interruptible

 

 

Okay...good info.  So I should ATOMIC block for the comparison on any IF statements as well.  Basically, if I even think about the variable I should ATOMIC block it.  I think I am also going  to also rework the code to get rid of the 16-bit variable in this area.

 

As for the math for the flash frequency...I was looking at it from the clock period being set to 0.002 second and 5000 overflows would be the 1 second.  It sure looks like 1 second right now.  I'm going to rework the math using the information above and check my timing.

 

Everyone...I appreciate the learning experience.

 

-Joe

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Dataweasel wrote:
Basically, if I even think about the variable I should ATOMIC block it.

Only those that are shared between ISR() and main()!

 

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:

 

I agree, something else is going on.  Try this sanity check:

 

 

Your code works exactly as I was expecting my code to work.  50% on, 50% off.  Going to look into the WGM02 item.

 

-Joe

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your code works exactly as I was expecting my code to work. 

So does YOUR CODE I modified above https://www.avrfreaks.net/commen...

John Samperi

Ampertronics Pty. Ltd.

www.ampertronics.com.au

* Electronic Design * Custom Products * Contract Assembly

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just looked at <util/atomic.h> and didn't understand the code.

You could replace the ATOMIC_BLOCK with the following and

see if things improve:
 

    while(1)
    {
        cli();
        if (intPulseCount >= 5000)
        {
            cli();
            {
                intPulseCount = 0;
            }
            sei();

            if (intFlash == 1)
            {
                intFlash = 0;
                PORTB &= ~(1<<PORTB0);
            }
            else
            {
                intFlash = 1;
                PORTB |= (1<<PORTB0);
            }
        }
        sei();
    }

 

The above is just a test, I wouldn't actually write the code this way.

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-mike wrote:
The above is just a test, I wouldn't actually write the code this way.
Realise that but possibly better is:

    while(1)
    {
        uint8_t old_sreg = SREG;
        cli();
        if (intPulseCount >= 5000)
        {
            cli();
            {
                intPulseCount = 0;
            }
            SREG = old_sreg;

            if (intFlash == 1)
            {
                intFlash = 0;
                PORTB &= ~(1<<PORTB0);
            }
            else
            {
                intFlash = 1;
                PORTB |= (1<<PORTB0);
            }
        }
        SREG = old_sreg;
    }