4cylinder RPM code

Hi

i am using this code to count the Rpms of my old car

The problem is that it coounts right when the car is at idle but when i accelerate it shows wrong rpms (i am at 2500 RPMs and the display shows 330 rpm then 770rpm etc )

/* RPM counter  4 cylinder engine  with signal from brakepoints of distributor using an 4N25 and 

oled display  SSD1306_128X64  */


#include "U8glib.h"
U8GLIB_SSD1306_128X64 u8g(U8G_I2C_OPT_NO_ACK);  // Display which does not send AC

//------code for RPM----------------

unsigned int rpm;

// rpm reading
volatile byte pulses;

// number of pulses
unsigned long timeold;

// The number of pulses per revolution

unsigned int pulsesperturn = 4;
void counter()
{
    //Update count
    pulses++;
}


/////////////////////////////////////////////////////////////  void setup ////////////////////////////////////////////////

void setup()   {
    Serial.begin(9600);
   
    //-----------code for RPM---------------
    //Use statusPin to flash along with interrupts
    
  // pinMode(encoder_pin, INPUT);
  
    //Interrupt 1 is digital pin 3, so that is where the IR detector is connected
    
    attachInterrupt(1, counter, RISING);
    // Initialize
    pulses = 0;
    rpm = 0;
    timeold = 0;



 // assign default color value
  if (u8g.getMode() == U8G_MODE_R3G3B2)
  {
    u8g.setColorIndex(255);     // white
  }
  else if (u8g.getMode() == U8G_MODE_GRAY2BIT)
  {
    u8g.setColorIndex(3);         // max intensity
  }
  else if (u8g.getMode() == U8G_MODE_BW)
  {
    u8g.setColorIndex(1);         // pixel on
  }
  else if (u8g.getMode() == U8G_MODE_HICOLOR)
  {
    u8g.setHiColorByRGB(255, 255, 255);
  }


    
}


void loop()
{
  

 u8g.firstPage();  
  do {
    Rpms();   //mpainei to onoma tou void me to display
  } while( u8g.nextPage() );   
  




 if (millis() - timeold >= 1000){
        //Uptade every one second, this will be equal to reading frecuency (Hz)//
        //Don't process interrupts during calculations
        detachInterrupt(1);
        //Note that this would be 60*1000/(millis() - timeold)*pulses if the interrupt
        //happened once per revolution
        rpm = (60 * 1000 / pulsesperturn )/ (millis() - timeold)* pulses;
        timeold = millis();
        pulses = 0;
        //Write it out to serial port
        Serial.print("RPM = ");
        Serial.println(rpm,DEC);
        //updateDisplay(rpm);
        //Restart the interrupt processing
        attachInterrupt(1, counter, RISING);
    }


    
}



void Rpms(void) { 
  u8g.setFont(u8g_font_fub11);
  u8g.setFontRefHeightExtendedText();
  u8g.setDefaultForegroundColor();
  u8g.setFontPosTop();
  u8g.drawStr(4, 20, "RPM");
  u8g.setPrintPos(70, 20); 
  u8g.print(rpm); 
  

}

i am using an arduino pro mini 328p 5v 16Mhz

is something wrong with the code? any mentions ?

i am planning to use also a small oscillator to see the output of the 4N25 if it works right in high revolutions

thanks in advance

// rpm reading
volatile byte pulses;

Wrong!

// number of pulses
unsigned long timeold;

Wrong.

is something wrong with the code?

There is way too much of it. Your problem has nothing, apparently, to do with the u8g thing. Get rid of all the code that deals with the u8g thing to confirm, or refute, that.

        rpm = (60 * 1000 / pulsesperturn )/ (millis() - timeold)* pulses;

60 is an int. 1000 is an int. So, int registers will be used.

Will 60 * 1000 fit in an int register?

How many pulses will you miss when you have the interrupt handler detached while you (mis)calculate rpm and print to the serial port?

I would use an Arduino Due which has a library for this type of application https://github.com/antodom/tc_lib

Try something like this:

/* RPM counter  4 cylinder engine  with signal from brakepoints of distributor using an 4N25 and
  oled display  SSD1306_128X64  */


#include "U8glib.h"
U8GLIB_SSD1306_128X64 u8g(U8G_I2C_OPT_NO_ACK);


unsigned int RPM;
volatile unsigned long pulseCount;
unsigned long PreviousMicros;


// The number of pulses per revolution


const unsigned int pulsesperturn = 4;


void counterISR() {
  //Update count
  pulseCount++;
}




/////////////////////////////////////////////////////////////  void setup ////////////////////////////////////////////////


void setup()   {
  Serial.begin(9600);


  // Initialize
  pulses = 0;
  rpm = 0;
  PreviousMicros = 0;


  //-----------code for RPM---------------
  //Use statusPin to flash along with interrupts


  // pinMode(encoder_pin, INPUT);


  //Interrupt 1 is digital pin 3, so that is where the IR detector is connected
  attachInterrupt(1, counterISR, RISING);


  // assign default color value
  if (u8g.getMode() == U8G_MODE_R3G3B2) {
    u8g.setColorIndex(255);     // white
  }
  else if (u8g.getMode() == U8G_MODE_GRAY2BIT) {
    u8g.setColorIndex(3);         // max intensity
  }
  else if (u8g.getMode() == U8G_MODE_BW)
  {
    u8g.setColorIndex(1);         // pixel on
  }
  else if (u8g.getMode() == U8G_MODE_HICOLOR)
  {
    u8g.setHiColorByRGB(255, 255, 255);
  }
}


void loop() {
  u8g.firstPage();


  do {
    Rpms();   //mpainei to onoma tou void me to display
  } while ( u8g.nextPage() );



  unsigned long curentTime = micros();
  unsigned long interval = currentMicros - PreviousMicros;
  
  // Update once per second

  if (interval >= 1000000UL) {
    PreviousMicros = currentMicros;


    // Grab the volatile data and reset the count
    unsigned long count;
    noInterrupts();
    count = pulseCount;
    pulseCount = 0;
    interrupts();


    unsigned long microsPerRev = interval * 4UL / count;
    const unsigned long microsPerMinute = 60UL * 1000UL;
    RPM = microsPerMinute / microsPerRev;


    Serial.print("RPM = ");
    Serial.println(RPM, DEC);
    //updateDisplay(rpm);
    //Restart the interrupt processing
  }
}

void Rpms(void) {
  u8g.setFont(u8g_font_fub11);
  u8g.setFontRefHeightExtendedText();
  u8g.setDefaultForegroundColor();
  u8g.setFontPosTop();
  u8g.drawStr(4, 20, "RPM");
  u8g.setPrintPos(70, 20);
  u8g.print(RPM);
}