[Concluded] Interrupt - timing - conflict . problem

My 328p AVR had the problem of the below sketch freezing up seconds after I’d start sending radio signals to it (signals which come at 10 pulses per second).

I set up the mark(x) function to help track the problem down, and was surprised to discover adding a 10ms delay at the start of the loop() function solves my problem! (As happens in the sketch below.)

But not knowing WHY this solves the problem, I fear it may be a “flakey” way to fix it, that won’t hold up as I add more to the sketch.

I need to know what was happening – along with what the correct fix is – to avoid more problems further down the road of development.

My theory is that Serial, Mirf, SPI, and Wire are all using interrupts, and one of them was starting exicution before another had a chanse to complete, resulting in a major conflict. (But I don’t know.)

Anyone see the real problem and answer?

byte inAA[10];
byte outAA[10];
byte maxMove,highSkin,lowSkin;
byte beatCnt = 0;
unsigned long aaHeart[10];
byte aaCnt = 0;
byte markBeat = 0;
boolean beatRising = false;
unsigned long newBeatMill = 0;
unsigned long oldBeatMill = 0;
unsigned long gloveMill = 0;
byte glovePin = 2;
byte gloveB = false;

// *** RADIO ***
#include <SPI.h>
#include <Mirf.h>
#include <nRF24L01.h>
#include <MirfHardwareSpiDriver.h> // radio
#include <Wire.h>

void mark(byte x)
{
  Serial.println(x);
  if (x == 1)
    delay(10);
}

byte heartBeats()
{
  unsigned long total = 0;
  for (byte i=0; i<10; i++)
    total += aaHeart[i];
  return (600000 / total);
}

void sendMinute()
{
  mark(8);
  outAA[0] = maxMove; // max move
  maxMove = 0;
  outAA[1] = inAA[1]; // battery
  outAA[3] = inAA[3]; // heat
  outAA[4] = heartBeats(); // heart beats
  beatCnt = 0;
  outAA[5] = highSkin;
  highSkin = 0;
  outAA[6] = lowSkin;
  lowSkin = 255;
  Wire.write(outAA,10);
  mark(9);
}

void setup()
{
  digitalWrite(glovePin,LOW);
  pinMode(glovePin,OUTPUT);
  Wire.begin(8);
  Wire.onRequest(sendMinute);
  SPI.begin();
  Serial.begin(9600); // 115200
  delay(10);
  
  // *** RADIO ***
  Mirf.cePin = 9;
  Mirf.csnPin = 10;
  Mirf.spi = &MirfHardwareSpi;
  Mirf.init();
  Mirf.setRADDR((byte *)"clie1");
  Mirf.setTADDR((byte *)"serv1");
  Mirf.payload = 10;
  Mirf.config();
}

void loop()
{
  mark(1);
  if (Mirf.dataReady())
  {
    mark(2);
    if (!gloveB)
    {
      mark(3);
      gloveB = true;
      digitalWrite(glovePin,HIGH);
    }
    mark(4);
    gloveMill = millis() + 500;
    Mirf.getData((byte *) &inAA);
    maxMove = max(inAA[0],maxMove);
    highSkin = max(inAA[2],highSkin);
    lowSkin = min(inAA[2],lowSkin);
    mark(5);
    if (inAA[4] > (markBeat + 25))
      beatRising = true;
    else if (inAA[4] < (markBeat - 25))
    {
      if (beatRising)
      {
        mark(6);
        beatRising = false;
        if (aaCnt > 8)
          aaCnt = 0;
        else
          aaCnt++;
        newBeatMill = millis();
        aaHeart[aaCnt] = (newBeatMill - oldBeatMill);
        oldBeatMill = newBeatMill;
      }
    }
    mark(7);
    if (beatRising)
      markBeat = max(markBeat,inAA[4]);
    else
      markBeat = min(markBeat,inAA[4]);
  }
  if (gloveB && (gloveMill < millis()))
  {
    gloveB = false;
    digitalWrite(glovePin,LOW);
  }
}

Your code doesn’t compile.

aarg: Your code doesn't compile.

You're right! The letter "M" got added to the end of a line somehow, now removed.

I'm compiling it for the Uno, using Adruino version 1.0.6.

Thanks for giving it a try.

One thing, your use of millis() won't survive a rollover event. You don't know, or don't care?

Need to be powered up for 71+ days before seeing that.

The combination of terse variable names and no comments makes it very hard to follow. Especially without knowing anything about the hardware. Or what it does.

CrossRoads: Need to be powered up for 71+ days before seeing that.

49 days.

aarg: One thing, your use of millis() won't survive a rollover event. You don't know, or don't care?

CrossRoads: Need to be powered up for 71+ days before seeing that.

The Lithium Ion battery in the transmitter only lasts 12 hours before it needs recharging, so "Don't care" is probably the right answer. Also, most people won't be leaving the base unit on for 71+ days. If they do, I doubt the rollover will cause a noticeable distortion in the collected data.

Your debug method seems complicated enough to cause side effects, thus inconclusive results. Can you try to get rid of them and use a simple delay(10) at the spot where you call mark(1) now?

Oops, 49.71 days. Knew there was a 71 in there. (2^32)-1 / 1000 / 60 / 60 / 24 = 49.71

aarg, I grasp what you are saying about it being too hard to figure out without better documentation, variable names, and knowledge of additional hardware.

Therefore, I conclude it is not possible to answer this question, and would like to withdraw it.

I was hoping someone would spot a glaring error in my code, but that does not seem to be the nature of the beast.

Thank you for taking it seriously and giving it a try.

This is an international forum and many members are currently asleep. I would wait a while for other people to check in.

By the way, you never commented on my suggestion in reply #8.

Comment on suggestion in reply #8:

aarg: Your debug method seems complicated enough to cause side effects....Can you try ... a simple delay(10) at the spot where you call mark(1) now?

I've tried it without any debugging function, without Wire.write, without Serial.write. None of it had any sigificant effect on the problem.

Only adding that ten millisecond pause before starting each next loop(), gives whatever has fallen behind, a chance to catch up; and in doing so, solvesthe problem.

CosmickGold: Comment on suggestion in reply #8:

I've tried it without any debugging function, without Wire.write, without Serial.write. None of it had any sigificant effect on the problem.

Only adding that ten millisecond pause before starting each next loop(), gives whatever has fallen behind, a chance to catch up; and in doing so, solvesthe problem.

Yes, but in the way that you posted, or the way I suggested?

OK. I just now tested it exactly as you said in comment #8, editing out all reference to mark(x), and putting a simple delay(10); at the top of the loop. That worked equallly well, no problem.

aarg: This is an international forum and many members are currently asleep.

Like me. No, wait, I was eating my lunch. :)