# Logging engine timing events

I'm trying to graph the timing curve on an old mechanical diesel. One sensor is on the flywheel looking for TDC. The other is on the #1 injector line looking for the fuel injection pulse. BTDC

The graph will show BTDC (before top dead center) on the Y-axis because the fuel is injected before it reaches TDC. So the pulse detector will probably be the first event entered. about 12 degrees after that the TDC detector will go LOW

I need to log the time of each event: BTDC,... and 2 TDCs to get the RPM. I'm not sure what the best way to do it would be. Should I have the BTDC pulse trigger an interupt, then log the time of the event and have the interupt routine stop everything to look for and log the LOW on TDC1 and again on TDC4? Or should I have both sensors trigger interupts and log the timing of the events? Or maybe have the second interupt inside the first interupt routine?

6000 RPM max to give you an idea of timing.

I think I would have the pulses cause interrupts and use each interrupt to save the value of micros() for a certain number of revolutions.

However at the relatively slow speed of the engine you may find that a repeating loop that uses digitalRead() or direct port reads (see direct port manipulation) which allow several pins to be read at the same time may be perfectly fast enough and a lot less trouble to debug than interrupts.

Then at your leisure you can work out the maths.

I don't think you can have interrupts within interrupts, but I would recommend strongly against it. An interrupt routine should do the absolute minimum in the shortest possible time.

...R

I think your best bet would be to use two external interrupts for the two inputs and use micros() to measure the interval between successive crank position interrupts (to get engine speed) and the interval between the two interrupts to get the injection timing.

Given that the frequency is a lot higher than you need, it would be worth averaging the values you calculate over a few dozen engine cycles to reduce noise in the result.

OK, here’s some code I wrote:

``````/*
* Finds time from TDC1, TDC4 and Pulsetime and calculates BTDC.
* First it looks for a LOW on the pulse detector to trigger a start. It starts counting and looks for a LOW on the TDC sensor. TDC1 stores the timing of the event.
* Then it looks for the second TDC LOw which gets stored in TDC4.
* The pulse comes before TDC1.*/

const int pulsePin = 2;       // digital pin 2, referred to as interupt 0, connected to the injector line pulse detector
const int tdcPin = 3;         // digital pin 3, referred to as interupt 1 connected to flywheel TDC sensor
long TDC4;                    //TDC on cyl 4, LOW on flywheel sensor TDC4-TDC1=RPM
long TDC1;                    //TDC on cyl 1, LOW on flywheel sensor
long pulseStart;
int rpmX;                     //The X axis on the graph not needed if excel does the math
int btdcY;                    //Before top dead center The Y axis on the graph, not needed?

void setup()
{
Serial.begin(9600);
pinMode(pulsePin, INPUT);
pinMode(tdcPin, INPUT);
Serial.println("CLEARDATA");           //PLX-DAQ stuff
Serial.println("LABEL, Rpm, Temp,");   //PLX-DAQ column labels

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

}

void loop()
{
Serial.print("DATA, TIME,,");
if (pulseStart != 0 && TDC1 != 0 && TDC4 != 0);         //if all the sensors have stored data in the variables send to excel to start the math process
{  Serial.print(pulseStart);Serial.print(",");Serial.print(TDC1);Serial.print(",");Serial.println(TDC4);
}
//  (TDC1 - TDC4) = microsecrotate             Subtracts the starting event TDC4, from the final event TDC1, finding the microsecs per revolution
//  rpmX = (60000000 / microsecrotate)           60 million micro seconds divided by the total time between events gives us the RPM

//  btdcY = ((TDC1 - pulseStart) / ((TDC1-TDC4) / 360))   The timing pulse happens how many degrees before TDC1? BTDC
void Pulsetime()
{
pulseStart = micros ();
)
void Tdc()
{
if (TDC1 == 0 && pulseStart != 0)               // if the flywheel triggers was it cylinder 1 or 4
{TDC1 = micros}                             // if cyl 1, store timing of event
else if (TDC1 != 0 && pulseStart != 0)           // if cyl 4
{TDC4 = micros}                             // store timing of the event
}
pulseStart = 0                                      //clear these variable so we know if they have been triggered
TDC1 = 0
TDC4 = 0
}
}
``````

It doesn’t verify properly but I only posted it to see if you think I’m on the right track.
I’m not sure where or how I would do the averaging.

The way your code is laid out it's very hard to follow. Use the autoFormat in the IDE.

You seem only to record millis() into a single variable.

``````pulseStart = micros ();
``````

That means it gets overwritten each time and you have no previous data with which to do arithmetic.

I would plan on taking, perhaps, 20 readings and then doing my maths. So create arrays for the time values and when all 20 are filled detach the interrupts. Then you can take as long as you like to do the maths.

...R

If all you are doing is a graph of injector timing and rpm, here's all you need. I left the calculations to you. It compiles, but I did not test it, so there could be errors.

``````#define INJECTORWAIT 0
#define TDCWAIT_1 1
#define TDCWAIT_2 2

unsigned long tdcStartTime;
unsigned long injectorStartTime;
unsigned long BTDC;
unsigned long oneRev;

const int pulsePin = 2;
const int tdcPin = 3;
int rpm;

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)
}
}

void loop () {
switch( state ) {
case INJECTORWAIT:
injectorStartTime = micros();
state = TDCWAIT_1;
}
break;

case TDCWAIT_1:
tdcStartTime = micros();
BTDC = tdcStartTime - injectorStartTime;
state = TDCWAIT_2;
}
break;

case TDCWAIT_2 :
oneRev = micros() - tdcStartTime;
calc_RPM (oneRev);
display ();
state = INJECTORWAIT;
}
break;
}
}

void calc_RPM( unsigned long oneRev ) {
// calculate rpm from tdc to tdc time
}

void display() {
// Show BTDC and RPM
}
``````

Robin2, Thanks for the info on autoformatt. I found it but apperantly the sketch has to be perfect to use it and my code was just a general idea of the plan. "Too many right parenthasis" to autoformat.

The problem when I try to verify:

``````   attachInterrupt(1,Pulsetime,FALLING);   //enable interrupt attached to pin 2 pulse detector
``````

Error message "pulsetime was not declared" I know this isn't the programming fora but any help with how to declare it would be appreciated.

I also think I see what you mean by I was only recording micros into pulseStart because the others didn't have the () after micros right?

``````pulseStart = micros ();    //compared to the other variables
{TDC1 = micros}
{TDC4 = micros}
``````

Soo you suggest each interrupt routine would have a for loop to store 20 consecutive events in an array. The first rpm might read 1500rpm, the second 1550, the third 1560.

The pulse events would also be treated this way. A minimum of 20 revolutions before any result would be available, then that would get averaged and dumped to the excel program. About every 600ms @2000rpm.

Since the timing "curve" is supposed to be a straight line; I'm wondering if I can visually avareage it by seeing how straight the line is but I will be working on this in my mind in case the line is curvy.

Lar3ry, My programming skills are weak so thanks for posting a general idea of a programming solution I have concerns about doing this without interrupts. If the engine is moving at 5000 RPM and a 5mm strip of shiney material flys past the laser tach sensor, it will only be in view for 70 microseconds. 140us @2500 Is that a long time for the Arduino? Could it end up being busy during this whole time and miss it? Shouldn't it be triggered as soon as the edge arrives to be accurate? Same with the diesel pulse detector though I'm not sure how long that pulse lasts.

I really appreciate the help.

graphing: Soo you suggest each interrupt routine would have a for loop to store 20 consecutive events in an array. The first rpm might read 1500rpm, the second 1550, the third 1560.

NO!

My idea is that each time an interrupt triggers it stores the current value of millis() or micros() into the next element of a global array.

And working out RPM is for a later stage of the program - after all the times have been collected.

And a 20mm strip of shiny material would be visible for 4 times as long. Does a 140 microsecond (avg) error matter?

...R

Yeah sorry, I worded that wrong. I know rpm will be calculated in excel. It would still require a for loop in the interrupt routine to store the raw data in a global array right? I'll find out what a "global" array is.

