Need help with interrupts

I’m writing some software to function as a repeater controller for amateur radio use. It has 2 main functions - 1, generate a morse code (CW) id every X minutes, and 2, trigger Push-To-Talk relay on the radio to ‘transmit’ whenever a received signal is present.

I have the code ‘mostly’ working, but here’s my hangup.

I’m trying to monitor an interrupt. The pin is normally pulled high via a pullup resistor. When the pin is falling, I want to run a routine ptt() which triggers the activation of a relay. I want this relay to remain activated as long as the pin remains pulled low. When the pin is no longer pulled low and goes high again, I want to disable the relay and run a second routine ct() - which generates a ‘beep’ - acting as a ‘roger beep’ or courtesy tone.

The interrupt is currently activating the relay when triggered, but it’s staying activated, and the routine for running the ‘beep’ is never running.

Care to take a peek. Code below.

/*
###########################################################

Program: KC8OZA-ID - Open Source Repeater Controller

Author: Josh Kittle, KC8OZA

Date: 06/2010

Written for use on the Arduino Platform

###########################################################
*/

/*
######################################

Includes and Variable Definitions

######################################
*/

#include <LiquidCrystal.h>
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
int dahfreq = 600;
int ditfreq = 600;
int multi = 40;
int pttPin = 14;
int txActvLED = 4;
int txStbyLED = 5;
int audioPin = 6; // This pin is the audio output for CWID and Courtesy Tonet
int cosPin = 2; // Only pins 2 and 3 can be used for interrupts, so we will use pin 2
volatile int cosState = HIGH ; // The value for this variable is ready from memory so you need to define it as volatile
volatile int returnfromptt = 0;
char countertxt[3];
int cur_id_timer;
int id_timer;
char cur_timetxt[5];

/*
##################

Setup Routine

##################
*/

void setup() {

// Define LCD Configuration Parameters

lcd.begin(20, 4); // Define the LED Size (4x20 LCD shown)

// Configure the Interrupt for COS

attachInterrupt(1, ptt, FALLING);
interrupts();

// Display the initialization information on the LCD

delay(3000);
lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(2, 2);
lcd.print(“Controller Init”);

// Define Pin Modes

pinMode(cosPin, INPUT); // This is used for sensing COS activity
pinMode(txActvLED, OUTPUT); // This LED is lit when TXing
pinMode(txStbyLED, OUTPUT); // This LED is lit when not TXing
pinMode(pttPin, OUTPUT);

delay(3000);

cwid(); // ID the repeater on power-up
}

/*
##################

Main Program

##################
*/

void loop() {
digitalWrite(txStbyLED, HIGH);
digitalWrite(txActvLED, LOW);

cur_id_timer = 20; // This is the value in seconds between CWID

digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)

digitalWrite(cosPin, cosState);

lcd.clear();

if (returnfromptt == 0){

while (id_timer < cur_id_timer) {

delay (1000);

cur_id_timer = cur_id_timer - 1;
itoa(cur_id_timer, cur_timetxt, 10);

lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(4, 2);
lcd.print(cur_timetxt);
lcd.print(" sec to ID");
}
}
else
{
digitalWrite(pttPin, LOW);
ct();
}

lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(3, 2);
lcd.print(“ID In Progress”);

cwid(); // Timer Expired - ID the repeater

}

/*
##################

CWID Routine

##################
*/

void cwid()
{
digitalWrite(txStbyLED, LOW);
digitalWrite(txActvLED, HIGH);
digitalWrite(pttPin, HIGH);

dah();
dit();
dah();
letter(); // K
dah();
dit();
dah();
dit();
letter(); // C
dah();
dah();
dah();
dit();
dit();
letter(); // 8
dah();
dah();
dah();
letter(); //O
dah();
dah();
dit();
dit();
letter(); // Z
dit();
dah();
letter(); // A

digitalWrite(pttPin, LOW);
digitalWrite(txStbyLED, HIGH);
digitalWrite(txActvLED, LOW);
}

/*
######################

PTT Relay Routine

######################
*/

void ptt() // This is the PTT Routine that will trigger when COS goes LOW.
{

// Change the LED state to indicate active TX

digitalWrite(txStbyLED, LOW);
digitalWrite(txActvLED, HIGH);

// Trigger the PTT Relay

digitalWrite(pttPin, HIGH);

int returnfromptt = 1;

}

/*
##########################

Courtesy Tone Routine

##########################
*/

void ct()
{
tone(audioPin, ditfreq*2, 200);
digitalWrite(pttPin, LOW);
Serial.println(“PTT LOW”);

}

/*
##################

CW Routines

##################
*/

void dit()
{
delay(1multi);
//digitalWrite(txPin, HIGH);
tone(audioPin, ditfreq);
delay(1
multi);
noTone(audioPin);
//delay(75);
//tone(txPin, freq, 225);
}

void dah()
{
delay(1multi);
//digitalWrite(txPin, HIGH);
tone(audioPin, dahfreq);
delay(3
multi);
noTone(audioPin);
//digitalWrite(txPin, LOW);
//tone(txPin, freq, 225);
}

void letter()
{
delay(2*multi);
}

attachInterrupt(1, ptt, FALLING);

You're only triggering on the falling edge. If you want to trigger the ISR, you need to say so (trigger on CHANGE). http://arduino.cc/en/Reference/AttachInterrupt

Also, at a glance it looks like you don't have any code in the ISR to handle the rising edge (release PTT) case. You'll key up and never release.

Also, I'd wait until after the tone/cwid is complete to release PTT, else no one will hear it.

since you need to take a long time (generate the tone and/or cwid) I'd move the code that does work out of the ISR and into loop(), and just set a flag inside the ISR. Then you may have some fun special cases, like can PTT be triggered after relase but before tone/cwid is complete?

-j

If I trigger on change, how do I determine I'm in the right states, and not backwards for instance?

I tried writing separate code for trigger on fall versus rise (separate lines of code) but it seemed like only the one that was first would happen - the second one was ignored.... Can it be done? If so, any example code?

If I trigger on change, how do I determine I’m in the right states, and not backwards for instance?

Inside your ISR routine you would perform a digital read on pin 3 (if using interrupt 1, pin 2 for interrupt 0) to find out the present state of the input pin that caused the interrupt.

Am I properly setting the interrupt pin as an input? Let me say that a different way - the arduino example on their wiki shows it set as an output, but I believe that's an error. I believe it should be an input.

See url:

http://www.arduino.cc/en/Reference/AttachInterrupt

Example

int pin = 13; volatile int state = LOW;

void setup() { pinMode(pin, OUTPUT); attachInterrupt(0, blink, CHANGE); }

void loop() { digitalWrite(pin, state); }

void blink() { state = !state; }

No. that example is setting the output pin for the LED on pin 13 (pinMode(pin, OUTPUT);).

While I would think it would be better form to explicitly set-up pin 2 as an input (and enable the internal pull-up if desired), it's not really required because the default mode for I/O pins is input mode following a reset or power up cycle and the attachInterrupt function 'knows' that interrupt 0 uses I/O pin 2.

Another poor part of that example is that it doesn't state what is wired to pin 2 to generate the interrupt. I would assume they would have pin 13 wired to pin 2, but that would create an awful fast blink that might just show up as a dim glow on the pin 13 led?

Lefty

Hmm.

So does that mean that I don't really need the following code in my sketch?

int cosPin = 2; // Only pins 2 and 3 can be used for interrupts, so we will use pin 2

and

digitalWrite(cosPin, cosState);

... the cospin is being used as an input. I think I misunderstood how the example code was working.

So does that mean that I don't really need the following code in my sketch?

int cosPin = 2; // Only pins 2 and 3 can be used for interrupts, so we will use pin 2

and

digitalWrite(cosPin, cosState);

