Need help troubleshooting variable not getting set by interrupt

This is running on an ATTINY84. I am stumped in that the statusMask variable is not ever being set by the main loop, though I know for a fact the interrupts are being called because a diagnostic LED tell's me so, I have ran the diagnostic LED in every conceivable spot, and the one place I never get a response is inside of if (statusMask), which makes no sense because the interrupt does indeed get triggered, any ideas?

#include <PinChangeInterrupt.h>


/*
 MatCat Airplane Lighting Contoller
 
 PWM and I2C connectivity to drive 3 channels of lighting.
 
 This code is open source and free to be used by whomever
 for non-commercial purposes.  Please keep the proper
 copyright when distributing this code, thank you!
 */


// PWM Outputs
unsigned int headlights = 2;
unsigned int navlights = 3;
unsigned int floodlights = 4;

// PWM Inputs
unsigned int headlightInput = 5;
unsigned int navlightInput = 6;
unsigned int floodlightInput = 7;

// Operational Parameters
int strobeFrequency = 1000;        // 1 second between pulses
int strobeLength = 30;             // 30ms strobe length
int strobeBreak = 50;              // 50ms break between fast pulses

volatile byte headlightLevel = 0;           // Output buffer level for headlights
volatile byte navlightLevel = 0;            // Output buffer level for navlights
volatile byte floodlightLevel = 0;          // Output buffer level for floodlights
volatile uint16_t oldHeadLevel = 0;          // Old lighting level
volatile uint16_t oldNavLevel = 0;
volatile uint16_t oldFloodLevel = 0;
volatile uint8_t oldHeadOut = 0;
volatile uint8_t oldNavOut = 0;
volatile uint8_t oldFloodOut = 0;
// Bit mask for keeping track of whats going on

static uint8_t HEADLIGHT_FLAG = 1;
static uint8_t NAVLIGHT_FLAG = 2;
static uint8_t FLOODLIGHT_FLAG = 4;
static uint8_t I2C_FLAG = 8;                 // Indicates the usage of I2C over PWM

uint8_t hasBound = 0;               // A variable to keep track of being bound

volatile uint8_t statusMask;        // This is the actual mask variable

// Timers to keep track of pulse widths
volatile unsigned long headlightTimer = 0;
volatile unsigned long navlightTimer = 0;
volatile unsigned long floodlightTimer = 0;

// Timers for other operations
unsigned long lastStrobeTime;       // Used for strobe timing
unsigned long lastFrameTime;        // When was the last PWM frame?
unsigned int secondTime;           // A special timer for measuring a second  
uint8_t rxFrequency = 0;                // Rated in HZ, normal is 50
uint16_t frameCount = 0;            // Count of frames per second

void setup()  { 
  // Setup inputs
  pinMode(headlightInput, INPUT);
  pinMode(navlightInput, INPUT);
  pinMode(floodlightInput, INPUT);

  // Setup outputs
  pinMode(headlights,OUTPUT);
  pinMode(navlights,OUTPUT);
  pinMode(floodlights,OUTPUT);

  // Setup a few variables
  lastStrobeTime = millis();
  secondTime = millis();
  // Let's do a little init. routine
  strobeLight(headlights,160,100);
  delay(100);
  strobeLight(navlights,160,100);
  delay(100);
  strobeLight(floodlights,160,100);
  delay(500);
  for (int a = 0; a < 5; a++) {
    strobeAll(160,160,160,30);
    delay(100);
  }
  // Ok now let's setup some interupts
  attachPcInterrupt(headlightInput,headlightOn,RISING);
  attachPcInterrupt(headlightInput,headlightOff,FALLING);
  attachPcInterrupt(navlightInput,navlightOn,RISING);
  attachPcInterrupt(navlightInput,navlightOff,FALLING);
  attachPcInterrupt(floodlightInput,floodlightOn,RISING);
  attachPcInterrupt(floodlightInput,floodlightOff,FALLING);

} 

void loop()  { 
  // Setup some local variables for reading
  // the values set by interrupts.  
  static uint16_t headLevel = 0;    
  static uint16_t navLevel = 0;
  static uint16_t floodLevel = 0;
  static uint8_t flagmask = 0;
  static uint8_t updateFlag = 0;    
  static long frameTime = 0;
  static uint8_t secondFlag = 0;
  static uint8_t strobeFlag = 0;
  
  if (statusMask) {
    // We need to update what's going on.
    noInterrupts();
    flagmask = statusMask;
    if (flagmask & HEADLIGHT_FLAG) {
     
      // Headlights have been updated
      if (headlightLevel != oldHeadLevel) {
        headLevel = headlightLevel;
        oldHeadLevel = headLevel;
        updateFlag |= HEADLIGHT_FLAG;
      }
      // Let's capture the frame time for calculation later
      frameTime = (uint16_t)(micros() - lastFrameTime);
      lastFrameTime = micros();
    }
    if (flagmask & NAVLIGHT_FLAG) {
      // Navs have been updated
      if (navlightLevel != oldNavLevel) {
        navLevel = navlightLevel;
        oldNavLevel = navLevel;
        updateFlag |= NAVLIGHT_FLAG;
      }
    }
    if (flagmask & FLOODLIGHT_FLAG) {
      // Floods have been updated
      if (floodlightLevel != oldFloodLevel) {
        floodLevel = floodlightLevel;
        oldFloodLevel = floodLevel;
        updateFlag |= FLOODLIGHT_FLAG;
      }
    }
  
    // We need to clear the status flags from interrupts
    statusMask = 0;
    
    interrupts();
  }
  // Let's handle the second timer.
  if ((millis() - secondTime) >= 1000) {
    // A second has passed, reset the timer
    secondTime = millis();
    secondFlag = 1;
  }
  
  // Let's handle the strobe timer.
  if ((millis() - lastStrobeTime) >= strobeFrequency) {
    // We need to do a strobe...
    lastStrobeTime = millis();
    strobeFlag = 1;
  }
  if (frameTime > 0) {
    // It's a new frame from the RX, let's do some math
    frameCount++;  // Add to frame count
    if (secondFlag) {
      // The frameCount variable will tell us, should be close to 50
      rxFrequency = frameCount;
      frameCount = 0;  // Reset it
    }
  }
  
  if (rxFrequency > 30) {
    // We probably have a valid PWM signal, so consider it bound
    hasBound = 1;
  } else {
    // Either we are not bound or lost bound...
    hasBound = 0;
  } 
  
  if (!hasBound) {
    // We need to continue looking for an input...
    strobeLight(headlights,50,15);      // Indicate looking for PWM
    delay(50);    
    strobeLight(navlights,50,15);      // Indicate looking for PWM
    delay(50);    
    strobeLight(navlights,50,15);      // Indicate looking for PWM
    delay(50);    
    strobeLight(navlights,50,15);      // Indicate looking for PWM
    delay(50);    
    strobeLight(navlights,50,15);      // Indicate looking for PWM
    delay(500);
  } else {
    // We are bound, so let's do what needs to be done
    // First we need to see if any of the levels need to be adjusted...
    uint8_t headOut = oldHeadOut;
    uint8_t navOut = oldNavOut;
    uint8_t floodOut = oldFloodOut;

    // Before we output the the power, we need to check if we have to strobe...
    if (strobeFlag) {
      // We need to do some strobing before actually setting the output
      strobeAll(255,255,255,strobeLength);
    }
    if (updateFlag & HEADLIGHT_FLAG) {
      // We need to change output of headlights
      headOut = map(headLevel,1000,2000,0,255);
      if (headOut < 40) {headOut = 0;}
      if (headOut > 210) {headOut = 255;}
      oldHeadOut = headOut;
      analogWrite(headlights,headOut);
    }
    if (updateFlag & NAVLIGHT_FLAG) {
      // We need to change output of headlights
      navOut = map(navLevel,1000,2000,0,255);
      if (navOut < 40) {navOut = 0;}
      if (navOut > 210) {navOut = 255;}
      oldNavOut = navOut;
      analogWrite(navlights,navOut);
    }
    if (updateFlag & FLOODLIGHT_FLAG) {
      // We need to change output of headlights
      floodOut = map(floodLevel,1000,2000,0,255);
      if (floodOut < 40) {floodOut = 0;}
      if (floodOut > 210) {floodOut = 255;}
      oldFloodOut = floodOut;
      analogWrite(floodlights,floodOut);
    }
  }
}
void strobeXTimes(int x) {
  for (int a = 0;a < x;a++) {
    strobeLight(headlights,255,20);
    delay(50);
  }
}
void strobeAll(byte valuehead,byte valuenav, byte valueflood, int duration) {
  analogWrite(headlights,valuehead);
  analogWrite(navlights,valuenav);
  analogWrite(floodlights,valueflood);
  delay(duration);
  analogWrite(headlights,oldHeadOut);
  analogWrite(navlights,oldNavOut);
  analogWrite(floodlights,oldFloodOut);
}

void strobeLight(int light,byte value,int duration) {
  analogWrite(light,value);
  delay(duration);
  if (light == headlights) {
      analogWrite(light,oldHeadOut);
  }
  if (light == navlights) {
      analogWrite(light,oldNavOut);
  }
  if (light == floodlights) {
      analogWrite(light,oldFloodOut);
 }
}

void headlightOn() {
  // Do something when we have headlight PWM coming in...
  headlightTimer = micros();   
}
void headlightOff() {
  // Do something when we have headlight PWM Pulse finished...
  headlightLevel = (uint16_t)(micros() - headlightTimer);
  statusMask |= HEADLIGHT_FLAG;
}
void navlightOn() {
  navlightTimer = micros();
}
void navlightOff() {
  navlightLevel = (uint16_t)(micros() - navlightTimer);
  statusMask |= NAVLIGHT_FLAG;
}
void floodlightOn() {
  floodlightTimer = micros();
}
void floodlightOff() {
  floodlightLevel = (uint16_t)(micros() - floodlightTimer);
  statusMask |= FLOODLIGHT_FLAG;
}
  1. Try working with just one think (say headlight) at a time. Get that running then extend it to cover the other things.

  2. I don't think you can do this

 attachPcInterrupt(headlightInput,headlightOn,RISING);
 attachPcInterrupt(headlightInput,headlightOff,FALLING);

Does the documentation explicitly say that you can have more that one interrupt attached to a pin AT THE SAME time?

Mark

holmes4:

  1. Try working with just one think (say headlight) at a time. Get that running then extend it to cover the other things.

  2. I don't think you can do this

 attachPcInterrupt(headlightInput,headlightOn,RISING);

attachPcInterrupt(headlightInput,headlightOff,FALLING);




Does the documentation explicitly say that you can have more that one interrupt attached to a pin AT THE SAME time?

Mark

The library does it in software, in all actuality pin change is called when there is a pin change on ANY of the pins on that port, and the library takes the ISR and does internal tracking of rising/falling and calls the appropriate callback. The callbacks themselves are working fine, one thing I found was changing UINT references to UNSIGNED INT and such made a difference and the main loop started to see something in the variable, then my RC TX battery died so now it's charging and I can't diagnose farther until then :).

Hi,
The PinChangeInt library masks pins at a hardware level so its not just any pin on a port, the hardware will not generate the interrupt if the individual pin is not configured for pin change interrupts.

I would suggest having a single interrupt using CHANGE rather than one for rising and one for falling, this is because internally the library will loop over your interrupts looking for the correct handler, more interrupts = more loops.

