Interruption function not working as it should

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

Hello everyone!

I'm just new at the forum and I have already got some experience with PIC and now I'm starting with AVR. I've tested some stuff using timer0/1, external interrupts and etc, but in this last project I got some trouble I could not solve by myself.

I'm trying to build a metronome with ATmega328P using Arduino Duemilanove breakboard, and the code is this so far:

#include 
#include 
#define F_CPU 16000000
#include 

#define T_TIC 1136
#define T_TOC 1432
#define PORT_TOM PORTB
#define SPEAKER 3

unsigned char beat = 4;
unsigned char ind = 4;

unsigned short tic = T_TIC;
unsigned short toc = T_TOC;

int main()
{	
	DDRC = 0x00;
	DDRB = 0xFF;
	PORTB = 0x00;

	TCCR1A = 0x00;	//CTC mode
	TCCR1B = (1<<WGM12) | (1<<CS12); //prescaler 256
	TIMSK1 |= (1<<OCIE1A);//INT TIMER1_COMPA
	
	OCR1A = 31250; //120bpm, just testing right now

	EIMSK = 0x03;	//INT1 e INT0 enable
	EICRA = 0x0F;	//INT1 e INT0 rising edge

	sei();
	while(1){}
}

void tom(int T) //plays a tone
{
	int i;
	for(i=0;i<26;i++){
		PORT_TOM |= (1<<SPEAKER);
		_delay_us(T);
		PORT_TOM &= ~(1<<SPEAKER);
		_delay_us(T);
	}	
}

ISR(TIMER1_COMPA_vect) //play a tone when TCNT1 = OCR1A, if ind=beat plays tic, otherwise toc
{
	if(ind < beat){
		tom(toc);
		ind++;
	}
	else{
		ind = 1;
		PORTB |= (1<<5);
		tom(tic);
		PORTB &= ~(1<<5);
	}
}

ISR(INT1_vect) //increase beat
{
	PORTB |= (1<<5);
	beat++;
	ind = beat;
	TCNT1 = 0;
	_delay_ms(200);
	PORTB &= ~(1<<5);
}

ISR(INT0_vect) //decrease beat
{
	PORTB |= (1<<5);
	beat--;
	ind = beat;
	TCNT1 = 0;
	_delay_ms(200);
	PORTB &= ~(1<<5);
}

The problem is:
When the ATmega328P enters INT1 or INT0 routine, instead of increase or decrease beat by 1 they add or subtract by 2. Anyone got any ideas? Is this code correct? In the simulator it works fine! I couldn't find any mistake by myself =S

Thanks in advance!

More info: Using AVR Studio 4 with WinAVR-20100110.
PS.: Did I post it in the right section? If not, I'm sorry, fell free to move it!

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

WElcome to Avrfreaks, do this

volatile unsigned char beat = 4;

and read this:
https://www.avrfreaks.net/index.p...

Hit the forum button near the upper left corner of the site and get familiar with it. :D

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

Thanks very much for the welcome and the suggestion!
Unfortunately it didn't work.

Also when I try to see the Disassembler, I get some "151: File not Found" errors in the middle of it. Could it give some hint to the problem or is it just a AVR Studio problem? The code compiles with no errors or warnings.

Edit: Ops, now you've edited I'm going to read that!

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

Add :

volatile unsigned char ind = 4;

I get that "file..." stuff too, ignore it so long as it builds, just keep steppin' . How do you know it's counting by 2 instead of 1 ?

Edit: Wait... are you using switches/buttons on the INT pins ? If so, in the future give these hardware details in your OP ( They could be digital signals on those pins... we don't know unless you tell us ). They need to be debounced if your answer's yes.

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

Thanks!
Now I get it about what is and why use volatile!
I've added the line above also and no change.
I've compiled using -O0 optimization (off) and no changes also.

AVR Studio version:

AVR Studio		4.18.684  
GUI Version		4, 18, 0, 670
AVR Simulator		1, 0, 2, 1
ATmega328P		1

Operating System
Major			6
Minor			1
PlatformID		2
Build			7600


Plugins:

VDMAvrGUI			
AvrPluginAvrAsmObject	1, 0, 0, 48
AvrPluginavrgccplugin	1, 0, 0, 11
Stk500Dll			1, 0, 1, 13

I could make it using if functions inside while(1) loop but it would be better if I could use ext. interruptions instead.

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

OK, what about my other questions ? Come on, don't make it a tooth pullin' session.

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

indianajones11 wrote:
OK, what about my other questions ? Come on, don't make it a tooth pullin' session.

Sorry, I didn't see your edit! Yes I'm using switches on both interruptions. That's why I've put the _delay_ms(200) lines on both interruptions. Isn't a delay enough to debounce the signal? It works the very same even with a second delay in each interruption.

By the way, I've came with a workaround: changing the ind++ line to ind = ind + 2;. Now both counter (ind) and limit (beat) are being added by two and it is working as intended :x

I think I will follow with the rest of the project by now, but I would like to solve this later. If I find the solution I will come post it here.

Thanks for your tips indianajones11! You really helped me!

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

Yes, 200ms is more than enough ( Even like (20-50)ms would work. It should be the FIRST thing done in the ISRs though. It should be possible to inc. by 1, of course, but it works for you now. Strange problem.

1) Studio 4.18 build 716 (SP3)
2) WinAvr 20100110
3) PN, all on Doze XP... For Now
A) Avr Dragon ver. 1
B) Avr MKII ISP, 2009 model
C) MKII JTAGICE ver. 1

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

