TIMER4 Input Capture Interrupt Causes Serial Communication Failure

Hi!

I'm trying to use the 3 Input Capture Pins on my Seeeduino Mega for an application. I've written some LabVIEW software that polls the Seeeduino Mega every 250ms (rather slow poll rate). I'm using a modified version of the LabVIEW for Arduino firmware where I've made a couple of my own application specific "Commands".

I've posted some code here on my TIMER setup and checkouts: Should I outsource my counting to something external? Micros is too slow. - #56 by system - Project Guidance - Arduino Forum

This shows that I have the basics of setting up the timers and synchronizing them. I'm essentially applying the same code here.

Everything works fine until I enable the Input Capture Interrupt for TIMER4 and "trigger" the input capture pin for TIMER4. When that interrupt fires it is locking up my serial communication with my computer long enough for the LabVIEW Arduino "Send Receive.vi" to throw an error.

This doesn't happen with either TIMER1 or TIMER5. I'm scratching my head on this one and could use some help troubleshooting.

Here is the code I'm running to setup my timers:

/*
	GTCCR: General Timer Control Register
	Bit 7 - TSM: Timer/Counter Synchronization Mode.  Setting this bit high will stop all counters so that they can be configured
	*/
	
	GTCCR = (1<<TSM)|(1<<PSRSYNC);

	/*Timer1 configuration
		Timers 4 and 5 follow a similar pattern so therefore only Timer 1 will be commented on here.
	*/
	
	/*
	TCCR1B: Timer/Counter 1 Control Register B
	ICNC1: Input Capture Noise Cancel.  Set to 1, noise is rejected by simply requiring that the signal is held high (on rising edge triggers) or low (on falling edge triggers) for at least 4 clock cycles (note: not prescaled clock cycles
	ICES1: Input Capture Edge Select.  0 = falling edge, 1 = rising edge.
	CS12 - 0: Clock select.  CS10 = 1, CS11 = 0, and CS12 = 0 means system clock w/ no prescaling.  See Table 17-6 in datasheet of ATmega 2560 chip.
	*/
	TCCR1B = (0<<ICNC1)|(1<<CS10)|(1<<ICES1);
	
	/*
	TCCR1A: Timer/Counter 1 Control Register A
	Set WGM11 and WGM10 to 0 (along with WGM13 and WGM12 in TCCR1B) in order to set the output to the TOP value of 0xFFFF.  See Table 17-2 in the ATMega 2560 datasheet.
	*/
	TCCR1A &= ~((1<<WGM10)|(1<<WGM11));
	
	/* 
	TIFR1 - Timer/Counter 1 Interrupt Flag Register
	*/	
	TIFR1 = (1<<ICF1)|(1<<TOV1); //Clear ICF1 (this is done by writing a logical 1 to it apparently in the manual).  This clears pending interrupts.  Clear OVERFLOW interrupt flag as well.  I found that for some reason Timer 5 initialized with an OVERFLOW interrupt which was resulting in a different value from Timer 1 and 4.  Clearing this interrupt with the timers stopped solved that problem.
	
	TIMSK1 = (1<<ICIE1)|(1<<TOIE1); //Timer 1 Interrupt Mask Register.  Enables Input Capture interrupt and Overflow Interrupt.  Disables the Output compare interrupts.
	
	/* Setup Timer 4 */
	TCCR4B = (0<<ICNC4)|(1<<CS40)|(1<<ICES4);
	TCCR4A &= ~((1<<WGM40)|(1<<WGM41));
	TIFR4 = (1<<ICF4)|(1<<TOV4);
	
	TIMSK4 = (1<<ICIE4)|(0<<TOIE4);		//Since the Timer 1 Overflow Interrupt is enabled, and all the timers are kept in sync, Timer 4 Overflow Interrupt is disabled
	
	/* Setup Timer 5 */
	TCCR5B = (0<<ICNC5)|(1<<CS50)|(1<<ICES5);
	TCCR5A &= ~((1<<WGM50)|(1<<WGM51));
	
	TIFR5 = (1<<ICF5)|(1<<TOV5);
	
	TIMSK5 = (1<<ICIE5)|(0<<TOIE5);		//Since the Timer 1 Overflow Interrupt is enabled, and all the timers are kept in sync, Timer 5 Overflow Interrupt is diabled
	
	TCNT1L = 0;
	TCNT1H = 0;
	TCNT4L = 0;
	TCNT4H = 0;
	TCNT5L = 0;
	TCNT5H = 0;
	
	/* Start the Timers together*/
	GTCCR = 0;

Here are my interrupt vectors:

ISR (TIMER1_CAPT_vect) {
	cli();
	TimerFlagNewEventInProgress();
	MyTimer.ICR1CaptureValue = ICR1;
	MyTimer.ICR1CaptureOverflow = MyTimer.TimerOverflow;
	sei();
}

ISR (TIMER1_OVF_vect) {
	MyTimer.TimerOverflow++;
}


ISR (TIMER4_CAPT_vect) {
	cli();
	TimerFlagNewEventInProgress();
	MyTimer.ICR4CaptureValue = ICR4;
	MyTimer.ICR4CaptureOverflow = MyTimer.TimerOverflow;
	sei();
}

ISR (TIMER5_CAPT_vect) {
	cli();
	TimerFlagNewEventInProgress();
	MyTimer.ICR5CaptureValue = ICR5;
	MyTimer.ICR5CaptureOverflow = MyTimer.TimerOverflow;
	sei();
}

If I change the line in my timer setup "TIMSK4 =" to:

TIMSK4 = (0<<ICIE4)|(0<<TCIE4);

Everything works fine. For some reason when the TIMER4 Input Capture Interrupt Service Routine fires, my controller is locking up and causing my communication with LabVIEW over RS232 to fail.

Thanks for your input!
-Nic

If I comment out the disable interrupt call "cli();" and set interrupt call "sei();" like so:

ISR (TIMER4_CAPT_vect) {
//	cli();
	TimerFlagNewEventInProgress();
	MyTimer.ICR4CaptureValue = ICR4;
	MyTimer.ICR4CaptureOverflow = MyTimer.TimerOverflow;
//	sei(); 
}

My code runs fine. But I've read that this isn't good practice because ICR4 combines ICR4H and ICR4L into a 16 bit value and shouldn't be interrupted while doing this. I'm really scratching my head on this one. Why is it that the exact same code being executed for TIMER1 and TIMER5 work fine, but when the code is executed for TIMER4 my controller locks up and my communication to LabVIEW fails.

The problem is not intermittent. It is very repeatable. When the cli(); line is executed in the TIMER4 Input Capture Interrupt Service Routing, my controller locks up.

There is no point in putting cli() into an ISR because interrupts are already disabled.

Also don't put sei() into it.

Thanks for your reply. If I don't need it then I'll just leave it out since it is causing problems 8).

Looking at AVR130 on Atmel Timers they don't disable and re-enable interrupts in their example code either. I know I read it somewhere but must have only applied if you are polling the flag and now using ISRs.