Enabling interrupt timers causes Arduino Mega reset

Not sure why my code keeps resetting but it starts working when I comment out this line of code "TIMSK5 |= 0b00000100; "
I am trying to integrate some timer driven RF transmit functionality developed by Anthony Stirk into my main project but it’s causing my mega to run through the following subroutine and hang when it hits the next “delay(50);” statement. This subroutine is the first thing called in my setup().

void initialise_interrupt() 
{
  // Start Timer5
  //cli();          // disable global interrupts
  TCCR5A = 0;     // set entire TCCR1A register to 0
  TCCR5B = 0;     // same for TCCR1B
  OCR5A = F_CPU / 1024 / RTTY_BAUD - 1;  // set compare match register to desired timer count:
  TCCR5B |= (1 << WGM12);   // turn on CTC mode:
  // Set CS10 and CS12 bits for:
  TCCR5B |= (1 << CS10);
  TCCR5B |= (1 << CS12);
  // enable timer compare interrupt:
  [b]// TIMSK5 |= 0b00000100; // <-- COMMENTED CODE TO STOP RESET
[/b]  //sei();
}

Basically what I am trying to do is setup a timer based interrupt on my mega, of which it has 6.
I’ve tried enabling different match interrupts, every timer I have and have been digging through the datasheet to no avail.
I’ve also tried commenting out other lines in the subroutine but the critical line of code is the "TIMSK5 |= 0b00000100; " line which activates my timer interrupt.

As far as I know no other libraries I have use timers at all so I have no idea what I’ve done wrong.

The test script which runs and works with timers is below:

/*
 Interrupt Driven RTTY TX Demo
  
 Transmits data via RTTY with an interupt driven subroutine.
  
 By Anthony Stirk M0UPU 
  
 October 2012 Version 5
  
 Thanks and credits :
 Evolved from Rob Harrison's RTTY Code.
 Compare match register calculation by Phil Heron.
 Thanks to : http://www.engblaze.com/microcontroller-tutorial-avr-and-arduino-timer-interrupts/
 RFM22B Code from James Coxon http://ukhas.org.uk/guides:rfm22b 
 Suggestion to use Frequency Shift Registers by Dave Akerman (Daveake)/Richard Cresswell (Navrac)
  
 This program is free software: you can redistribute it and/or modify
 it under the terms of the GNU General Public License as published by
 the Free Software Foundation, either version 3 of the License, or
 (at your option) any later version.
  
 This program is distributed in the hope that it will be useful,
 but WITHOUT ANY WARRANTY; without even the implied warranty of
 MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 GNU General Public License for more details.
  
 See <http://www.gnu.org/licenses/>.
 */
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/crc16.h>

#define ASCII 7          // ASCII 7 or 8
#define STOPBITS 2       // Either 1 or 2
#define TXDELAY 0        // Delay between sentence TX's
#define RTTY_BAUD 300    // Baud rate for use with RFM22B Max = 600
 
#define RADIOPIN 13 
 
char datastring[80];
char txstring[80];
volatile int txstatus=1;
volatile int txstringlength=0;
volatile char txc;
volatile int txi;
volatile int txj;
unsigned int count=0;
 
 
//rfm22 radio1(RFM22B_PIN);
 
void setup()
{
  Serial.begin(115200);
  Serial.println("Start");
  initialise_interrupt();
  setupRadio();
}
 
void loop()
{
 
  sprintf(datastring,"$$$M0UPU,%04u,RTTY TEST BEACON RTTY TEST BEACON",count); // Puts the text in the datastring
  unsigned int CHECKSUM = gps_CRC16_checksum(datastring);  // Calculates the checksum for this datastring
  char checksum_str[6];
  sprintf(checksum_str, "*%04X\n", CHECKSUM);
  strcat(datastring,checksum_str);
  delay(1000);
  count++;
}
 
void initialise_interrupt() 
{
  Serial.println(TIMSK5);
  // initialize Timer1
  cli();          // disable global interrupts
  TCCR5A = 0;     // set entire TCCR1A register to 0
  TCCR5B = 0;     // same for TCCR1B
  OCR5A = F_CPU / 1024 / RTTY_BAUD - 1;  // set compare match register to desired timer count:
  TCCR5B |= (1 << WGM12);   // turn on CTC mode:
  // Set CS10 and CS12 bits for:
  TCCR5B |= (1 << CS10);
  TCCR5B |= (1 << CS12);
  // enable timer compare interrupt:
  TIMSK5 =2;
  sei();          // enable global interrupts
  Serial.println((1 << OCIE1A));
  Serial.println(TIMSK5);
}
ISR(TIMER5_COMPA_vect)
{
  switch(txstatus) {
  case 0: // This is the optional delay between transmissions.
    txj++;
    if(txj>(TXDELAY*RTTY_BAUD)) { 
      txj=0;
      txstatus=1;
    }
    break;
  case 1: // Initialise transmission, take a copy of the string so it doesn't change mid transmission. 
    strcpy(txstring,datastring);
    txstringlength=strlen(txstring);
    txstatus=2;
    txj=0;
    break;
  case 2: // Grab a char and lets go transmit it. 
    if ( txj < txstringlength)
    {
      txc = txstring[txj];
      txj++;
      txstatus=3;
      rtty_txbit (0); // Start Bit;
      txi=0;
    }
    else
    {
      txstatus=0; // Should be finished
      txj=0;
    }
    break;
  case 3:
    if(txi<ASCII)
    {
      txi++;
      if (txc & 1) rtty_txbit(1); 
      else rtty_txbit(0);   
      txc = txc >> 1;
      break;
    }
    else
    {
      rtty_txbit (1); // Stop Bit
      txstatus=4;
      txi=0;
      break;
    } 
  case 4:
    if(STOPBITS==2)
    {
      rtty_txbit (1); // Stop Bit
      txstatus=2;
      break;
    }
    else
    {
      txstatus=2;
      break;
    }
 
  }
}
 
void rtty_txbit (int bit)
{
  if (bit)
  {
    digitalWrite(RADIOPIN,HIGH); // High
  }
  else
  {
    digitalWrite(RADIOPIN,LOW); // Low
  }
}
 
void setupRadio(){
  //pinMode(RFM22B_SDN, OUTPUT);    // RFM22B SDN is on ARDUINO A3
  pinMode(RADIOPIN,OUTPUT);
  
  digitalWrite(RADIOPIN, LOW);
  delay(1000);
/*  rfm22::initSPI();
  radio1.init();
  radio1.write(0x71, 0x00); // unmodulated carrier
  //This sets up the GPIOs to automatically switch the antenna depending on Tx or Rx state, only needs to be done at start up
  radio1.write(0x0b,0x12);
  radio1.write(0x0c,0x15);
  radio1.setFrequency(RADIO_FREQUENCY);
  radio1.write(0x6D, 0x04);// turn tx low power 11db
  radio1.write(0x07, 0x08);
  */// delay(500);
}
 
uint16_t gps_CRC16_checksum (char *string)
{
  size_t i;
  uint16_t crc;
  uint8_t c;
 
  crc = 0xFFFF;
 
  // Calculate checksum ignoring the first two $s
  for (i = 5; i < strlen(string); i++)
  {
    c = string[i];
    crc = _crc_xmodem_update (crc, c);
  }
 
  return crc;
}

*Modified adding code tags.

I haven't studied your code. It is long and you did not post it within code tags that make it much easier to read.

I wonder if your problem is similar to a problem I had that had similar symptons. My problem was that there was an interrupt ready to trigger as soon as the interrupt was switched on and then there was a string of thousands of interrupts that just overwhelmed the CPU. In part my problem was solved by clearing the interrupt flag before I switched on the interrupt. But you also need to ensure that there are no stray signals causing lots of interrupts you know nothing about.

...R

Your code is setting the mask to 4, while the original code is setting it to 2. Is that intentional?

Sorry about the lack of code tags, first time posting here.

The flag was different because I was in the process of playing with the mask to see if it makes any difference. It didn’t work.

I’ve wrapped all my register writes with interrupt enable/disables and print statements:

void initialise_interrupt() 
{
	Serial.print("Initalising RF interrupt ");
	delay(2000);
  // initialize Timer1
        cli();          // disable global interrupts
	Serial.print(". ");
  TCCR5A = 0;     // set entire TCCR1A register to 0
	sei();
	delay(100);
	cli();
	Serial.print(". ");
  TCCR5B = 0;     // same for TCCR1B
	sei();
	delay(100);
	cli();
	Serial.print(". ");
  OCR5A = F_CPU / 1024 / RTTY_BAUD - 1;  // set compare match register to desired timer count:
	sei();
	delay(100);
	cli();
	Serial.print(". ");
  TCCR5B |= (1 << WGM12);   // turn on CTC mode:
	sei();
	delay(100);
	cli();
	Serial.print(". ");
  // Set CS10 and CS12 bits for:
  TCCR5B |= (1 << CS10);
	sei();
	delay(100);
	cli();
	Serial.print(". ");
  TCCR5B |= (1 << CS12);
	sei();
	delay(100);
	Serial.print(". ");
	cli();
  // enable timer compare interrupt:
  TIMSK3 |= 0b00000010;
	sei();
	Serial.print(". ");

	delay(1000);
	Serial.println("_/ RF Interrupts Initialised");
}

This is all that’s happening in setup up to this point:

void setup()
{ 
  Serial.begin(115200);
  // enable global interrupts
  Serial.println("BOOT");
  initialise_interrupt();
  initLEDs(10); 
...

This gives me the following serial output.

BOOT
Initalising RF interrupt . . . . . . . BOBOOT
BOOT
BOOT
BOOT
BOOT
BOOT
BOOT
BOOT
BOOT
...

This indicates that it sets up all the control registers correctly but for some reason goes into an infinate boot loop once Timer5 compare interrupt is activated.
Robin2, I would love to see what’s going on with regards to other interrupts running around but I have no idea how to monitor theses.

Cheers,

danwil02

rebooting is what happens if you get an interrupt that you don't have a matching ISR for.

// enable timer compare interrupt:
  TIMSK3 |= 0b00000010;

Shouldn't that be TIMSK5 ?

I haven't seen you post your ISR code; are you sure you have the vector name spelled exactly right? (It will NOT cause an error if you use the wrong name...)

Now that you have put your code into code tags I have had a look at it.

I don't know if it is the cause of the problem but your IRS is far far far too long. It should only have 3 or 5 lines of code so that it concludes as quickly as it possibly can.

And you certainly should NOT be trying to transmit anything from within an ISR.

...R

your ISR is far far far too long. It should only have 3 or 5 lines of code

Not really. Any particular path through that code IS only a few lines of code; it's just got a lot of decision points, which should not be that much of a problem.

Thanks for your help guys, I now have something that seems to be running well though I’ll need to wait until Monday to test as the receiver is at uni.

Putting the right interrupt handler got me past the initialise_interrupt() without going into a boot loop.
This got me back to where I was previously: Using code off the internet to manage my transmission for me. This wasn’t working so I’ve rewritten something that allows everything else to actually work without getting stuck as before. I still need to test it however.
On it’s own the code I initially posted works fine but I needed a faster ISR here as there is a lot more going on.

ISR(TIMER5_COMPA_vect)
{
	if(RF_msg_sending && (MsgCharCount<txStrLength))
	{
		RFout = txstring[MsgCharCount];
		RFbit = RFout & (1<< MsgBitCount);

		// Increment counters accordingly. 
		if(MsgBitCount<8)
		{
			MsgBitCount++;
		}
		else
		{
			MsgBitCount=0;
			MsgCharCount++;
		}
		rtty_txbit(RFbit);
	}
	else if(MsgCharCount>=txStrLength)
	{
		RF_msg_sending=false;
	}
}

All this does is send one bit at a time when the timer goes off @ 300 baud until the message is sent (hopefully).
It certainly beats forcing the rest of my program to wait 0.3 sec to send my RF message (using delay()).

		RFbit = RFout & (1<< MsgBitCount);

This is a relatively slow construct on AVRs. Instead of keeping a bit count, keep a bitmask that you shift in your ISR, something like:

if (txbitmask) {
   RFout = txtstring[msgCharCount];
   RFbit = RFout & txbitmask;
   txbitmask <<= 1;   // Shift mask to next bit
   rtty_txbit(RFbit);  // send the bit
   if (txbitmask == 0) { // end of this character?
	MsgCharCount++; // next character
        if (MsgCharCount < txStrLength) {
          txbitmask = 1;  //first bit of next char
        }  // else leave txbitmask at zero, which prevents additional transmitting.
   }
}

Note that RFBit will be zero/nonzero, rather than zero/one…
Also your code will be much shorter if tty_txbit() is NOT another function (make it a macro or inline.) If you avoid having your ISR call additional functions, you usually save a whole bunch of additional context save/restore.
Note that this doesn’t send any “start bits” or “stop bits”, which are normally required for async serial…

Thanks for the tips westfw.

Unfortunately the only code I can get to work in isolation is a semi cut down version of the example I’m working off:

ISR(TIMER5_COMPA_vect)
{
	if(TxEnable)
	{
		switch(txstatus) {
		case 2: // Grab a char and lets go transmit it. 
			if ( txj < txstringlength)
			{
				txc = txstring[txj];
				txj++;
				txstatus=3;
				rtty_txbit (0); // Start Bit;
				txi=0;
			}
			else
			{
				txstatus=0; // Should be finished
				TxEnable=false;
				txj=0;
			}
			break;
		case 3:
			if(txi<ASCII)
			{
				txi++;
				if (txc & 1) rtty_txbit(1); 
				else rtty_txbit(0);   
				txc = txc >> 1;
				break;
			}
			else
			{
				rtty_txbit (1); // Stop Bit
				txstatus=4;
				txi=0;
				break;
			} 
		case 4:
			if(STOPBITS==2)
			{
				rtty_txbit (1); // Stop Bit
				txstatus=2;
				break;
			}
			else
			{
				txstatus=2;
				break;
			}
		}
	}
}

But when integrated into my main program I get an infinite boot loop as soon as this ISR is activated by new GPS data.
I’ve tried many versions of my own code which seem to not disturb the main program but don’t operate correctly. This seems to operate correctly but only when nothing else is going on.
Some background on what is else is going on in my program: Accelerometer, Magnetometer, Gyroscope and Pressure sensor data are read and written to an SD card. There is some computation work done on the pressure sensor values to see what state the rocket is in (basic apogee detection stuff). The accelerometer and magnetometer have attached interrupts which just set data ready flags (not the best way to handle this I know but it works pretty well).

I’m pretty well stuck now, hopefully someone has some helpful suggestions but if I can’t make it work I guess I’ll go back to the method where I suspend my program for 0.3sec to send GPS coords.

        switch(txstatus) {
        case 2: // Grab a char and lets go transmit it. 
        case 3:
        case 4:

1) You really should Name your states:

  enum txstates { IDLE, STARTBIT, DATABITS, STOPBITS } txstatus;

or something like that.

2) it makes me really nervous that you don't have a "default" case. I'd guess that you're using state 0 as "idle", and the main program code changes the state to 2 when there is actually something to transmit, but that's less than obvious.

3) How does the main program know it can send a new string?

I'll see if I can come up with some full code. Too bad I co-opted 300bps to mean 1Mbps in my OS. I guess I'll try 1200. (sounds like fun!)

try this, after modifying the TXPINNO, TXPORT, TXBIT appropriately?

#define RTTY_BAUD 1200


char *volatile txmsg = NULL;
enum txstates {TXIDLE, TXSTARTBIT, TXDATA, TXSTOPBIT1, TXSTOPBIT2};
uint8_t txstate;  // (make this a uint8_t to save time/space in the ISR,
                  //   Since an actual enum would be an int.)
volatile uint8_t txbitmask;

/*
 * Define the pin number and port/bit for transmitting serial data.
 * Make sure they match!
 */
#define TXPINNO 1
#define TXPORT PORTE
#define TXBIT PORTE1


#define CLEARTX TXPORT &= ~(1<<TXBIT)
#define SETTX TXPORT |= 1<<TXBIT

void setup() {
  pinMode(13, OUTPUT);
  pinMode(8, OUTPUT);
  /*
   * Initialize SWSerial output
   */
  pinMode(TXPINNO, OUTPUT);
  cli();          // disable global interrupts
  SETTX; // Set to idle state (1)
  txstate = TXIDLE; // initial state
  TCCR5A = 0;     // set entire TCCR1A register to 0
  TCCR5B = 0;     // same for TCCR1B
  OCR5A = F_CPU / 1024 / RTTY_BAUD - 1;  // set compare match register to desired timer count:
  TCCR5B |= (1 << WGM12);   // turn on CTC mode:
  // Set CS10 and CS12 bits for:
  TCCR5B |= (1 << CS10) | (1 << CS12);
  // enable timer compare interrupt:
  TIMSK5 |= 0b00000010;
  sei();
}

void sendBackgroundMsg(char *msg)
{
  if (txstate != TXIDLE) {
    // Output already in progress.  This is an error; light pin13 LED
    digitalWrite(13, 1);
  } else {
    txmsg = msg;
    txstate = TXSTARTBIT;
  }
}

void loop() {
  delay(1000);
  digitalWrite(8, 1);
  sendBackgroundMsg("This is a test that is pretty long and should take some time to print\n");
  delay(1000);
  digitalWrite(8, 0);
}

ISR(TIMER5_COMPA_vect)
{
  switch (txstate) {
    case TXIDLE:
      break;  //stay idle

    case TXSTARTBIT:
      CLEARTX;  // Clear output for start bit
      txstate = TXDATA;
      txbitmask = 1;  //first bit of next char
      break;

    case TXDATA: {
        uint8_t RFout = *txmsg; // get current character
        uint8_t RFbit = txbitmask;  // current bit of current char.
        if (RFout & RFbit) {
          SETTX;
        } else {
          CLEARTX;
        }
        RFbit <<= 1;   // Shift mask to next bit
        if (RFbit == 0) { // end of this character?
          txstate = TXSTOPBIT1;
        }
        txbitmask = RFbit;  // save new bit position
        break;
      }

    case TXSTOPBIT1:
      SETTX;
      txstate = TXSTOPBIT2;
      break;

    case TXSTOPBIT2:
      if (*txmsg++ == 0) {  // Any more characters in string?
        txstate = TXIDLE;   // Nope: go to idle
      } else {
        txstate = TXSTARTBIT;  // Yep: start the next character
      }
      break;

    default:
      PORTB |= 1 << PORTB7; // Set LED for error
      break;
  }
}

Are you trying to write code to send serial data?

If so, this demo might be of interest.

...R

westfw:
try this, after modifying the TXPINNO, TXPORT, TXBIT appropriately?

I’m using pin 13 as the TX pin, but I’m not sure what to change your #define TXPORT and TXBIT to?
I think it should be:

#define TXPINNO 13
#define TXPORT 13
#define TXBIT 1

Also how does the code below do the digitalWrite on the pin?

#define CLEARTX TXPORT &= ~(1<<TXBIT)
#define SETTX TXPORT |= 1<<TXBIT

And how does this set an LED high for error?

PORTB |= 1 << PORTB7; // Set LED for error

Apart from that I’ve set the baud to 300 and if I can get those defines sorted I’m very keen to test this - thanks westfw :slight_smile:

Robin2:
Are you trying to write code to send serial data?

If so, this demo might be of interest.

…R

Thanks for that Robin2, I had a look into it but anything I do along these lines utilities delays which I am trying to avoid by using interrupt timers. Also using softwareserial I would get only 60% to 70% Tx success rate at best.

danwil02: Thanks for that Robin2, I had a look into it but anything I do along these lines utilities delays which I am trying to avoid by using interrupt timers. Also using softwareserial I would get only 60% to 70% Tx success rate at best.

My code only uses delayMicroseconds() while it is trying to sync with the incoming data stream - looking for a break in transmission so it can distinguish the HIGH of the idle signal from a HIGH corresponding to a series of 1 bits. Once it has sync'ed it doesn't use delay at all - only interrupts.

You could probably replace that delayMicroseconds() with another timer ISR but I'm not sure if it is worth the trouble considering how slow serial data is. If the transmitter is usually silent the sync will happen very quickly.

The code doesn't use delayMicroseconds() when it is just transmitting.

...R

I'm using pin 13 as the TX pin, but I'm not sure what to change your #define TXPORT and TXBIT to?

I think you'll want to use

#define TXPINNO 13
#define TXPORT PORTB
#define TXBIT PORTB7

Also how does the code below do the digitalWrite on the pin?

You'll want to read one of the existing tutorials on "Direct port IO"; but basically, while the arduino functions use pin numbers and HIGH/LOW values, lower-level (faster) software uses internal "port names" and bitmasks.

And how does this set an LED high for error?

You'll have to remove the "error" code, since you're already using the LED pin (13) for your transmission.