Interrupt problem counting pulses

Hi all,

Today I started using interrupts and learning about them. I use a small anemometer to play around with.
I have been reading about interrupts and using them for this purpose. I made a small sketch...Small problem, after a while the Serial port "hangs" and does not update. I have seen that no delay() or Serial.println should be used in an ISR. So I put them in my void loop () (see code below). Is there anyone that can explain me where the problem might be? All feedback is welcome!

/*
   This code counts the amount of pulses from an anemometer
   which has 3 pulses per rotation. With thise pulses an average windspeed can be calculated

*/

volatile byte WindCount;
float WindSpeedMs;
float WindSpeedKmH;
float Rotations;
float Distance;
int RotationsPerSecond;
unsigned long LastMillis = 0;


void setup() {// put your setup code here, to run once:
  Serial.begin (9600);//open serial monitor
  attachInterrupt  (digitalPinToInterrupt(2), PulseCount, RISING);//attach interrupt anemometer to pin 2

}

void loop() {// put your main code here, to run repeatedly:

  if (millis() - LastMillis == 5000) { //when 5 seconds passed
    detachInterrupt (digitalPinToInterrupt(2)); //detach intterupt
    Rotations = WindCount / 2.0; //count rotations and devide by 2
    Distance = Rotations * 0.4398; // 1 rotation is about 44 cm
    WindSpeedMs = Distance / 5; // distance divided by 5 seconds is speed in m/s
    WindSpeedKmH = WindSpeedMs * 3.6; //from meters per second to kmh
    Serial.println("Rotations");//print values on serial bus
    Serial.println(Rotations);
    Serial.println("Distance");
    Serial.println(Distance);
    Serial.println("Windspeed");
    Serial.println(WindSpeedKmH);
    WindCount = 0;//reset values
    LastMillis = millis();//reset LastMillis
    attachInterrupt(digitalPinToInterrupt(2), PulseCount, RISING);//attach the interrupt

  }

}

void PulseCount() {
  WindCount++;
}
    detachInterrupt (digitalPinToInterrupt(2)); //detach intterupt

Detaching, and re-attaching the interrupt handler is not usually done. Just disable interrupts, copy the relevant data, enable interrupts, and then perform the calculations.

You have too many global variables. The scope of variables should be as small as possible.

You should briefly disable interrupts and take a copy of the WindCount and set it to zero - like this

noInterrupts();
   copyOfWindCount = windCount;
   windCount = 0;
interrupts();
Rotations = copyOfWindCount / 2.0;

and then you don't need to detach and attach the interrupt

...R

Hi all! Thank you all for the quick replies! The serial monitor keeps working now. I am generally quite patient but 49 days is a bit too much.
About the Interrupt()/noInterrupt function I need to reed a bit more since I can't seem to find out the distance between the 2...

rmvansomeren:
About the Interrupt()/noInterrupt function I need to reed a bit more since I can't seem to find out the distance between the 2...

What do you mean by "the distance between the 2" ?

If you mean the time that interrupts are disabled, then it should be a short as possible.

...R

Robin2:
What do you mean by "the distance between the 2" ?

OPs autocorreckts me thinks? distance = difference?

noInterrupts();
Temporarily stops interrupts from happening. Allows you to read/write variables shared with the interrupt code without fear of having them modified part way through by an interrupt.

interrupts();
Re-enables normal interrupt handling.

As Robin2 says the amount of code between noInterrupts() and interrupts() should be as small as possible.
Usually it is just: read/write a variable (or two).
Things to absolutely avoid: delay(), loops, floating point maths. Do these things after taking a copy of the shared variable(s) you need.

Yes, sorry! I meant difference. I basically copy the value of WindCount and then I reset it to 0 in the new code. Now I run into another issue. And that is the fact that probably I need to debounce the reed switch of the anometer because I sometimes read values that do not make sense...

rmvansomeren:
And that is the fact that probably I need to debounce the reed switch of the anometer because I sometimes read values that do not make sense...

You need to tell us what is the expected shortest interval between legitimate pulses.

Your interrupt code probably needs to be like this

void PulseCount() {
  if (millis() - previousPulseTime >= debounceInterval) {
     WindCount++;
     previousPulseTime = millis();
   }
}

The variable previousPulseTime should be volatile unsigned long and the debounceInterval should be roughly 50% of the shortest time between legitimate pulses.

...R

I see your solution. Thing is I am using an anemometer. So the debounce in this case should happen in the ISR right? Which probably means a hardware solution would be better? I am just thinking out loud... I read in the datasheet that for the anemometer 1 revolution/seconds equals to 2.4km/h. Which means a lot of pulses per second.

Which means a lot of pulses per second.

Not really a lot compared with many encoders. That means you have about 240 pps at 240km/h wind speed.

Yes, you normally need to debounce in the ISR, if it's just a reed switch. You can reduce the ISR overhead by conditioning the signal first, with an RC filter and/or comparator with hysteresis. If you do a really good job of signal conditioning, you don't need to debounce at all.

At 240 pps the interval between pulses is about 4000 microsecs which is a long time for an Arduino. In the code I suggested in Reply #8 I would set the debounceInterval at 2 millisecs or 3 millisecs and see what happens.

You could add hardware debouncing but it may not be necessary.

...R

Thank you for your replies. I will try to apply it this evening and see how it works! In a later stage of my project I need to apply way more code so I need to keep the ISR as short as possible.

I tried the suggested debounce code and I understand what it is doing. But my readings now do not match at all...eventhough I turn the anemometer for around 10 times in 5 seconds I get a maximum of 1 rotation. I am using a debounce interval of 5ms in a volatile variable...I have no clue about the pulse time though since I do not have a datasheet of the anemometer

rmvansomeren:
I tried the suggested debounce code and I understand what it is doing.

Please post the actual code you are trying so we can see exactly what you have done.

...R

Agree, post your actual code.

Also Arduino has built in hardware for capturing either pulse counts or durations. It will be more accurate than using millis(), but won't necessarily solve your de-bounce problem (if indeed you have one).

Ideally you'd also be using an oscilloscope to see what kind of signal you are dealing with, rather than trying to find out what works by guesswork and experiment.

Apologies. See the code here:

/*
   This code counts the amount of pulses from an anemometer
   which has 3 pulses per rotation. With thise pulses an average windspeed can be calculated

*/

volatile byte WindCount;
int CopyOfWindCount;
float WindSpeedMs;
float WindSpeedKmH;
float Rotations;
float Distance;
int RotationsPerSecond;
unsigned long LastMillis = 0;
volatile unsigned long debounceInterval = 5;
volatile unsigned long previousPulseTime;


void setup() {// put your setup code here, to run once:
  Serial.begin (9600);//open serial monitor
  attachInterrupt  (digitalPinToInterrupt(2), PulseCount, RISING);//attach interrupt anemometer to pin 2

}

void loop() {// put your main code here, to run repeatedly:

  if (millis() - LastMillis >= 5000) { //when 5 seconds passed
    noInterrupts(); //detach intterupt
    CopyOfWindCount = WindCount;
    WindCount = 0;//reset values
    interrupts();
    Rotations = CopyOfWindCount / 2.0; //count rotations and devide by 2
    Distance = Rotations * 0.4398; // 1 rotation is about 44 cm
    WindSpeedMs = Distance / 5; // distance divided by 5 seconds is speed in m/s
    WindSpeedKmH = WindSpeedMs * 3.6; //from meters per second to kmh
    Serial.println("Rotations");//print values on serial bus
    Serial.println(Rotations);
    Serial.println("Distance");
    Serial.println(Distance);
    Serial.println("Windspeed");
    Serial.println(WindSpeedKmH);
    LastMillis = millis();//reset LastMillis
   // attachInterrupt(digitalPinToInterrupt(2), PulseCount, RISING);//attach the interrupt

  }

}

void PulseCount() {
  if(millis() - previousPulseTime >= debounceInterval){
    WindCount++;
    previousPulseTime = millis();
  }
}

Unfortunatly I do not have a scope

Use your PC sound card: Poor man's oscilliscope

Less jitter and more efficient (only 1 call to millis()) if you do it this way...

void PulseCount() {
  if(millis() - previousPulseTime >= debounceInterval){
    WindCount++;
    previousPulseTime += debounceInterval;
  }
}

Also applies to the timing in your main loop. Although I can't see it making a difference in this case.

You can also use the Arduino hardware to count pulses. But you can't debounce in software in this case. This is for an Uno...

#define DELAY_TIME 5000
uint32_t delayStartTime;
uint16_t lastCount;

void setup()
{
  // Configure T1 to count pulses on pin 5
  pinMode(5, INPUT);
  TCCR1A = 0x00;
  TCCR1B = 0x07;
  TCNT1 = 0;

  // Start debug output
  Serial.begin(9600);
  while (!Serial);
  Serial.println("T1 Pulse Counter");

  // Start delay between sending CAN packets
  delayStartTime = millis();
}


void loop()
{
  // Every DELAY_TIME interval...
  if (millis() - delayStartTime >= DELAY_TIME)
  {
    // Get pulse count from hardware
    uint16_t currentCount = TCNT1;
    uint16_t pulses = currentCount - lastCount;
    lastCount = currentCount;

    Serial.println(pulses);

    // Time to next delay
    delayStartTime += DELAY_TIME;
  }
}

rmvansomeren:
.I have no clue about the pulse time though since I do not have a datasheet of the anemometer

How many pulses does it produce per revolution - I have been assuming it is only 1

If it produces 1 pulse per rev and you rotate it 10 times in 5 seconds that should produce a pulse every 500 millisecs. At that very slow speed the debounce interval may need to be 50 rather than 5

I suggest you change this

volatile byte WindCount;

to this

volatile unsigned long WindCount;

and also change CopyOfWindCount to unsigned long

It may be that there are more than 255 pulses and your byte value is overflowing and rolling over back to 0

...R

volatile byte WindCount;
int CopyOfWindCount;

Why are the real counter and the copy different types?

PaulS:

volatile byte WindCount;

int CopyOfWindCount;



Why are the real counter and the copy different types?

Yes sorry, stupid mistake. I changed it. Eventhough it does not change that much.

@Robin2 I can hear the anemometer switch 2 times per revolution. If I look on the serial monitor the numbers really do not make sense. When I keep turning it sometimes prints 0. At other times It spikes up to 35 revolutions...