... the cospin is being used as an input. I think I misunderstood how the example code was working.

Yes, that does not make a lot of sense. You have defined pin 2 as an input but later you perform a digitalWrite to it. All that will do is either turn on or off the internal pull-up resistor for pin 2 depending on the value of cosState, probably not what you intended? Also it seems you have another problem in that you do a attachInterrupt(1, ptt, FALLING); yet you are using pin 2 for your interrupt. Interrupt 1 uses pin 3, interrupt 0 uses pin 2?

If you require the internal pull-up for pin 2 to be enable you should perform a digitalWrite(cosPin,HIGH) just once in the setup function.

Lefty WA6TKD

Thanks for the clarification. I was definitely screwed up :) I'm doing external pullups, so I'm not intending to use the internal ones.

The inconsistancy in the interrupt is because ... I changed which pin I was using, got things a little out of sync there.

I'm rewriting some code now - breaking the ptt routine into two separate routes... a 'close relay' routine, and an 'open relay' routine. This seems to make more sense than doing ct() elsewhere - since every time I open the relay I want to play the tone - so why not just code it that way - I figure.

A little variable magic here, and some tweaking, I think I'll be ready to connect the arduino and test.

I LOVE this Arduino :)

Ok I think I’m really close here.

I’ve made the following changes - the system boots, runs an initial cwid(), and goes into the while loop… starting the countdown timer.

As the timer expires, the id plays and PTT does what it’s supposed to.

But… it get stuck in a loop and keeps executing the cwid() over and over. Gotta be a simple error in the variable state - care to peek for a second set of eyes?

/*
###########################################################

Program: KC8OZA-ID - Open Source Repeater Controller

Author: Josh Kittle, KC8OZA

Date: 06/2010

Written for use on the Arduino Platform

###########################################################
*/

/*
######################################

Includes and Variable Definitions

######################################
*/

#include <LiquidCrystal.h>
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
int dahfreq = 600;
int ditfreq = 600;
int multi = 40;
int pttPin = 14;
int txActvLED = 4;
int txStbyLED = 5;
int audioPin = 6; // This pin is the audio output for CWID and Courtesy Tonet
int cosPin = 2; // Only pins 2 and 3 can be used for interrupts, so we will use pin 2
volatile int cosState = HIGH ; // The value for this variable is ready from memory so you need to define it as volatile
volatile int pttState = 0;
char countertxt[3];
int cur_id_timer = 20;
int id_timer;
char cur_timetxt[5];

/*
##################

Setup Routine

##################
*/

void setup() {

// Define LCD Configuration Parameters

lcd.begin(20, 4); // Define the LED Size (4x20 LCD shown)

// Configure the Interrupt for COS

attachInterrupt(1, pttEval, CHANGE);
interrupts();

// Display the initialization information on the LCD

delay(3000);
lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(2, 2);
lcd.print(“Controller Init”);

// Define Pin Modes

pinMode(cosPin, INPUT); // This is used for sensing COS activity
pinMode(txActvLED, OUTPUT); // This LED is lit when TXing
pinMode(txStbyLED, OUTPUT); // This LED is lit when not TXing
pinMode(pttPin, OUTPUT);

delay(3000);

cwid(); // ID the repeater on power-up
}

/*
##################

Main Program

##################
*/

void loop() {
digitalWrite(txStbyLED, HIGH);
digitalWrite(txActvLED, LOW);

digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)

// digitalWrite(cosPin, cosState);

lcd.clear();

if (pttState == 0){

while (id_timer < cur_id_timer) {

delay (1000);

cur_id_timer = cur_id_timer - 1;
itoa(cur_id_timer, cur_timetxt, 10);

lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(4, 2);
lcd.print(cur_timetxt);
lcd.print(" sec to ID");
}
}
else
{
pttOpen();

}

lcd.clear();
lcd.setCursor(2, 0);
lcd.print(“KC8OZA Repeater”);
lcd.setCursor(3, 2);
lcd.print(“ID In Progress”);

cwid(); // Timer Expired - ID the repeater

}

/*
##################

CWID Routine

##################
*/

void cwid()
{
pttClose();

dah();
dit();
dah();
letterspace(); // K
dah();
dit();
dah();
dit();
letterspace(); // C
dah();
dah();
dah();
dit();
dit();
letterspace(); // 8
dah();
dah();
dah();
letterspace(); //O
dah();
dah();
dit();
dit();
letterspace(); // Z
dit();
dah();
letterspace(); // A

pttOpen();

}

/*
######################

PTT Relay Routine

######################
*/

void pttEval() // Evaluate PTT State
{
if (pttState == 0)

{
pttClose();
}

else
{

pttOpen();

}
}

void pttClose() // This is used to close the PTT relay (trigger PTT)
{

// Change the LED state to indicate active TX

digitalWrite(txStbyLED, LOW);
digitalWrite(txActvLED, HIGH);

// Trigger the PTT Relay ON

pttState = 1;

digitalWrite(pttPin, HIGH);

}

void pttOpen() // This is used to open the PTT relay (drop the PTT signal)
{
tone(audioPin, ditfreq*2, 200); // This is our courtesy tone

digitalWrite(txStbyLED, HIGH);
digitalWrite(txActvLED, LOW);

// Trigger the PTT Relay OFF

pttState = 0;

digitalWrite(pttPin, LOW);

}

/*
##################

CW Routines

##################
*/

void dit()
{
delay(1multi);
//digitalWrite(txPin, HIGH);
tone(audioPin, ditfreq);
delay(1
multi);
noTone(audioPin);
//delay(75);
//tone(txPin, freq, 225);
}

void dah()
{
delay(1multi);
//digitalWrite(txPin, HIGH);
tone(audioPin, dahfreq);
delay(3
multi);
noTone(audioPin);
//digitalWrite(txPin, LOW);
//tone(txPin, freq, 225);
}

void letterspace()
{
delay(2*multi);
}

I think it’s more likely a structure problem in your main loop function. cwid(); // Timer Expired - ID the repeater is the last statement in the loop function so it will of course ID every cycle of the main loop. This probably needs to be placed somewhere inside a for or while loop that triggers it at the proper times.

When you post code in the future, if you highlight it all then then press the # (insert code) button on the top of the edit window, it will place all your code into a nice formated window which is easier to read and scroll.

Lefty WA6TKD

KC8OZA,

Would be useful if you gave a clearer description of what needed to be accomplished.

Well, I’ve made some progress. Still a few kinks, but very close.

To give a little better information for those who might be reading this who aren’t ‘hams’ - the purpose of this device is simple - to provide a logic interface between a transmitter and a receiver, providing for the machine to identify itself via morse code every 10 minutes, to provide for sensing of an interrupt to determine when to key a transmitter, and to provide for sensing of ‘end of transmission’ to play a ‘roger beep’ or ‘courtesy tone’.

I’ve got interrupts working, and basic functionality pretty much doing what i want - but it seems like something is unstable. Triggering interrupts is generating the beep I want, but currently not keying the transmitter ( I think I can find that problem) but seems to ‘rapidly change state’ in the beginning - and then settle to a stable state. Sort of what you’d see with a switch needing debouncing. Do I need that sort of logic here?

I want it to do this

…while loop… ID every 10 minutes.

When interrupt = CHANGE (falling) trip a ptt relay and hold it open.
When interrupt = CHANGE (rising) close the ptt relay and play a beep.

DO you guys see a simpler way to code this?

Current code - for those interested in how this is evolving.

/*
  ###########################################################
  #  Program: KC8OZA-ID - Open Source Repeater Controller   #
  #  Author: Josh Kittle, KC8OZA                            #
  #  Date: 06/2010                                          #
  #  Written for use on the Arduino Platform                #
  ###########################################################
*/



/*
    ######################################
    #  Includes and Variable Definitions #
    ######################################
*/

#include <LiquidCrystal.h>
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);

int dahfreq = 600;
int ditfreq = 600;
int multi = 40;
int pttPin = 14; 
int txActvLED = 4;
int txStbyLED = 5;
int audioPin = 6;                       // This pin is the audio output for CWID and Courtesy Tonet
int cosPin = 2;                         // Only pins 2 and 3 can be used for interrupts, so we will use pin 2
volatile int cosState = HIGH ;      // The value for this variable is ready from memory so you need to define it as volatile
volatile int pttState = 0;
int InterruptState;
char countertxt[3];
int original_id_timer = 300;
int cur_id_timer = original_id_timer;
int id_timer;
char cur_timetxt[5];

/*
    ##################
    #  Setup Routine #
    ##################
*/
    
void setup() { 
Serial.begin(9600);
  // Define LCD Configuration Parameters
  
      lcd.begin(20, 4);  // Define the LED Size (4x20 LCD shown)

  // Configure the Interrupt for COS
  
      attachInterrupt(1, pttEval, CHANGE);
      interrupts();
      
   
  // Display the initialization information on the LCD
 
      delay(3000);
      lcd.clear();
      lcd.setCursor(2, 0);
      lcd.print("KC8OZA  Repeater");
      lcd.setCursor(2, 2);
      lcd.print("Controller  Init");
 
 
  // Define Pin Modes
    
      pinMode(cosPin, INPUT);       // This is used for sensing COS activity
      pinMode(txActvLED, OUTPUT);  // This LED is lit when TXing
      pinMode(txStbyLED, OUTPUT);  // This LED is lit when not TXing
      pinMode(pttPin, OUTPUT);
  
      delay(3000);     
      
      cwid(); // ID the repeater on power-up
}

/*
    ##################
    #  Main Program  #
    ##################
*/

void loop() {
  Serial.println("Begin Loop Routine");
  digitalWrite(txStbyLED, HIGH);
  digitalWrite(txActvLED, LOW);
  digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)
  lcd.clear();
  
  // Stuck in this loop while counting to do the ID
  
  while (id_timer < cur_id_timer) {
  Serial.println("in the while loop");
   delay (1000);
     
    cur_id_timer = cur_id_timer - 1;
    itoa(cur_id_timer, cur_timetxt, 10);
    
    lcd.clear();
    lcd.setCursor(2, 0);
    lcd.print("KC8OZA  Repeater");
    lcd.setCursor(4, 2);
    lcd.print(cur_timetxt);
    lcd.print(" sec to ID");
    
   }
   
  // When the loop ends - run the ID 
    Serial.println("Run CWID Routine");
  cwid();  // Timer Expired - ID the repeater

}

/*
    ##################
    #  CWID Routine  #
    ##################
*/

void cwid()
{
  Serial.println("running cwid");
  lcd.clear();
  lcd.setCursor(2, 0);
  lcd.print("KC8OZA  Repeater");
  lcd.setCursor(3, 2);
  lcd.print("ID In Progress");
    
  pttClose();
  
  dah(); 
  dit(); 
  dah(); 
  letterspace();  // K
  dah(); 
  dit(); 
  dah(); 
  dit(); 
  letterspace(); // C
  dah(); 
  dah(); 
  dah(); 
  dit(); 
  dit(); 
  letterspace(); // 8
  dah(); 
  dah(); 
  dah(); 
  letterspace(); //O
  dah(); 
  dah(); 
  dit(); 
  dit(); 
  letterspace(); // Z
  dit(); 
  dah(); 
  letterspace(); // A

  pttOpen();
cur_id_timer = original_id_timer;


}


/*
    ######################
    #  PTT Relay Routine #
    ######################
*/

void pttEval() // Evaluate PTT State
{
InterruptState = digitalRead(cosPin);

if (InterruptState == HIGH)

{
  pttOpen();
} 
else {
  
  pttClose();
 
}
}



