Confused by oscilloscope - square wave signal to RPM

OK. After that, post your latest code.

*/
// Measures the HIGH width, LOW width, frequency, and duty-cycle of a pulse train
// on Arduino NANO Pin 8 (ICP1 pin).
// Note: Since this uses Timer1, Pin 9 and Pin 10 can't be used for
// analogWrite().

// Adafruit NeoPixel RGBW strips (2x8 LEDS) 

#include <MedianFilterLib.h>

//MedianFilter < volatile > medianFilter ( 5 ); // 5 samples
volatile MedianFilter<uint32_t> medianFilter(5); // 5 samples
unsigned long pulse = 0;
float sensorRPM;
int RPMno = 4000; // base rpm for led activation
int space = 100; //delays for led startup sequence

//Input for headlight circuit
#define headlights 10
bool dimmer = LOW;
unsigned long millisDimmer = 0;

//NeoPixels
#include <Adafruit_NeoPixel.h>
#define PIN 6
#define NUM_LEDS 16
Adafruit_NeoPixel strip = Adafruit_NeoPixel(NUM_LEDS, PIN, NEO_GRBW + NEO_KHZ800);
// full brightness
uint32_t r = strip.Color  (200, 0, 0, 0);
uint32_t g = strip.Color  (0, 180, 0, 0);
uint32_t b = strip.Color  (0, 0, 255, 0);
uint32_t w = strip.Color  (0, 0, 0, 120);
uint32_t y = strip.Color  (200, 120, 0, 0);
// dimmed
uint32_t rd = strip.Color  (8, 0, 0, 0);
uint32_t gd = strip.Color  (0, 7, 0, 0);
uint32_t bd = strip.Color  (0, 0, 10, 0);
uint32_t wd = strip.Color  (0, 0, 0, 3);
uint32_t yd = strip.Color  (10, 5, 0, 0);
// off/no color
uint32_t o = strip.Color   (0, 0, 0, 0);

bool ledState = 1;

unsigned long millisLeds = 0;

void setup()
{
  //initiate neopixels
   strip.begin();
  //set strip to nothing, flushes potential random colors
  strip.clear();
  strip.fill(o);
  strip.show();
  ledStartup();
   
  //assign input for relay
  pinMode(headlights, INPUT);
  digitalWrite(headlights, HIGH); //enables pullup resistor - HIGH on an input pin
                                 //also, it inverts signal: < 3v = HIGH, > 3v = LOW
  
  Serial.begin(9600);
  while (!Serial);

  // For testing, uncomment one of these lines and connect
  // Pin 3 or Pin 5 to Pin 8
  // analogWrite(3, 64);  // 512.00, 1528.00, 2040.00, 25.10%, 490.20 Hz
  // analogWrite(5, 64);  // 260.00, 764.00, 1024.00, 25.39%, 976.56 Hz

  noInterrupts ();  // protected code
  // reset Timer 1
  TCCR1A = 0;
  TCCR1B = 0;
  TCNT1 = 0;
  TIMSK1 = 0;

  TIFR1 |= _BV(ICF1); // clear Input Capture Flag so we don't get a bogus interrupt
  TIFR1 |= _BV(TOV1); // clear Overflow Flag so we don't get a bogus interrupt

  TCCR1B = _BV(CS10) | // start Timer 1, no prescaler
           _BV(ICES1); // Input Capture Edge Select (1=Rising, 0=Falling)

  TIMSK1 |= _BV(ICIE1); // Enable Timer 1 Input Capture Interrupt
  TIMSK1 |= _BV(TOIE1); // Enable Timer 1 Overflow Interrupt
  interrupts ();

  millisLeds = millis();
  millisDimmer = millis();
}

volatile uint16_t Overflows = 0;

ISR(TIMER1_OVF_vect)
{
  Overflows++;
}

ISR(TIMER1_CAPT_vect)
{
  static uint32_t previousRisingEdgeTime = 0;

  uint32_t thisRisingEdgeTime;
  uint16_t overflows = Overflows;

  // If an overflow happened but has not been handled yet
  // and the timer count was close to zero, count the
  // overflow as part of this time.
  if ((TIFR1 & _BV(TOV1)) && (ICR1 < 1024))
    overflows++;

  // Interrupted on Rising Edge
  thisRisingEdgeTime = overflows; // Upper 16 bits
  thisRisingEdgeTime = (thisRisingEdgeTime << 16) | ICR1;

  //uint32_t pulseDuration = thisRisingEdgeTime - previousRisingEdgeTime;
  pulse = thisRisingEdgeTime - previousRisingEdgeTime;
  previousRisingEdgeTime = thisRisingEdgeTime;

  // This is a good place to add 'pulseDuration' to the median filter.
  // Make sure the median filter is marked 'volatile'.
  //medianFilter.AddValue (pulseDuration);
}

