Problem with tacho

Hi There still a newbie at this I have found a sketch for a tacho i want to monitor a small engine I am using a magnetic pickup already fitted on the engine and this runs through a LM339 to convert the signal. the rpm is very erratic and will only register up to about 2000rpm I have tried it with serial monitor with the same affect Any help please?

This is the sketch

LiquidCrystal lcd(9, 8, 7, 6, 5, 4);

void setup(){ pinMode(PMS_PIN, INPUT); pinMode(LED_PIN, OUTPUT); lcd.begin(16, 2); }

void loop(){ t2 = millis(); if(t2 >= (t1 + 1000)){ rps = hits; hits = 0; t1=t2;

lcd.clear(); lcd.print("RPM: "); lcd.print(rps*60); }

if(digitalRead(PMS_PIN) == HIGH){ if(!counted){ counted = true; hits++; } } else { counted = false; }

digitalWrite(LED_PIN, digitalRead(PMS_PIN)); }

If you're going to post code, at least make sure it compiles: that code doesn't have an include for the LCd library, nor does it declare a couple of variables.

And stick it in code tags: the # above the ;) :sweat_smile: smilies.

I said i was a newbie I have been changing so many things i am loosing where i am

define PMS_PIN 2 // Pin for signal from Photomicrosensor

define LED_PIN 13 //Using Arduino's Internal LED; just as an indicator

boolean counted=false; int t1=0,t2=0; int hits=0; int rps=0; //********************************************************************************************

void setup() /----( SETUP: RUNS ONCE )----/ {

pinMode(PMS_PIN, INPUT); pinMode(LED_PIN, OUTPUT);

Serial.begin(9600); // Used to type in characters

}/--(end setup )---/

void loop() /----( LOOP: RUNS CONSTANTLY )----/ {

t2 = millis(); if(t2 >= (t1 + 1000)){ rps = hits; hits = 0; t1=t2;

}

if(digitalRead(PMS_PIN) == HIGH){ if(!counted){ counted = true; hits++; } } else { counted = false; }

//Write it out to serial port Serial.print(rps*60,DEC); Serial.println(" RPM "); delay(5);

//**************************************************

digitalWrite(LED_PIN, digitalRead(PMS_PIN)); }

//* --(end main loop )-- */

Code should be displayed within code tags - the # button so it looks like the following. Don't use excessive white space. Don't use very long comment lines as they make the code hard to see here.

I have made a few changes marked // change but I don't have the hardware to test the code

#define PMS_PIN 2 // Pin for signal from Photomicrosensor
#define LED_PIN 13 //Using Arduino's Internal LED; just as an indicator

boolean counted=false;
unsigned long t1=0;  // change - values for millis() MUST be unsigned long
unsigned long t2=0;
int hits=0;
int rps=0;
//********************************************


void setup()   /*----( SETUP: RUNS ONCE )----*/
{
  pinMode(PMS_PIN, INPUT);
  pinMode(LED_PIN, OUTPUT);
   
  Serial.begin(9600);  // Used to type in characters

}/*--(end setup )---*/

//======================
void loop()   /*----( LOOP: RUNS CONSTANTLY )----*/
{

  t2 = millis();
//  if(t2 >= (t1 + 1000)){
  if (t2 - t1 >= 1000) {    // change - correct way to use millis
    rps = hits;
    hits = 0;
    t1 = t1 + 1000;   // keeps better time
                                      // moved here so it prints every second
    Serial.print(rps);  // change -- keep simple until it works
    Serial.println("  RPM ");  // change
  } 

  if(digitalRead(PMS_PIN) == HIGH){
    if(!counted){
      counted = true;
      hits++;
    }
  } else {
    counted = false;
  }

    //Write it out to serial port
//    Serial.print(rps*60,DEC);  // change --- moved up
//    Serial.println("  RPM ");
//    delay(5);
   
   digitalWrite(LED_PIN, digitalRead(PMS_PIN)); // why ????
}
//* --(end main loop )-- */

...R

The correct way to deal with this is to STOP trying to sample the sensor using digitalRead() (polling) and use an interrupt!. This is just what the external interrupts are provided for!

Mark

As Holmes says, using an interrupt is the only way to go. Your current approach has many flaws. Such as Counting a HIGH pulse more than once. Simply because you read it many times before the sensor moved out of range. Missing HIGH pulses as your sketch is too busy updating the display to notice them. (LCD.clear() takes quite a long time at this time scale) The start of your sampling period is based on the start of the next second. (This will give inconsistent results as you should be starting your count from a predetermined position of the crank)

Using an interrupt service routine overcomes all of these issues. because: It only occurs on the rising (or falling) edge of the signal so no danger of counting the pulse more than once. It WILL get fired every time. Even in the middle of an update to the display You can use it to restart the counter at a consistent position of the crankshaft

Thanks for all the advise. Can you give me an example of code to use an Interupt

You can use an interrupt for this but why bother if it is not necessary. Interrupts cause all sorts of headaches for the unwary.

I think it would be better to explore and learn from the capabilities of the existing code first.

...R

Hi Robin2, This is great thanks so much One more question, The pickup i am using will measure 1 pulse per rev and the sketch is measuring pulses per second therefor i expect to multiply by 60 to read rev / minute but i only need to multiply by about 6 why is this so?

but i only need to multiply by about 6 why is this so?

You are probably counting each pulse about 10 times.

Your look for the signal being high, you then look again in a few microseconds and it's still high so gets counted again. Finally after about 10 cycles of the loop, the sensor has moved out of range.

KenF:

but i only need to multiply by about 6 why is this so?

You are probably counting each pulse about 10 times.

Your look for the signal being high, you then look again in a few microseconds and it's still high so gets counted again. Finally after about 10 cycles of the loop, the sensor has moved out of range.

But the "counted" flag is there to obviate that, if I read the code correctly.

should i try and fix this or will the count stay constant and i will calibrate as is?

if(digitalRead(PMS_PIN) == HIGH){ if(!counted){ counted = true; hits++; } } else { counted = false;

I cant see any other reference to counted What am i looking for or what do i need?

I cant see any other reference to counted What am i looking for or what do i need?

No worries. I missed one of your curley braces so it looked like that executed every time through the loop.

It's fine.

Looks to me- although I haven't walked through every line- that the idea there is to make sure "hits" only increments the first time it sees the sensor high.

  • Let's start with sensor low: if(digitalRead(PMS_PIN) == HIGH) fails.

  • Sensor goes high and so next time through loop() if(digitalRead(PMS_PIN) == HIGH) passes.

  • So in there, is counted true or false? Well if this is the first ever time the sensor was high, counted must be false from when it was initialised, so hits is incremented and counted is set true for next time

  • Let's say loop() runs faster than the sensor moves out of range, so we test the sensor again and if(digitalRead(PMS_PIN) == HIGH) is still good.

  • But this time, counted is true from the previous run thru loop() so hits is not incremented. That can happen many times for the same instance of the sensor being high.

  • Eventually (in computer terms....), the sensor is out of range and if(digitalRead(PMS_PIN) == HIGH) fails so counted is set false.

That looks ok to me, but as I say I haven't checked every line.

Are you positive there's only one pulse per revolution?

great explanation I am positive 1 pulse per rev I have worked out it needs to be multiplied by almost3.5 to make it accurate I am still confused why its out this much if the sketch is ok

stevedarman: great explanation I am positive 1 pulse per rev I have worked out it needs to be multiplied by almost3.5 to make it accurate I am still confused why its out this much if the sketch is ok

I think you need to walk through that code line-by-line from top to bottom, from the case where the sketch has fired up but not seen a high sensor yet, through where the sensor goes high (with loop() running either slower or faster than the sensor moves out of range), until the sensor goes off againe, following all the "ifs" and tabulate the values of all the variables.

Can you post your current version inside code tags please?

Good point: I’m going off Robin’s version in reply#3.

What figures are you getting? And what is the approximate RPM?

Possibly you are getting bounce on the reading. Debouncing could improve accuracy. In other words, once you get a reading, ignore further positive readings until a certain number of mS has elapsed. Enough to debounce but not so much that you might miss the next revolution.