My interrupts are disabling themselves!

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

Note: I changed the subject line to better address the symptoms after more exploration. What I didn't originally know in this first post was that the interrupts weren't incrementing my global variables because the interrupts were not firing for some reason which is the problem I'm now trying to figure out.

I have two global variables defined at the top of my program like so:

volatile uint16_t global_uart_index;
volatile uint8_t honeywell_tx_timeout_timer;

I have a function that sends out a string to the honeywell PLC controller whereupon I expect a response about 50ms thereafter. I know how many bytes the response should be (9) so I want to wait until a response has come or there's been a timeout.

Here's the loop I wait in until either exit scenario happens:

while ( (global_uart_index < 9) && (honeywell_tx_timeout_timer < 4) );

global_uart_index was set to 0 right before the wait loop and is incremented in an interrupt each time a character is received on the wire:

ISR(USART0_RX_vect)
{
	if (global_uart_index < GLOBAL_UART_ARRAY_SIZE)
		global_uart_array[global_uart_index++] = UDR0;
		
}

Also, I have an overflow loop continuously overflowing every second. I know the timing is correct since I can blink a light on and off using this routing and I get 1Hz:

ISR(TIMER0_OVF_vect)
{
	second_timer++;
	if (second_timer > 56)
	{
	   honeywell_tx_timeout_timer++;
           second_timer=0;
	}
}

Ok. So, the wait loop never terminates. I've put in little printfs in all the interrupt routines and I can see they are being called yet they don't seem to be modifying my volatile global variables like they should - they both stay 0 so I don't get out of the loop.

Anyone know why my variables aren't being changed? This is all in the same file.

Last Edited: Thu. Apr 23, 2009 - 09:36 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

the snippits look fine, with the exception that you may have trouble with the uint16_t type, as reads to it are not atomic. Though in this case since the value is so small, it should not be a problem. (having said that, does it really need to be a 16 bit value?)

Are you sure you have enabled the interrupts?

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

Well, I guess I have a different symptom now.

I added in some extra code to break out of the wait loop if it took too long:

i=0;
	
	while ( (global_uart_index < 9) && (honeywell_tx_timeout_timer < 4) )
	{
		_delay_ms(1000);
		
		i++;
		if (i == 5) break;
	}

This gets me out of the loop after 5 seconds. In that 5 second time period, the timer overflow isn't called as I don't see it's printf thing happening. It happens before and after the wait loop but not during. Somehow the interrupt is being deactivated.

I looked all over for cli's and sei's and I see a couple of spots in other parts of the program where I'll put in a cli but after a couple of statements I have a sei to reactivate.

Just to be sure, I put a sei() right before the wait loop starts and it still doesn't make a difference.

I have no clue as to why the interrupts are stopping during the wait loop. Argh!

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

Woops glitch, didn't see your post up there.

I can't recall why I needed a 16-bit index so I've changed it to an 8-bit variable.

Unfortunately, that change hasn't made much of a difference. The timeroverflow function still isn't being called during the wait loop.

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

ok so global interrupts are enabled, but are you sure that the individual timer and USART interrupts are enabled.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

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

I turn on the timer overflow in the initialization routine:

TCCR0B |= (1 << CS02) | (1 << CS00);  //1024 prescaler
TIMSK0 |= (1 << TOIE0);  //Overflow interrupt

Those two timer registers are never touched again in the program.

As for the UART, I enable the receive character interrupt right before I enter the wait loop. Here's another snipet where I include the extra line:

UCSR0B |= (1<<RXCIE0);   //Enable receive interrupt

	global_uart_index = 0;  //Reset index in anticipation of new message from honeywell
	honeywell_tx_timeout_timer = 0;  //Reset watchdog

	
	sei();  //Just to see if this helps...
	i=0;
	
	while ( (global_uart_index < 9) && (honeywell_tx_timeout_timer < 4) )
	{
		_delay_ms(1000);
		
		i++;
		if (i == 5) break;
		
	}

Even if I didn't enable the receive interrupt, the watchdog timer should still be incrementing every second and get me out of the wait loop. It's just odd that the watchdog timer isn't incremented during the wait.

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

I'm going to stop this thread and make a new one with a new subject line since the current subject doesn't define the problem anymore.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
fizgig wrote:
I know the timing is correct since I can blink a light on and off using this routing and I get 1Hz:

ISR(TIMER0_OVF_vect)
{
	second_timer++;
	if (second_timer > 56)
	{
	   honeywell_tx_timeout_timer++;
           second_timer=0;
	}
}

I don't see the light blinking in this code. Please post what you test. You might also want to post second_timer's definition and perhaps also the asm output.

(You already started to divide (timer interrupt from the uninteresting rest), it's a good path to conquer :-) )

JW

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

Quote:

I'm going to stop this thread and make a new one with a new subject line since the current subject doesn't define the problem anymore.

Why? If you go back and edit the "subject" of your first post in this thread you change the title anyway.

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

Yeah, I put the light in later. Here's the new code that I'm actually using:

ISR(TIMER0_OVF_vect)
{
	second_timer++;
	if (second_timer > SECOND_OVERFLOW_NUMBER)
	{
		second_timer=0;
		honeywell_tx_timeout_timer++;
		read_output_timer++;
		PORTC ^= (1<<PC0);
	}
}

That PORTC line at the end is the light that blinks. I also show another variable I'm incrementing (read_output_timer) that doesn't have anything to do with this so I didn't want to confuse people.

I wish I could provide assembly code but I'm not sure how. I wrote a couple of programs by hand in assembly a long time ago but haven't looked back since C. I'm not sure how to get assembly code from C and how to line up C instructions with the assembly instructions.

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

Make the avrgcc toolchain generate a .lss file . If you are on an AVR Studio project then there is a check box in the project settings to get it generated.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

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

Looks like the makefile I use already causes an .lss file to be made. I've attached it.

After looking through it, it looks like the wait loop starts at 1031.

Last Edited: Sat. Apr 25, 2009 - 06:16 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I sure could use some help on this one. I really have nothing else I can check. I've tried all debugging steps I can think. If anyone can give me anything to try I would be very grateful.

I'm starting to wonder if there's a bug in the compiler or in my chip.

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

You must have ignored a warning, saying: "F_CPU not defined for " . I doubt you have a 1MHz crystal - which is what delay.h assumes as default - and Based on your comments, you have a 14.7456MHz crystal, so the instead of 1000ms delay it lasts in fact around 68ms, and 5 loops of that last around 340ms - and that's not even one single timer interrupt.

I assume you used some macros dependent on F_CPU also for the baudrate settings, so they might be also off.

Next time, strip down your example code to the absolute necessary minimum to demonstrate the problem.

JW

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

Interesting. You might be onto something there but I'm not sure.

I do define the cpu speed at the top of my program like this:

#define F_CPU 14745600UL

I put a couple of lines at the beginning of my program that toggles the LED at 1Hz like so:

	cli();
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	sei();

Notice I disable interrupts temporarily so I know that the toggling of the light is due to these lines and not the overflow interrupt.

I always check the light when the program first starts and I see 1Hz there as well so it looks like the delay functions are working ok. Should I modify the delay.h file to comment out the 1MHz definition anyway? Seems like it works now.

After the above code toggles the light, interrupts are re-enabled as shown and the overflow interrupt takes over toggling the light at 1Hz and I see that happening as well.

It's just the dang wait loop that causes the interrupts to stop and I can't figure out why.

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

fizgig wrote:
Interesting. You might be onto something there but I'm not sure.

I am. ;-) (To be honest, too many times it does not pay out :-( )

fizgig wrote:

I do define the cpu speed at the top of my program like this:

#define F_CPU 14745600UL

Humm. Where is "top of my program"? Couldn't that be AFTER you included that damn' delay.h?

Also, what about the warnings? Do you use -Wall? Do you get any warnings?

fizgig wrote:

I put a couple of lines at the beginning of my program that toggles the LED at 1Hz like so:

	cli();
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	PORTC ^= (1<<PC0);
	_delay_ms(1000);
	sei();

We might face a language problem here: you have put this sequence in AFTER you posted the .lss file, have you? (sorry, my English is not better than this) I can't see it in .lss.

fizgig wrote:
Notice I disable interrupts temporarily so I know that the toggling of the light is due to these lines and not the overflow interrupt.

No, you don't know that. You'd need to have a clear separator, say a wait for a button press. Now, you have 3 very quick flashes - 60ms delay so all that above happens in about 1/3 of a second, too fast to be seen clearly - and then the 1sec flash. You could see that if you concentrate on it, but better use the "wait until some pin goes low" method.

fizgig wrote:
It's just the dang wait loop that causes the interrupts to stop and I can't figure out why.

No, the interrupts won't stop. Your method of detecting the stopped interrupts is flawed.

JW

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

Appreciate the comments. It's good to understand what I shouldn't assume.

Yes, I keep modifying the program by adding in light blinks here and there to see if I can figure this problem out so I'm afraid the .lss file wont show some of the later debugging additions I'm adding in my desperation to get this solved.

I do have the F_CPU defined currently as the first line of my file (above all else) so it should hopefully be picked up by the delay routine.

I'll remove the light blinking from the interrupts so I can be sure that the blinking light is only from my calibration routine at the start. I'll do that when I get home tonight. Then I'll add in a button press for the separation.

Thanks again for your input. It's very valuable.

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

Should I modify the delay.c file to comment out the F_CPU inside it?

I'm not really sure what the best way is to handle specifying F_CPU.

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

Quote:

I'm not really sure what the best way is to handle specifying F_CPU.

A -D in the Makefile as it keeps the definition in one place and ensures it is defined in advance of all #include's

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

clawson wrote:
Quote:

I'm not really sure what the best way is to handle specifying F_CPU.

A -D in the Makefile as it keeps the definition in one place and ensures it is defined in advance of all #include's

Oh, yes; and exactly that might've been the way how the 1MHz crept in WITHOUT causing a single warning... Any tool creating makefile with -DF_CPU=1000000UL ?

JW

[edit] bah stupid me, the explicit redefinition would cause at least a warning... [/edit]

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

Ok, I'll take out the F_CPU definition at the top of my file and add this line to my makefile:

F_CPU = 14745600

Shouldn't I still edit the delay.h file since I see the following near the top of it:

/** \file */
/** \defgroup util_delay : Convenience functions for busy-wait delay loops
    \code
    #define F_CPU 1000000UL  // 1 MHz
    //#define F_CPU 14.7456E6
    #include 
    \endcode

It sort of looks like the code might be commented out so it might not matter but it's hard to really know for sure - it's kind of hard for me to parse. What tips me off is if I remove the F_CPU line from my makefile, it still compiles with no warnings. (There is no definition of F_CPU in the main file anymore - or anywhere else for that matter) so I don't know why I don't get a warning.

This is all being done in programmer's notepad with a makefile I've used for quite some time (attached in case it helps)

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

Quote:

and add this line to my makefile:

and modify the CFLAGS or compiler invocation rule to actually use this (with a suffixed "UL") in -D ?

If you want to see an example use Mfile and it already has all this (and everything else you require in a Makefile)

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

There's a clock frequency setting in studio as well, under project, configuration options, general.

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

Ok. I've used mfile to generate my makefile (neat little tool!). Was confused at first because I didn't see a dropdown to change the frequency. It shows you how in the actual makefile itself. Thanks all for the tips.

Can't wait to go home and try this out!

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

Well, I had my "aha" tonight.

Another interrupt was apparently grabbing the attention of the chip and wouldn't let go. It was the interrupt that the ethernet chip uses to signal that it has a packet ready to pick up.

When I disable that interrupt and use polling to check for the packet, the problem goes away.

Not sure why I have this problem now, the method I've been using worked in the past. I might have messed up the initialization code of the ethernet chip causing it to keep the line high or something.

I'll figure out the details in the morning but with polling everything works!

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

Did you disable the interrupt in the ENC28J60 or the AVR?

Maybe you did not acknowledge the interrupt, and that it kept on interrupting your AVR?

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

I was doing a search on a similar problem and came across this posting. In my problem, my interrupts were firing at approximately 4 times the rate they should have been. In my make file, F_CPU was defined as 8000000 and should have been 16000000. However, that did not fix the problem. The root of my problem was that I used sei(); prior to finishing initializing values for serial comm. Interrupts have to be turned on only AFTER you have properly initialized serial communication. I don't know if that was done here in the above problem, but I thought if someone got to this page they would benefit from my frustration. A short blurb is avaiable in the atmega 168 datasheet on pg 176 about it under UART Initialization..

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

Interesting thought. I do have an initialization procedure and the first line is cli() and the last line is sei(). All the stuff in between is configuring interrupts, timers, data direction on GPIO, etc...

As such, I don't think that was the problem.

As I stated above, I use polling now and haven't really cared to go back to interrupt-based checking. My main loop (where the polling takes place) cycles so fast that it doesn't really matter. The packet interrupt actually just changed the status of a global flag bit that I would later check during the main loop anyhow so it doesn't really make a bunch of difference. There's more communication with the ethernet chip with this approach but I don't think that's a big deal.

Network performance metrics like packet response time haven't been affected so I'm cool for now. Eventually I'll dig into the problem further with a scope and see what's going on and see if I can get back to interrupt-based communication but that will take a back seat for now.

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

@jayjay1974

Sorry, didn't see your post there.

I disabled the interrupt in both the ethernet chip and the AVR so the interrupt line is completely unused at the moment.

Not sure what you mean by "acknowledging" an interrupt. I thought the interrupt function kind of handled all that stuff in avr-gcc. You can see my interrupt function (when I was trying to do interrupt-based communication) doesn't do much:

ISR(INT0_vect)
{
	ethernet_packet_received=1;
}

I believe that was the same code I used on previous versions that worked. Perhaps I'm wrong.

Last Edited: Tue. May 19, 2009 - 05:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's been quite a while since I've worked with the ENC28J60 but usually external devices that can issue interrupts need an acknowledge so they release the interrupt line. Usually in the form of reading some register or setting or clearing a bit somewhere. They don't know when the ISR is entered ;)

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

Interesting. That's definitely worth another look. I'll go look at the spec sheet right now.

ps. I updated my post above to show the correct interrupt code. I had a timer overflow in there at first. Now it's correct (and even simpler).

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

OP,

Is it possible you are using level detection instead of edge detection.

A

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

Well Andrew, I think you hit on something there.

I had changed the circuit a little bit and had moved from using INT1 to INT0. Check out my initialization code:

//Setup ethernet packet interrupt line on AVR to INT0 (PD2)
	EICRA |= (1 << ISC11); //Falling edge of int0 causes interrupt
	EIMSK |= (1 << INT0);  //Enable int0

The falling edge statement is wrong. It still reflects what it was from INT1. It should be:

EICRA |= (1 << ISC01); //Falling edge of int0 causes interrupt

Looking at the AVR datasheet, the INT0 interrupt was set to "The low level of INT0 generates an interrupt request." instead of the falling edge since I was configuring the falling edge of INT1 and not INT0.

I've never intentionally used this mode of interrupt before so I'm not sure what a steady low level on the interrupt does. I guess, judging by what I saw happening, that the interrupt procedure gets called over and over again and nothing else happens until the line goes high again which effectively freezes the program.

I can't wait to test the code out when I get back home to see if this makes the difference. Sure sounds like this was the problem.

Thanks everyone for the input and thanks Andrew for the great question!

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

Well, I finally got next to the circuit board and made the change above and that did the trick!

Big thanks Andrew!