void loop()
{
  static uint32_t previousMedian;
  uint32_t thisMedian;

  if ( millis() >= millisDimmer + 250)
   {
    dimmer = digitalRead(headlights);
    millisDimmer = millis();
   }
  
  noInterrupts();
  // This would be where you get 'thisMedian'
  // from the median filter.
  thisMedian = medianFilter.AddValue (pulse);
  interrupts();

  // If a sample has been measured
  if (thisMedian != previousMedian  && thisMedian >= 0.75*previousMedian)
   {
     // Display the pulse length in microseconds
     //Serial.print("Pulse time (microseconds): ");
     //Serial.println(thisMedian / 16.0, 2);
    
     //measure revs from pulse_length
     //first calculate how many sparks happening per second, 1000ms divided by delay between sparks
     float sparksPerSec = 1000/((thisMedian / 16.0)*0.001);
     //next multiply by 60 for sparks per minute - two revs per spark
     sensorRPM = sparksPerSec*120;
     Serial.println(sensorRPM);
     //Serial.println(thisMedian);
     previousMedian = thisMedian; 
    }

  if (millis() >= millisLeds + 50)
  {
    if (dimmer = LOW){
     leds(); }
    else {
     ledsDimmed(); }  
     millisLeds = millis();
  }
}


//__      ______ _____ _____  
// \ \    / / __ \_   _|  __ \ 
//  \ \  / / |  | || | | |  | |
//   \ \/ /| |  | || | | |  | |
//    \  / | |__| || |_| |__| |
//     \/   \____/_____|_____/ 


//     _ _                               
//  __| (_)_ __ ___  _ __ ___   ___ _ __ 
// / _` | | '_ ` _ \| '_ ` _ \ / _ \ '__|
//| (_| | | | | | | | | | | | |  __/ |   
// \__,_|_|_| |_| |_|_| |_| |_|\___|_|                             

//void dimmer (void)
//{
//  
// val = digitalRead(headlights);
//  if (val == LOW) leds();
//   else            ledsDimmed();
//   
//}


//  _          _     
// | | ___  __| |___ 
// | |/ _ \/ _` / __|
// | |  __/ (_| \__ \
// |_|\___|\__,_|___/  font = ogre
                                                                 
void leds(void) 
{  
  
if (sensorRPM > 0 && sensorRPM < RPMno) 
 {
  strip.clear();
  strip.fill(o); //----------------
  strip.show(); 
 }
 
else if (sensorRPM > RPMno && sensorRPM < (RPMno + 90)) 
 {
  strip.clear();
  strip.fill(g, 14);  //gg--------------
  strip.show(); 
 }
 
else if (sensorRPM > (RPMno + 90) && sensorRPM < (RPMno + 220)) 
 {
  strip.clear();
  strip.fill(g, 12);  //gggg------------
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 220) && sensorRPM < (RPMno + 330)) 
 {
  strip.clear();
  strip.fill(g, 10);  //gggggg----------
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 330) && sensorRPM < (RPMno + 440)) 
 { 
  strip.clear();
  strip.fill(y,8);   //yyyyyyyy--------         
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 440) && sensorRPM < (RPMno + 550)) 
 {
  strip.clear();
  strip.fill(y,6);   //yyyyyyyyyy------
  strip.show();
 }
 
else if (sensorRPM > (RPMno + 550) && sensorRPM < (RPMno + 660)) 
 {
  strip.clear();
  strip.fill(y,4);         //yyyyyyyyyyyy----
  strip.show();  
 }
 
else if (sensorRPM > (RPMno + 660) && sensorRPM < (RPMno + 770)) 
 {
  strip.clear();strip.fill(r,2);         //rrrrrrrrrrrrrr--         
  strip.show(); 
 }
 
else if (sensorRPM > (RPMno + 770) && sensorRPM < (RPMno + 880)) 
 {
  strip.clear();
  strip.fill(r);        //rrrrrrrrrrrrrr
  strip.show(); 
 } 
 