To keep this fast, you can use PCInt::pinState instead of using digitalRead to figure out whether it is a rising or falling edge i.e. -

This might even fix your issue.

void calcThrottlePulse()
{
  if(PCintPort::pinState)
  {
    ulThrottleInStart = micros();
  }
  else
  {
    unThrottleInShared = (micros() - ulThrottleInStart);
    bUpdateFlagsShared |= THROTTLE_FLAG;
  }
}

More optimisations here -

Duane B

rcarduino.blogspot.com

A single CHANGE interrupt is the way to go.

Mark

This is not the PinchangeInt library, it is the PinchangeInterrupt library for the ATTINY core, I dunno why everyone want's to focus on the interrupts when they work and the last bit of code does not even apply to this library... At either rate I AM open to ideas and if everyone thinks it's better to keep track of the actual rise/fall change myself vs letting the library do it I can, but the way it's coded and how simple it is I do not see it making much of a difference...

This type of project is REALLY tough without a serial port!!! I just have no IDEA how to tell what is going on... I think I will use a pin for debugging and just bitbang to my oscilloscope to try to debug...

Ok I wrote a little function to bitbang a byte to a pin so I could see on oscilloscope... now I need to make one that can do multibyte so I can see whats going on with timer variables because I couldn't find any issues with any variable that is just a byte. The flags are working fine, though the output level goes to 255 every time it binds, so that tells me the timer isn't working properly for some reason.

Hi
Fair enough comment on the library. I guess that most people see attaching two different handlers to a single pin as being something that is not supported at the hardware level, if it works using the library, it maybe by a side effect rather than a design intention - its better not to build projects that rely on side effects.

I can appreciate that it must be very hard to do these things without serial, but testing a single channel version using the single ISR with change interrupts will simplify the task a little.

Duane B

DuaneB:
Hi
Fair enough comment on the library. I guess that most people see attaching two different handlers to a single pin as being something that is not supported at the hardware level, if it works using the library, it maybe by a side effect rather than a design intention - its better not to build projects that rely on side effects.

I can appreciate that it must be very hard to do these things without serial, but testing a single channel version using the single ISR with change interrupts will simplify the task a little.

Duane B

EXT interrupts are limited and do hardware RISING/FALLING detection and can only detect either one or the other at a time, but PC interrupts are triggered whenever any pin on the entire port change. All the library does when an interrupt is given is check if the pin state is high or low, and call the appropriate callback for RISING/FALLING.

It's certainly a timing issue at this point somewhere, because I did have it working perfectly with only a single in/out, but I wrote 90% of that code at work with no chip on me to test as I went along hehe. I ended up writing a little routine set to output bytes and ints to a digital pin which I can see with my oscope, I tested the single byte version this morning and it worked quite well to see some of the variables, so when I get home I can now check the counters and timers and see exactly what sort of measurement result on the width timing it is coming up with, it has to be higher then it should because it binds but assumes it is getting a full power signal reguardless of the pulse width.

Just for reference here are the few routines I came up with to see variables on scope, it's simple and basic and works hehe:

void debugPulse(int time) {
// A simple debugging routine to pulse pin 0 to debug variables using a scope
  digitalWrite(0,HIGH);
  delayMicroseconds(time);
  digitalWrite(0,LOW);
}
void debugPrintByte(byte bOut) {
// A simple function to pulse out a byte in bits, 50us width = 1, 10us width = 0
  if (bOut & 1) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 2) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 4) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 8) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 16) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 32) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 64) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 
  delayMicroseconds(5);
  if (bOut & 128) {
    debugPulse(50);
  } else {
    debugPulse(10);
  } 

}
void debugPrintInt(int iOut) {
  // This function will outpit all 16 bits of an int on pin 0
  static byte singleByte = 0;
  singleByte |= (255 & (iOut & 1));
  singleByte |= (255 & (iOut & 2));
  singleByte |= (255 & (iOut & 4));
  singleByte |= (255 & (iOut & 8));
  singleByte |= (255 & (iOut & 16));
  singleByte |= (255 & (iOut & 32));
  singleByte |= (255 & (iOut & 64));
  singleByte |= (255 & (iOut & 128));
  debugPrintByte(singleByte);
  delayMicroseconds(20);
 singleByte = iOut >> 8;
  debugPrintByte(singleByte);
}

