Logging engine timing events

lar3ry you have been so helpful. Thanks for taking the time to express solutions so clearly.

    Serial.print("DATA, TIME,,,");             // Opens PLX-DAQ data line

No idea what "Opens PLX-DAQ data line" means, but you probably don't want to print the header every time you go through the loop. It will probbaly print a few hundred times between state changes.

That line put right after the beginning of the loop has worked great. It lets PLX-DAQ know data is coming and how many columns I want. I started a post about PLX-DAQ that has some simple code lines and how I used them. Basically opens up the data line. Without it no data goes into excel. I've been testing the PLX-DAQ /excel graphing stuff with my solar box temp sensor. Graphing temp (and rpm for the x-axis, ha) in excel. How fast is that temp spinning anyway? The LABEL command/ line, in the setup function prints the headers and runs just once.

So you don't think I even need averaging because using the interrupts will make it accurate enough? Interrupts and averaging is overkill? I guess I can put it back in later if values are all over the place.

I straightened up the sketch a bit and autoformatt works now so it's easier for everyone to read:

/* Finds time from TDC1, TDC4 and pulseStart. It's a finite state machine. Holds in a loop until the proper state occurs.
 * First it looks for a LOW on the pulse detector to trigger an interrupt that logs the timing of the event. The *INJECTORWAIT state isn't looking for a LOW on the pin.
 * The interrupt does that. The INJECTORWAITT state is looking for data stored because the pin went LOW. Same with *the other 2 states The injector pulse comes before TDC1. How many degrees before top dead center BTDC is what we *are looking for */
#define INJECTORWAIT 0
#define TDCWAIT_1 1
#define TDCWAIT_2 2

volatile unsigned long tdcStartTime;   //declared volatile because it's a global variable used in a function
volatile unsigned long injectorStartTime;
volatile unsigned long tdcStartTime_2;
volatile unsigned long tdcStartTime_1;
unsigned long oneRevTime;
unsigned long btdcY;
unsigned long rpmX;
const int pulsePin = 2;
const int tdcPin = 3;

byte state = INJECTORWAIT;

void setup() {
  Serial.begin(9600);
  pinMode(pulsePin, INPUT);
  pinMode(tdcPin, INPUT);
  while (digitalRead(tdcPin) == HIGH) {    //wait here until we are at TDC (any TDC)
  }
}
attachInterrupt(0,PulseIRPT,FALLING);  //enable interrupt attached to pin 2 pulse detector
attachInterrupt(1,TdcIRPT,FALLING);    //enable interrupt attached to pin 3 flywheel laser sensor

void loop () {
  Serial.print("DATA, TIME,,,");             // Opens PLX-DAQ data line

  while (state == INJECTORWAIT)  {
    if (injectorStartTime > 0 ) {
      state = TDCWAIT_1;
    }
  }
  while (state == TDCWAIT_1) {
    if (tdcStartTime > 0 ) {
      tdcStartTime_1 = tdcStartTime;      //here we stored the value from the tdcIRPT interrupt routine intdcStartTime_1
      tdcStartTime = 0;                   //set to 0 so the next state knows if it's been retriggered.
      state = TDCWAIT_2;
    }
  }
  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
    }
    int z;
    z = calculate (unsigned long a, unsigned long b, unsigned long c);  //call to the calculate function      
    delay 10000;
    state = INJECTORWAIT;
  }
}   //end of loop so interrupt and other functions go here
void TdcIRPT() {                     // the interrupt routines
  tdcStartTime = micros();                 
}
void pulseIRPT() {
  injectorStartTime = micros();
}
int calculate  (unsigned long tdcStartTime_1,  unsigned long injectorStartTime, unsigned long tdcStartTime_2) {
  unsigned long a;
  unsigned long b;
  unsigned long c;
  int btdcY;
  long rpmX;
  unsigned long Btdc;
  Btdc = a - b;                 // BTDC in microseconds
  oneRevTime = c - a;           // time for one revolution in microseconds
  rpmX = 60000000/oneRevTime;   //x-axis data to send to excel
  btdcY = ((Btdc/oneRevTime)/360);  //y-axis data to send to excel
  Serial.print(btdcY);
  Serial.print(",");
  Serial.println(rpmX);
}

[ A few errors left,.. and I'm not completely sure I wrote the calculate function right. But we're getting closer.

Now this is the project guidance fora and I've been asking all programming questions. I'm curious about the hardware part of this. I'm using a laser tach to detect the TDC mark on the flywheel. Hacked it to find the trigger for the atmel chip inside. It amplifies the photodiode's output with a LM358 op amp before sending it to a 4050 hex buffer. It's meant to take a CMOS input and output enough current for 2 TTL loads. It connects to the INT (not) pin 12 on a AT89S52 atmel chip. I hooked an LED/resistor to it and it goes LOW when a reflector is placed in front of it. It uses a seperate 9 volt battery than the Arduino,.. should I use an optocoupler between the laser tach and the Arduino digital pins?

For the pulse detector I'm using a ferret 765. It turns the pulse into a coil pulse that a tach can pick up. It also uses an op amp, the LM324 quad op amp to amplify the signal from the piezo pulse detector. Then sends it to a PIC chip. Same question. Optocoupler? I'll be building my own detector circuits eventually,.. but until then I'm using what I have.

A few comments on the code:

attachInterrupt(0,PulseIRPT,FALLING);  //enable interrupt attached to pin 2 pulse detector
attachInterrupt(1,TdcIRPT,FALLING);    //enable interrupt attached to pin 3 flywheel laser sensor

These will error. Just as you cannot declare functions within other functions, you can only do certain things outside functions. What you can do is to include libraries, declare variables, define new datatypes, and define macros or values.

What you cannot do is to write statements that will perform operations.

So you will need to place these statements inside a function. Inside setup() would be the best place.

  while (digitalRead(tdcPin) == HIGH) {	//wait here until we are at TDC (any TDC)
  }

This is a good idea, but only if you then set injectorStartTime to 0 after the end of the while(). this ensures that you will not enter the INJECTORWAIT routine until the next injector pulse after any TDC.

The following comments are in the nature of ensuring that we do everything in order, with nothing extra happening unexpectedly.

Right after the above statement, place a statement injectorStartTime = 0; This will ensure that we do not satisfy the if in the INJECTORWAIT routine until we get an injector pulse. When we do, we are interested in the next two TDC pulses, so in these states, try this (lines with <***** are added or changed):

  while (state == INJECTORWAIT)  {
    if (injectorStartTime > 0 ) {
      state = TDCWAIT_1;
      tdcStartTime = 0;   // get set to wait for non zero in TDCWAIT_1  <*****
    }
  }
  while (state == TDCWAIT_1) {
    if (tdcStartTime > 0 ) {
      tdcStartTime_1 = tdcStartTime;      //here we stored the value from the tdcIRPT interrupt routine intdcStartTime_1
      tdcStartTime = 0;                   //set to 0 so the next state knows if it's been retriggered.
      state = TDCWAIT_2;
    }
  }
  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
      injectorStartTime = 0;    // get set to wait for non zero in INJECTORWAIT  <*****
    }

I’ll wait to comment on your function call and the calculate() function, as it’s getting late. There are several things you don’t quite get about functions yet, but I think you are getting a better understanding of all this.

Ya know I kept meaning to ask you what that while statement was for :slight_smile:
I’m not sure it’s needed. No matter what, the loop will wait for the first injector pulse right?
I can see why it injectorStartTime needs zeroed

Moving the attach interrupt function calls inside the setup function worked great,…
once I paid attention to what was lower case and what was upper case.

tdcStartTime = 0;   // get set to wait for non zero in TDCWAIT_1  <*****

zeroing that makes sense.
Not because we are overloading (We don’t just use it to store one value. We reuse it to store both of the TDC values) a variable. Because the interrupt routine would just stuff another value in there.
If it isn’t zero the next state will think it has been triggered. So it needs cleared each time we are done with it.

I fixed some of the calculate () function problems. So here’s the lastest:

/* Finds time from TDC1, TDC4 and pulseStart. It's a finite state machine.
* Holds in a loop until the proper state occurs.
* First it looks for a LOW on the pulse detector to trigger an interrupt
* that logs the timing of the event.
*The INJECTORWAIT state isn't looking for a LOW on the pin.
 * The interrupt does that.
* The INJECTORWAITT state is looking for data stored because the pin went LOW.
* Same with the other 2 states. The injector pulse comes before TDC1.
* How many degrees before top dead center BTDC is what we *are looking for */
#define INJECTORWAIT 0
#define TDCWAIT_1 1
#define TDCWAIT_2 2

//declared volatile because it's a global variable used in a function
volatile unsigned long tdcStartTime;   
volatile unsigned long injectorStartTime;
volatile unsigned long tdcStartTime_2;
volatile unsigned long tdcStartTime_1;
unsigned long oneRevTime;
unsigned long btdcY;
unsigned long rpmX;
const int pulsePin = 2;
const int tdcPin = 3;

byte state = INJECTORWAIT;

void setup() {
  Serial.begin(9600);
  pinMode(pulsePin, INPUT);
  pinMode(tdcPin, INPUT);
  while (digitalRead(tdcPin) == HIGH) {	   //wait here until we are at TDC (any TDC)
  }
  injectorStartTime = 0;
  attachInterrupt (0,pulseIRPT,FALLING);  //enable pin 2 pulse interrupt
  attachInterrupt (1,tdcIRPT,FALLING);   //enable pin 3 tdc interrupt

}

void loop () {
  Serial.print("DATA, TIME,,,");             // Opens PLX-DAQ data line

  while (state == INJECTORWAIT)  {
    if (injectorStartTime > 0 ) {
      state = TDCWAIT_1;
      tdcStartTime = 0;
    }
  }
  while (state == TDCWAIT_1) {
    if (tdcStartTime > 0 ) {
      tdcStartTime_1 = tdcStartTime;      //we stored the value from the tdcIRPT interrupt
      tdcStartTime = 0;        //set to 0 so the next state knows if it's been retriggered.
      state = TDCWAIT_2;
    }
  }
  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
      injectorStartTime = 0;
    }
    unsigned long z;      //a call to the calculate function
    z = calculate (unsigned long tdcStartTime_1,  unsigned long injectorStartTime,
    unsigned long tdcStartTime_2);
    delay 10000;
    state = INJECTORWAIT;
  }
}   //end of loop so interrupt and other functions go here
void tdcIRPT() {                     // the interrupt routines
  tdcStartTime = micros();                 
}
void pulseIRPT() {
  injectorStartTime = micros();
}
int calculate  (unsigned long a, unsigned long b, unsigned long c); 
{ //calculate function
  int btdcY;  //degrees before top dead center shouldn't be more than 30
  long rpmX; // can be up to 6000
  unsigned long Btdc;
  Btdc = a - b;                 // BTDC in microseconds
  oneRevTime = c - a;           // time for one revolution in microseconds
  rpmX = 60000000/oneRevTime;   //x-axis data to send to excel
  btdcY = ((Btdc/oneRevTime)/360);  //y-axis data to send to excel
  Serial.print(btdcY);
  Serial.print(","); 
  Serial.println(rpmX);
}

Well, you're getting there. Have a look at your code for the TDCWAIT_2 state:

  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
      injectorStartTime = 0;
    }
    unsigned long z;      //a call to the calculate function
    z = calculate (unsigned long tdcStartTime_1,  unsigned long injectorStartTime,
    unsigned long tdcStartTime_2);
    delay 10000;
    state = INJECTORWAIT;
  }

In the IDE, if you put your cursor just after a parenthesis, a square bracket, or a curly brace, the IDE editor will show the matching character by drawing a rectangle around it. In the above code, place your cursor on the opening curly brace of the while loop. You will see that the closing curly brace is the one after the line state = INJECTORWAIT; , but that means that it doesn't matter if we've satisfied the if, so regardless of whether or not we have found the second TDC, we will try to calculate our values, and that while loop runs REAL quick. So you will want to move the highlighted closing curly brace up to just after the closing curly brace of the if statement. This will keep it in the while() until we have the second TDC, and only drop through when we have it (and thus, have all three values).

Now the call to calculate(). First, you do not need the variable z, so there is no use assigning the result of calculate() to it. In fact, you do not need a return value at all, so your calculate function can be a void type. The rule is that if a function CAN return a value, it is legal to return one, but it is also legal to not return one.

The next thing is this line:

z = calculate (unsigned long tdcStartTime_1,  unsigned long injectorStartTime,
    unsigned long tdcStartTime_2);

The unsigned long types redeclare variables that are already declared, and worse, variables that already have values we want to keep. The redecalarations actually create new variables of the same name as the ones already declared, and initialize them to 0. So when you call the function, you are calling it with three variables that contain 0.

The delay(10000) is not required. and will actually cause the Arduino to do absolutely nothing for 10 seconds. When you arrive back from the delay(), you will start looking for another injector pulse. You can delay if you really only want a reading every ten seconds, but I would have thought you wanted data more quickly than that.

Finally (for this chunk of code), you want to call calculate ONLY after you find TDC 2, which means that you want to call it from within the while(). As well, you can only drop out of TCDWAIT_2 if you change the state, so you must change it within the if.

So the above code should now look like this:

  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
      calculate (tdcStartTime_1, injectorStartTime, tdcStartTime_2);
      injectorStartTime = 0;  // must be after call to calculate()
      state = INJECTORWAIT;
    }
  }

Now for the function itself. It's up to you, and a matter of taste, what you call your parameter variables, but in the long run you will find it easier to read and/or troubleshoot your functions later if you give them meaningful names. So instead of a, b, and c, you should call them something like tdc1Time, injTime, and tdc2Time. It isn't important what they are called, but anything that helps when you look at your code a year or two later is worth doing.

So really the only thing to do in your function is to change it to void instead of int, though it's not necessary to do so, and to change the parameter names if you want to. And of course, you will want to double-check your actual math in the function.

Once you get this going, please let us know how it goes. I may just try something like this on my tractor, a Ford 545 Industrial. I have a number of projects in mind for the tractor, including a temperature sensor and real-time-clock tied to an AC circuit that will control when a battery charger and recirculating block heater are activated. -35C is not kind to tractors. I also have a difficult time knowing when my loader bucket is level, so an Arduino with an accelerometer could be a winner.

Well we got the error codes worked out. It compiles. http://forum.arduino.cc/index.php?topic=218124.msg1595836#msg1595836

Come back here to find out I have some logic errors to work on.

In fact, you do not need a return value at all, so your calculate function can be a void type. The rule is that if a function CAN return a value, it is legal to return one, but it is also legal to not return one.

The next thing is this line:

When you say "return a value" you mean calculate() would actually be used in,..? say an equation somewhere else in the program right? but since we are using the function to dump 2 values into excel. calculate() isn't used anywhere. So use void for the type.

The unsigned long types redeclare variables that are already declared, and worse, variables that already have values we want to keep. The redecalarations actually create new variables of the same name as the ones already declared, and initialize them to 0. So when you call the function, you are calling it with three variables that contain 0.

Compile didn't catch that.

I put a delay() in there just to have some adjustment available later. Something to tweek. Might not be needed. Excel can handle 5000 values. @2500 RPM that's about 41 samples a second. I can probably control the rate-of-change for the RPMs to end up with a nice curve. Hit disconnect in the PLX-DAQ window and save the sheet. If I try checking the curve while going down the road, slow RPM changes might clump data points. 10 secs is definately too much.

Cleaned-up the function call and put it in the right place. Thanks, I see why it goes there.

So really the only thing to do in your function is to change it to void instead of int, though it's not necessary to do so, and to change the parameter names if you want to. And of course, you will want to double-check your actual math in the function.

Good point about the math.

Once you get this going, please let us know how it goes. I may just try something like this on my tractor, a Ford 545 Industrial. I have a number of projects in mind for the tractor, including a temperature sensor and real-time-clock tied to an AC circuit that will control when a battery charger and recirculating block heater are activated. -35C is not kind to tractors. I also have a difficult time knowing when my loader bucket is level, so an Arduino with an accelerometer could be a winner.

Oh yeah. Weather is getting warmer. I ordered up some parts that should be in this week. I'll probably use an optocoupler. Working toward building my own circuits to read the pulse detector and TDC,.. then there are a few more things I'd like on the same graph. I think this will help alot of us with the old mechanical diesels. Could lead to a huge fuel savings across the country/nations. When we figure out the proper timing curve for each engine and how to properly change it. Code as it sits now:

/* Finds time from TDC1, TDC4 and pulseStart. It's a finite state machine.
 * Holds in a loop until the proper state occurs.
 * First it looks for a LOW on the pulse detector to trigger an interrupt
 * that logs the timing of the event.
 *The INJECTORWAIT state isn't looking for a LOW on the pin.
 * The interrupt does that.
 * The INJECTORWAITT state is looking for data stored because the pin went LOW.
 * Same with the other 2 states. The injector pulse comes before TDC1.
 * How many degrees before top dead center BTDC is what we *are looking for */
#define INJECTORWAIT 0
#define TDCWAIT_1 1
#define TDCWAIT_2 2

//declared volatile because it's a global variable used in a function
volatile unsigned long tdcStartTime;   
volatile unsigned long injectorStartTime;
volatile unsigned long tdcStartTime_2;
volatile unsigned long tdcStartTime_1;
unsigned long oneRevTime;
unsigned long btdcY;
unsigned long rpmX;
const int pulsePin = 2;
const int tdcPin = 3;

byte state = INJECTORWAIT;

void setup() {
  Serial.begin(9600);
  pinMode(pulsePin, INPUT);
  pinMode(tdcPin, INPUT);
  while (digitalRead(tdcPin) == HIGH) {       //wait here until we are at TDC (any TDC)
  }
  injectorStartTime = 0;
  attachInterrupt (0,pulseIRPT,FALLING);  //enable pin 2 pulse interrupt
  attachInterrupt (1,tdcIRPT,FALLING);   //enable pin 3 tdc interrupt

}

void loop () {
  Serial.print("DATA, TIME,,,");             // Opens PLX-DAQ data line

  while (state == INJECTORWAIT)  {
    if (injectorStartTime > 0 ) {
      state = TDCWAIT_1;
      tdcStartTime = 0;
    }
  }
  while (state == TDCWAIT_1) {
    if (tdcStartTime > 0 ) {
      tdcStartTime_1 = tdcStartTime;      //we stored the value from the tdcIRPT interrupt
      tdcStartTime = 0;        //set to 0 so the next state knows if it's been retriggered.
      state = TDCWAIT_2;
    }
  }
  while (state == TDCWAIT_2) {
    if (tdcStartTime > 0) {
      tdcStartTime_2 = tdcStartTime;
      calculate (tdcStartTime_1, injectorStartTime, tdcStartTime_2);
      injectorStartTime = 0;
    }
  }
  delay (10);
  state = INJECTORWAIT;
}
 //end of loop so interrupt and other functions go here
void tdcIRPT() {                     // the interrupt routines
  tdcStartTime = micros();                 
}
void pulseIRPT() {
  injectorStartTime = micros();
}
void calculate  (unsigned long tdc1us, unsigned long injus, unsigned long tdc2us) 
{                //calculate function
  int btdcY;  //degrees before top dead center shouldn't be more than 30
  long rpmX; // can be up to 6000
  unsigned long Btdc;
  Btdc = tdc1us - injus;                 // BTDC in microseconds
  oneRevTime = tdc2us - tdc1us;           // time for one revolution in microseconds
  rpmX = 60000000/oneRevTime;   //x-axis data to send to excel
  btdcY = ((Btdc/oneRevTime)/360);  //y-axis data to send to excel
  Serial.print(btdcY);
  Serial.print(","); 
  Serial.println(rpmX);
}

I really couldn't have done it without you,.. or if I did it wouldn't have turned out so simple and elegant. I really like the way it flows. Not too complicated for beginners to understand but enough to learn from. With all your great comments I think it will be a good topic/thread to learn from.

graphing: When you say "return a value" you mean calculate() would actually be used in,..? say an equation somewhere else in the program right? but since we are using the function to dump 2 values into excel. calculate() isn't used anywhere. So use void for the type.

Right. Consider this function:

float hypotenuse (float sideA, float sideB) {
    float hyp = sqrt( ( sideA * sideA ) + ( sideB * sideB ) );
    return hyp;
}

We can call it from multiple places in a program, using different variables, or even numbers not in variables.

distanceToTown = hypotenuse (distanceToHighWay, distance toGridRoad)
HypotenuseValue = hypotenuse(44.5, X); // where X is a float variable containing a value

The unsigned long types redeclare variables that are already declared, and worse, variables that already have values we want to keep. The redecalarations actually create new variables of the same name as the ones already declared, and initialize them to 0. So when you call the function, you are calling it with three variables that contain 0.

Compile didn't catch that.

It doesn't, because it is not an error. It is perfectly legal to re-use variable names that duplicate other previously declared variable names. It is usually not a good idea for beginners to do, because it's usually inadvertent, and usually results in logical errors that are difficult to find. But it IS legal.

Oh yeah. Weather is getting warmer. I ordered up some parts that should be in this week. I'll probably use an optocoupler. Working toward building my own circuits to read the pulse detector and TDC,.. then there are a few more things I'd like on the same graph. I think this will help alot of us with the old mechanical diesels. Could lead to a huge fuel savings across the country/nations. When we figure out the proper timing curve for each engine and how to properly change it.

I took a look at my tractor today, and it looks like I don't have an electrical injector pulse I can tie into. It will also be quite difficult to get at the pulley that rotates at the RPM of the engine, so I might not be doing what you are.

I really couldn't have done it without you,.. or if I did it wouldn't have turned out so simple and elegant. I really like the way it flows. Not too complicated for beginners to understand but enough to learn from. With all your great comments I think it will be a good topic/thread to learn from.

Glad I could help. As for elegant... well, elegance is something that comes with thinking about the problem, then rethinking and rethinking again, until you have the problem whittled down to the bare minimum you need to do in order to solve the problem. As you gain experience in programming, you will find this process coming naturally. I have often thrown out two or three attempted approaches to a problem, before suddenly realizing that I could do the whole thing very simply, with 1/10 the code required for the first attempt.

BTW, I just now noticed something in your code.

Have a look at your calculate() function. Check the end of the first line. Is there something there that shouldn't be?

I took a look at my tractor today, and it looks like I don't have an electrical injector pulse I can tie into. It will also be quite difficult to get at the pulley that rotates at the RPM of the engine, so I might not be doing what you are.

I think the VW TDI engines have one but any VW diesel before '92 is mechanical. No electrical injector pulse for a computer to read,.. because no computer. I had to buy a piezo clamp that clamps onto the injector line. Using an op amp I might be able to get the Arduino to detect it on an analog port. Much like the piezo knock tutorial. Tiny tach sells the piezo clamps for about $60 each. Once I get it to work, I'll probably try building a shield. Sooo no pullys you can point a laser tach at?I suppose they are up behind a cover.

I've been trying to figure out your riddle. The parenthasis are in the right place, I didn't put a semicolon at the end where it didn't belong. All I can think of is the,.. oh I don't even know what they are called, not parenthasis or brackets. Maybe squiggly brackets. In most cases they are at the end, after the parameters but it was moved a line down in the calculate() function. changed it to this: here V

void calculate  (unsigned long tdc1us, unsigned long injus, unsigned long tdc2us) {
  int btdcY;  //degrees before top dead center shouldn't be more than 30

Is that it? Seems small. Compile and verify don't notice it.

The piezo sensor is a good idea. I'll have to have a look at that. As for the pulley, yes, it's well hidden by covers. Very difficult to access. Come summer, if I get time, I'll explore the possibilities.

As for the riddle, I was looking at previous code (from reply #22) when I wrote that. Ignore.

Looks like all you need to do now is get the sensors working. 8)

I know. Only 4 wires to connect to the Arduino. I bought a connector shield so those will plug right in. I am really looking forward to the first test, then hooking it up to some cars that are getting much better fuel mileage and power than I am. I'll get everything on a protoboard, ready for the optocouplers to arrive. I hope stuff gets here before Thursday so I can put it together during the next blizzard.

You put alot into this so It's your's too. I'll keep this post updated.

It might be awhile but possibly my next project,.. A thing that turns the TV down when it gets too loud and turns it back up when too soft. That's for another topic. Just planting the seed. :)

Way off topic: water bottles that snap together like leggos?

A short progress report: I got the optocoupler wired-up. Made a saddle to support the laser tach so it's pointing at the flywheel. It's been very cold out, really too cold for my laptop. I got the car warmed-up and the laptop in the passenger seat. Checked to see what was happening in the serial monitor and DATA, TIME,

Soo it's reading the laser tach but not the pulse detector. The pulse detector has an LED that flashes to the RPM so I know it's working. If I hook the laser tach up to the pulse side I can flash an LED back at pin2 so both channels of the optocoupler are working.

I tried hacking into a different spot, with about as much luck. I tried the sketch with a couple push buttons and got it to graph the results. No debounce made for weird results. but was still fun to see I had to push TDC to get DATA,TIME to show up continueing to push TDC did nothing. Everything after that was pulse, TDC1, TDC2 rinse repeat.

Probably mentioned I am using a laser tach for the TDC sensor. I don't think I could build my own sensor for cheaper than I can buy a laser tach. Plus the laser tach is nice to have on it's own. You might be able to shine it on a pully even if you can't get to it, through a slot in the grill. Once you put the shiney tape on it.

I'll try again tomorrow. Getting close. Hey how cold do these Arduinos work down to?