void pttClose() // This is used to close the PTT relay (trigger PTT)
{
 Serial.println("pttClose Routine"); 
 // Change the LED state to indicate active TX
 
  digitalWrite(txStbyLED, LOW);
  digitalWrite(txActvLED, HIGH);
 
 // Trigger the PTT Relay ON
 
  int pttState =     1;
Serial.println("PTT State 1");
  digitalWrite(pttPin, HIGH);
 
 }


void pttOpen() // This is used to open the PTT relay (drop the PTT signal)
{
  Serial.println("pttOpen Routine");
  tone(audioPin, ditfreq*2, 400); // This is our courtesy tone
  
  digitalWrite(txStbyLED, HIGH);
  digitalWrite(txActvLED, LOW);
  
  // Trigger the PTT Relay OFF
  
  int pttState = 0;
  Serial.println("PTT State 0");
  digitalWrite(pttPin, LOW);

}

/*
    ##################
    #  CW Routines   #
    ##################
*/

void dit()
{
  delay(1*multi);
  //digitalWrite(txPin, HIGH);
  tone(audioPin, ditfreq);
  delay(1*multi);
  noTone(audioPin);
  //delay(75);
  //tone(txPin, freq, 225);
}

void dah()
{
  delay(1*multi);
  //digitalWrite(txPin, HIGH);
  tone(audioPin, dahfreq);
  delay(3*multi);
  noTone(audioPin);
  //digitalWrite(txPin, LOW);
  //tone(txPin, freq, 225);
}

void letterspace()
{
  delay(2*multi);
}

A couple of comments.

If you think you are getting flaky interrupts you might indeed be suffering the equivalent of contact bounce from the carrier detect signal that activates the interrupt. You can try a R/C filter the signal to slow it down some, would have to see the circuit the generates the signal to say how.

In your ISR routine you are doing an awful lot of code. This probably won't be a problem as the interrupts shouldn't be happening very often in computer time. But in general practice one normally tries to keep the code as lean and mean as possible while inside the ISR function as all the while you are in the ISR all other interrupts are disabled until the ISR returns to the main loop. What one normally does is to just set or reset volatile int variables (used as flags) that the main loop function can test and take action on. In your case the functions pttClose() and pttOpen() are called while you are still in the ISR function. These functions have serial commands that take some time to run in computer time. It would be better if you just set flags inside to ISR and then have the main loop test them and call pttOpen and pttClose from the main loop and then reset the flags.

Lefty

Yeah, what Lefty said. [u]Way[/u] too much time spent in the ISR; set a flag, get out, handle the flag somewhere else (e.g. loop()).

-j

To echo the others, remove both the 'Serial.println' and 'tone' statements from the Interrupt Service Handler. Serial I/O take a looong time and, I may be wrong but 'tone' may wait for the specified duration parameter before it returns as well.

Nodding head... Understood. Will rethink this one this evening and see where I can get with it.

Trying to figure out if I need to represent 2 states or 3..... Obviously there is a 'did somebody just talk = yes' state, and a 'did somebody just talk = no' state.... my brain is thinking there is a third combination somewhere... Will see where my creativity leads tonight. I'm really stinking close.

Should the CW identifier interrupt PTT keyed mode do it's thing and then come back to PTT keyed mode.

If I understand the question - I'm fine with the CWID taking place 'over top' or while the interrupt is triggered, yes. I may want to make that more intelligent later, but right now, at this stage, I'm just going for basic functionality.

'Trying' to walk before I run, anyway.

Still trying to sort out the number of states.....

Heres where my brain is.

I think I need a 'tx mode', 'rx mode with beep', and 'rx mode no beep'

Because In the loop, I guess its going to detect 'do I need to turn on the transmitter (due to interrupt), its going to detect 'do I need to drop the transmitter and beep (because somebdoy talked), or 'do I need to keep the transmitter dropped, but not beep???

Thoughts? Confusing myself. lol.