Since it's time we're talking about; the averaging math would only require the first and last of the 20 for a total, divided by 20. Smaller array of just 2? ooorr take the first, skip 18 and read the last, subtract divide by 20?

I've read that each flywheel tooth represents 2.5degrees. That 5mm represents about 3 degrees of crank rotation. OK let's check. @ 2500 rpm, 1 rev = 60 / 2500=24ms per rev 24ms / 360degrees= 67us per degree Sooo every degree = 67us @2500rpm With a 5mm reflector it can be 3 degrees difference from the rising edge to the falling edge.

If it detects somewhere after the rising edge it can be off by over 2 degrees, with a 5mm strip. If it reads closer to the falling edge on the first reading and closer to the rising edge the next it could be 4 degrees off. I can now see how averaging 20 cycles WOULD clean this up alot but I still wouldn't use a 20mm strip and expect it to be precise.

Real problem would come when trying to subtract pulseStart from TDC1 if TDC1 can be up to 2 degrees off.

Interrupts catching the leading edge of an event seems the way to go for accuracy but averaging will reduce the need for accuracy. When it comes time to run this thing it will be fun to test.

Well, if you do want to use interrupts (yes, they will be more accurate within the detection), that's fine. But keep the code I posted in mind. The thing is, that while the tdc and injector pulse are short, they are quite far apart.

``````void tdcIRPT() {
tdcStartTime = micros();
}

void pulseIRPT() {
injectorStartTime = micros();
}
``````

Only that? Yes! you capture the time of the event in the interrupt, then when you return, you have plenty of time to change a state, calculate a time difference, calculate RPM, etc., before the next interrupt arrives. Again, if this is all the Arduino has to do, you can even make tight loops for each state, checking for the presence of a time in the appropriate variable. When it becomes non-zero, you deal with the variable. Now your loop() can look like this...

``````void loop () {

while (INJECTORWAIT) {
if (injectorStartTime > 0 ) {
state = TDCWAIT_1;
}
}

while (TDCWAIT_1) {
if (tdcStartTime > 0 ) {
tdcStartTime_1 = tdcStartTime;
state = TDCWAIT_2;
}
}

while (TDCWAIT_2) {
if (tdcStartTime > 0) {
tdcStartTime_2 = tdcStartTime;
state = INJECTORWAIT;
}
}

// You now have the three times you need for all your
// calculations.

InjectorToTDC = tdcStartTime_1 - injectorStartTime;
oneRevTime = tdcStartTime_2 - tdcStartTime_1
tdcStartTime_1 =0;
injectorStartTime = 0;
tdcStartTime_2 = 0;
}
// and around the loop() we go again.
}
``````

graphing: Yeah sorry, I worded that wrong. I know rpm will be calculated in excel. It would still require a for loop in the interrupt routine to store the raw data in a global array right? I'll find out what a "global" array is.

I have no managed to follow @Iar3ry's comments in parallel with my own and, for all I know his proposal is essentially the same.

First off, a global variable is one that is declared before setup() and is thus accessible everywhere in the program. (Local variables are declared within a function and are only accessible within that function).

You don't need a for loop to access elements of an array. You just need the appropriate index value. If the index value is also a global value initialized to 0 the content of the ISR could be as simple as

myISR() { injectorMillis [ injectorIndx ] = millis(); injectorIndx ++; }

And in loop() you could test to see if injectorIndx exceeds 20 (or whatever) and then detach the ISR.

It would probably be wise to have the array bigger than 20 in case additional interrupts add data before the detach takes effect.

I understand your thinking about just needing the first and 20th time but I assumed you might like to look at the individual intervals also. That's entirely your decision - it's not a programming issue.

...R

Keeping an array of historical values would enable you to do an accurate rolling average, but I think a simple decaying average would be all you need and would be much simpler to implement.

All you'd do is have a couple of unsigned long variables used by the the interrupt handlers to record the last interrupt time. You'd calculate the interval since the last interrupt by simple subtraction and then save the new time. The subtraction will give you an interval. You would apply a decaying average to that to calculate the smoothed value. Here's a simple decaying average:

``````average = ( (15 * average) + (1 * newValue) ) / 16;
``````

The figures 15 and 1 determine how much smoothing is applied, and the divisor (16) is just the sum of the other two values.

Thanks for all the help. I'll take some time to soak it up and write some possible code solutions. Lar3ry, I am curious about how this works: "So, all you need to do is to start with 0 in your time variables,"

``````void tdcIRPT() {
tdcStartTime = micros();
}

void pulseIRPT() {
injectorStartTime = micros();
}
``````

How do you know micros = 0? Because it's before the loop starts? Would it be better to just say tdcStartTime = 0

I'll work through the rest of the tips and come back with better questions and/or possible answers.

OK I wrote some code.
I tried to make it easier to read, more like other code I’ve seen because it’s still just a general idea and can’t be verified.:

``````/*
* Finds time from TDC1, TDC4 and pulseStart.
* First it looks for a LOW on the pulse detector to trigger and logs the timing of the event. Then looks for a LOW on the TDC sensor. TDC1 stores the timing of that event.
* Then it looks for the second TDC LOW
* The pulse comes before TDC1.*/

const int pulsePin = 2;       // digital pin 2, referred to as interupt 0, connected to the injector line pulse detector
const int tdcPin = 3;         // digital pin 3, referred to as interupt 1 connected to flywheel TDC sensor
unsigned long TDC4;                    //TDC on cyl 4, LOW on flywheel sensor TDC4-TDC1=RPM
unsigned long TDC1;                    //TDC on cyl 1, LOW on flywheel sensor
unsigned long pulseStart;
unsigned long BTDC
unsigned long aveBTDC
unsigned long averpmTime
long int rpmX;                     //The X axis on the graph not needed if excel does the math ?long?
long int rpmTime
int btdcY;                    //Before top dead center The Y axis on the graph, not needed? 35 degrees max sooo int
int average                   //variable for averaging calculations
int count                   //variable for keeping the loop count for averaging

void setup()  {
Serial.begin(9600);
pinMode(pulsePin, INPUT);
pinMode(tdcPin, INPUT);
Serial.println("CLEARDATA");           //PLX-DAQ stuff
Serial.println("LABEL, Rpm, Temp,");   //PLX-DAQ column labels
count=0
attachInterrupt(1,PulsetimeIRPT,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
void PulsetimeIRPT()  {                   // Interrupt when injector pulse detector goes LOW
pulseStart = micros();                    // puts the value from micros into the variable pulseStart
)                                                            //maybe increment the count,    count++ right here?
void TdcIRPT()  }
if (TDC1 = 0)   {                          //First time around?
TDC1 = micros()                            //store the timing
BTDC = TDC1 - pulseStart                   //We have pulseStart and TDC so do the math
else rpmTime = micros() - TDC1              //TDC isn't 0 so it's the second time around. Time per revolution is stored in rpmTime.
}                                            //can I do that or does micros have to be stuffed into a variable's skin first?
if (pulseStart != 0 && rpmTime != 0 && count < 16)  {
averpmTime = ((15 * averpmTime) + rpmTime)/ 16;
aveBTDC =    ((15 * aveBTDC) +  BTDC) / 16
}
if (count = 16) {
Serial.print(aveBTDC);Serial.print(",");Serial.println(averpmTime);
count = 0
}
pulseStart = 0                                      //clear these variable so we know if they have been triggered
TDC1 = 0
rpmTime = 0
}
``````

I tried it without TDC4 but can I do that?

``````else rpmTime = micros() - TDC1     //can I do that or does micros have to be stuffed into a variable's skin first?
``````

Also need to figure out where to increment the averaging count and how to get it to wait for the pulse detector before running another loop?

graphing: Lar3ry, I am curious about how this works: "So, all you need to do is to start with 0 in your time variables,"

``````void tdcIRPT() {
tdcStartTime = micros();
}
``````

void pulseIRPT() {      injectorStartTime = micros(); }

``````

How do you know micros = 0? Because it's before the loop starts?
Would it be better to just say tdcStartTime = 0
``````

