Why can't get precise 1 sec ON/OFF pulse - Attiny85?

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

Hi!
I'm trying to write a program where I can simple setup a time constant for ON and OFF on PB0 on my Attiny85.

Everything is working but the result should be lets say in my test to blink the LED on PB0 1sec ON and 1sec OFF.

Now, when I check the signal on my bench scope I figured out does I have inaugurated times.

 

I get some time the ON/OFF signal 1.96sec, then I have a ON/OFF period 1.46sec etc.

 

I use the internal oscillator which is 8MHz.

A prescaller of 64

 

Is in my code something wrong?

Or is this inacuracity normal when using the internal osc?

 

Here is my code:

 

#define F_CPU 8000000UL					// Define CPU frequency in Hz. for Attiny85
#include <avr/io.h>
#include <avr/interrupt.h>

// configuring the variables

volatile uint16_t Time_counter = 0;			// Counts the time in ms max up to 65535ms
volatile uint8_t Time_sec = 0;				// Counts the time in sec
volatile uint8_t Time_min = 0;				// Counts the time in min
volatile uint8_t Time_hour = 0;				// Counts the time in hour
volatile uint8_t PB0_ON_time = 1;			// PB0 ON time in sec
volatile uint8_t PB0_OFF_time = 1;			// PB0 OFF time in sec

static inline void Timer_On(void)			// Attiny85 Timer setup
{
  TCCR0A |= (1 << WGM01);				// clear timer on compare match (TCCR = Timer/Counter Control Register)
  TCCR0B |= (1 << CS00) | (1 << CS01);	                // clock source and prescaler 64 setup 
  OCR0A = 124;						// trigger interrupt every ([1 / (8E6 / 64)] * (124+1)) = 1ms
  TIMSK |= ( 1 << OCIE0A );				// enable output compare match interrupt - for Attiny85
  sei();							// enable interrupts
}

ISR(TIM0_COMPA_vect)						// TIMER0 ISR triggered depend on the Timer setup
{

	Time_counter++;						// count the time in ms max up to 65535ms

}

int main(void)
{

	DDRB |= (1 << PB0); 					// set PB0 on PORTB as Output
	PORTB &=~ ( 1 << PB0 ) ; 				// clear PB0
	Timer_On();						// Start timer

while(1)
   {

//============      Create sec, min, hour       ==========================

			if (Time_counter >= 1000)				// 1000ms = 1sec
			{
				Time_sec +=1;					// 1sec reached
				Time_counter = 0;

					if (Time_sec >= 60 )
					{
						Time_min++;			// 1min reached
						Time_sec = 0;
					}

					if (Time_min >= 60)
					{
						if (Time_hour >= 200)
						{
							Time_hour = 0;
						}

						Time_hour++;			// 1hour reached
						Time_min = 0;
					}
			}

//============    End creating sec, min, hour   ==========================

//============      PB0 control        ==========================

if (Time_sec >= PB0_OFF_time)					// Check if desired time is arrived
{
	if (!(PORTB & (1 << PB0)))				// Check if PB0 is set or not
	{
		PORTB |= ( 1 << PB0 );				// Set PB0 if not set before
	}

	if (PB0_ON_time == (Time_sec - PB0_OFF_time))		// Check if desired ON time is reached
	{
		PORTB &=~ ( 1 <<PB0 );				// Clear PB0 if PB0_ON_time reached
		Time_sec=0;					// Reset Time_sec time counter to 0
	}
}

//============      End PB0 control    ==========================

   }

}

 

Could this be more accurate with the internal osc?

Thanks in advance for any suggestion.

My best regards.

Last Edited: Wed. Jul 24, 2019 - 03:25 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

jodank wrote:

volatile uint16_t Time_counter = 0;			// Counts the time in ms max up to 65535ms
			if (Time_counter >= 1000)				// 1000ms = 1sec

This need to be atomically protected or the test could be half way through accessing when the ISR hits.

 

PS does the timer really need 1ms granularity anyway? In most systems the "tick" is more like 5/10/20/50ms. It only needs to be as granular as the most accurate thing you need to time. If you are only really using 1s periods then even 100ms would do. In which case it would never need to count above 10 and could fit easily into an atomic uint8_t. Even at 10ms you only need to count to 100 so again that fits into an 8 bit count.

Last Edited: Wed. Jul 24, 2019 - 03:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

AVR is an 8-bit microcontroller. When sharing a variable of 16 bits or more with interrupt processing, perform atomic operation in the foreground.

 

For example,
1, Read Low byte (0xFF) in foreground when Time_counter is 0x00FF.
2, An interrupt occurs and Time_counter is updated to 0x0100.
3, Read high byte (0x01) in foreground.
4, As a result, the value obtained in the foreground is the wrong 0x01FF.

 

Measures are easy. Prepare a local variable and copy it by atomic operation.

 

    uint16_t tmp;


    cli();
    tmp = Time_counter;
    sei();


    cli();
    Time_counter = 0;
    sei();

 

 

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

avr-gcc also has:

 

https://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html

 

specifically for handling atomicity.

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

Offhand I see two problems with your code.

 

1) You are comparing Time_counter and Time_sec using >=, but always setting the time values back to 0.  This can result in value slippage over time.  It would be better (but still not perfect) to subtract the interval value rather than setting the time value back to 0:

if (Time_counter >= 1000)
{
    Time_counter -= 1000;   // subtract interval instead of setting to 0
    Time_sec++;
}

2) More critically, you are not protecting your shared variables (Time_counter in this case)  from corruption.  You must do this when sharing variables between an ISR and main code.  One example of such corruption would look like this:

 

-- main code reads low byte of Time_counter in preparation for doing >= 1000 test.  Call that value X

-- ISR runs and modifies Time_counter to X+1

-- main code reads high byte of Time_counter (now X+1)

-- comparison now uses an invalid value of Time_counter (low byte of X, high byte of X+1)

 

There are different ways to prevent this, but all require that the ISR not be allowed to run while main code is accessing (either reading or writing) the shared variable.  One method is as follows (note that every access must be protected, not just this one):

 

cli();                              // prevent ISR from running
uint16_t temp_time = Time_counter;  // get valid copy of shared variable
sei();
if (temp_time >= 1000)
}
....
}

I don't know if these issues are causing the problem you're seeing, but they definitely need to be fixed.  And about the internal clock, it is only guaranteed to within a few percent.  Using an external crystal or oscillator will give much more accurate timing.

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

Thanks for the quick replay, I really appreciate that.

 

I changed the code now in this way and didn't use any more uint16_t variable.

