Why this LED blinking code doesn't work?

Go To Last Post
79 posts / 0 new

Pages

Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

HI!
I wanna make a LED blinking than if I press the PB0 the blinking should accelerate any time I press the PB0.

here is my code what I wrote, but I can't realize the problem why my LED won't accelerate when I press the PB0 button.

 

#include <avr/io.h>
#define F_CPU 1000000UL 
#include <util/delay.h>

int PB0_bounce_counter = 0;	
int PB1_bounce_counter = 0;	
int Debounce = 50; 
int Delay = 500; 


void Wait( int ms_sec )
{
	int i;

	for( i = 0; i < ms_sec; ++i )

	_delay_ms( 1 );
}


int main(void){
	
    
	DDRB = (1<<PB1) | (1<<PB2) | (1<<PB3) | (1<<PB4);
	PORTB = (1<<PB1) | (1<<PB2) | (1<<PB3) | (1<<PB4);
	_delay_ms(2500);
	
	PORTB &=~ (1<<PB1) & (1<<PB2) & (1<<PB3) & (1<<PB4);
			
   while (1) 
  
    {	

	if(PINB & (1<<0))
	{
        	PB0_bounce_counter++; 
			
			if (PB0_bounce_counter==Debounce)
			{
	        		Delay = Delay - 50 ;
			}
			
	}
	
	else
	{
       		PB0_bounce_counter = 0;
	}
		
		
	PORTB = (1<<4);
	Wait(Delay);
	
	PORTB &=~(1<<4);	
	Wait(Delay);
		
    }
}

Actually, the LED is blinking but on a constant frequency.

 

Any suggestions pls.

Thank you.

Last Edited: Wed. Sep 12, 2018 - 09:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Show your connections.  Check the voltage right at the AVR pin for the button.

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

It looks like you need to hold the button down for 50 seconds before the delay gets smaller.

 

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

Show your connections.  Check the voltage right at the AVR pin for the button.

I use the original EasyAVR5 dev. board, not my wiring up etc...

 

 

 

It looks like you need to hold the button down for 50 seconds before the delay gets smaller.

 

 

I don't think it has anything to do with seconds of waiting.

I don't set up any seconds etc.

The 50 cycles should count the switch bouncings between the pressed and released state.

I assume after 50 bouncing the switch is released physically.

 

I played around again and I figured out actually maybe I missing the keypress reading, the keypress reading is inaccurate.

 

Here is a bit modded code for testing if my code is working, and yes, this code is working but the PB4 is not blinking.

My goal should be to have two switches and one blinking LED.

With the switches, I should manipulate the blinking time of the LED.

 

This is the modded code what is working but not as it should:

 

#include <avr/io.h>
#define F_CPU 1000000UL 
#include <util/delay.h>

int PB0_bounce_counter = 0;	
int PB1_bounce_counter = 0;	
int Debounce = 50; 
int Delay = 500; 


void Wait( int ms_sec )
{
	int i;

	for( i = 0; i < ms_sec; ++i )

	_delay_ms( 1 );
}


int main(void){
	
    
	DDRB = (1<<PB1) | (1<<PB2) | (1<<PB3) | (1<<PB4);
	PORTB = (1<<PB1) | (1<<PB2) | (1<<PB3) | (1<<PB4);
	_delay_ms(2500);
	
	PORTB &=~ (1<<PB1) & (1<<PB2) & (1<<PB3) & (1<<PB4);
			
   while (1) 
  
    {	

	if(PINB & (1<<0))
	{
        	PB0_bounce_counter++; 
			
			if (PB0_bounce_counter==Debounce)
			{
	        	//	Delay = Delay - 50 ;
	        	PORTB ^=(1<<1);
			}
			
	}
	
	else
	{
       		PB0_bounce_counter = 0;
	}
		
	/*	
	PORTB = (1<<4);
	Wait(Delay);
	
	PORTB &=~(1<<4);	
	Wait(Delay);
	*/	
    }
}

 

If I let the PB4 blinking part live in the code the keypress won't be recognized.

I mean this part at the end of the code:

 

    PORTB = (1<<4);
	Wait(Delay);
	
	PORTB &=~(1<<4);	
	Wait(Delay);

What did I do wrong?

 

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

In my experience, the best way to de-bounce a switch is to use a timer, as the number of bounces per switch closure can vary.   Can you put a scope in the input pin to verify the actual timing?

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

You have not enabled pull-up on PB0. Is the button connected to GND or to VCC?

 

If you want react to button immediately during function Wait() then try

 

void Wait( int ms_sec )
{
int i;

    if (! (PINB & (1<<0)))  {break;}  // if button pressed then get out of the function

	for( i = 0; i < ms_sec; ++i )
	_delay_ms( 1 );
}

 

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

Let's see how PB0_bounce_counter counts up after the switch opens in the # 1 program.

 

Loop 1st time: (time = 0 sec)
PB0_bounce_counter++ is executed. (PB0_bounce_counter = 1)
The LEDs are turned on and off. (It takes about 1 sec)

Loop 2nd: (time = 1 sec)
PB0_bounce_counter++ is executed. (PB0_bounce_counter = 2)
The LEDs are turned on and off. (It takes about 1 sec)

 

Loop 50 th time: (time = 49 sec)
PB0_bounce_counter++ is executed. (PB0_bounce_counter = 50)
Delay = Delay - 50 is executed.
The LEDs are turned on and off. (It takes about 1 sec)

Loop 51 th time: (time = 50 sec)
PB0_bounce_counter++ is executed. (PB0_bounce_counter = 51)
Delay = Delay - 50 is executed. (Delay = 450)
The LEDs are turned on and off. (It takes about 1 sec)

 

Loop 61 th time: (time = 60 sec)
PB0_bounce_counter++ is executed. (PB0_bounce_counter = 61)
Delay = Delay - 50 is executed. (Delay = -50)
The LEDs are turned on and off. (It takes about 1 sec)

Since it no longer has a flashing interval, it can not recognize blinking.

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

This is a remarkably simple piece of code for someone who's on this forum for 8 years and has 150 posts to his/her name.

I wonder what's the story behind that...

 

But more to the point.

The whole approach to the problem is sort of flawed.

It lacks structure and invites for an ad-hoc addition of random bits and pieces, which leads to an un maintainable mess.

So instead of letting myself being lured into that I'll give some general / extendable structure to build some projects around.



void main( void) {
    
    Timer_Initialize();     // This should generate a regular (for example 1ms) timer tick.
    
    for( ;;) {
        
        while( !Timer_Event( )) // Should be true every timer tick (1ms).
            ;
            
        Scan_Keys( );
        Handle_Leds( );
    }
}

 

The idea is to make your whole program a slave to the same regular timer tick.

In ARM hardware it is very common to have a hardware timer on the silicon which is especially designed for this task.

It is a very simple timer, and it can not do much more than generate periodic interrupts.

This saves silicon area compared to a lot of other uC families where a full blown complex timer is used for this simple task.

 

But the main part is that it divides and separates your program into separate blocks.

Because all those blocks are called at a regular interval, there is no more need for delay() functions, which waste cpu cycles and much worse, block operation of other tasks.

Each block should check some bits, (keyboard input) or output something (set a port pin for a led) and then return as fast as feasible.

 

This method works pretty well together with state machines.

A state machine for blinking a led could be something like:

#define LED_RED		( 1 << 4)
#define DELAY_TIME	500

void Handle_Leds( void) {
	enum {
		Init,
		On,
		Off,
		Delay,
	};

	static uint8_t State = Led_Init;	// Must be static to remain it's value after exit.
	static uint8_t State_Next;
	static uint16_t Time;
	
	switch( State) {
	case Init:
		DDRB |= LED_RED;
		State = On;
		break;
	case On:
		PORTB |= LED_RED;
		Time = Delay_Time;
		State_Next = Off;
		State = Delay;
		break;
	case Off:
		PORTB &= ~LED_RED;
		Time = Delay_Time;
		State_Next = On;
		State = Delay;
		break;
	case Delay:
		if( Time > 2) {
			// Minimum delay is 2. To get into, and get out of this delay state again.
			--Time;
		}
		else {
			State = State_Next;
		}
		break;
	}
}

This may seem like a lot of typing at first, but it makes your Led function timing independent from the "other stuff".

While you are focusing on the leds, you do not have to think about the keys.

While you are focusing on the keys, you do not have to think about the leds.

 

If you want to have some kind of communication between different "threads", the easiest way would be to do that with global variables.

There are other ways, but I think this is enough for a single post.

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

jodank wrote:

 

It looks like you need to hold the button down for 50 seconds before the delay gets smaller.

 

 

I don't think it has anything to do with seconds of waiting.

I don't set up any seconds etc.

The 50 cycles should count the switch bouncings between the pressed and released state.

I assume after 50 bouncing the switch is released physically.

 

 

   while (1) 

    {	

	if(PINB & (1<<0))
	{
        	PB0_bounce_counter++; 

			if (PB0_bounce_counter==Debounce)
			{
	        	//	Delay = Delay - 50 ;
	        	PORTB ^=(1<<1);
			}

	}

	else
	{
       		PB0_bounce_counter = 0;
	}

	/*
	PORTB = (1<<4);
	Wait(Delay);

	PORTB &=~(1<<4);
	Wait(Delay);
	*/
    }
}

 

 

What did I do wrong?

 

 

 

I think you don't truly understand how "_delay_ms()" works?

 

Like it was pointed out before, your debounce is flawed! You have to push the button for 50 seconds for anything to happen!
Try reducing your "Debounce" to for example a value of 1, then hold the button for more than one second and see what happens! wink

- Brian

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

jodank wrote:
I use the original EasyAVR5 dev. board, not my wiring up etc...

Show your connections, as I said.  Tell the voltage right at the pin, as I said -- when the button is pressed, and when the button is not pressed.  Is an active-high button just floating?  It would cause the symptoms.  I did not ask the questions just to up my post count.

 

So, that is the first step.  If indeed you sniff at those results -- "of COURSE the signal levels are proper; here are the values..." -- then analysis can move forward, with e.g  the delay discussions.

 

I'm out.

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: 1

Note that writing programs works better if you take a little time up front to plan the exact sequence of what you hope is going to happen. Don't just sit in front of your C editor and type things in as they occur to you. This gets increasingly more important the more complex your designs get.
.
For buttons to be "responsive" to an end user you probably want to be checking their state something like every 50ms. (1,000 times more often than you currently do).

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

jodank wrote:

It looks like you need to hold the button down for 50 seconds before the delay gets smaller.

 

 

I don't think it has anything to do with seconds of waiting.

I don't set up any seconds etc.

The 50 cycles should count the switch bouncings between the pressed and released state.

I assume after 50 bouncing the switch is released physically.

....

What did I do wrong?

Think it through.  Every time you go through your main loop, it takes 1 second (2 500 ms delays as you blink the LED).  How many times do you have to go through the main loop to detect a button push and modify the delay time? 50, so 50 seconds.

 

Whether you use a timer or _delay_ms, you need to have a small timing 'tick' which you use for both timing operations, debouncing and blinking.  10 ms is a common value because it's a nice round number.  Call that timing tick 'T', so let's say T = 10ms.  Then you use 2 software counters - variables that your software increments or decrements each T ms - and associated software flags.  For debouncing you might want to count 2 to 5 ticks, while for your blink you would count 50 ticks for a 1-second blink (500 ms on, 500 ms off).

 

The key to understand is that your main loop now will be happening rather fast - every T ms.  And you get slower events by running software counters in that fast main loop.  Here is a rough outline, you can work out how this might be turned into actual code:

 

LOOP   // every T ms

   if (button_pushed)

      if (--button_timer_count == 0)

         reduce_blink_delay

         button_timer_value = auto_repeat_value

   else   // button not pushed

      button_timer_count = initial_value

 

   if (LED_state_flag == ON)

      turn_LED_on

   else

      turn_LED_off

   if (--blink_delay_count == 0)

      LED_state_flag = !LED_state_flag

      blink_delay_count = blink_delay

 

   _delay_ms(T)

END LOOP

 

Once you see how this works, you can easily add a second button to increase the blink delay in the same manner.  You can also then use a hardware timer to generate the ticks, and simply look for a timer tick flag in the main loop.  Lots of ways to do this, both with and without timer interrupts.

 

EDIT: things get rather interesting as you keep holding the button down after the debounce.  Do you want to force the user to release the button and then push again to decrement the delay again?  Or do you want to set up an auto-repeat the way most keys work?  Either one is easy.  I've modified the pseudo-code for auto-repeat.  Think about how you would change it for release-and-push-again.

 

Last Edited: Thu. Sep 13, 2018 - 03:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok, here is the schematic:

 

The voltage on the pin when the button is released is 0V.

The voltage when the pin is pressed is 4.63V.

Thanks for your time and helping to point me in the right direction.

 

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

Just out of interest why do it that way up? If you have the switch connect to Gnd not Vcc you can switch to using the resistor built into the AVR. No needfor the external 10K. It does mean the button reads 0 when closed and 1 when open but that can simply be inverted in the software.

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

clawson wrote:

Just out of interest why do it that way up? If you have the switch connect to Gnd not Vcc you can switch to using the resistor built into the AVR. No needfor the external 10K. It does mean the button reads 0 when closed and 1 when open but that can simply be inverted in the software.

 

The xtinies can even invert it in hardware.

 

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

clawson wrote:

Just out of interest why do it that way up? If you have the switch connect to Gnd not Vcc you can switch to using the resistor built into the AVR. No needfor the external 10K. It does mean the button reads 0 when closed and 1 when open but that can simply be inverted in the software.

Also, the idea of running Vcc off the board to switches and such just bothers me personally.  Much better IMO to run Gnd if possible.  

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

Ok guys, why the pins are set to gnd?
Because of the dev board manufacturer made it that way.
Yes I can make the pins setup to tristate, or set a pull-down or pull-up resistor,

but I never set them another way on my board.

 

You say if I understand correctly, I should setup the internal resistor on my input pin and trigger the button to GND and not to VCC?

 

I can do that also, no problem.

 

But, after reading your suggestions here about my code, I really realized does I made a BIG MASSIVE MESS of code. :-)

My debouncing code is not worth the time to "repair" I think so.

 

 

The biggest problem in such of code like mine posted here is the "_delay_ms" function, which will alter the whole cpu for a meaning of time...

 

I would appreciate if somebody can share a piece of debouncing code for beginners.

Before I used Bascom and there was the function for debouncing and I made some filters around the buttons on my hardware, 

but in C it is a bit hard for me to understand everything.

 

Maybe I'm the most stupid guy on the planet but I don't shame me to ask for help.

I'm also not a guru in this stuff and I really can't post good post even if I'm here in this community over 8 years.

I'm a hobbyist in avr and not a regular user of avr stuff, and I developed my projects in Bascom for years.

 As I know, this forum is a better place for C developers than for Bascom developers.

This is the reason behind my low post number and 8 years of been registered here.

I also read the rules of this forum and didn't found any point where is forbidden, to be

here or to must meet some post numbers to be here, or even I don't find any point

of disagreeing of posting maybe for you stupid but for me hard to understand questions. 

 

I'm trying to be honest and to follow the rules of this community.

I don't wanna post and hijack somebody's post just to be there, but I read really frequently 

the posted text here on this community, so I'm learning new stuff.

I also know does here are several persons, who are real gurus in AVR and C programming, I really appreciate that.

But they are not the gods from who we stupid peoples should have fear to ask stupid questions.

That type of behaviors, from my point of view, is intellectually humiliating and it is a question of the personality of that persons.

To be on top of a science is very cool, but don't forget what it's like to be a beginner.

 

I think I answered all the questions or not?

 

So, let us be friends and focus on AVR debouncing problem if you agree.

Last Edited: Sun. Sep 16, 2018 - 05:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Here are some posts I wrote a few years ago that might be helpful to you.

 

https://www.embeddedrelated.com/...

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

That's what about I talk.

Thank you so much, man!

 

My best regards.

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

I think you should re-read my post #8 again.

I was just curious to your background, and I try to adopt to people with more experience in other subjects then programming.

 

I thought the way of adding structure into your program by making the key scan and led functions non-blocking was a valuable idea for you with which you can make yourself a better programmer. Using long software delays is almost always a bad solution, and it makes your program harder to understand if your uC has to do different things.

 

The only difficult part is to set up a hardware timer for "proper" timing.

But if approximate timing is acceptable you can easily use a software delay instead of a timer interrupt to add some cadence to your program.

Like so:

void main( void) {
    
    for( ;;) {
        _delay_ms(1);       // Wait a bit for a somewhat regular interval.
        do_keyboard( );
        do_leds( );
        do_relays( );
        // etc...
    }
}

The main idea is to separate funcionality from each other. This technique can be used in C, Basic, or any other programming language.

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

Paulvdh:
Thank's Paul for the direction and kk6gm gave me also a good tutorial in his #18 post,  what I'm now reading and trying to use it in my code.

 

I appreciate your time and also other peoples time too.

 

I will be out for some time and trying t rewrite my code to a less massive messy code... laugh

But I  will be back! with another messy code! cool

My best regards.

Thank you.

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

HI!
Here is my new version of the code what I wrote based on information from the internet and examples what I found and studied.

I had troubles with the timer setup but I hope I did it well.

Can somebody pls check my counter setup and maybe give some suggestion if I have done something wrong?

 

Actually, the code is working, the switches are debounced and the toggling speed on the PB0 is well done, but when I calculate the

timing on paper and when I compare it on my scope I get differences...

 

I use Attiny13 uC.

Here is my code:

 

#define F_CPU 1200000UL
#include <avr/io.h>
#include <avr/interrupt.h>

int counter = 0;                
int Delay = 1;
int Step = 100;

int SPS_PB0 = 1; 
int SWP_PB0 = 1; 

int SPS_PB3 = 1; 
int SWP_PB3 =1; 

static inline void Timer_On(void)
{
	TCCR0A |= (1 << WGM01);				// clear timer on compare match
	TCCR0B |= (1 << CS01);				// clock prescaler 8
	OCR0A = 25;					// trigger interrupt every ([1 / (1.2E6 / 8)] * 25 = 200uS) ??
	TIMSK0 |= ( 1 << OCIE0A );			// enable output compare match interrupt
	sei();                                          // enable interrupts
}


ISR(TIM0_COMPA_vect)					// TIMER0 ISR
{
	counter++;
	
	/****************************************** START rev up LED ***************************************/
	if ((PINB & (1 << PB4)) != SPS_PB0)   
	{
		if(!SWP_PB0)                     
		{
			Delay=Delay-Step; 
			
			if (Delay < 100)
			{
				Delay=1;
				PORTB = (1 << PB2);
			}
			else
			{
				PORTB &=~ (1 << PB2 );
			}
			
			counter = 0;           
			SWP_PB0 = 1;               
		}
		else
		SWP_PB0 = 0;  	      
	}
	SPS_PB0 = (PINB & (1 << PB4));        
	/****************************************** END rev up LED *****************************************/
	
	
	/****************************************** SLOW down LED ******************************************/
	if ((PINB & (1 << PB3)) != SPS_PB3)   
	{
		if(!SWP_PB3)                     
		{
			Delay = Delay + Step;
			
			if (Delay > 1000)
			{
				Delay=1000;
				PORTB = (1 << PB2);
			}
			else
			{
				PORTB &=~(1 << PB2);
			}
			
			counter = 0;
			SWP_PB3 = 1;              
		}
		else
		SWP_PB3 = 0;  	      
	}
	SPS_PB3 = (PINB & (1 << PB3));        
		
	/****************************************** END slow down LED **************************************/
	
	/* Toggle output based on the Delay setup */
	if (counter == Delay)
		{
			PORTB ^= (1 << PB0);
			counter = 0;
		}
}

int main(void)
{
	
	DDRB |= (1 << PB0) | (1 << PB2) ;	
	PORTB |= (1<<PB4) | (1<<PB3);	
	Timer_On();	
	
	while(1)
		{
			
		}
	return 0;
}

 

When I try this code with CKDIV8 fuse bit unchecked I get on the scope a signal which is 22uS off = 0V and 22uS on = +5V.

Should I get around 200uS?

 

When I try this code with CKDIV8 fuse bit checked I get on the scope a signal which is 176uS off = 0V and 1762uS on = +5V.

That's for me correct because the clock is divided by 8 but I still don't get a signal on and off state 200uS.

 

Should CKDIV8 fuse bit be checked or not?

 

Is this a better code than my previous direction/version?

What do I missing? 

 

Thank you.

 

 

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

[1 / (1.2E6 / 8)] * 25 = 167 us

 

--Mike

 

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

Is it really 1.2MHz anyway??

 

EDIT: Ah OK, I see it's one of these "odd" AVRs that has a 9.6MHz (rather than 8MHz) default clock. So I guess that without CKDIV F_CPU is 9,600,000 and with CKDIV8 it is 1,200,000

Last Edited: Thu. Sep 20, 2018 - 10:55 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-mike wrote:

[1 / (1.2E6 / 8)] * 25 = 167 us

 

--Mike

 

Oh!
Thanks for this correction mate.

 

clawson:

Yes, this uC has an internal clock of 9.6MHz, so I made the calc: 9.6/8=1.2E6MHz.

Therefore I used this F and the CKDIV8 should be checked I assume.

 

So, is actually my signal on the scope with 176uS On and 176uS Off enough accurate compared to the math I made?

 

Is this Timer right configured?

 

Thanks.

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

There's a +1 missing too:

 

[1 / (1.2E6 / 8)] * (25 + 1) = 173.33 us

 

--Mike

 

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

avr-mike wrote:

There's a +1 missing too:

 

[1 / (1.2E6 / 8)] * (25 + 1) = 173.33 us

 

--Mike

 

 

Can you pls help me to understand the equation now?

I'm confused now.

 

Thanks.

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

jodank wrote:

avr-mike wrote:

There's a +1 missing too:

 

[1 / (1.2E6 / 8)] * (25 + 1) = 173.33 us

 

--Mike

 

 

Can you pls help me to understand the equation now?

I'm confused now.

 

Thanks.

To count N timer clock cycles you need to load a timer OCR register with N-1.  It's just how they work - I can explain more if you need to know why.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1
			if (Delay < 100)
			{
				Delay=1;
				PORTB = (1 << PB2);
			}

You mean Delay = 100, right?

 

Also, using global variables in your ISR is not a good approach.  If a variable is only used in the ISR it should be declared 'static' inside the ISR.  If the variable is to be shared between an ISR and main code, it does need to be global or static to the file, but then it must be declared 'volatile'.

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

kk6gm:

I would appreciate if you can a bit more explain the timer setup on a maybe of a more average way if it is actually possible.

I have done a lot of project with timer/counter in Bascom but even there I run into troubles until I finally figured out how it works and where I made my errors.

All that because I don't really understand that maybe simple function of an AVR...

 

Here is my I hope final code:

#define F_CPU 1200000UL
#include <avr/io.h>
#include <avr/interrupt.h>

volatile int counter = 0;				// Interrupt cycle counter
volatile int Delay = 500;				// PB0 toggle speed setup value	- changeable through PB3 & PB4
static int Delay_max = 1000;			        // ~100ms toggle ( slow toggle )
static int Delay_min = 100;				// ~10ms toggle	 ( fast toggle )
static int Step = 100;					// toggle step setup for PB3 & PB4

static int SPS_PB4 = 1;
static int SWP_PB4 = 1; 

static int SPS_PB3 = 1;
static int SWP_PB3 =1; 

static inline void Timer_On(void)
{
    TCCR0A |= (1 << WGM01);				// clear timer on compare match
    TCCR0B |= (1 << CS01);				// clock prescaler 8
    OCR0A = 25;					        // trigger interrupt every ([1 / (1.2E6 / 8)] * (25+1) = 173.3uS)
    TIMSK0 |= ( 1 << OCIE0A );			        // enable output compare match interrupt
    sei();                                              // enable interrupts
}

ISR(TIM0_COMPA_vect)					// TIMER0 ISR
{

    counter++;

    /****************************************** START rev up LED ***************************************/
    if ((PINB & (1 << PB4)) != SPS_PB4)
    {
        if(!SWP_PB4)
        {
            Delay = Delay-Step; 

            if (Delay <= Delay_min)
            {
                Delay = Delay_min;
                PORTB = (1 << PB2);
            }
            else
            {
                PORTB &=~ (1 << PB2 );
            }

            counter = 0;
            SWP_PB4 = 1; 

        }
        else
        SWP_PB4 = 0;
    }
    SPS_PB4 = (PINB & (1 << PB4));
    /****************************************** END rev up LED *****************************************/

    /****************************************** SLOW down LED ******************************************/
    if ((PINB & (1 << PB3)) != SPS_PB3)
    {
        if(!SWP_PB3)
        {
            Delay = Delay + Step;

            if (Delay >= Delay_max)
            {
                Delay = Delay_max;
                PORTB = (1 << PB2);
            }
            else
            {
                PORTB &=~(1 << PB2);
            }

            counter = 0;
            SWP_PB3 = 1;      

        }
        else
        SWP_PB3 = 0;
    }
    SPS_PB3 = (PINB & (1 << PB3));        

    /****************************************** END slow down LED **************************************/
}

int main(void)
{

    DDRB |= (1 << PB0) | (1 << PB2) ;
    PORTB |= (1<<PB4) | (1<<PB3);
    Timer_On();	

    while(1)
        {
            /* Toggle output based on the Delay setup */
            if (counter == Delay)

            {
                PORTB ^= (1 << PB0);
                counter = 0;
            }

        }

    return 0;

}

I assume I defined everything correct.

My final goal should be to click a relay on PB0.

 

Thank you all of you for helping.

My best regards/

Last Edited: Fri. Sep 21, 2018 - 08:05 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Is there a question or questions about the last posted code #30 ?

 

Once you learn about "volatile", you also need to learn about "atomic access" to multi-byte variables.

 

A small style point:

jodank wrote:
if (counter == Delay)

I always do (counter >= Delay)  It costs you nothing to use the comparison, it operates identically, and helps to protect code from "running away".

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: 1

A few random thoughts:

 

What is your counter counting?  From your comments it is counting ticks of 173.3 us.  Why?  That sure sounds like a strange value to me.  Why not set your timer up for 1ms or 5ms or 10ms?

 

If you do want to do all your calculations in units of milliseconds (which is a good idea IMO), and if your tick happens e.g. every 5ms, simply add 5 to your counter in the ISR, rather than incrementing it.

 

Agree with theusch that >= is a more bulletproof test than ==, especially when you start adding other time-consuming code to your main loop.  This raises the question, what if you do go over, so that e.g. counter equals 505 when you make the comparison?  One way to help keep long-term timing accuracy would be to do counter -= Delay rather than counter = 0.  (and BTW, you know you can write Delay -= Step, right?)

 

On the subject of atomic access, here's a quick explanation of one possible failure mode:

MAIN: fetch Delay LSB

Timer interrupts

ISR: modify Delay and exit

MAIN: fetch modified (!!) Delay MSB and perform calculation

 

Now you're working with a Delay value that is half old and half new, i.e. garbage.  The problem happened because the AVR cannot read or write a 16-bit int in a single operation, but must perform multiple operations, and an interrupt can occur in the middle of those multiple operations.

 

A common solution is to disable interrupts (or at least any interrupt that can affect the data you're after), access the data, then re-enable interrupts.  One method in your case could be:

 

cli();
bool time_to_blink = (counter >= Delay) ? true : false;  // allow safe access of both counter and Delay
sei();
if (time_to_blink)
{
}

Another method is to set single-byte flags in an ISR, then check them (which is a safe operation for the AVR) in main code.  For example, you could have this in your main code:

 

if (dec_Delay)
{
    Delay -= Step; // ignoring range checking for clarity
    dec_Delay = false;
}
else if (inc_Delay)
{
    Delay += Step;
    inc_Delay = false;
}

I'll let you figure out how dec_Delay and inc_Delay would be set in your ISR.

 

One last comment for now.  Why are you using signed integers for your count and delay?  Those values will never go below zero.  I have a little mantra that is helpful, especially for small (especially 8-bit) embedded processors:

 

Never use floats when longs will do

Never use longs when ints will do

Never use ints when bytes will do

Never use signed when unsigned will do

 

Last Edited: Fri. Sep 21, 2018 - 10:39 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Nice to see you've got an timer interrupt going.

About the CKDIV8 Fuse. My advise is to not program it. You want a (relatively) fast F_CPU, especially during software development and/or when current consumption is not of premium concern.

Higher clock rates gives more opportunity for your uC to do stuff.

The main reason for the CKDIV8 fuse is probably  in situations where the uC has to run on low voltages, where the clock has to be de-rated to be within speck ( Edit: See kk6gm's remark in #36). The combination of low voltage and low F_CPU can make for a power efficient design.

 

So instead of using the CKDIV8 fuse, you should use another preload value for OCR0A. Start with 8*25 = 200 to get similar timing to what you had with the CKDIV8 fuse programmed.

Then later adjust the OCR0A value to get a timing that is easy to remember (such as 1ms or 10ms, just as kk6m suggests in #32).

If your program has to check different things between each timer tick, you do not want this interval to be too small, your uC needs time to do those things in the allotted interval, and this is another reason you want a relatively high F_CPU.

 

For most programs it is also recommended to keep the time spent in interrupts as low as possible.

The main reason is that it is hard to write reliable programs if you get into the area of interrupts which have to interrupt other interrupts.

Your code is much easier to understand and maintain if you do most of the processing in a flat structure in main().

So this is where we get into the area of interacting between interrupts and the main(), non interrupt code.

In your example from #22  -> #30 you added the volatile keyword to your counter variable, which is good.

(Have you read some background on what it does, and why it is neccasary?)

 

The next step will be to move the code from your ISR to main.

The minimum version of your ISR would be:

 

ISR(TIM0_COMPA_vect)					// TIMER0 ISR
{
    counter++;
}

 

In your main() function you can then use this counter variable for all kind of timing.

It is very common to write a few functions as an interface library for timing purposes.

For example, the first thing you should have learned about working with ISR's is the volatile keyword.

To interact with shared variables with ISR's you then also need to use cli() and sei() (or other methods) to ensure your program works correctly.

So the first (very simple) function of your timer library could be a simple function to read your counter value.

uint16_t counter_read( void) {
    // Simple atomic read of volatile counter.
    uint16_t temp;

    cli();
    temp = counter;
    sei();
    return temp;
}

 

The next thing you want to have is to be able to wait for your ISR to occur.

You could do that for example with:

void counter_wait( void) {
    // Wait untill the next ISR timer tick. (counter gets incremented).

    uint16_t tmp = counter_read();

    while( counter_read() == temp)
      ;
}

 

And then you can extend that with delay functions, a stopwatch or other timing related stuff you want.

A trivial next step is to use your counter functions for timing.

For example:


 uint16_t start, end;

 start = counter_read( );
 do_something( );
 end = counter_read( );

 printf( "do_domething took: %d ms \n", end - start);

 

A subtle but very important trick is to always calculate time intervals as the difference between the end  *** MINUS *** the start.

The reason is that your counter value can and will overflow, and this can easily lead to errors.

For example, if you use a 1ms timer ISR, a 16 bit counter will overflow about every minute. (an uint16_t can count to maximum of 65535 before it overflows to 0).

 

By calculating the difference, the calculated interval will wrap back to what it should be without errors.

I wrote a little example:

#include <stdio.h>
#include <stdint.h>

int main( void) {

	uint16_t start, end, interval;

	start = 65000;    // Just before a simulated overflow.
	end = 200;        // Just after a simulated overflow.
	interval = end - start;

	printf(" Interval = %d\n", interval);
}

 

You can compile and run this example on your pc (if you have a C compiler installed.

I saved the program as "asdf.c" and compiled to the output (with "-o") asdf.

On Linux you have to add the pathname to all "unknown" locations. The current working directory usually is not searched for programs.

So "./asdf.exe" means: Execute the program asdf.exe in the current working directory ( "./" is a reference to the current working directory).

paul@dualcore:~$ gcc asdf.c -o asdf.exe
paul@dualcore:~$ ./asdf.exe
 Interval = 736

With the information I gave above you should be able to verify why 736 is the right answer here.

 

Edit: Return temp, instead of counter in 2nd code snippet (Tnx avr-mike #34).

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

Last Edited: Sun. Sep 23, 2018 - 06:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Paulvdh wrote:

uint16_t counter_read( void) {
    // Simple atomic read of volatile counter.
    uint16_t temp;

    cli();
    temp = counter;
    sei();
    return counter;
}

 

I'm sure you meant to write return temp;

 

--Mike

 

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

Paulvdh wrote:
I don't really understand why they bothered to implement that fuse.

I'll leave the determination about a Good Thing or not to you experts.  But the datasheet, for [every?] AVR8 model, has an explanation on why there is [universal?] use of CKDIV8 on virgin chips.

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.

Last Edited: Sun. Sep 23, 2018 - 01:24 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Paulvdh wrote:
I don't really understand why they bothered to implement that fuse.

Because AVRs cannot run at high clock rates when powered by low (but in-spec) Vcc.  So if your system is designed for a low (but in-spec) Vcc, without CKDIV8 or equivalent, it would be running out of spec when powered up on the internal oscillator.  Having CKDIV8 or equivalent assures that the internal oscillator clock rate will be valid for any valid Vcc.  This chart is from a '328, but all AVRs have similar restrictions.

 

Last Edited: Sun. Sep 23, 2018 - 02:57 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The fuse just sets the initial value of the CLKPS (clock prescale) bits in CLKPR.

You are able to change this value in your code to achieve any clock speed, even

full speed, while leaving the fuse as-is.

 

--Mike

 

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

HuH!
Huge writings, you are really amazing how much info you are shared with me.

Thank's for such of helping.

 

By my side is the book from Kernighan &  Ritchie "The C programming language 2nd edition" and I digging non-stop in it.

 

I corrected my timing in the code from 173.3uS to 200uS.

Here is my example snippet from my code :

 

The old code:

static inline void Timer_On(void)
{
    TCCR0A |= (1 << WGM01);				// clear timer on compare match
    TCCR0B |= (1 << CS01);				// clock prescaler 8
    OCR0A = 25;					        // trigger interrupt every ([1 / (1.2E6 / 8)] * (25+1) = 173.3uS)
    TIMSK0 |= ( 1 << OCIE0A );			        // enable output compare match interrupt
    sei();                                              // enable interrupts
}

 

New code:

static inline void Timer_On(void)
{
	TCCR0A |= (1 << WGM01);				// clear timer on compare match
	TCCR0B |= (1 << CS01);				// clock source and prescaler 8 setup ( Timer/Counter Clock Sources datasheet pg.58)
	OCR0A = 29;					// trigger interrupt every ([1 / (1.2E6 / 8)] * (29+1) = 200uS)
	TIMSK0 |= ( 1 << OCIE0A );			// enable output compare match interrupt
	sei();                                          // enable interrupts
}

This gives me a better calculation than the old code as you guy's told before, also on my scope it is really 200us...

 

The first point with the timer implementation was because of the switch-debouncing problem in my code,

but now I think  I wish to do something more also, but I have no clue how to achieve that.

Any suggestion please would be nice.

 

In my main code I don't really figured out how to wait 4ms until the PB0 is cleared:

int main(void)
{
	
	DDRB |= (1 << PB0) | (1 << PB2) ;	
	PORTB |= (1<<PB4) | (1<<PB3);	
	Timer_On();	
	
	while(1)
		{
			
			/* Toggle output based on the Delay setup */
			if (counter == Delay)
			
			{
				//PORTB ^= (1 << PB0);
		        PORTB |= (1 << PB0);
		        
		          WAIT 4ms ???
		        
		        PORTB &= ~(1 << PB0);
				
				counter = 0;
			}
			
		}
		
				
	return 0;

 

Let's say, every 200ms PB0 goes from High to Low.

But the switching time on PB0 from High to Low should be delayed to 4ms and not another 200ms.

 

About the CKDIV8:

If I understand you, that should be programmed if I use the uC on a lover voltage.

On the spec. diag. from the datasheet, I can figure out does if my uC is powered with 5v then I 

have nothing to worry, because the safe operation area Fmax vs VCC is around 20MHz.

So, set or not set the CKDIV8 makes no root changes but in calculations...

From the datasheet, I have the information does the Attiny13 comes with the CKDIV8 fuse enabled by default when it is new.

 

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

Again, why are you choosing a 200us timer interrupt tick?  Why not something more human-brain friendly like 1ms (or 5ms or 10ms)?  If you have a reason, that's fine, but you're not communicating it with us.

 

As for your question, I think you should start thinking in terms of states.  You can define states with an enum statement, or straight defines.  Here is one skeletal structure that would work, using a variable Delay count and a fixed 4MS_DELAY count.  There are other ways that would also work, like the leap-frogging technique in the tutorial I linked to.

 

enum {PB0_OFF, PB0_ON};

uint8_t state = PB0_OFF;
...
while (1)
{
    if ((state == PB0_OFF) && (counter >= Delay))
    {
        PORTB |= (1 << PB0);
        state = PB0_ON;
        counter -= Delay;
    }
    else if ((state == PB0_ON) && (counter >= 4MS_DELAY))
    {
        PORTB &= ~(1 << PB0);
        state = PB0_OFF;
        counter -= 4MS_DELAY;
    }
}

Note that I didn't deal with atomically accessing the counter variable, which you need to do if it is being incremented in the ISR.

Last Edited: Sun. Sep 23, 2018 - 09:00 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 Thanks for the quick replay kk6gm.

Again, why are you choosing a 200us timer interrupt tick?

 I'm working on it to migrate the code from the TIMER0 ISR and just let the counter++ in there but I have jet some problems with it, I won't give up.

 

So to say, when I make changes to the timer interrupt tick I have some problems with the debouncing again.
When I change my code to:
 

OCR0A = 149;		// trigger interrupt every ([1 / (1.2E6 / 8)] * (149+1) = 1ms)

The debouncing stuff is meesy and won't work again, it is to slow, I have to figure out how to separate the switch pressing 

code from the timer isr and use them in the main of my program and use the timer also for the debouncing....

 

 

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

jodank wrote:

So to say, when I make changes to the timer interrupt tick I have some problems with the debouncing again.
When I change my code to:
 

OCR0A = 149;		// trigger interrupt every ([1 / (1.2E6 / 8)] * (149+1) = 1ms)

The debouncing stuff is meesy and won't work again, it is to slow, I have to figure out how to separate the switch pressing 

code from the timer isr and use them in the main of my program and use the timer also for the debouncing....

The solution to this is to make all your delay values defines (or calculations) based on your tick interval, rather than hard-wired.  It might look something like this:

 

#define TICK_US 1000        // 1ms
#define DEBOUNCE_US 20000   // 20ms
#define DEBOUNCE_TICKS (DEBOUNCE_US/TICK_US)    // so 20 ticks
#define PB0_ON_US 200000
#define PB0_ON_TICKS (PB0_ON_US/TICK_US)
#define PB0_OFF_US 4000
#define PB0_OFF_TICKS (PB0_OFF_US/TICK_US)

You can even derive your OCR0A value this way

 

#define TICK_FREQ (1000000/TICK_US)     // 1000 Hz for 1000us tick
#define T0_PRESCALE 8
#define T0_TICK_VAL ((F_CPU/T0_PRESCALE/TICK_FREQ)-1)    // 149

Always define or calculate values that depend on other values in this manner, rather than hard-coded "magic numbers".

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

Hmmm...
I have somehow to separate the timer and the other stuff (PB0 on-off, switch bouncing etc...).

 

From now, I'm not so happy with this debouncing technique.

The button interacting is working in the ISR. I assume that is not a good implementation.

 

Ok, I need some time pls to understand all this what you are shared with me.

 

Thanks, guys.

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

jodank wrote:

Hmmm...
I have somehow to separate the timer and the other stuff (PB0 on-off, switch bouncing etc...).

 

From now, I'm not so happy with this debouncing technique.

The button interacting is working in the ISR. I assume that is not a good implementation.

 

Ok, I need some time pls to understand all this what you are shared with me.

 

Thanks, guys.

OK, think about this.  What if this was your entire timer ISR:

volatile bool timer_tick = false;

ISR(...)
{
    timer_tick = true;
}

And your main code:

// main while(1) loop here
...
if (timer_tick)
{
    timer_tick = false;
    // do here what you would do in ISR
}
...

 

Last Edited: Mon. Sep 24, 2018 - 06:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

@kk6gm #43

Setting a bool in your ISR also works, and it's also extensible, you could handle multiple bools and even at different rates.

Disadvantage is that if code gets more complex the ISR grows to handle those different flags.

 

With incrementing a counter in the ISR you have a very short ISR and you can still use as many derived timer vars as will fit in the uC.

A simple counter increment is also an easy way for making variable timing stuff.

If you want to do variable timing with bools in the ISR, more communication between the ISR and the main program is needed, and you have to maintain the code in 2 different places when anything changes.

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

Ok, I have two questions which are not clear for me in my "thinking" brain:

 

1. Timers have several types of operation mode.

I set up my timer in my program to work in CTC mode.

That mean's the compare register will be cleared when it reached the counter value in TCNTx.

 

In my program, I set that value in OCR0A to 29 ticks, so the timer will reset itself to 0 when the TCNTx value is 29.

That means in my case it would happen in every 200us.

 

Let's say, there is the code from above:

volatile bool timer_tick = false;

ISR(...)
{
    timer_tick = true;
}

In the ISR the timer_tick will every 200us set to true. ( I used the 200us because it is set in my code, no matter what time is set... )

 

the code in the main is doing something with the timer_tick.

That is also fine.

 

But, what if I need to do two or more job what have to be timed also?

Or what if some job should be done after let's say 600us? ( that is out of the 200us, should I implement a separate counter to count to 3? because 3cycle * 200us = 600us)...
Let's say:

 

If (timer_tick) then LED = ON  // every 200us the LED will go to on state.

 

But the led should go off in let's say 4us from the moment when it was lit up?

 

This should also be covered by the timer I assume.

Should I do something like this in my code?

 

If (timer_tick) then LED = ON

check timer_tick for 4 more ticks and then

LED = OFF

Actually, that different time sharing through one timer goes me over my head...

I don't know if I could really describe what I mean.

 

 

2. Should the ISR routine be written before the MAIN ?

Is there a regulation in C or can I write my ISR's after the MAIN also?

 

 

Thanks.

 

 

 

 

 

Last Edited: Tue. Sep 25, 2018 - 06:33 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

jodank wrote:
That mean's the compare register will be cleared when it reached the counter value in TCNTx.
Err no, wrong way round. The TCNT register will be cleared when it matches the compare register value.

jodank wrote:
Or what if some job should be done after let's say 600us?
So you use your 200us interrupt for multiple purposes:

volatile uint8_t tick_200;
volatile uint8_t tick_600;

ISR(TMR_vect) {
    static count = 0;
    tick_200 = 1;
    count++;
    if (count >= 3) {
        count = 0;
        tick_600 = 1;
    }
}

Now tick_200 becomes set every 200us but tick_600 only becomes set every 600us (because there's a count to 3 each time). Obviously you could extend this as long as the timer runs at your fastest required rate and your other processes are some integer multiple (like X3) of the core rate. So this one time interrupt could be running 10 or more different things. In fact think of your Windows/Linux machine - right now it is running 50..100 processes and they are all being triggered from a single 10ms timer.

2. Should the ISR routine be written before the MAIN ?

Is there a regulation in C or can I write my ISR's after the MAIN also?

Very much a question of personal taste - do you want the reader to have to look at the top or the bottom of the file to find main() so they can then follow on from that to all the other functions it calls. With ISR() (because they aren't actually called from main()) it really does not matter where you put it (many would perhaps put it in a separate interrupt.c or perhaps a timer.c or something). But consider the case of "normal" functions. In C code such as:

int main(void) {
    PORTB = add(123, 207);
}

uint16_t add(uint8_t a, uint8_t b) {
    return a + b;
}

then as the C compiler reaches the call to add() in main() it does not yet know that add is a function that takes two uint8_t and returns one uint16_t so you have two ways to tell it. One is to declare the function above main:

uint16_t add(uint8_t, uint8_t);

int main(void) {
    PORTB = add(123, 207);
}

uint16_t add(uint8_t a, uint8_t b) {
    return a + b;
}

Now the compiler has what it needs to know to prepare the inputs and accept the output when it comes to the use of add() in main. The other way to do this is:

uint16_t add(uint8_t a, uint8_t b) {
    return a + b;
}

int main(void) {
    PORTB = add(123, 207);
}

Because a function definition also acts as a declaration, by putting the function before main() where it is used there is no need for a separate declaration.

 

Now folks argue both ways as to which of these is "better". To be honest the formal, separate declaration of the function is probably the better approach (you will do this a lot when you switch to C++) but some argue that this requires two things (the declaration and the definition) to be maintained. Suppose I change it so that parameter 'a' is now uint16_t not uint8_t I might need to make this change in two places. 

 

The functions defined before invocation approach avoids this and is simply less typing - so some might prefer that.

 

I'm ambivalent myself - seen too many example of both - so will accept either. Having said that, when I open a 2000 line main.c I will often jump to the end hoping to find the main() function that invokes everything else at the bottom - sometimes I'm lucky and it is there!

 

As I said above ISR() are a bit different. They have a "void ISR(void)" interface anyway but nothing in your code calls them either so they don't need to be declared before use - so you can put them where it suits you.

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

Sorry because I was a few days out, actually I practiced and tried to understand the timer more closely to me etc.

 

I understand the timer in this meaning, am I right?

 

The timer is a completely self-standing device inside the uC.

Like a stopwatch in a man's hand.

A stopwatch has also different modes to make some measurements.

So the timer in the uC has also his modes, like CTC, OVf etc.

To set up the timer we need a prescaller to slow down the timing compared to the core timing of the uC,

to select a clock source, to set up the mode in which we wish to use the timer etc.

Like the man with the stopwatch who takes measurements of a runner average speeds.

The stopwatch is set in a mode to take several results and then calculates the average time...

This could be let's say the CTC mode of an avr timer. ( of course, CTC has nothing to do with the average speed takeing. )

 

But the timer inside the avr won't stop even if the program in the main is doing some other stuff.

Like the man with the stopwatch.

If the runner is stumbling after a period of time, the man with the stopwatch will count forward uninterrupted.

This is in nut shell the way I understand the timer in my imagination.

 

So, I set a task for myself to do :

To use a Timer in CTC mode and toggle the PB0 on the Attiny13 with a puls, which should 20ms be high and 100ms low.

Here is my code:
 

#define F_CPU 1200000UL			    // Define CPU frequency in Hz. - for Attiny13
#include <avr/io.h>
#include <avr/interrupt.h>

volatile uint8_t tick_20ms;                  // variable to set when 20ms achieved  
volatile uint8_t tick_100ms;                // variable to set when 100ms achieved

static inline void Timer_On(void)
{
	TCCR0A |= (1 << WGM01);		    // clear timer on compare match
	TCCR0B |= (1 << CS01);		    // clock source and prescaler 8 setup ( Timer/Counter Clock Sources data sheet pg.58)
	OCR0A = 149;			    // trigger interrupt every ([1 / (1.2E6 / 8)] * (149+1) = 1ms)
	TIMSK0 |= ( 1 << OCIE0A );	    // enable output compare match interrupt 
	sei();                              // enable interrupts
}

ISR(TIM0_COMPA_vect)			    // TIMER0 ISR
{
	static int counter = 0;             // defined the counter variable
	counter++;                          // incrementing the counter by 1 every 1ms
	
	if (counter >= 120)		    // 120 - 100 = 20cycle = 20ms
	{
		tick_20ms=1;	            // the counter achieved the 20ms
		counter = 0;                // reset the counter to zero
	} 
	
	if (counter >= 100)		    // 100cycle = 100ms
	{
		tick_100ms=1;               // the counter achieved the 100ms
	}
	
	
}

int main(void)
{
	
	DDRB |= (1 << PB0);                 // set PB0 as output
	PORTB &=~ ( 1 << PB0 );             // clear PB0
	Timer_On();	                    // start timer
	
while(1)
{
	if (tick_100ms)
	{
	    PORTB |=( 1 << PB0 );           // set PB0 to high   
	}

	if (tick_20ms)                       // 20ms high
        { 
            PORTB &=~( 1 << PB0 );           // clearing PB0 after 20ms 
	    tick_20ms = 0;
	    tick_100ms = 0;	
	}	
}

}

After checking the PB0 with my scope I got a nice signal with 20ms high and 100ms low state.

Is there something I should do in some other way?
Is this code good enough for a beginning in C?

 

Thank you for your time helping me.

My best regards.

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

Nothing jumps out at me that is problematic.  Given the approach you are taking, your code seems to be clean and efficient.  The one thing I would point out is that there is no need for a signed 16-bit integer for 'counter'.  Since it only goes between 0 and 120 (or at most one or two beyond, in some future worst case), you should be using a uint8_t for counter.

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

I would use

 

static uint8_t counter = 0;

 

for 2 reasons: the size of int varies with implementation and you need only 8 bits. In general it's better to use variables with a fixed size for portability.

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

A button is a deceptive thing: it seems impossibly simple and yet it is a devil to code for.   

 

Personally I don't understand why anyone does it anymore.  There are several working (but underdocumented and absurdly complex) libraries for Arduino that handle buttons: one object for each button.  Here is one of the best:  https://github.com/t3db0t/Button

 

People here especially beginners just don't get how much easier it is to work in Arduino instead of standard register-diddling C.   This is most true for the basic microcontroller operations like button reading, I2C, SPI, and, well, everything.   People smarter than you have spent many many hours writing and debugging these freely-downloadable Arduino libraries.  [Most] are excellent examples of good coding procedures because the code gets downloaded, tested, and studied by so many people.    Granted the IDE sucks, but it is possible to change the sizes and colors on just about everything in the Arduino IDE, it's just hard to find out how to actually do it.

 

Beginners think that they are learning how to do "real" coding when they "advance beyond" Arduino.  That is simply not true.   Your "professional" status as a programmer depends on a very simple equation:  (How much you got paid to solve a client's programming issue) / (How much time that you spent writing the code that solves your client's programming issue).  And nobody ever gets paid for studying the AVR's complex internal register and opcode structure.  So why do it if you don't have to and aren't getting paid to do it? 

 

Everything is simpler to do in Arduino.  The most complex algorythm is going to have the same code (more or less) in both AvR Studio C and Arduino because they are both using the same compiler (AVRGCC more or less).  It's in the added functionality of the supplemental keywords that Arduino gives you that makes code writing so much faster and debugging so much easier.   Look at all the time spent above on some silly relatively simple button implementation.  With Arduino libraries, I can write working and functional code for an additional button switch in less time than it takes me to physically add the pushbutton to the prototype circuit board.

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

jodank, if I were tutoring you I would suggest that, now that you have code working with flags communicating between ISR and main code, you try to achieve the same result using 'counter' alone, without flags.  The more ways you learn about how to communicate between an ISR and main code, the more tools you'll have in your toolbox.

Pages