Need help with interrupts

Thoughts? Confusing myself. lol.

Sometimes it's helpful to write out the basic concept with a simple flowchart.

lol. my brain is hurting trying to do that heheh. But yeah thats what I'm doing now. Figured Id open it up to comments anyway. Like 'hey dummy, you're making this harder than it has to be'. lol :)

'hey dummy, you're making this harder than it has to be'.

Yes, that is a common trap that begineer programmers can fall into. It really helps to read lots of example code to see how the C language functions and structures work and how what may seem complex can be reduced to just a few statements.

One example in your case is your use of interrupts. While it can certainly be designed that way, the speed of the basic controller (fast) Vs the speed of QSO turnovers (slow) is such that you could easily accomplish the tasks without using interrupts. Just scanning for change of state of a input pin with digitalRead() statements and taking action on them via IF statements would work just fine.

Lefty

I'm no beginner. I've been writing really bad code for 20 years. Ask anybody who's ever seen it :) It takes years of practice to write code as awfully as some of mine has been :)

lol.

Is this more along the lines of what you were thinking for an interrupt-less version?

It doenst seem to be working though… like I’m stuck in the while loop. Should I change it to an if loop instead of a while loop?

/*
  ###########################################################
  #  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;                          // COS Detect Pin
int waskeyed = 0;
int cosState;
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)
      
  
  // 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);     
        digitalWrite(txStbyLED, HIGH);
        digitalWrite(txActvLED, LOW);
       
      cwid(); // ID the repeater on power-up
}

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

void loop() {
  Serial.println("Begin Loop Routine");
  lcd.clear();
  
  // Stuck in this loop while counting to do the ID
 cosState = digitalRead(cosPin);
 
if (cosState == 0) 
{
  Key();

  int waskeyed = 1;
}
 
  else
  {
    if (waskeyed == 1)
    { 
     Key();
     ct();
     unKey();  
}


  
  
  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");
    
  Key();
  
  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

  unKey();
  
cur_id_timer = original_id_timer;


}


/*
    ##################
    #  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);
}


void unKey()
{
  digitalWrite(txStbyLED, HIGH);
  digitalWrite(txActvLED, LOW);
  digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)
  Serial.println("unKey");
}

void Key()
{
  digitalWrite(txStbyLED, LOW);
  digitalWrite(txActvLED, HIGH);
  digitalWrite(pttPin, HIGH); // Ensure the transmitter is keyed by forcing the pin HIGH
  Serial.println("Key");
}

void ct()
{
   tone(audioPin, ditfreq*2, 200);
}
if (cosState == 0)
{
  Key();

  [glow]int waskeyed[/glow] = 1;
}

This variable is not the same as the global variable of the same name. Having local and global variables with the same names is rarely a good idea.

I’m trying to modify the global variable - my intent was not to do locally scoped variables. What do I need to do differently?

… just drop the ‘int’ ?

.. just drop the 'int' ?

You got it.

Having different variable scope capabilities is one of the hallmarks of the C language (and many other languages) and is always encouraged in C tutorials and courses.

I however tend to use (maybe overuse) mostly global variables in my sketches (other then inside FOR and other control statements). My style of development tends to be build a little hardware, write a little code then test it out, build a little more hardware, write more code and test it, lather and repeat. Using global variables tends to work better for that style, and while the finished project may not be of C text book standard, it tends to have few and easier bugs with the incremental build/write/test then if I built all the hardware at once then all the code at once.

I only occasionally write separate functions as it just seems easier to keep adding to the main loop function as I go. At least I know that I'm not always writing using the best standards and practices, but the typical smaller applications that I use a Arduino for tend to not need tons of variables and functions to work and are not hard to deal with globally.

Anyway just rambling I guess and at least I gave up using the BASIC language thanks to the Arduino platform, I really have come to like C (but the jury is still out for C++ ;) )

Lefty

Sorry to be jumping around so much - but we’re under active rapid development… hehe.

For play, I’ve removed the ‘interrupt’ side of things (and I’m thinking I’ll be putting them back) just to try and whip the looping part of things - its really close. The input pin state changes are triggering the output pin state changes - but when the transmitter kicks in it immediately drops (runs the key() then unkey() - it should be staying keyed. Still looking to see whats going on there.

… many code changes. The latest revision (been renaming stuff to make it make more sense in my head)

/*
  ###########################################################
  #  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 = 3;                          // COS Detect Pin
int waskeyed = 0;
int cosState;
char countertxt[3];
int original_id_timer = 15;
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)
      
  
  // 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);     
        digitalWrite(txStbyLED, HIGH);
        digitalWrite(txActvLED, LOW);
       
      cwid(); // ID the repeater on power-up
}

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

void loop() {
  Serial.println("Begin Loop Routine");
  lcd.clear();
  
  // Stuck in this loop while counting to do the ID
 cosState = digitalRead(cosPin);
 
if (cosState == 0) 
{
  Serial.println("IF cosstate ==0");
  Key();

  waskeyed = 1;
}
 
  else
  {
    
    Serial.println("IF cosstate == 1");
    if (waskeyed == 1)
    { 
      Serial.println("IF waskeyed ==1");
     // Key();
     ct();
     unKey();  
     
    waskeyed = 0;
}


  
  
  if (id_timer < cur_id_timer) {
  Serial.println("another second elapsed");
   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
  {
    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");
    
  Key();
  
  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

  unKey();
  ct();
cur_id_timer = original_id_timer;


}


/*
    ##################
    #  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);
}


void unKey()
{
  digitalWrite(txStbyLED, HIGH);
  digitalWrite(txActvLED, LOW);
  digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)
  Serial.println("unKey");
}

void Key()
{
  digitalWrite(txStbyLED, LOW);
  digitalWrite(txActvLED, HIGH);
  digitalWrite(pttPin, HIGH); // Ensure the transmitter is keyed by forcing the pin HIGH
  Serial.println("Key");
}

void ct()
{
   tone(audioPin, ditfreq*2, 200);
   Serial.println("ct");
}

Ok… Found a problem.

I’ve got a variable that seems to be changing value. And Im not sure why.

cosstate should always be 0 (because its never goes to 1)… at least not as far as I can detect.

But it seems to be going to 1… and then invoking the wrong section of the ‘if’. Causing it to key up, key down, loop, key up, key down, loop.

See below. Pay attention to the top section (if clause) of the loop section.

COS is at V+ (Squelch Closed)
another second elapsed
Begin Loop Routine
COS is at V+ (Squelch Closed)
another second elapsed
Begin Loop Routine
COS is at GND (Squelch Open)
Key
Begin Loop Routine
COS is at V+ (Squelch Closed)
We had been previously keyed
ct
unKey
another second elapsed
Begin Loop Routine
COS is at V+ (Squelch Closed)
another second elapsed
Begin Loop Routine
COS is at GND (Squelch Open)
Key
Begin Loop Routine
COS is at GND (Squelch Open)
Key
Begin Loop Routine
COS is at V+ (Squelch Closed)
We had been previously keyed
ct
unKey
another second elapsed

current code is this

/*
  ###########################################################
  #  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 = 3;                          // COS Detect Pin
int waskeyed = 0;
int cosState;
char countertxt[3];
int original_id_timer = 15;
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)
      
  
  // 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);     
        digitalWrite(txStbyLED, HIGH);
        digitalWrite(txActvLED, LOW);
       
      cwid(); // ID the repeater on power-up
}

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

void loop() {
  Serial.println("Begin Loop Routine");
  lcd.clear();
  
  // Stuck in this loop while counting to do the ID




 cosState = digitalRead(cosPin);
 
if (cosState == 0) 
{
  Serial.println("COS is at GND (Squelch Open)");
  Key();

  waskeyed = 1;
}
 
  else
  {
    
    Serial.println("COS is at V+ (Squelch Closed)");
    if (waskeyed == 1)
    { 
      Serial.println("We had been previously keyed");
     // Key();
     ct();
     unKey();  
     
    waskeyed = 0;
}






  
  
  if (id_timer < cur_id_timer) {
  Serial.println("another second elapsed");
   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
  {
    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");
    
  Key();
  
  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

  unKey();
  ct();
cur_id_timer = original_id_timer;


}


/*
    ##################
    #  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);
}


void unKey()
{
  digitalWrite(txStbyLED, HIGH);
  digitalWrite(txActvLED, LOW);
  digitalWrite(pttPin, LOW); // Ensure the transmitter is not keyed by forcing the pin LOW (use pulldown instead??)
  Serial.println("unKey");
}

void Key()
{
  digitalWrite(txStbyLED, LOW);
  digitalWrite(txActvLED, HIGH);
  digitalWrite(pttPin, HIGH); // Ensure the transmitter is keyed by forcing the pin HIGH
  Serial.println("Key");
}

void ct()
{
   tone(audioPin, ditfreq*2, 200);
   Serial.println("ct");
}

cosstate should always be 0 (because its never goes to 1)... at least not as far as I can detect.

The cosState variable is set on every pass through loop:

cosState = digitalRead(cosPin);

If it is taking on unexpected values, it looks like a hardware problem. What is connected to cosPin, and how is it wired?

It's connected to an external voltage source thats normally high (common ground to the arduino, though).

Maybe I'll add a buffer transistor and pull from the 5v bus and see if it likes that better. I guess thats more 'correct' anyway.

Ok the buffer transistor with a pulldown has me CLOSE. Getting the 'bad delay' i was afraid of with the loop routine - i just dont want to wait the 1 second for the loop to progress to pick up a key... back to interrupts now after this slight distraction. At least the electronics are now good, and I have an idea of what im doing with the code... very close now I think.

very close now I think.

Or as Paul Simon sang " The closer you get the more your just slip sliding away" ;)

I think Alabama said something similar... :)

Just wait until I get really distracted and put a GPS disciplined real time clock in her, heh.... maybe.