Interrupts without delays?!?!?

I did not ask what they do. I asked why they are in those specific locations.

Not sure... This code was based on the code example for seeedstudio.

http://themakersworkbench.com/node/399

RobDrizzle:
Not sure...

Thank you for your honesty.

This code was based on the code example for seeedstudio. http://themakersworkbench.com/node/399

Ugh. Not good.

If you're interested, this will hopefully give you some knowledge regarding interrupts and critical sections...

On to your code... The NbTopsFan1 and NbTopsFan2 need to be protected. Everything else needs to run free. I suggest access functions. Let's start with something to set the value...

void SetNbTopsFan( volatile int& NbTopsFan, int NewValue )
{
  uint8_t SaveSREG;

  SaveSREG = SREG;
  cli();
  NbTopsFan = NewValue;
  SREG = SaveSREG;
}

Replace these two lines...

void loop()
{
//GPM Stuff!!!
NbTopsFan1 = 0; //Set NbTops to 0 ready for calculations
NbTopsFan2 = 0;

With these...

void loop()
{
//GPM Stuff!!!
SetNbTopsFan( NbTopsFan1, 0 ); //Set NbTops to 0 ready for calculations
SetNbTopsFan( NbTopsFan2, 0 );

Which leaves this bad boy...

Calc1 = (NbTopsFan1 + NbTopsFan2); //adding both sensor's pulses together

Another access function to protect the variable...

int GetNbTopsFan( volatile int& NbTopsFan )
{
  uint8_t SaveSREG;
  int rv;

  SaveSREG = SREG;
  cli();
  rv = NbTopsFan;
  SREG = SaveSREG;
  return( rv );
}

The line above is changed to this...

Calc1 = (GetNbTopsFan(NbTopsFan1) + GetNbTopsFan(NbTopsFan2)); //adding both sensor's pulses together

If there are any more sei or cli calls in your sketch, post the code.

Thank you... I'll give it a try. Your write up is a bit over my head... lol. I'm just getting started with arduinos and we never had C in college (instead we had fortran 78 shakes head

That was the only set.

I have to sit and study the load you just dropped on me... because this is some heavy...

Finally, the sei and cli I marked above need to be removed...

Good luck.

int GetNbTopsFan( volatile int& NbTopsFan )
{
uint8_t SaveSREG;
int rv;

SaveSREG = SREG;
cli();
rv = NbTopsFan;
SREG = SaveSREG;
return( rv );
}

Should the bold "int" be "void"?

No. The purpose of the function is to safely return the value of one of the NbTopsFan variables.

Doesn't seem to work... I'm kinda confused as to how this is counting the pulses for only a second, not to mention everything else. Is there something I can read on access functions? I searched and didn't get much. Well this is what I have thus far...

unsigned long gpm;
unsigned long pressure;
unsigned long battery;
unsigned long pastTime = 0;
unsigned long actualTime = 0;
unsigned long currentTime;
long interval = 1000;
volatile int NbTopsFan1;
volatile int NbTopsFan2;
int pulse;
int Calc2;
int Calc1;
int hallsensor1 = 2;
int hallsensor2 = 3;

void rpm1 ()
{
  NbTopsFan1++;
}

void rpm2 ()
{
  NbTopsFan2++;
}

void SetNbTopsFan( volatile int& NbTopsFan, int NewValue )
{
  uint8_t SaveSREG;

  SaveSREG = SREG;
  cli();
  NbTopsFan = NewValue;
  SREG = SaveSREG;
}

int GetNbTopsFan( volatile int& NbTopsFan )
{
  uint8_t SaveSREG;
  int rv;

  SaveSREG = SREG;
  cli();
  rv = NbTopsFan;
  SREG = SaveSREG;
  return( rv );
}

void setup()
{
  Serial.begin(9600);
  pinMode(menuswitchPin, INPUT);
  pinMode(selectswitchPin, INPUT);
  pinMode(logswitchPin, INPUT);
  pinMode(resetswitchPin, INPUT);
  pinMode(hallsensor1, INPUT); //initializes digital pin 2 as an input
  pinMode(hallsensor2, INPUT);
  attachInterrupt(0, rpm1, RISING); //and the interrupt is attached mube pin 2 
  attachInterrupt(1, rpm2, RISING); // must be pin 3
  menubuttonState = digitalRead(menuswitchPin);
  logbuttonState = digitalRead(logswitchPin);
}

void loop()
{
//GPM Stuff!!!

  SetNbTopsFan( NbTopsFan1, 0 );   //Set NbTops to 0 ready for calculations
  SetNbTopsFan( NbTopsFan2, 0 ); 
currentTime = millis();
  if ((currentTime - pastTime) >= interval){
  actualTime = (currentTime - pastTime); //actualTime used to pulse/second count
  pastTime = currentTime; //resetting pastTime for next pass
}
  Calc1 = (GetNbTopsFan(NbTopsFan1) + GetNbTopsFan(NbTopsFan2)); //adding both sensor's pulses together
  Calc2 = (actualTime / 1000); //1000 milliseconds per second
  pulse = (Calc1 / Calc2); // pulses/seconds
  gpm = (pulse / 26.0926); // 26.0926 pulses per GPM

The original code basically does this...

  1. Reset counters
  2. Wait for one second
  3. Calculate rate

Another way to look at the steps...

  1. After one second has elapsed, calculate the rate and reset the counters

Each pass through loop, you will need to check if one second has elapsed. Which you are conveniently already doing...

currentTime = millis();

if ((currentTime - pastTime) >= interval)
{
actualTime = (currentTime - pastTime); //actualTime used to pulse/second count

// At this point, one second has elapsed

pastTime = currentTime; //resetting pastTime for next pass
}

Note: For further research search for "blink without delay".

From the problem description above, you need to calculate the rate and reset the counters when one second has elapsed...

currentTime = millis();

if ((currentTime - pastTime) >= interval)
{
actualTime = (currentTime - pastTime); //actualTime used to pulse/second count

// Calculation and reset code goes here

pastTime = currentTime; //resetting pastTime for next pass
}

Which leads you to this...

currentTime = millis();

if ((currentTime - pastTime) >= interval)
{
actualTime = (currentTime - pastTime); //actualTime used to pulse/second count

// Calculate GPM and reset counters
Calc1 = (GetNbTopsFan(NbTopsFan1) + GetNbTopsFan(NbTopsFan2)); //adding both sensor's pulses together
Calc2 = (actualTime / 1000); //1000 milliseconds per second
pulse = (Calc1 / Calc2); // pulses/seconds
gpm = (pulse / 26.0926); // 26.0926 pulses per GPM

SetNbTopsFan( NbTopsFan1, 0 ); //Set NbTops to 0 ready for calculations
SetNbTopsFan( NbTopsFan2, 0 );

pastTime = currentTime; //resetting pastTime for next pass
}

Continued....

But, there is a serious (hidden) flaw in the code. Reading the counters and resetting the counters has to be combined into one atomic operation. So, we need to combine the two access functions into one...

int GetAndSetNbTopsFan( volatile int& NbTopsFan, int NewValue )
{
  uint8_t SaveSREG;
  int rv;

  // Save interrupt state and disable interrupts
  SaveSREG = SREG;
  cli();

  // The following cannot and will not be interrupted...

  // Save the current value so it can be returned
  rv = NbTopsFan;
  // Set the value
  NbTopsFan = NewValue;

  // Restore the interrupt state (enable interrupts)
  SREG = SaveSREG;

  // Return the saved value
  return( rv );
}

Combining the whole mess...

int GetAndSetNbTopsFan( volatile int& NbTopsFan, int NewValue )
{
  uint8_t SaveSREG;
  int rv;

  // Save interrupt state and disable interrupts
  SaveSREG = SREG;
  cli();

  // The following cannot and will not be interrupted...

  // Save the current value so it can be returned
  rv = NbTopsFan;
  // Set the value
  NbTopsFan = NewValue;

  // Restore the interrupt state (enable interrupts)
  SREG = SaveSREG;

  // Return the saved value
  return( rv );
}
  currentTime = millis();

  if ((currentTime - pastTime) >= interval)
  {
    actualTime = (currentTime - pastTime); //actualTime used to pulse/second count

    // Calculate GPM and reset counters
    Calc1 = (GetAndSetNbTopsFan(NbTopsFan1,0) + GetAndSetNbTopsFan(NbTopsFan2,0)); //adding both sensor's pulses together
    Calc2 = (actualTime / 1000); //1000 milliseconds per second
    pulse = (Calc1 / Calc2); // pulses/seconds
    gpm = (pulse / 26.0926); // 26.0926 pulses per GPM

    pastTime = currentTime; //resetting pastTime for next pass
  }

Questions?

Is there something I can read on access functions? I searched and didn't get much.

I don't know. I can't remember how that term came into my head.

Basically, it's a function that serves just one purpose: provide access to data. In your case, the function provides atomic (interrupts disabled) access to data that is shared with an interrupt service routine. You use a function so you don't have to keep retyping the same code (disable interrupts, save data, enable interrupts).

Thanks... I'll give it a shot when I get home from work. Thank you very much for the commented access function code. I'll have to study it later.

Thank you... Very much! It works great! For the first time in 8 months of working on this project I was able to accurately measure flow and pressure. Fantastic! Also, I have been studying what you did, It's slowly starting to make sense.

Excellent. Glad to have helped.

I have just found this topic and I had to log on to say thank you also. This finally works the way I need it to Thanks to your input here. Appreciate The effort. :slight_smile: