RPM counter spiking when LED code added in or slow RPMs.

Hello,

I have been working on this project for a while and have been searching the boards for this problem. Here is what I'm trying to do. I do have to say that I am still not great with code.

I have a hand crank connected to a shaft. I'm using a photo diode and LED to read 4 stripes on the shaft as it rotates. This will be operated by hand so the RPMs will be low. I used 4 stripes to try an get a bit better response with the low RPMs.

For hardware I'm using a ATmega328 Adafruit Pro Trinket 5v which they say is equivalent to a Pro Mini. For the LEDs I'm using the dotstar product which is a 4 wire LED with separate clock and data. To control those LEDs I'm using the FastLED library which supposedly handles interrupts ok.

Here is the problem I'm having:

When I comment out the LED calls the RPM value is stable and pretty much stays within range. Usually staying under 300. When I add the LED back in the RPM value climes way up like it's just adding on top of itself and climes up to the thousands then overflows and resets. I put the code between the interrupt off and on so was hoping that would help. The LED works as desired and don't flicker at all.

Here is the code. Any help would be greatly appreciated.

Thank you.

#include <Wire.h> // Enable this line if using Arduino Uno, Mega, etc.
#include <Adafruit_GFX.h>
#include "Adafruit_LEDBackpack.h"
#include <SPI.h>
#include "FastLED.h"

//Pin Info for 7 Segment LED readout
#define DATA_PIN 6
#define CLOCK_PIN 8
Adafruit_7segment matrix = Adafruit_7segment();

//Breaks LED string into 3 sections
#define NUM_LEDS_1 9
#define NUM_LEDS_2 26
#define NUM_LEDS_3 9
#define NUM_LEDS_ALL (NUM_LEDS_1 + NUM_LEDS_2 + NUM_LEDS_3)
CRGB ledsAll[NUM_LEDS_ALL];
CRGB* leds1 = ledsAll;
CRGB* leds2 = ledsAll + NUM_LEDS_1;
CRGB* leds3 = ledsAll + NUM_LEDS_1 + NUM_LEDS_2;

//Interrupt pin for photo diode
#define interruptPin 3 

volatile int rpmcount = 0;//see http://arduino.cc/en/Reference/Volatile
int rpm = 0;
unsigned long lastmillis = 0;

void setup(){
 Serial.begin(19200); 
 matrix.begin(0x70); //Start LED

 FastLED.addLeds<DOTSTAR, DATA_PIN, CLOCK_PIN, RGB>(ledsAll, NUM_LEDS_ALL);
 
 pinMode(interruptPin, INPUT_PULLUP);
 attachInterrupt(digitalPinToInterrupt(interruptPin), rpm_fan, FALLING);
}

void loop(){
 
  
 if (millis() - lastmillis >= 1000){  /*Uptade every one second, this will be equal to reading frecuency (Hz).*/
 
 detachInterrupt(0);    //Disable interrupt when calculating
 rpm = rpmcount * 15;   // 15 because of 4 pulses per rotation
 
 rpm = constrain (rpm, 5, 255); 

//if I remove section below RPM works fine
 matrix.println(rpm);
 matrix.writeDisplay(); //Right out RPM to led
 bolt(); //Fade up middle LED Section to full
 bolt1(); //if RPM higher than 200 light up
 bolt3(); //if RPM higher than 150 light up
//if I remove section above RPM works fine
 
 rpmcount = 0; // Restart the RPM counter
 lastmillis = millis(); // Uptade lasmillis
      
  attachInterrupt(0, rpm_fan, FALLING); //enable interrupt
  }
}


void rpm_fan(){ /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  rpmcount++;
}
void bolt(){ 
  int rpm2 = rpm;
  rpm2 = map (rpm2, 0, 200, 0, 255);
  rpm2 = constrain (rpm2, 5, 255);
  fill_solid( leds2, NUM_LEDS_2, CRGB(rpm2,rpm2,rpm2));
  FastLED.show();
 
}
void bolt3(){
  int rpm3 = rpm;
  if (rpm3 >= 150)
      {
       fill_solid( leds3, NUM_LEDS_3, CRGB(255,255,255));
      }
    else
      {
       fill_solid( leds3, NUM_LEDS_3, CRGB(0,0,0));
      }
    FastLED.show();
}
void bolt1(){ 
  int rpm1 = rpm;
  if (rpm1 >= 200)
      {
       fill_solid( leds1, NUM_LEDS_1, CRGB(255,255,255));
      }
    else
      {
       fill_solid( leds1, NUM_LEDS_1, CRGB(0,0,0));
      } 
}

Interrupt 0 is not on pin 3 on a ATmega328p.

For some reason they ported it to pin 3 on this hardware.

I'm guessing a bit here, but it sounds like the your interrupt routine is being called when you don't think it is.

Instead of trying to control if the interrupt is enabled or not, you can add a flag which enables and disables the actual counting e.g.

volatile int rpmcount = 0;//see http://arduino.cc/en/Reference/Volatile
volatile byte rpm_enable;  // <-- add this

void enable_rpm(){
  noInterrupts (); 
  rpm_enable= 1;
  interrupts ();
}

void disable_rpm(){
  noInterrupts (); 
  rpm_enable= 0;
  interrupts ();
}


void rpm_fan(){ /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  if( rpm_enable ){
    rpmcount++;
  }
}

Then in the main loop(), replace your attach/detach interrupt with enable_rpm(), disable_rpm()

Yours,
TonyWilk

For some reason they ported it to pin 3 on this hardware.
Pinouts | Introducing Pro Trinket | Adafruit Learning System

On that pinout pin 3 is labeled INT1 not INT0. So it's interrupt 1 not interrupt 0.

Thanks Tony.

That didn't seem to help though. It's strange if I just use it to readout the RPMs to the LED it's smooth an clean. Maybe it has something to do with the FastLED library.

mrrandom:
Thanks Tony.

That didn't seem to help though. It's strange if I just use it to readout the RPMs to the LED it's smooth an clean. Maybe it has something to do with the FastLED library.

So you need to figure out if it's some strange interrupt thing going on or the way you are using the RPM value to update your LEDS.

Since the main bit of your loop runs once per second, try this:

// rpm = rpmcount * 15;   // 15 because of 4 pulses per rotation COMMENTED OUT

rpm= rpm+1;   // just increment rpm once per second as a test

rpm = constrain (rpm, 5, 255);

First time thru the loop 'rpm' will get set to 5 (because you have the constrain()), then it sould just add 1 each time... nothing to do with the interrupt.

See if that does what you expect.

Yours,
TonyWilk

P.S. oh, and post all of your current code now you changed it some :slight_smile:

Instead of counting up the number of pulses Lets try calculating the time between pulses. Let me know if this code works?

#define ClockPin 3 // Must be pin 2 or 3
//#define DataPin 9 // can be any other pin

      // My Encoder has 400 Clock pulses per revolution
      // note that 150000.0 = (60 seonds * 1000000 microseconds)microseconds in a minute / 400 pulses in 1 revolution)
      // change the math to get the proper multiplier for RPM for your encoder
     // yours has 4 pulses in 1 revolution
      // note that 15000000.0 = (60 seonds * 1000000 microseconds)microseconds in a minute / 4 pulses in 1 revolution)
#define Multiplier 15000000.0 // don't forget a decimal place to make this number a floating point number
volatile long count = 0;
volatile long EncoderCounter = 0;
volatile float SpeedInRPM = 0;

void onPin3CHANGECallBackFunction(){ 
    static uint32_t lTime; // Saved Last Time of Last Pulse
    uint32_t cTime; // Current Time
    cTime = micros(); // Store the time for RPM Calculations
    int32_t dTime; // Delt in time
/*
// Encoder Code
    bool DataPinVal = digitalRead(DataPin);
// We know pin 3 just went high to trigger the interrupt
// depending on direction the data pin will either be high or low
    EncoderCounter += (DataPinVal) ? 1 : -1; // Should we step up or down?
// End Encoder Code
*/
// calculate the DeltaT between pulses
    dTime = cTime - lTime; 
    lTime = cTime;
//    SpeedInRPM = Multiplier / ((DataPinVal) ? dTime: (-1 * dTime)); // Calculate the RPM Switch DeltaT to either positive or negative to represent Forward or reverse RPM
    SpeedInRPM = Multiplier / dTime; // Calculate the RPM Switch DeltaT to either positive or negative to represent Forward or reverse RPM
}


void setup() {
  Serial.begin(115200); //115200
  // put your setup code here, to run once:
  pinMode(ClockPin, INPUT);  
//  pinMode(DataPin, INPUT);
  attachInterrupt(digitalPinToInterrupt(ClockPin),onPin3CHANGECallBackFunction,RISING);
}

void loop() {
//  long Counter;
  float Speed;
  noInterrupts (); 
// Because when the interrupt occurs the EncoderCounter and SpeedInRPM could be interrupted while they 
// are being used we need to say hold for a split second while we copy these values down. This doesn't keep the 
// interrupt from occurring it just slightly delays it while we maneuver values.
// if we don't do this we could be interrupted in the middle of copying a value and the result get a corrupted value.
//  Counter = EncoderCounter;
  Speed = SpeedInRPM;
  interrupts ();

// use the speed and counter values for whatever you need to do.

  static unsigned long SpamTimer;
  if ( (unsigned long)(millis() - SpamTimer) >= (100)) {
    SpamTimer = millis();
 //   Serial.print(Counter );
 //   Serial.print("\t");
    Serial.print(Speed , 3);
    Serial.print(" RPM");
    Serial.println();
    SpeedInRPM = 0; // if no pulses occure in the next 100 miliseconds then we must assume that the motor has stopped
  }
}

This method may provide a way to skip readings then come back and check the RPM every so often without having to keep track of the number of pulses.
Z

zhomeslice:
Instead of counting up the number of pulses Lets try calculating the time between pulses. Let me know if this code works?
snip

Woa... the OP got his RPM reading to work (sorta) already...

When I comment out the LED calls the RPM value is stable and pretty much stays within range. Usually staying under 300.

Yes, counting a small number of pulses over a second might be skewed by taking time out of every second to update a matrix display, but dumping their code for something else is hardly a learning experience is it?

Could be that the display routines they are using messes with interrupts for 100mS or so? Could be something else... but we're debugging !

In the end timing spaces (and averaging them - it is hand-cranked) may give more accurate results, or simply having a lot more stripes on the encoder wheel.

Yours,
TonyWilk

Woa... the OP got his RPM reading to work (sorta) already

I understand The OP area has it (sorta) working. I'm concerned that his method needs a way to quickly handle rpm on the pulse level rather than the time / counts level.
OP:

When I comment out the LED calls the RPM value is stable and pretty much stays within range. Usually staying under 300. When I add the LED back in the RPM value climes way up like it's just adding on top of itself and climes up to the thousands then overflows and resets. I put the code between the interrupt off and on so was hoping that would help. The LED works as desired and don't flicker at all.

Sounds like a faster way to calculate rpm would help. The chance I propose would have the RPM calculated at the time of each pulse occurrence. No more counting which is causing OP's issue as I see it.
Z

zhomeslice:
Instead of counting up the number of pulses Lets try calculating the time between pulses.

I agree that this is a better approach.

I am using a QRE1113 reflective optical detector to detect a blob of white paint on a rotating disc on a small DC motor. This is the code in my ISR

void revDetectorISR() {
	isrRevMicros = micros();
	isrRevCount ++;
	newRevMicros = true;
}

and in loop() I have this code

if (newRevMicros == true) {
    noInterrupts();
        latestIsrMicros = isrRevMicros;
        revCount = isrRevCount;
        newRevMicros = false;
    interrupts();
}
revMicros = latestIsrMicros - prevIsrMicros;

With a slow moving shaft you might get some "bounce" and one way to deal with that is to use the detector ISR to turn itself off and turn a HardwareTimer on. When the HardwareTimer triggers an interrupt that ISR will turn the detector ISR back on. The Hardware Timer interval should be a long as possible without the risk of missing a legitimate pulse.

...R

Thanks for all the awesome help! It ended up being a really stupid mistake on my part. I defined my interrupt as pin 3 but called on 0 interrupt so the interrupt was never on. I guess it kinda worked because I did attach it correctly in setup.

Sorry to waste your time. I do like some of those ideas though and will play with them as they might be a bit more responsive. Everything updates now by the second and it makes the leds a bit jumpy up and down. Trying to figure out if I can smooth between values.

Thanks again!

#define interruptPin 3 

attachInterrupt(digitalPinToInterrupt(interruptPin), rpm_fan, FALLING);


detachInterrupt(0);    //Should be detachInterrupt(interruptPin);

attachInterrupt(0, rpm_fan, FALLING); //Should be attachInterrupt(interruptPin, rpm_fan, FALLING);