The problem is a bit solved but still not precise.

This means, now I have a stable timing but also wrong timing.

The ON time is 815ms and the OFF time is also 815ms but it should be 1sec or the whole period of the signal should be near as possible to 2sec.

 

Here is the code how it looks like now:

 

#define F_CPU 8000000UL					// Define CPU frequency in Hz. for Attiny85
#include <avr/io.h>
#include <avr/interrupt.h>

// configuring the variables

volatile uint8_t Time_counter = 0;			// Counts the time in ms max up to 255ms
volatile uint8_t Tick_counter = 0;			// Count to 10 which is equal to 1sec
volatile uint8_t Time_sec = 0;				// Counts the time in sec
volatile uint8_t Time_min = 0;				// Counts the time in min
volatile uint8_t Time_hour = 0;				// Counts the time in hour
volatile uint8_t PB0_ON_time = 1;			// PB0 ON time in sec
volatile uint8_t PB0_OFF_time = 1;			// PB0 OFF time in sec

static inline void Timer_On(void)			// Attiny85 Timer setup
{
	TCCR0A |= (1 << WGM01);				// clear timer on compare match (TCCR = Timer/Counter Control Register)
	TCCR0B |= (1 << CS00) | (1 << CS01);	        // clock source and prescaler 64 setup
	OCR0A = 124;					// trigger interrupt every ([1 / (8E6 / 64)] * (124+1)) = 1ms
	TIMSK |= ( 1 << OCIE0A );			// enable output compare match interrupt - for Attiny85
	sei();						// enable interrupts
}

ISR(TIM0_COMPA_vect)					// TIMER0 ISR triggered depend on the Timer setup
{

	Time_counter++;					// count the time in ms max up to 255ms

}

int main(void)
{

	DDRB |= (1 << PB0); 				// set PB0 on PORTB as Output
	PORTB &=~ ( 1 << PB0 ) ; 			// clear PB0
	Timer_On();					// Start timer

while(1)
   {

//============      Create sec, min, hour       ==========================

			if (Time_counter >= 100)			// 100ms
			{
				Tick_counter +=1;
				Time_counter -= 100;
					if (Tick_counter == 10)
					{

						Time_sec +=1;		// 1sec reached
						Time_counter -= 100;
						Tick_counter -= 10;

					}
	
					if (Time_sec >= 60 )
					{
						Time_min++;		// 1min reached
						Time_sec = 0;
					}

					if (Time_min >= 60)
					{
						if (Time_hour >= 200)
						{
							Time_hour = 0;
						}

						Time_hour++;		// 1hour reached
						Time_min = 0;
					}
			}

//============    End creating sec, min, hour   ==========================

//============      PB0 control        ==========================

if (Time_sec >= PB0_OFF_time)					// Check if desired time is arrived
{
	if (!(PORTB & (1 << PB0)))				// Check if PB0 is set or not
	{
		PORTB |= ( 1 << PB0 );				// Set PB0 if not set before
	}

	if (PB0_ON_time == (Time_sec - PB0_OFF_time))		// Check if desired ON time is reached
	{
		PORTB &=~ ( 1 <<PB0 );				// Clear PB0 if PB0_ON_time reached
		Time_sec=0;					// Reset Time_sec time counter to 0
	}
}

//============      End PB0 control    ==========================

   }

}

 

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

In the main time, I rechecked the code again and still can not find out the problem, but I changed the IC to another new one Attiny85 

and wrote the same content to it.

 

After measuring with my scope here are the two results from the two IC's with the same program in it:
 

First Attiny85:

The ON time is 815ms

The OFF time is 815ms but it should be 1sec

The whole period of the signal is 1.63sec

 

Second Attiny85:
The ON time is 840ms

The OFF time is 860ms but it should be 1sec

The whole period of the signal is 1.7sec

 

Diferencial between the two IC's 70ms.

Is this normal or my code is not good in some way?

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

When Tick_counter >= 10, you are subtracting 200, not 100 from Time_counter.

 

Also, you are still not protecting your shared variable from corruption.  The possible error is smaller (just 1 timer tick, I think), but you are still performing operations on the shared variable in two different contexts, and there is a potential clash.  And your code is now clumsier and less clear - go back to a uint16_t counter and protect is properly.

 

Finally, Time_sec will never exceed 2, because at 2 you are setting it back to 0.

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

jodank wrote:

...

Diferencial between the two IC's 70ms.

Is this normal or my code is not good in some way?

 

The RC oscillators are not accurate, so some variation is expected.

I would suggest you toggle a pin in the interrupt, to confirm the actual interrupt rate on a frequency counter.  (addit: or scope or multi-meter ...)

You now have a 8 bit counter, but it's normal to avoid writes in two places with such interrupt/timing vars.

 

You can wrap MOD 100 inside the interrupt, and then either set a boolean, or increment a 1/10ths counter that can be read-only checked to pace your 1s timebases.

 

100ms is a fairly coarse tick rate, you may prefer to run 2 byte counters of MOD 10 and MOD 100 inside the interrupt, and poll the MOD 100 for 10ms LSB precisions, for other common tasks like debounce or chirps.

The main loop only needs to poll that for flyback (This < Previous), to increment seconds, it does not need to modify it.

Last Edited: Thu. Jul 25, 2019 - 01:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Who-me wrote:
I would suggest you toggle a pin in the interrupt, to confirm the actual interrupt rate on a frequency counter.

Agree, good idea (or a scope).

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

How do you know that your scope is accurate?

#1 This forum helps those that help themselves

#2 All grounds are not created equal

#3 How have you proved that your chip is running at xxMHz?

#4 "If you think you need floating point to solve the problem then you don't understand the problem. If you really do need floating point then you have a problem you do not understand." - Heater's ex-boss

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

Brian Fairchild wrote:
How do you know that your scope is accurate?

 

Modern scope  are, and you can use a Sound Card as a useful Oscilloscope, and those are good to with a couple of hundred ppm.

Most modern Multimeters also include a Hz scale, that is also quite good precision.

USB-Uarts can also be used as calibrate-checks, and the xtal-less ones are usually some fraction of a percent (they DPLL to the USB frames), and the Xtal ones are commonly better than 50ppm

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

Brian Fairchild wrote:
How do you know that your scope is accurate?

How do you know that your frequency counter is accurate? :)

 

Either the scope has a calibrator of known frequency, or you've used it recently to view signals of known pulse width / frequency.  Or wire up an oscillator >= 550 kHz, find the signal on a digital readout radio, and compare that frequency with what the scope reports.

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

How do you know that your scope is accurate?

I know it's accurate, it's never given a wrong reading!! surprise

 

Lots of misc boards have oscillator modules...find one & probe it.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

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

I would try to simplify things. One option would be just to do the time additions inside the isr. There may come a time when that type of thing is not a good idea, this isn't one of them as those additions are trivial and nothing else critical is going on.

 

One way would be like the following, where instead of a big if/else mess, you make it more readable (and will ultimately compile to the same code most likely)-

 

ISR(TIM0_COMPA_vect){
    if( ++Time_counter < 1000 ) return;
    Time_counter = 0;
    if( ++Time_sec < 60 ) return;
    Time_sec = 0;
    if( ++Time_min < 60 ) return;
    Time_min = 0;
    if( ++Time_hour < 24 ) return;
    Time_hour = 0;
}

But you are still left with problems to deal with. Those split up times are not going to be very nice to use. So I would think about just keeping a ms time (up to 60sec in this example, and if you use a uint32 instead, it becomes quite large). The unsigned math means you do not have to reset the ms time, and keeping the wait time to a 'safe' number means the unsigned math will always work (you just have to keep some distance between the max number and you max ms number). Now you have a blocking wait where you pass in the time in ms you want to wait. From there you can modify to make it non-blocking, and increase the max wait time.

 

volatile uint16_t time_ms;

ISR(TIM0_COMPA_vect){ ++time_ms; }

uint16_t now(){
    for(;;){
        //without turning off irq's
        //get twice, if match, is good
        uint16_t t1 = time_ms;
        if( t1 == time_ms ) return t1;
    }
}

void wait(uint16_t ms){
    //limit so unsigned math always works
    //60 seconds is a nice round number
    if( ms > 60000 ) ms = 60000;
    uint16_t t1 = now();
    while( now() - t1 < ms );
}

while(1){
    //pb0 off
    wait( 1000 );
    //pb0 on
    wait( 1000 );
}

I have not tested or compiled this, but I think its mostly correct.

 

Your internal osc is not very accurate to begin with- check the datasheet (I'm seeing a +/-10% number, but have not read much more- they have graphs also). You also have an OSCCAL register you can tweak if wanted.

Last Edited: Thu. Jul 25, 2019 - 07:41 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

kk6gm wrote:
Also, you are still not protecting your shared variable from corruption.
It doesn't matter any more - it's now a uint8_t which is naturally atomic.

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

Unfortunately, the OP is doing a read modify write on the variable. The read and the write are atomic by nature but what happens in between isn’t.

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

How precise do you want? Sounds like your clock is running fast. Maybe you need a crystal.

The largest known prime number: 282589933-1

Without adult supervision.

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

Unfortunately, the OP is doing a read modify write on the variable. 

Ah sorry. I thought we were just talking about:

			if (Time_counter >= 100)			// 100ms

which is an atomic access. Missed:

				Time_counter -= 100;

which is a very odd thing to do - why would one subtract. Surely if it exceeded 100 you just want to set it back to 0 and:

				Time_counter = 0;

would also be atomic.

Actually the more I look at that code the more puzzling it becomes. If you take the entire sequence:

			if (Time_counter >= 100)			// 100ms
			{
				Tick_counter +=1;
				Time_counter -= 100;
					if (Tick_counter == 10)
					{

						Time_sec +=1;		// 1sec reached
						Time_counter -= 100;

Then it just reached 100 or 101 or something. The first -= 100 takes it back to 0/1 but now "Tick_counter" also reached ten and overflowed so now you so a second -=100. It's a uint8_t so -99 or whatever is actually 0x9D which means it actually jumps forward to 157 ??

 

Was this all part of some devious/cunning plan? Or a design over-sight?

 

(sorry, I haven't read all the details above - perhaps this was already mentioned??)

 

PS I much prefer the simplicity of code such as Curt shows in #15. It's very clear, concise and not (AFAICS) prone to any kind of error. I don't think a handful of comparative tests in an ISR is going to be that great an overhead.

Last Edited: Thu. Jul 25, 2019 - 01:41 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

;

clawson wrote:

				Time_counter -= 100;

which is a very odd thing to do - why would one subtract. Surely if it exceeded 100 you just want to set it back to 0 and:

In general it is safer.  Suppose the >= 100 test was delayed until Tick_counter was 102.  Wouldn't happen in this particular code, but could happen as a general rule.  If you just set Tick_counter back to 0 then you've lost (slipped) those two extra ticks forever.  But if you subtract 100 then you have kept long term accuracy.

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

kk6gm wrote:
 But if you subtract 100 then you have kept long term accuracy.
Not if you do it twice in one cycle ;-)

 

Seems to me to be a better idea to keep all your counter incrementing/resetting in one place (the ISR) then you won't have atomicity/synchronization issues.

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

clawson wrote:

Not if you do it twice in one cycle ;-)

Can't argue with you there. :)

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

Hi folks!

I sitting here and try to take steps with your writings.

I really appreciate your infos, there are so many planty new moments for me, from what I learn so much.

I'm not a pro, but a hobbyist and I like to make some gadgets for fun and therefore I use AttinyXX uC's.

I'm also learning C cos I migrated from Bascom to C a year ago or so.

 

However, lets do things right... smiley

 

I know my scope is accurate, it is not a big deal, big brand but a good one for my home DIY time.

It is a Siglent SDS 1052DL+ scope, I use it very frequently on electronic repairing stuff, DIY projects etc...

But I will check it with an 1KHz signal generator.

 

I like the code as curtvm wrote in #15 but I have to reach the level in C to think in such of programming logic.

 

What I'm not sure and not understand completely is:

 

1.
If I would write a code like mine is, with a  IF/ELSE mess, would that be more complicated for the uC to do the operations than if I write the same code like in #15 is?

I'm not sure do you got the point what I wish to ask, but I hope.

 

I think, such of code like #15 would be simpler and more clear to understand for the reader, but the uC have no problem to finish the task both way...

 

2.

I realized the problem in my code where I change the variable which is located in the ISR in the main code after you pointed me to that.

That's a bad think I assume.

If I would let the counter Time_counter in the ISR to count to the max value which is 255 without resetting it, what will happen when the variable reach the number higher than 

the unit8_t can handle?

 

3.

I'm confused now about the protecting of the  shared variable from corruption.

As far as I use a variable on a 8bit uC declared as uint8_t it must not be protected.

It should be safe from corruption I assume and learned this in that way.

How can I be sure do I have or not to protect the shared variables?

I know, it was my mistake to use the variable as a 16bit variable in a 8bit uC.

In this case I really should think about the protection.

 

Should I change/reset to zero the Time_counter in the ISR?
That sound me more logically, cos in the ISR could be changed the Time_counter value until the same variable is modified in the main program.

And that would lead in a trouble I assume... surprise

 

 

 

 

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

jodank wrote:

I'm confused now about the protecting of the  shared variable from corruption.

As far as I use a variable on a 8bit uC declared as uint8_t it must not be protected.

It should be safe from corruption I assume and learned this in that way.

Not in a read-modify-write operation,  Example:

-- main code loads 8-bit variable X

-- interrupt runs, ISR modifies X

-- main code performs operation on old X and writes it out.  New X has been lost.

 

An example would be a counter where the ISR increments the counter while the main code decrements it.  You can miss the ISR increment completely.

 

Quote:
How can I be sure do I have or not to protect the shared variables?

If it's multi-byte (for 8-bit device), always protect it.  If it's single byte, protect it if main code does a read-modify-write and if ISR can modify the variable (as opposed to just reading it).

 

There is also the technique, for reading only, of doing multiple reads of the variable in main code until you get two that are the same.  Then you know that the ISR has not modified the variable during the main code fetch.

 

Quote:
I know, it was my mistake to use the variable as a 16bit variable in a 8bit uC.

Not at all.  In fact, I think your use of a 16-bit variable was more clear and appropriate, and I would recommend you get that working and correct.

 

Last Edited: Thu. Jul 25, 2019 - 04:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

jodank wrote:
As far as I use a variable on a 8bit uC declared as uint8_t it must not be protected.
This is only true if you use things like "if (var == something)" or "var = new value". that actual access to "var" in these cases is one opcode (for an 8 bit var) so there's no chance that an interrupt can occur while it's being read or written. But as soon as you start doing stuff like "var = var op something" (a "read , modify, write" or just "RMW") then between the read and the write an interrupt might occur and change the underlying value of "var". So say it was 17 and you are about to add 7 the new value should be 24. But after it has been read but before you write it back the interrupt occurs and increments it to 18. You are now still going to write 24 back but 18+7 should have been 25. 

 

As noted above there's a very simplistic thing you can do which is:

cli();
var = var + 7;
sei();

but C gives no guarantee that the CLI, operation and SEI will occur in that order. Also suppose interrupts were off at the start a simple sei() just turned them on when they shouldn't be. Slightly better then is:

old_state = SREG;
cli();
var = var + 7;
SREG = old_state;

which should put things back how they were. But again C does not guarantee the order here (there's a technical thing called "sequence points" needed)

 

So, better is:

ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    var = var + 7;
}

that does the SREG/state/cli thing but guarantees the sequence. Now the bit in brackets cannot be interrupted. If an interrupt would occur to change "var" then it will be delayed until after this bit finishes so in the example var will be written as 24 then the delayed interrupt would increment it to 25.

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

>If I would write a code like mine is, with a  IF/ELSE mess, would that be more complicated for the uC to do the operations than if I write the same code like in #15 is?

 

It would compile to the exact same code. Don't worry about the compiler, it is quite smart. Use code that is easiest to read/understand. There may be times you have to adjust your code so the compiler produces 'better' code, but in most cases it is not worth 'micromanaging' the compiler. If you do not generate an asm listing file, I would start doing that. I add a step in every project I create to run avr-objdump -d project.elf > list.txt (there are various objdump options, and depending on what you use for ide, it may also have an option to do this all for you). Then you can see what the compiler does with your c code.

 

>Should I change/reset to zero the Time_counter in the ISR?

 

I would do as I suggested, let the timer counter run free. Let's say you have 2 led's and you want them to blink at different rates. How would you do that when you reset the timer counter? If you let the timer counter run without resetting to 0, you can use as many timers as you want. Each timer event only needs to know 'now' (start time) and 'ms' (how long).

 

This is an example of what I do for delay timers (usually in c++ which ends up looking nicer, and I use some unused timer- rtc, cpu count, nco, but in a tiny85 you don't have many choices). Again, this is untested and just an idea which could be refined further.

 

#include <avr/io.h>
#include <avr/interrupt.h>
#include <stdint.h>
#include <stdbool.h>

typedef struct {
    uint16_t start; //start time
    uint16_t ms;    //ms
    bool expired;   //timer expired
} timer_t;

timer_t led1, led2;

bool expired(timer_t* t){
    if( ! t->expired && now() - t->start >= t->ms ){
        t->expired = true;
    }
    return t->expired;
}

void restart(timer_t* t){
    t->start = now();
    t->expired = false;
}

void set_ms(timer_t* t, uint16_t ms){
    t->start = now();
    t->expired = false;
    if( ms ) t->ms = ms;
}

//other code here

int main(){
    //setup stuff

    set_ms( &led1, 100 );
    set_ms( &led2, 500 );

    for(;;){
        if( expired( &led1 ) ){
            //toggle led1 here
            set_ms( &led1, 0 ); //0 = use previously set ms
        }
        if( expired( &led2 ) ){
            //toggle led2 here
            set_ms( &led2, 0 );
        }

    }
}

 

Now you can blink as many led's as you want, each at a different rate (ok, maybe 3 led's for a tiny85).

 

toward the bottom, I have a few video links that show blinking lights on a samd10 xplained mini-

https://github.com/cv007/Samd10XplainedMini

this is using the similar technique I described to get each led using its own timer

(various blink routines, some morse code messages at various rates per led, etc)

 

Last Edited: Thu. Jul 25, 2019 - 07:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes yes guy's it is much more clear now for me.

I think I got the point.

 

Let's say, my understanding of the problem:

A Counter++ is in the ISR counting.

 

To RMW the Counter variable in the main code I must stop the ISR, until my program do things with the Counter variable in the main/while loop...

 

If I would let say change the Counter in my while loop and didn't stop the ISR I can get a corrupted information at the end and a wrong working program.

 

In nutshell this is how I understand the problem.

 

I'm reading and searching through the Net and this forum, what I can realize, many people have problems to understand this "simple" 1ms task

and many many writings are out here, but I can't find a simple but good or even usable code example.

 

Can somebody show a link or came up with a correct example but for newbies how to toggle a LED 500ms ON 500ms OFF for Attiny85 or similar uC?

So we can maybe get a better understanding.

 

In the meantime, I reading and remembering what you guys wrote when I was asking for help in some other project.
Actually I bookmark/print out all the good stuff's, that's the way I take care about tutorials, writings, explanations... :-)

 

In this post #53 was written:

Note that as long as the counter is 8 bits, you can read it and set it safely in your main code, because those operations are atomic on 8-bit values.  If you needed to go to a 16 or 32-bit counter, or if you needed to do a RMW (read-modify-write) in the main code, like counter++, then you'd have to take extra precautions.

 

     In the first part is written:

... you can read it and set it safely in your main code ...

 

     And than:

... or if you needed to do a RMW (read-modify-write) in the main code, like counter++, then you'd have to take extra precautions. ...

   

Now it is for me not clear, if I use uint8_t on a 8-bit uC, and wish to RMW do I have or do I have not to protect the variable?

 

The bolded text says to me does I don't have to take any precaution if I use 8-bit value even if I RMW it.

Then in the 3. quotation I should use precaution.

Does this means only if I use 16 or 32-bit values?

 

In my code what I posted in my first post here I made a huge mistake, I used a uint16_t but didn't protected the variable.

That's ok and my mistake, according to this explanation above.

 

But when I changed from uint16_t to uint8_t then I got from the same person in #8 who wrote the text what I quoted above in another post on this forum a warning 

I again didn't used protection...

 

That is totally confusing me now...

 

Thank you very much for such of hel what you do here.

Really glorious things.

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

Here is a method I like to use for multiple actions based either on time or on software flags.

 

//
//  LeapFrog.c
//
//
//  Created by Michael Silva on 7/25/19.
//

#include <stdio.h>

volatile uint16_t Tick = 0;

ISR...()
{
    Tick++;
}

bool deadline_reached(uint16_t deadline)
{
    cli();
    bool result = ((int16_t)(Tick - deadline) >= 0) ? true : false;
    sei();
    return result;
}

main()
{
    uint16_t dl_1;  // deadline values in units of Tick
    uint16_t dl_2;
    uint16_t dl_3;
    bool flag3 = false;
    bool flag4 = false;
    // code to set deadlines and flags

    while (1)
    {
        if (deadline_reached(dl_1))
        {
            // do dl_1 task
            dl_1 += INTERVAL_1;
        }
        if (deadline_reached(dl_2))
        {
            // do dl_2 task
            dl_2 += calc_dl_2();
        }
        else if (deadline_reached(dl_3) || flag3)
        {
            // do dl_3 task
            cli();
            dl_3 = Tick;
            sei();
            dl_3 += INTERVAL_3
            flag3 = false;
        }
        else if (flag4)
        {
            // do flag2 task
            flag4 = false;
        }
    }
}

The Tick counter is free-running, never getting reset.  It could be one byte, 2 bytes or even 4 bytes, depending on timing resolution and maximum interval required.  The maximum interval (including any loop latency) that can be timed is the max positive integer value, so 127 ticks for 1 bytes, 32767 ticks for 2 bytes.

 

The Tick value is fetched for every deadline_reached call.  If necessary the fetch can be protected with cli()/sei(), or with ATOMIC_BLOCK(ATOMIC_RESTORESTATE), or by using the 2 identical reads method.

 

Some control over priority is achieved in the while loop by using if() (will test for task run every time through) or by using else if() (can prevent multiple lower-priority tasks from running in the same loop, thus giving higher priority tasks lower response latency).

 

Note also that the use of flags can be combined with deadline values.  One example of this could be a display update task, where the display could be updated every 500 ms but could also be updated immediately on some event such as a keypress.  In this example, the next deadline is correctly calculated even if the task ran early due to the flag being set.

 

Finally note that the next deadline can be a calculated value as well as a constant value.  One example might be button auto-repeat with rate speedup as the button is held down longer.

Last Edited: Thu. Jul 25, 2019 - 08:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

jodank wrote:

Now it is for me not clear, if I use uint8_t on a 8-bit uC, and wish to RMW do I have or do I have not to protect the variable?

 

If you look at the created assembler, it may be clearer. 

You need to consider possible delays, in more complex code, which may mean the next interrupt fires, right in the middle of a C line, after you have read the var, but before you write it.

ie if you write in more than ONE place (one inside INT, one outside), you have to worry about the possible relative timing of those writes. 

 

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

Thank you for that fast response and nice example.

 

I can clearly see how you managed the ISR, how you stop [ cli() ] and start [ sei() ] the ISR whenever you RMW the Tick variable.

 

If I understand it correctly, I could also manage this way like you in my code the Time_counter .

 

Like so:

 

ISR(TIM0_COMPA_vect)
{

    Time_counter++;                        

}



While(1)

{

     something...



             cli();

             Timer_counter = 0;
             
             sei();



     something....

    

}

 

This way I can actually switch back and use a 16-bit value and have no problem with the ISR and corrupting the data in the variable...

Am I right?

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

>Can somebody show a link or came up with a correct example but for newbies how to toggle a LED 500ms ON 500ms OFF for Attiny85 or similar uC?

 If all you want is a blocking delay, you don't need to get very complicated-

#define F_CPU 8000000ul
#include <avr/io.h>
#include <stdint.h>
#include <util/delay.h>

void delay_ms(uint16_t ms){
    for( ; ms; _delay_ms( 1 ), ms-- );
}
void setup(){
    DDRB = (1 << PB0); //pb0 output
}
int main(){
    setup();
    for(;;){
        PORTB |= (1 << PB0);
        delay_ms( 500 );
        PORTB &= ~(1 << PB0);
        delay_ms( 500 );
    }
}

Its not going to be precise when using the _delay_ms in a loop, but close enough and will not make any difference when your inaccurate internal osc is the major cause of error in accuracy. Also, you will not know that the led is only on for 490ms instead of an exact 500ms.

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

A stable waveform with a wrong frequency in a timing/blinking program means either incorrect system clock, or wrong timer programming parameters.

 

Check and verify the system clock first.  Never trust the F_CLK setting in software to be your real clock speed.

 

What is your real system clock speed?   How do you know this? Have you programmed the CKOUT to put the system clock on the COUT pin so that you can measure it with an oscilloscope and verify that system clock is what it should be?  Are you using a crystal?  Were the fuses changed to "external crystal"?

 

Is this ATtiny85 still configured as it came from the factory?   If not, what has been changed?  If this is school project and the tiny85 has been used previously in other lab exercises, then what changes were made to the fuses previously?  Did they adjust the phase-lock loop and not tell the next class?  Have you adjusted the phase-lock loop settings?

 

Fixed that...The F_CLK says 8 Megahertz and the o-scope is showing 8MHz on the COUT pin.

 

Turn off everything in software but the timer that can count to one second using system clock and pre-scalers. Use the timer1 if needed.  Determine the exact count and pre-scaler to get 500milliseconds.  Write a program that does nothing but count to this interval and then toggles one output pin.  Do not use interrupts.  

When it works for exactly one second blinking, go back and add all the real-time keeping code.  

When that works convert the timer interval to 0.001 second.  Enable an interrupt that updates a long unsigned volatile variable each 0.001 second.   Have the main code check if this counter has a remainder of zero when divided by 500.  If yes, then reenable the blink code.  If the output pin is high on this cycle, do the real-time code.

 

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

Here is a program I wrote right now to test the frequency by toggeling the PB0 through the ISR on ~1KHz.

 

#define F_CPU 8000000UL				// Define CPU frequency in Hz.
#include <avr/io.h>
#include <avr/interrupt.h>

static inline void Timer_On(void)		// Attiny85 Timer setup
{
	TCCR0A |= (1 << WGM01);			// clear timer on compare match (TCCR = Timer/Counter Control Register)
	TCCR0B |= (1 << CS00) | (1 << CS01);	// clock source and prescaler 64 setup
	OCR0A = 62;				// trigger interrupt every ([1 / (8E6 / 64)] * (62+1)) = .5ms ~ 1 KHz
	TIMSK |= ( 1 << OCIE0A );		// enable output compare match interrupt - for Attiny85
	sei();					// enable interrupts
}

ISR(TIM0_COMPA_vect)				// TIMER0 ISR triggered depend on the Timer setup
{

	PORTB ^=( 1 << PB0 );			// Toggle PB0

}

int main(void)
{

	DDRB |= (1 << PB0); 			// set PB0 on PORTB as Output
	PORTB &=~ ( 1 << PB0 ) ; 		// clear PB0
	Timer_On();				// Start timer

while(1)
   {

   }
}

 

So, I checked with my scope the signal and I got a reading:

One period is 980uS and the scope show around 1.02641 KHz.

 

I checked my scope calibration with the 1 KHz signal generator from the scope and the signal is measured exactly 1.00000 KHz.

It means the scope is measuring really accurate.

 

 

Last Edited: Thu. Jul 25, 2019 - 09:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

That’s around 2% error. Well within the expected tolerance of the internal clock.

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

Blow the AVR with a hot air gun and some freeze spray and enjoy the fun.

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

jodank wrote:

Here is a program I wrote right now to test the frequency by toggeling the PB0 through the ISR on ~1KHz.

One period is 980uS and the scope show around 1.02641 KHz.

 

 1.02641k*(62+1)*64*2 = 8276970.24MHz as SysCLK or 3.462% error

A smarter test is to use the same dividers your final interrupt will use, and here you then expect a 500Hz frequency from a 1ms toggle.

Leave the pin toggle in place, when you write the rest of the code.
 

 

jodank wrote:

I checked my scope calibration with the 1 KHz signal generator from the scope and the signal is measured exactly 1.00000 KHz.

It means the scope is measuring really accurate.

Did you measure the 1kHz on an external frequency counter ? If the scope measures itself, it does not show you "the scope is measuring really accurate"

Any half-decent digital scope will have a crystal/oscillator inside, so you could expect low tens of ppm, or better.  (WAY better than any AVR RC Clock )

If you have a GPS 1pps signal, you can check your scope accuracy. 

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

I'm also learning C cos I migrated from Bascom to C a year ago or so.

Went to the dark side...

 

 Can somebody show a link or came up with a correct example but for newbies how to toggle a LED 500ms ON 500ms OFF for Attiny85 or similar uC?

Well, in Bascom one might do it something like below, using a Mega328P, (Arduino Nano), with its 16 MHz Xtal, and an LED on Portd.3:

 

(Bascom can configure the Timer/Counter in one statement, but I'm used to doing it myself, so that I am sure exactly how it is being configured.)

 

JC

 

'File:  Nano LED Toggle V1.bas
'Bascom  July 27, 2019   JC

'This program Toggles an LED On/Off at 1 Hz using
'Timer/Counter #1 in CTC mode, firing an ISR firing at 1 KHz, (1/mSec)

'Hardware is an Arduino Nano.
'Mega328P  16 MHz Xtal  5V project
'Nano PCB LED: LED PortB.5  (Arduino: "D13")
'LED1 PortD.3

   $regfile = "m328pdef.dat"                                'Specify the uC
   $crystal = 16000000                                      '16 MHz

   $hwstack = 64                                            'Hardware Stack
   $swstack = 64                                            'Software Stack
   $framesize = 64                                          'Frame Space

   'Hardware Aliases:
   Led1 Alias Portd.3                                       'Led1 Red High is On

   'Now Configure the Port's Pins for Input or Output mode.
   Config PortD.3 = Output                                  'LED1


   'Variables:
   Dim Regval As Byte                                       'Register Value for manual config
   Dim TicCnt As Word                                       'Timer Tic Counter

   'Hardware Startup Delay:
   Waitms 10

Inits:
   TicCnt = 0                                               'Init Tick Tock ISR Counter

StartUp:
   'Config Timer/Counter #1 Manually:
   'Setup to fire ISR at 1 kHz, i.e. once every 1 mSec.
   'Using this for LED flashing at 1 Hz.
   'Set it up for CTC Mode. Input is from the PreScaler.
   'Clock = 16 MHz, (Ext. Xtal)
   'PreScaler = 64
   '16 MHz / 64 -> 250,000 Hz into the Timer/Counter.
   'T/C divide by 250 gives 1 KHz Interrupt Rate.
   'Top = 249, as rollover to 0 counts as 1.

   'First Turn On the Timer/Counter1 Module.
   'Read the Power Reduction Register, set the Tim1 bit to 0 to Enable it.
   'Then write it back.  This does NOT alter any other module's activity.
   'Bitwise AND with 1111 0111   All bits unchanged, except Bit 3 = 0.
   Regval = Prr                                             'Read in current Power Reduction Register Values
   Regval = Regval And &B11110111                           'Force Bit 3 = 0
   Prr = Regval                                             'Save new value

   'Now set up the Timer/Counter#1 Control Registers for CTC Mode, PreScale Div by 64:
   Tccr1a = &B00000000                                      'Timer/Counter Control Register A
   Tccr1b = &B00001011                                      'Timer/Counter Control Register B
   Tccr1c = &B00000000                                      'Timer/Counter Control Register C

   'Now set the TOP value for the A Output Compare Register.
   'Set for Divide by 250, TOP = 249d = 1111 1001 B
   'Write the High Byte First!
   Ocr1ah = &B00000000
   Ocr1al = &B11111001

   'Now Set The Interrupt Mask:
   Timsk1 = &B00000010                                      'Timer/Counter1 Interrupt Mask Register

   'Now Enable the Interrupt and define the ISR handler:
   Enable Oc1a                                              'CTC Timer Counter 1 Intr
   On Oc1a Heartbeat                                        'Timer/Counter1 CTC A Intr

   Enable Interrupts                                        'Global Enable Interrupts

Main:

   Do
      'Main Loop does nothing.
      'The entire program, toggling the LED, runs in the ISR

   Loop

Heartbeat:
   'Timer/Counter #1 ISR for flashing an LED.
   'ISR fires at 1000 Hz, (q 1 mSec)
   'TicCnt counts from 0 to 999 (mSec)
   'LED is ON  for counts from 0 - 499
   'LED is OFF for counts from 500 - 999, plus roll over to 0

   TicCnt = TicCnt + 1                                      'Incr Tic count at 1 KHz rate

   If TicCnt < 500 Then
      Set Led1                                              'LED On
   Else
      Reset Led1                                            'LED Off
   End If
   If TicCnt >= 999 Then
      TicCnt = 0                                            'Rollover
   End If
   Return

 

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

If you really want to have some fun, switch to C++

 

https://godbolt.org/z/A5Gv0u

 

I didn't like my previous C example, got a little carried away with trying a C++ version :)

 

 

 

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

#38 

Thanks for the link, It looks interesting for me. :-)

 

I tested again my scope with an old oscillator device what I got with one of my old fashion analog scope maybe 30 years ago.

The device generate a square waveform of 1MHz.

When I attached my scope to the output I got a reading of 999.984KHz

I think it is pretty accurate.

The picture of my old oscillator device is in the attachment.

 

Here #60 is a very good explanation of an atomicity failure, I put this link here just for other readers who maybe find this post and didn't found the other one.

 

About

... let the timer counter run free.

I'm not sure what would happen with a let's say uint8_t variable in this situation:

Let's say the ISR overflow in every 1ms.

The Timer_couner increments every time by 1.

In a certain time the Timer_counter will achieve the max number it can hold, which is 255 cos it is an 8-bit variable.

The ISR count's again and the Timer_counter increment to 256 and then again to 257 ... what is actually an error.

 

What will happen with the Timer_counter value if I never reset it to 0 but let the ISR free running?

Will the Timer_counter reset themself to 0 after reached the max value?

 

// Let's say the ISR overflow in every 1ms

volatile uint8_t Time_counter = 0;

ISR(TIM0_COMPA_vect)
{
    
    Timer_counter++;
    
} 

Attachment(s): 

Last Edited: Fri. Jul 26, 2019 - 08:48 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

jodank wrote:
The ISR count's again and the Timer_counter increment to 256 and then again to 257 ... what is actually an error.
The next value after 255 in an unsigned 8 bit variable is 0. That is:

 

... 253, 254, 255, 0, 1, 2, 3 ...

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

jodank wrote:

In a certain time the Timer_counter will achieve the max number it can hold, which is 255 cos it is an 8-bit variable.

The ISR count's again and the Timer_counter increment to 256 and then again to 257 ... what is actually an error.

 

What will happen with the Timer_counter value if I never reset it to 0 but let the ISR free running?

Will the Timer_counter reset themself to 0 after reached the max value?

As noted, the sequence will be 253, 254, 255, 0, 1, 2, ...

 

The nice thing is that the difference, viewed as a signed value, will always be < 0 if the deadline is not reached, and >= 0 if it has been reached.  With the limitation that the interval can never be more than the max positive integer that the variable can represent.

 

Suppose the previous deadline was 250, and 10 was added to that, giving a new deadline (with 8-bit rollover) of 4.  As Timer_counter ticks up and overflows, the difference (int8_t)(Timer_counter - deadline) will be as follows:

 

(int8_t)(251-4) = -9

(int8_t)(252-4) = -8

(int8_t)(253-4) = -7

(int8_t)(254-4) = -6

(int8_t)(255-4) = -5

(int8_t)(0-4) = -4    Timer_counter rolled over to 0 here

(int8_t)(1-4) = -3

(int8_t)(2-4) = -2

(int8_t)(3-4) = -1

(int8_t)(4-4) = 0    Deadline reached

 

Last Edited: Fri. Jul 26, 2019 - 04:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:
(int8_t)(0-4) = 0    Deadline reached

oops, I think you meant (int8_t)(4-4) = 0

 

Jim

 

Click Link: Get Free Stock: Retire early! PM for strategy

share.robinhood.com/jamesc3274
get $5 free gold/silver https://www.onegold.com/join/713...

 

 

 

 

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

Right, will fix.  Thanks.

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

Ok, than it is safe to let freely running the ISR and counting "forever" some variable if needed.

And not RMW the variable which is in the ISR. So I could avoid some atomicity conflict...

 

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

In your main code you will only be reading the Tick variable, not writing.  But, unless it is only 8 bits, you still need to protect the main code read accesses so the multi-byte access is not interrupted.d

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

Ok, I experimented a bit and went back to some fundamental stuff with my programm and wrote a small code which will toggle the PB0.

What do you think about this code?
Is it good enough for average use in DIY projects?

 

Is it better to use Atomic_Block or cli()/sei() function?

Now I have a good timing on my scope, but how good is my code now compared to my original code?

I mean the ISR and timing handling parts...

 

Is it better to toggle the PB0 like PIN = ( 1 << PB0 ); 

Or is it the same as PORTB ^=~( 1 << PB0 ); 

 

I also found a very handy source , the AVR LIB Reference in case somebody need it.

It helped me to figure out how to use the Atomic_Block...

 

#define F_CPU 8000000UL
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/atomic.h>

volatile uint16_t Tick_500ms = 500;

static inline void Timer_On(void)		// Attiny85 Timer setup
{
	TCCR0A |= (1 << WGM01);			// clear timer on compare match (TCCR = Timer/Counter Control Register)
	TCCR0B |= (1 << CS00) | (1 << CS01);	// clock source and prescaler 64 setup ( Timer/Counter Clock Sources data sheet pg.58)
	OCR0A = 124;				// trigger interrupt every ([1 / (8E6 / 64)] * (124+1)) = 1ms ~ 1KHz
	TIMSK |= ( 1 << OCIE0A );		// enable output compare match interrupt - for Attiny85
	sei();					// enable ISR
}

ISR(TIM0_COMPA_vect)				// TIMER0 ISR triggered depend on the Timer setup
{
	Tick_500ms--;
}

int main(void)
{

	DDRB |= (1 << PB0); 			// set PB0 on PORTB as Output
	PORTB &=~ ( 1 << PB0 ) ; 		// clear PB0
	Timer_On();				// Start timer

while(1)
   {
	ATOMIC_BLOCK(ATOMIC_FORCEON)  	        // Stop the ISR (Prevent the Tick_500ms from data corruption)
	{
		if (!Tick_500ms)		// Check if 500ms reached
		{
			Tick_500ms=500;		// Reset the Tick counter
			PINB = ( 1 << PB0 );	// Toggle PINB0
		}
	}

   }
}

 

Last Edited: Sat. Jul 27, 2019 - 09:32 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

If you want precise toggling of the port bit, then do it in the isr and avoid the atomic block stuff. Even more precision is gained by using the output compare feature of the timer which will give you precision to the clock cycle.

Some AVRs have the port toggle feature by writing to the PIN register. This is better than the PORT read/modify/write. The output compare will also do port toggle.

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

jodank wrote:

Is it better to use Atomic_Block or cli()/sei() function?

That comes down to personal preference,

I prefer the cli()/sei(), as it is clear what you are doing. With Atomic_Block it is less obvious where that ceases to apply, or even what it actually does, without jumping on google...

 

 

jodank wrote:

Now I have a good timing on my scope, but how good is my code now compared to my original code?

I mean the ISR and timing handling parts...

If all you want to do, is run a HH:MM:SS clock, then almost anything will do.  In your case, you just need to ensure the longest code path where INT are disabled, never exceeds 1ms

 

For more general use, it is better to avoid disable of interrupts, as that as a global impact across ALL code, and if you disable for too long, things can get missed on a rare basis, which is a pain to debug.

 

Notice with your setups, you can avoid needing a 16b integer variable entirely, if you move to a 2ms interrupt divided by 250. 

 

From a code clarity viewpoint, it is clearer if the timing wrap is all done in one place, rather than split in multiple places.

ie here you increment in INT, but check and reset outside INT - that's ok in a tiny example, but in a large project, who-does-what gets confused.

Cleaner is to manage timing wrap inside the interrupt, and set a timing boolean flag, or a tracking counter, that is polled and cleared elsewhere.

Flags are simpler, on slower events, a tracking counter you may use if you expect there is a chance an interrupt poll may be missed.

 

eg If you come along later, and want to fine tune the interrupt to a more precise 500ms, it is clear where to do that.

 

You currently have a deviation from expected SysCLK of 3.462% error, which is ok for the age of device you are using (but 'poor' in 2019, You can nudge the constants here to calibrate this particular MCU to within 0.164% (room temp))

Newer MCU devices are better, and of course, external xtals/oscillators are a lot better too...

What timing precision do you need ?

 

New ATtiny data : 

"During Reset the calibration values for the OSC20M are loaded from fuses . There are two different calibration bit fields. The Calibration bit field (CAL20M) in the Calibration A register
(CLKCTRL.OSC20MCALIBA) enables calibration around the current center frequency. The Oscillator Temperature Coefficient Calibration bit field (TEMPCAL20M) in the Calibration B register
(CLKCTRL.OSC20MCALIBB) enables adjustment of the slope of the temperature drift compensation. 

For applications requiring more fine-tuned frequency setting than the oscillator calibration provides, factory stored frequency error after calibrations are available."

- but their step size is 1.5% typical, so that last sentence sounds useful.

 

I see these new ATtiny store errors, at 3V and 5V, in a S.7/1024 format, so just under 0.1% LSB steps, with max numeric range +/- 12.4%

 

Last Edited: Sat. Jul 27, 2019 - 11:02 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

If you want to do the math in the ISR, do it all in the ISR, and just use a one-byte flag to communicate to main code (thus, no possible data corruption).

 

volatile uint8_t Tick_flag = 0;     // or false

ISR(TIM0_COMPA_vect)				// TIMER0 ISR triggered depend on the Timer setup
{
static uint16_t Tick_500ms = 500;

    if (--Tick_500ms == 0)
    {
        Tick_500ms = 500;
        Tick_flag = 1;      // or true
    }
}

int main(void)
{

    DDRB |= (1 << PB0); 			// set PB0 on PORTB as Output
    PORTB &=~ ( 1 << PB0 ) ; 		// clear PB0
    Timer_On();				// Start timer

    while(1)
    {
        if (Tick_flag)
        {
            Tick_flag = 0;  // or false
            PINB = ( 1 << PB0 );	// Toggle PINB0
        }
    }
}

Notice now that Tick_500ms is completely hidden from main code, and communication between ISR and main code is only through setting and reading (but not read-modify-write RMW) an 8 bit variable, so no possible data corruption, and no need to ever turn off interrupts.

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

Ok, what I'm interested in is now, why you declared the Tick_500ms as static and inside the ISR?

In this case, it will always be declared whenever the ISR is triggered.

Could it not be declared once outside the ISR?

Could it not be volatile and declared outside the ISR?

 

I mean, if it is volatile or static in this case makes not a big difference cos nothing/nobody will access the 

variable outside the ISR and so I think the data corruption can't be achieved.

 

Do I maybe missing something?

 

Thanks for the nice code example and explanation.

 

 

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

Having lots of global variables is considered bad practice. Having tick_500ms local to the isr makes more sense. Static means the variable is created once but lives on. As far as the AVR is concerned, whether it is global or static, there is no difference. If it were global, volatile would not be needed as it is not shared between two or threads of execution.

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

A general rule is to keep your variable scope as limited as possible (essentially, keep your variables hidden from all functions that don't need to access them).  Declaring Tick_500ms as static inside the ISR immediately tells you (and enforces in the compiler) that nobody else will be able to access the variable, and causes the variable contents to be persistent between function calls.  You don't have to bother searching the code for any other accesses, and you know there cannot possibly be any data corruption.  Always keep your variables in the smallest bag possible.  

 

Fancier languages even let you have local functions, where functions are declared inside other functions and are only visible inside those outer functions.  It is a very cool thing to do.

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

I'm thinking to try to playing with an external crystal resonator but I'm a bit afraid cos I'm not sure which fuse I should select.

I can remember some time ago when I was trying the same thing and I "locked" several tinyes with the wrong configured fuses.

After all I bring them back but I'm not sure what I have to tick in real.

 

So, I would like to use an 16MHz external crystal resonator for clock the uC.

Is there any code what I should program in my program and what fuse should I select in my programmer to achieve the task?

 

Picture attached.

 

Thanks.

 

Attachment(s):