Briggs and Stratton Digital Tachometer

Hello All,

I am an Electrical Engineering undergraduate from Southern Illinois University. I am a part of the SAE Baja team and we are using a 10hp motor to power our buggy. I am currently trying to use a signal from the engine to create a digital readout through an LCD using an Arduino Uno. Attached you can see my signal conditioning. When an oscilloscope is hooked to the emitter I receive a reading of 60Hz which is right on with about 1,600 RPM. When the Arduino reads out I get nothing close to this. I am new to coding and the Arduinos so I'm sure there is an error somewhere. I sometimes get hectic readings from the arduino. Also, when my AvgRPM = 13 I am getting ActRPM reading of 60 which is not correct with the process I have set up to find ActRPM. I hope that you can help steer me in the right direction.

volatile byte rpmcount;
volatile byte AvgRPM = 0;
volatile byte ActRPM;
int i=0;
int RPMCount0=0;
int RPMCount1=0;
int RPMCount2=0;
int RPMCount3=0;
int RPMCount4=0;


void rpm_count()
{
  rpmcount++;
}

void setup()
{
  Serial.begin(4800);
  attachInterrupt(0, rpm_count, RISING);
}

void loop()
{
  i = 0;
  
  for (i; i < 5; i++) {
    rpmcount = 0;
    delay(100);
    switch(i) {
      case 0:
        RPMCount0 = rpmcount;
        break;
       case 1:
         RPMCount1 = rpmcount;
        break;
        case 2:
         RPMCount2 = rpmcount;
        break;
        case 3:
         RPMCount3 = rpmcount;
        break;
        case 4:
         RPMCount4 = rpmcount;
        break;
    }
  }
  
    AvgRPM = (RPMCount0 + RPMCount1 + RPMCount2 + RPMCount3 + RPMCount4) / 5;
    ActRPM = AvgRPM * 10 * 60 / 2; // Multiply AvgRPM * 2 (1/10 second case loop = .5 seconds multiply by 2 for 1 second)
                               // * 60 (60 seconds) / 2 (2 sparks for every 4 cycles)
    Serial.print(RPMCount0);
        Serial.print(',');
    Serial.print(RPMCount1);
        Serial.print(',');
    Serial.print(RPMCount2);
        Serial.print(',');
    Serial.print(RPMCount3);
        Serial.print(',');
    Serial.print(RPMCount4);
        Serial.print(','); 
    Serial.print(AvgRPM);
    Serial.print(" RPM = ");
    Serial.print(ActRPM);
    Serial.print('\n');
  }

Thanks,
Alex Watson

SignalConditioning.jpg

    delay(100);

You are going to miss a lot of counts unless you have under 10 Hz, which you don't.

The symptoms suggest your input is not triggering your interrupt when you expect it to. Maybe you need to do some external signal conditioning before you can trigger a digital input from this.

I am, not sure why you are counting pulses over five small intervals and averaging them. Is it to avoid number overflow in rpmcount? If so, you may find that with a signed byte you still get overflow at quite low rpms. You may need to reduce the sample interval even further to avoid that if you plan to take the engine over about 2000 rpm. When your delay() interval gets small compared to the time taken to complete the loop, you may find that the overall loop period is significantly different to the nominal delay period. It's best to use the timing technique demonstrated in the 'blink without delay' example sketch to get an accurate loop period.

Otherwise, the basic logic looks as if it should produce a value for AvgRPM that matches the number of samples in the loop period.

What you do with AvgRPM to calculate ActRPM looks a bit dodgy to me. Remember that the AvgRPM you have calculated is the average number of pulses in 100 ms (ish), not in 0.5 seconds. Your comment implies a 4-stroke cycle so the engine turns twice per input pulse.

So the calculation should probably be:

ActRPM = AvgRPM * 10 * 60 * 2; // *10 converts pulse count to Hz, *60 converts to pulses per minute, *2 converts to revs per minute

rather than:

ActRPM = AvgRPM * 10 * 60 / 2;

If you are going to keep the averaging approach, RPMCount0 .. RPMCount4 are crying out to be put into an array.

The name AvgRPM is misleading since it is storing the average count per sample interval not revs per minute.

I think you could simplify this a fair bit by just accumulating pulse counts in a local variable until a long enough time has elapsed, for example a second. Then multiply the total up to get revs per minute and you are done. No need to buffer and average them. That would only be needed if you intended to produce a time average to smooth out variations over a longer period, but there's no point thinking about that type of thing until the basic instantaneous measurement is producing sensible values.