Hum, you're right. I've put the delay in the first line of the ISR. I have tried changing the beat++ to beat = beat + 1 and this causes no change also. Ok! Enough for today, I hope I find it out later or someone that cross this topic has a solution!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
_delay_us(T); 

You don't want to send a variable into _delay_us(), it requires a constant known at compile time.

Quote:
When the ATmega328P enters INT1 or INT0 routine, instead of increase or decrease beat by 1 they add or subtract by 2.
Because interrupts are a bad way to debounce switches. The delay in the interrupts does not prevent the interrupt flag from being set again, so as soon as you exit the interrupt, the ISR is called again.

Regards,
Steve A.

The Board helps those that help themselves.

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

Koshchi wrote:

_delay_us(T); 

You don't want to send a variable into _delay_us(), it requires a constant known at compile time.

Quote:
When the ATmega328P enters INT1 or INT0 routine, instead of increase or decrease beat by 1 they add or subtract by 2.
Because interrupts are a bad way to debounce switches. The delay in the interrupts does not prevent the interrupt flag from being set again, so as soon as you exit the interrupt, the ISR is called again.

YAY! There goes my hero! haha
I thought the flag was cleared in the end of the ISR, but it seems not! I've cleared the flag by software in the end of the ISR to be sure it wouldn't be called again:

ISR(INT1_vect)
{
	PORTB |= (1<<5);
	_delay_ms(200);
	beat++;
	ind = beat;
	TCNT1 = 0;
	PORTB &= ~(1<<5);
	EIFR |= (1<<INTF1); //clear INT1 flag
}

ISR(INT0_vect)
{
	PORTB |= (1<<5);
	_delay_ms(200);
	beat--;
	ind = beat;
	TCNT1 = 0;
	PORTB &= ~(1<<5);
	EIFR |= (1<<INTF0);  //clear INT0 flag
}

And now it works! :D

What would you recommend to debounce switches, besides external int.?

And for the delay suggestion, as there are only two possible values of T on the whole program I could write two different functions that use different constants.

Thank you all very much!

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

Since you use a timer, why do you need delay_ms?Have the timer and code do the delays for you. The timer isr can also be used to read and debounce your inputs - no need for external interrupts. Actually, for a simple app like this, you don't need interrupts at all, but that is an architectural decision. I wrote a tutorial on the traps of using interrupts, do a search!

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

Quote:

What would you recommend to debounce switches, besides external int.?

You simply would not believe how often this comes up here so there are literally hundreds of prior threads to read through. They all point to the same thing - first the article written by famous embedded engineer Jack Ganssle on the subject of debounce - you need to know the details of what you are dealing with and that explains it. Next you will find links to software solutions by both users "danni" (Peter Danneger) and "theusch" (Lee Theusch) that show the typical AVR solution. Bottom line is that you use a timer interrupt and poll the button states regularly maintaining some "previous history". When the history has "settled" you can report a firm button state. The only use of buttons on external interrupts might be to wake the AVR from a sleep mode but as soon as it awakes the ext-int source is disabled and the regular timer interrupt polling takes over. The ext-int is re-enabled just before sleep resumes.

(isn't it about bloomin' time that someone finally put all this together in a Tutorial Forum article so a link could be given to that each time the questions arises??)

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

Thanks very much! I will be reading your tutorial soon!

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

Quote:
I thought the flag was cleared in the end of the ISR
No, it is cleared at the start of the ISR.

Regards,
Steve A.

The Board helps those that help themselves.