else if (sensorRPM > (RPMno + 880) && sensorRPM < (RPMno + 1100)) 
 {
  strip.clear();
  strip.fill(w);        //wwwwwwwwwwwwwwww 
  strip.show(); 
 }


else if (sensorRPM > (RPMno + 1100)) 
 {
  strip.clear();
  strip.fill(o);
  strip.show(); 
 }
 
}

//  _          _        ___ _                              _ 
// | | ___  __| |___   /   (_)_ __ ___  _ __ ___   ___  __| |
// | |/ _ \/ _` / __| / /\ / | '_ ` _ \| '_ ` _ \ / _ \/ _` |
// | |  __/ (_| \__ \/ /_//| | | | | | | | | | | |  __/ (_| |
// |_|\___|\__,_|___/___,' |_|_| |_| |_|_| |_| |_|\___|\__,_|
                                                          
void ledsDimmed(void)  
{  
  
if (sensorRPM > 0 && sensorRPM < RPMno) 
 {
  strip.clear();
  strip.fill(o); //----------------
  strip.show(); 
 }
 
else if (sensorRPM > RPMno && sensorRPM < (RPMno + 90)) 
 {
  strip.clear();
  strip.fill(gd, 14);  //gg--------------
  strip.show(); 
 }
 
else if (sensorRPM > (RPMno + 90) && sensorRPM < (RPMno + 220)) 
 {
  strip.clear();
  strip.fill(gd, 12);  //gggg------------
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 220) && sensorRPM < (RPMno + 330)) 
 {
  strip.clear();
  strip.fill(gd, 10);  //gggggg----------
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 330) && sensorRPM < (RPMno + 440)) 
 { 
  strip.clear();
  strip.fill(yd,8);   //yyyyyyyy--------         
  strip.show(); 
  } 
  
else if (sensorRPM > (RPMno + 440) && sensorRPM < (RPMno + 550)) 
 {
  strip.clear();
  strip.fill(yd,6);   //yyyyyyyyyy------
  strip.show();
 }
 
else if (sensorRPM > (RPMno + 550) && sensorRPM < (RPMno + 660)) 
 {
  strip.clear();
  strip.fill(yd,4);         //yyyyyyyyyyyy----
  strip.show();  
 }
 
else if (sensorRPM > (RPMno + 660) && sensorRPM < (RPMno + 770)) 
 {
  strip.clear();
  strip.fill(rd,2);         //rrrrrrrrrrrrrr--         
  strip.show(); 
 }
 
else if (sensorRPM > (RPMno + 770) && sensorRPM < (RPMno + 880)) 
 {
  strip.clear();
  strip.fill(rd);        //rrrrrrrrrrrrrr
  strip.show(); 
 } 
 
else if (sensorRPM > (RPMno + 880) && sensorRPM < (RPMno + 1100)) 
 {
  strip.clear();
  strip.fill(wd);        //wwwwwwwwwwwwwwww 
  strip.show(); 
 }

else if (sensorRPM > (RPMno + 1100)) 
 {
  strip.clear();
  strip.fill(o);
  strip.show(); 
 }
}

//  _          _      __ _             _               
// | | ___  __| |___ / _\ |_ __ _ _ __| |_ _   _ _ __  
// | |/ _ \/ _` / __|\ \| __/ _` | '__| __| | | | '_ \ 
// | |  __/ (_| \__ \_\ \ || (_| | |  | |_| |_| | |_) |
// |_|\___|\__,_|___/\__/\__\__,_|_|   \__|\__,_| .__/ 
//                                             |_|    
void ledStartup(void)
{
  strip.fill(o);
  strip.show();
  delay(space);
  
  strip.fill(gd, 14);
  strip.show();
  delay(space);
  
  strip.fill(gd, 12);
  strip.show();
  delay(space);
  
  strip.fill(gd, 10); 
  strip.show();
  delay(space);
  
  strip.fill(yd,8);
  strip.show();
  delay(space);
  
  strip.fill(yd,6);
  strip.show();
  delay(space);
   
  strip.fill(yd,4);
  strip.show();
  delay(space);
  
  strip.fill(rd,2);
  strip.show();
  delay(space);
  
  strip.fill(rd);  
  strip.show();
  delay(space);
//---------------------------  
  strip.fill(wd);
  strip.show();
  delay(space*2);
//----------------------------
  strip.clear();
  strip.fill(rd);  
  strip.show();
  delay(space);

  strip.clear();
  strip.fill(rd,2);
  strip.show();
  delay(space);

  strip.clear();
  strip.fill(yd,4);
  strip.show();
  delay(space);

  strip.clear();
  strip.fill(yd,6);
  strip.show();
  delay(space);

  strip.clear();
  strip.fill(yd,8);
  strip.show();
  delay(space);
  
  strip.clear();
  strip.fill(gd, 10); 
  strip.show();
  delay(space);
  
  strip.clear();
  strip.fill(gd, 12);
  strip.show();
  delay(space);
  
  strip.clear();
  strip.fill(gd, 14);
  strip.show();
  delay(space);
  
  strip.fill(o);
  strip.show();
}

This should also be marked a volatile because it is updated in an ISR and read elsewhere, possibly asynchronously.

Since pulse is > 1 byte, you need to suspend interrupts, during the time you make a copy of it, in order to avoid reading it in a half updated state (non-atomic read operation).

The second part you have effectively done, however you are doing much more while interrupts are suspended than necessary.

EDIT

This would minimise the time interrupts are suspended:

noInterrupts();
unsigned long pulseCopy = pulse ;
interrupts();
  // This would be where you get 'thisMedian'
  // from the median filter.
  thisMedian = medianFilter.AddValue (pulseCopy);  // pulseCopy
//if (dimmer = LOW){
if (dimmer == LOW){

== for comparison
= for assignment

Over the past couple days I have spent hours deliberating over the code and, honestly, I give up.

I removed the median filter library, because I found it to be doing precisely jack. Or - probably more accurately - I can't get it to work. In the previous posted code thisMedian was just reading effectively straight from the ISR without any filtering happening. I also removed the RPM calculation for simplicity, to lower calculations, and instead called the neopixels based on the timing directly.

I thought I had the smoothing/timing situation sorted, but it turns out the timing is just too narrow at higher revs to filter out spurious results. The problem is, ultimately, the arduino has no idea what a spurious result is as they just come too thick and fast for averaging out the numbers to work. I found alot of "averages" would be averages of the spurious readings, rendering the new averages moot...

I tried using an array to average, both of the timing output itself and the RPM calcuation, and if Im using about 10 indices, its simply not accurate enough. If I use more than that, the program crashes. If i use 10, and then average every second array-average, it also crashes.

Frankly I'm at my wit's end with this project and I'm spending too much time thinking about it. To add insult to injury, my old sketch with pulsein() for some reason doesn't work reliably anymore despite no hardware changes (well I added a jumper between pins 3 and 8 and changed the headlight input from pin 8 to pin 10 but so what?). I have an idea to get revs from the can-bus, so I think I might give that a go instead and scrap the IGT wire. It seems to simply be too noisy for any reliable RPM reading. I've tried using an OBD reader that communicates over I2C before the pulsin() sketch, and it was way too slow and actually surprisingly innaccurate.

Thanks for the help everyone. I may revisit this at a later stage but for now I'm putting this project to sleep.

Before you do that, you haven't answered my question. Do you have access to a variable frequency, variable mark-to-space ratio square wave generator? Do you have access to an oscilloscope?

I can tell from your latest post that you don't know for sure why it isn't working, and why it's crashing. Fundamentally, you don't know whether the problem is with your code, or with the incoming signals, or with electrically induced noise.

You need to do these things:

1/ Using your signal generator, test it extensively on your workbench across the full range of frequencies it is expected to handle. Ditto with the M/S ratios. If it crashes on your bench, debug it and fix it. If the readings are wrong, or update too slowly, or anything else that you don't like, debug it and fix it. Continue until it is working perfectly and you are completely confident in your code.

ONLY THEN:

2/ Put your oscilloscope in your car, get someone to drive it while you look closely at the trace. Check for spikes, jitter, noise, etc. Measure peak voltages, measure timings and ratios. Work out what you must do to tame any spikes and condition the signal so it's suitable to be connected to your Arduino.

3/ Build and test the signal conditioning circuitry (if, indeed, any is necessary).

4/ Then install your Arduino in the car. If it crashes, you know for sure it isn't the code, but rather some kind of problem arising from the electrically noisy environment.

The most important thing is to get a 'scope and a sig gen, and get it working on your bench first. Until you do that, you are working blind.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.