How will there ever be a signal at pin 2 if pin 2 is grounded? - Scotty

I have signal conditioning made externally for the input of the signal. I attached it to my original post. To answer the question about ground, that was a good catch. I forgot to include it in the (rough) drawing, but I have a resistor between my signal out and ground. When hooked to an oscilloscope I'm getting a clean sine wave input into the Arduino. I assumed because of this when I set up the interrupt on falling edge it would catch this every time, I could be wrong here. Maybe someone can clear that up for me.

The reason I went with the switch case is for lack of knowledge of a better way to do it. I had planned to update my LCD display every .5 seconds. This is why my delay is set to 100ms over 5 periods pushing me to output roughly every .5 seconds. I read the blink without delay and that defiantly looks like better way to approach the code.

Then I averaged the code so that I would basically receive an average rpm over the .5 second period. Maybe I should scratch that and just purely display what the RPM is at every .5 seconds, and heck maybe .5 seconds is to slow of a refresh rate. What do you guys think? I'm open to any suggestions or coding changes.

As to the reason I have the code set up to AvgRPM * 10 * 60 / 2. With a pulse at 60Hz, I should receive 6 falling edges every 10ms. If so my AvgRPM = 6. 6 * 10 * 60 / 2 = 1800 RPM which if I'm correct should be the RPM of a 60Hz frequency. Please correct me if I am approaching this the wrong way.

Watson91:
As to the reason I have the code set up to AvgRPM * 10 * 60 / 2. With a pulse at 60Hz, I should receive 6 falling edges every 10ms. If so my AvgRPM = 6. 6 * 10 * 60 / 2 = 1800 RPM which if I'm correct should be the RPM of a 60Hz frequency. Please correct me if I am approaching this the wrong way.

A 60 Hz pulse corresponds to how many rpm?

60 pulses per second equals 3600 pulses per minute. If you are counting ignition pulses on one cylinder of a four-stroke engine you will get one pulse every two revolutions so that corresponds to 7200 rpm. Am I misunderstanding your setup?

PeterH:

Watson91:
As to the reason I have the code set up to AvgRPM * 10 * 60 / 2. With a pulse at 60Hz, I should receive 6 falling edges every 10ms. If so my AvgRPM = 6. 6 * 10 * 60 / 2 = 1800 RPM which if I'm correct should be the RPM of a 60Hz frequency. Please correct me if I am approaching this the wrong way.

A 60 Hz pulse corresponds to how many rpm?

60 pulses per second equals 3600 pulses per minute. If you are counting ignition pulses on one cylinder of a four-stroke engine you will get one pulse every two revolutions so that corresponds to 7200 rpm. Am I misunderstanding your setup?

I agree that I am somewhat confused by this reading also. I originally calculated what you are telling me it should be. When I hooked the oscilloscope to the engine signal I was receiving a 60Hz frequency at idle of the emitter leg of my signal conditioning. Idle of this briggs should roughly be around 1800 and 3200 is at the top of our RPM band. So, maybe there is some misunderstanding in what I was reading or I had something hooked up wrong.

But, according to the values you gave me which is AvgRPM * 10 * 60 * 2 this will be 7200 RPM.

If 60Hz AvgRPM = 6, 6 * 10 * 60 * 2 = 7200

Hi Alex

Not that it will save you a lot of RAM in this example, but when constructing the average rather than storing 5 values you could use a cumulative moving average. It would reduce your sampling loop to this:

void loop() {
  AvgRPM = 0;                  // reset the average
  for (int i = 1; i <= 5; i++) {      // note the subtle change from 0 up to 1 up
    rpmcount = 0;
    delay(100);                  // let the interrupts do their thing
    AvgRPM = AvgRPM + (rpmcount - AvgRPM)/i;
  }
// rest of loop()
}

The beauty of it is it's simple to tune it to any number of samples you like, without needing to change structure of the code or number of variables or size of the memory footprint. Simply change the upper limit of your for() loop.

Normal caveats about untested code, insufficient caffeine etc apply :slight_smile:

More info on how that's derived at Moving average - Wikipedia

Cheers ! Geoff

edit: peterH's fix applied

strykeroz:
Normal caveats about untested code, insufficient caffeine etc apply :slight_smile:

Would it work better with "AvgRPM = 0;" moved above the for loop? :wink:

PeterH:
Would it work better with "AvgRPM = 0;" moved above the for loop? :wink:

well spotted! And,yes.