[chbot] AVR external interrupt and Makefile example

[Charles Manning]

sei()/cli() is not necessarily non-portable because you generally use a macro 
that you can redefine whereever it is needed.

What is important though is to guard against reenabling interrupts when not 
intended.

For example consider:

void clear_counter(void)
{
	cli();
	temp = counter;
	sti()
	return temp;
}

int a_n_other(void)
{
	cli();
	...A...
	clear_counter()
	...B...
	sti()
}

or

main()
{

	/* initialisation  at start with interrupts off */
	...C..
	clear_counter();
	...D...
	/* Everything set up OK, now turn on interrupts */
	sei();
	...
}


At first glance that looks like the code section ..B.. in a_b_other is 
protected by the cli()/sti(). Nope. The sti() in clear_counter() reenabled 
interrupts.

Same with using clear_counter() during start up. It would have turned on 
interrupts too soon!

ATOMIC_BLOCK can help, but there is a way to do this manually too by 
remembering if interrupts were on or off.

eg
void clear_counter(void)
{
	uint8_t istate = interrupt_GetAndDisable()
	temp = counter;
	interrupt_Enable(istate);
	return temp;
}

Have a look at http://github.com/cdhmanning/avrprojects
for examples. 

-- Charles


On Sunday 15 November 2009 10:11:58 Richard Jones wrote:
> Many thanks Col,
>
> You located the 'bug' I referred to in my original post.
> 11 improvement opportunities to go!
> However I was unaware of the ATOMIC_BLOCK c99 feature and have to confess
> its portability is a real benefit compared to adding sei and cli
> instructions.
>
> Richard
>
> On Thu, 12 Nov 2009 17:40:15 +1300, Col <colp at paradise.net.nz> wrote:
> > Hi Richard
> >
> > In the interest of the group I would like to point out your supplied
> > code contains a race condition.
> >
> > Basically the count4ms is 16 bit variable on an eight bit micro. If an
> > interrupt occurs 1/2 way through reading the variable you could get
> > unexpected results.
> >
> > My suggestion is something like
> >
> >
> > #include <util/atomic.h>
> >
> > ATOMIC_BLOCK( ATOMIC_FORCEON )
> > {
> > 	temp = count4ms;
> > }
> > if ( temp >= 305 ) {
> >
> >
> > This is outlined in the libc user manual.
> > http://www.nongnu.org/avr-libc/user-manual/group__util__atomic.html
> >
> >
> > Cheers
> > Col.
> >
> > _______________________________________________
> > Chchrobotics mailing list Chchrobotics at lists.linuxnut.co.nz
> > http://lists.ourshack.com/mailman/listinfo/chchrobotics
> > Mail Archives: http://lists.ourshack.com/pipermail/chchrobotics/
> > Web site: http://kiwibots.org
> > Meeting Dates:
> > Tue 15 December 2009 6.30pm
> > Tue 16 February 2010 6.30pm
> > When replying, please edit your Subject line to reflect new content.
>
> _______________________________________________
> Chchrobotics mailing list Chchrobotics at lists.linuxnut.co.nz
> http://lists.ourshack.com/mailman/listinfo/chchrobotics
> Mail Archives: http://lists.ourshack.com/pipermail/chchrobotics/
> Web site: http://kiwibots.org
> Meeting Dates:
> Tue 15 December 2009 6.30pm
> Tue 16 February 2010 6.30pm
> When replying, please edit your Subject line to reflect new content.