You don't. And you don't care what micros() is at any given time. Yes, you want tdcStartTime to start with a value of 0, and that's what it has in it when it's declared. It must be declared global and volatile (volatile because it can change in the interrupt.) The other time variables should all be global and volatile as well.

Here's the code again, with some comments...

``````void loop () {
// we start by entering this while loop because of our state.
// In this loop, we check the value in injectorStartTime
// it was set to 0, and if it's still 0, we just keep looping
// in this while loop
// if it's not 0, we must have had a pulse interrupt (the injector
// pulse), so we now set the state to TDCWAIT_1 and that makes us drop
// out of thi while loop. Now we have the micros() value in
// injectorStartTime, put there by the interrupt
while (INJECTORWAIT) {
if (injectorStartTime > 0 ) {
state = TDCWAIT_1;
}
}

// Same sort of loop here. We stay in this loop until tdcStartTime
// becoames non-zero (as a result of the tdc interrupt), and
// that causes us to enter the if, set tdcStartTime_1 to the value
// we got in tdcStartTime. Then we change the state to TDCWAIT_2
// and that causes us to drop out of the while loop.
// I also made a small correction in this while loop. tdcStartTime
// should be set back to 0 for the next state.
while (TDCWAIT_1) {
if (tdcStartTime > 0 ) {
tdcStartTime_1 = tdcStartTime;
tdcStartTime = 0;
state = TDCWAIT_2;
}
}

// This is essentially the same loop as above, the only difference
// being that we set the new tdcStartTime into tdcStartTime_2.
// Note that this is the second TDC after the TDC after the cylinder 1 injector
// pulse, so the variables tdcStartTime_1 and tdcStartTime_2 are
// one revolution apart and the difference between the two values
// is the time in microseconds for one revolution.
// We then set state to INJECTORWAIT to wait for the next
// injector pulse
while (TDCWAIT_2) {
if (tdcStartTime > 0) {
tdcStartTime_2 = tdcStartTime;
state = INJECTORWAIT;
}
}

// You now have the three times you need for all your
// calculations.

InjectorToTDC = tdcStartTime_1 - injectorStartTime;  // in microseconds
oneRevTime = tdcStartTime_2 - tdcStartTime_1;     // in microseconds

// I apologize for leaving out one important variable in this bit of code.
// Here we set the time variables back to aero in order to let us
// go back through the loop, with the same initial conditions.
// At this point, we have awaited the injector pulse, two TDC
// pulses, and are all set to start again.
// Note that at this point in the code, we have OODLES of time
// before the next cylinder 1 injector pulse
injectorStartTime = 0;
tdcStartTime = 0;

tdcStartTime_1 = 0; // we don't really need to set these two variables back to zero
tdcStartTime_2 = 0;  // because we don't need to test for non-zero in the loops.
}
// and around the loop() we go again.
}
``````

Yeah, did I mention my coding skills suck. That lost me. For example. You mentioned state so I looked up state machine. You used a while command. Which will be a great help in getting the program to wait for a pulse or LOW. But my turorial shows the "functionality is simply to repeat statement while condition expressed in the statement is true"

# define INJECTORWAIT 0 sets it to 0, and that can't be changed right? It'll always be 0?

`````` while (INJECTORWAIT) {   //Since INJECTORWAIT is a const and will always be = 0 and isn't
//connected to a pin or anything it will always = 0 right?
if (injectorStartTime > 0 ) {         //shouldn't InjectorStartTime = digitalRead(buttonPin)  or something?
state = TDCWAIT_1;                    // Shouldn't micros() be used somewhere to put the time into a variable?
}[code]

And what is the purpose of this line?
byte state = INJECTORWAIT;
``````

[/code]

I've done some more state machine searching. I'm on dialup. It takes 6 minutes for this topic to load and there are many posts on this forum I can't get to ever load. utube is out of the question. So I'll be using links from other forums to help people better understand what Lar3ry is doing. Many of the links were promoting a state machine library. None I found used while instead of switch case,.. but I did notice some similarities with lar3ry's example. This one was some help: http://hacking.majenko.co.uk/finite-state-machine It helped me understand why INJECTORWAIT and the others were defined as a set value. As a label for each possible state, instead of a number that doesn't identify it as clearly.

Possible answer to my own question of "And what is the purpose of this line? byte state = INJECTORWAIT;" Other than initializing state as a byte instead of int, etc., I think that might be pointing to which state to check first. and I believe putting "state=" at the end of the while statement points to which state to check next.

As I understand more I'll post it for those who find this in a search. Thanks lar3ry for pointing me in a direction that should get this to work well.

graphing:

# define INJECTORWAIT 0 sets it to 0, and that can't be changed right? It'll always be 0?

`````` while (INJECTORWAIT) {   //Since INJECTORWAIT is a const and will always be = 0 and isn't
//connected to a pin or anything it will always = 0 right?
if (injectorStartTime > 0 ) {         //shouldn't InjectorStartTime = digitalRead(buttonPin)  or something?
state = TDCWAIT_1;                    // Shouldn't micros() be used somewhere to put the time into a variable?
}
``````

And what is the purpose of this line? byte state = INJECTORWAIT

You are right. In my haste to change my pseudo-code to real code, I did not change those while() conditions.

You will want:

``````    while (state == INJECTORWAIT)  {
if (injectorStartTime > 0 ) {
state = TDCWAIT_1;
}
``````

//shouldn't InjectorStartTime = digitalRead(buttonPin) or something? // Shouldn't micros() be used somewhere to put the time into a variable?

No, because the injector interrupt routine already ran because of a pulse on that pin. and No, because the interrupt routine already grabbed the micros() value at the time that interrupt happened.

Here's the interrupt routine:

``````void pulseIRPT() {
injectorStartTime = micros();
}
``````

So, when you get to loop(), the first while() runs until the injector interrupt happens, causing injectorStartTime to be set to the micros() value and become > 0), and state becomes TDCWAIT_1.

I can’t find the lightbulb icon so you’ll just have to imagine a lit bulb over my head,… and maybe a microwave ding.
I think it finally soaked in. Thanks so much for your help and patience.
The interrupt isn’t triggering the states,… the fact that the interupt placed a value into a variable is what they are looking for.

``````/* 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
#define AVERAGEWAIT

unsigned long tdcStartTime;
unsigned long tdcStartTime_1;
unsigned long tdcStartTime_2;
unsigned long injectorStartTime;
unsigned long Btdc;
unsigned long aveBtdc;
unsigned long oneRevTime;
unsigned long aveRevTime;
unsigned long btdcY;
unsigned long rpmX;

const int pulsePin = 2;
const int tdcPin = 3;
int count = 0;
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
void tdcIRPT() {                             // the interrupt routines
tdcStartTime = micros();
}
void pulseIRPT() {
injectorStartTime = micros();
}
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;
state = AVERAGEWAIT;
}
}
while (state == AVERAGEWAIT) {                    // You now have the three times you need for all your calculations.
Btdc = tdcStartTime_1 - injectorStartTime;        // BTDC in microseconds
oneRevTime = tdcStartTime_2 - tdcStartTime_1;     // time for one revolution in microseconds
injectorStartTime = 0;                            // gets zeroed every time
tdcStartTime = 0;
if (count < 16)  {
aveRevTime = ((15 * aveRevTime) + oneRevTime)/ 16;                //a decaying average allows me to do this without an array
aveBtdc =    ((15 * aveBtdc) +  Btdc) / 16;
count++;
else {
rpmX = (60000000 / aveRevTime);                                  //oodles of time so calculate RPM
btdcY = ((aveBtdc) / ((oneRevTime) / 360));                      //don't waste the oodles
Serial.print(btdcY);Serial.print(",");Serial.println(rpmX);      //Dumps the average results into excel columns D&E
count = 0;                                                       // zeroing variables that get used for averaging
state = INJECTORWAIT;                                           //sends it back to the beginning to wait for an injector pulse
}
}
}
// and around the loop() we go again.
}
``````

Not sure I had to set up another state “AVERAGEWAIT” to do these last calculations.
Could I have written it this way?

``````while (state == TDCWAIT_2) {
if (tdcStartTime > 0) {
tdcStartTime_2 = tdcStartTime;
//without the state=INJECTORWAIT here it should just go to the next line right?
}
}             // You now have the three times you need for all your calculations.
Btdc = tdcStartTime_1 - injectorStartTime;        // BTDC in microseconds
oneRevTime = tdcStartTime_2 - tdcStartTime_1;     // time for one revolution in microseconds
injectorStartTime = 0;                            // gets zeroed every time
tdcStartTime = 0;
if (count < 16)  {
aveRevTime = ((15 * aveRevTime) + oneRevTime)/ 16;   //a decaying average allows me to do this without an array
aveBtdc =    ((15 * aveBtdc) +  Btdc) / 16;
count++;
else {
rpmX = (60000000 / aveRevTime);                                  //oodles of time so calculate RPM
btdcY = ((aveBtdc) / ((oneRevTime) / 360));                      //don't waste the oodles
Serial.print(btdcY);Serial.print(",");Serial.println(rpmX);      //Dumps the average results into excel columns D&E
count = 0;                                                       // zeroing variables that get used for averaging
state = INJECTORWAIT;                                           //sends it back to the beginning to wait for an injector pulse
``````

When I try to verify I still get," ‘pulseIRPT’ was not declared in this scope"
I can’t find anything that mentions how to declare it.

When I try to verify I still get," 'pulseIRPT' was not declared in this scope" I can't find anything that mentions how to declare it.

Functions cannot be declared within functions. You start (usually) with two functions in your sketch. They are called setup() and loop(). They are supplied by the Arduino IDE, which calls them from within functions that the IDE uses to set things up so it's easier for folks to write code.

A function declaration looks like these (comments in the code):

``````// this function does not return a value, nor does it take any arguments.
// you see this one all the time. it will run once, then loop() will be called, which keeps on running

void setup () {
}

// this is a trivial function that adds five to a variable that is passed to it
// and returns the new value

int addFive ( int startval)  {
startval += 5;
return startval;
}

// you can call the above function like this (assume all variable passed are declared as int)

cakes = addFive ( cakes);  // adds 5 to the variable cakes
newval = addFive(oldval);  // adds 5 to the variable oldval, and assigns the result to newval

// obviously, these could be done much more easily in other ways, but you can pass multiple arguments
// to a function (the parameters must be declred, of course), and the function can be quite complex

unsigned long frob ( long diddle, int farkle, unsigned int hiccup) {
// here we might find calculations based on the contents of the three arguments,
// and the function can be very complex indeed, with if, switch, while statements,
// if might have IO (serial, I2C, SPI, etc.), and so on
//  and at the end, it will have a
return variablename;
}
``````

So, if you can't declare a function inside another function, how do you declare it?

Well, it's like this...

``````void setup() {
// all your setup stuff goes here, including calls to functions, if any
}

void loop() {
// all your loop stuf goes here, including calls to any functions, if any
}

void tdcIRPT() {                             // the interrupt routines
tdcStartTime = micros();
}

void pulseIRPT() {
injectorStartTime = micros();
}
``````

You expressed a desire to have the following post moved to this thread from your other one. I don't have credentials to move replies, so here's a duplicate.

``````void loop () {
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.

``````      //without the state=INJECTORWAIT here it should just go to the next line right?
``````

Not at all sure which statement you are referring to. You have placed some code after the TDCWAIT_2 loop. Are you wanting to remove the statement setting INJECTORWAIT in the TDCWAIT_2 loop? If so, yes, it will just drop through into the next bit of code. That next bit of code, however, if it does not depend on a state that you set in TDCWAIT_2, will execute EVERY TIME through loop(), whether or not it has gathered all the data you are looking for.

I would either do as you have done; setting another state (you used AVERAGEWAIT for some unknown reason. You are not waiting for anything), or, alternatively, do your calculations within TDCWAIT_2. Better still, write a function that takes Btdc pneRevTime, calculated in TDCWAIT_2, call it from within TDCWAIT_2, and in that function, do anthing you want with the data.. average if you want (no idea why you want to), display it, and so on.