Again, fair enough, as you say pin change interrupts work very differently on the tiny - I had a quick search and also turned up this reference to a serial debug option for the attiny which you might find useful -

Duane B

Yeah if I cant figure it out with my current method I will end up using the software serial, I also found a special ISP for the tiny (based on arduino ISP) that uses the MISO pin to relay serial via the arduino to serial which I may try.

Hi,
I have just had an idea of what your problem might be, when one channel falls, the other rises simultaneously - the logic of figuring out which pin is doing what inside the library might not be written for this situation of simultaneous pin changes.

Do you have a link to the source code of the library ?

Duane B

http://code.google.com/p/arduino-tiny/source/browse/trunk/libraries/PinChangeInterrupt/PinChangeInterrupt.cpp

From a quick look at the code, I can't see anything that would lead to one of the interrupts being dropped, but it sounds like this could be your problem. Off to the airport now, but this is what I would investigate.

Duane B

Success! After using my debugger code, which BTW had errors posted above, but I changed it to show working code. Basically it boiled down to variable decelerations! In one case an int was being put into a byte so that was screwing up that value, then in a few places I had unsigned int's being put into signed int's as well. After finding those bugs and fixing all works as well as should be, and BTW the timing is quite accurate, using the scope I can easily measure the width of the PWM outputs, and compared to the measurements my interrupts are measuring it is pretty much spot on and the action is quite smooth as can be seen in the video below.

Pictured below are screenshots from the scope showing the frameTime variable which is a calculation of the width of the pulse from the RX, normally 1000us is the lowest, and 2000us is the highest, the first is the throttle stick all the way down and reads 1116us which is within normal limits and indeed matches the width from the RX quite damn closely as measured with the scope. The second picture is with throttle stick at 50% and reads 1525us, and the final picture is with the throttle at 100% and the reading being 1927. all measurements are very consistent, I am quite happy now!

IMAGE007.BMP (150 KB)

IMAGE008.BMP (150 KB)

IMAGE009.BMP (150 KB)

I must say this project has been a hugely important learning experience for me, it really helped me grasp some deeper concepts I hadn't really wrapped myself around yet, I am also starting to realize just how much is possible if you think outside of the arduino core :).

This project is far from done, I still have code to put in to make it work as an I2C slave as well, this way it will support either PWM input OR I2C, main reason is because I am also building a custom 433MHz bidirectional radio system for this airplane and once that is complete it will have an I2C bus, this way I can give very precise commands and also read some data back from the controller, but until that is complete I want to still be able to fall back on standard RC receiver PWM. I also want to remove all use of delays in the code and use timers, it really isn't all that required per-say on this project, but I want my code to be as efficient as possible.

I will be sure to come back and post a video of the airplane working with all of the high powered lights once I have it installed on the plane :).

Basically it boiled down to variable decelerations!

It comes down to understanding what debugging is all about: trust nothing. You cannot presume the fault is with certain parts of your code and not others.

Chances are that whatever you think is working is actually causing you problems - that's why bugs are hard to find and often times firms hire people not involved in writing the code to debug it - to bring in a different perspective and an open mind.

Understanding that is the first step to debugging.

I am also starting to realize just how much is possible if you think outside of the arduino core

You can think of arduino as a dumbed down avr: to present an easy solution for people who don't want to or are unable to read the datasheet. Fortunately, it allows you also some flexibility in coding directly to the core (not as efficiently as otherwise) so you can grow out of it.