Pages: [1]   Go Down
Author Topic: Briggs and Stratton Digital Tachometer  (Read 1551 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.

Code:
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 (27.13 KB, 625x463 - viewed 51 times.)
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 474
Posts: 18696
Lua rocks!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
    delay(100);

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

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12549
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
Code:
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:
Code:
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.
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Pottstown, PA
Offline Offline
Sr. Member
****
Karma: 5
Posts: 317
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
« Last Edit: April 03, 2012, 09:48:51 am by Watson91 » Logged

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12549
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


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?
Logged

I only provide help via the forum - please do not contact me for private consultancy.

Offline Offline
Newbie
*
Karma: 0
Posts: 3
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


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
« Last Edit: April 03, 2012, 05:41:42 pm by Watson91 » Logged

Brisbane, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1121
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
Code:
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  smiley

More info on how that's derived at http://en.wikipedia.org/wiki/Moving_average#Cumulative_moving_average

Cheers ! Geoff

edit: peterH's fix applied
« Last Edit: April 04, 2012, 03:29:44 pm by strykeroz » Logged

"There is no problem so bad you can't make it worse"
- retired astronaut Chris Hadfield

UK
Offline Offline
Shannon Member
****
Karma: 222
Posts: 12549
-
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Normal caveats about untested code, insufficient caffeine etc apply  smiley

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

I only provide help via the forum - please do not contact me for private consultancy.

Brisbane, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1121
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset


Would it work better with "AvgRPM = 0;" moved above the for loop? smiley-wink
well spotted! And,yes.
Logged

"There is no problem so bad you can't make it worse"
- retired astronaut Chris Hadfield

Pages: [1]   Go Up
Jump to: