Ignition using the blink without delay principle

Projects General Guidance
1

Hello Forum.

I want to advance the ignition timing in a linear or close to linear fashion on my pit bike to increase performance.

My thought is that you could intercept the signal from the pickup on the flywheel that goes to the CDI box with a microcontroller. The point is to calculate the RPM of the engine and alter the signal to the cdi box so that it reaches the cdi box before the actual signal is given by the pickup.

The way i was going at this was, I would have the microcontroller count how long it takes between two signals from the pickup. Then i would make an array that has a given ignition time (Maybe a percentage of the milliseconds it takes for any given revolution?) to use for every RPM.

The program needs to keep track of the RPM by counting every pickup signal and calculating the RPM, and in between the signals, it needs to also fire the spark by enabling an output to the CDI's Input. The ignition output should be enabled according to the values given by the array, in the form of some milliseconds after the last pickup signal was recieved.

It seems to me that you have to make the microcontroller keep track of 2 different things at the same time, i find it hard to grasp which tools i should use to accomplish that.

Could i somehow use a modified version of the blink without delay code? I want the code for every interrupt to first write the time (milliseconds) since last rotation to an and resetthe timing.

Replies will be appreciated!

Can someone translate this into code for me please

CDI_advance.ino (395 Bytes)

2

Keeping track of different things will present no difficulty. Have a look at the demo Several Things at a Time

I'm not sure I properly understand your proposed use of the array. Is it your intention to have different amounts of advance at different RPMs? If so, perhaps that can simply be calculated from the speed

Assuming you can detect the TDC signal from your flywheel the Arduino can measure the time between pulses to determing the speed and can cause an I/O pin to toggle at any time you choose after detecting a pulse. I am using that sort of thing to measure the speed of a small DC motor - each pulse causes an interrupt and the Interrupt Service Routine (ISR) records the value of micros(). Subtracting the present value from the previous value gives you the time for a revolution. This is the code in my ISR

void sensorInterrupt() {

	unsigned long ISRmicros;
	static unsigned long prevISRmicros;
	ISRmicros = micros();
	ISRinterval = (micros() - prevISRmicros);
	prevISRmicros = ISRmicros;
	ISRrevCount ++;
	newISR = true;
}

When the main part of the code sees that newISR is true it knows there has been another pulse.

I think modern engines may produce a lot more than one pulse per revolution.

Whether this has any beneficial impact on your bike's performance is another matter entirely.

...R

3

Thank you for the reply.

Yes i want to advance the ignition as the rpms go up.

Im having a hard time implementing this into my sketch, i need to have other things in mind when im writing the flywheel interval code. Like, I need to be able to point to the array and be able to use the Interval when writing the code for the ignition output.

4

ettla:
Im having a hard time implementing this into my sketch, i need to have other things in mind when im writing the flywheel interval code. Like, I need to be able to point to the array and be able to use the Interval when writing the code for the ignition output.

I can't figure what might be in your mind unless you post the code you are trying.

If you have not got to the stage of writing actual code you could write it out in pseudo code - i.e. something that looks like a bit like code and expresses the calculations without being legitimate C/C++ code.

This is just a wild guess but maybe you need to reduce the RPM to a number - for example 0-500 = 0; 501-1000 = 2 etc. And then use those numbers as an index into the array.

By the way, I would keep the code simple and just use microsecs per rev rather that waste resources calculating RPM.

...R

5

Thank you so much for responding.

Your advice is really helping, it was 2 years since i coded even a little bit of python code so i dont have the basic thinking of coding in my mind yet.

Im thinking of starting at it with general youtube lessons all over again. It's always like this for me, i need to know some programming for a small project and i finally learn it, then i forget it until i come up with another fantastic idea that i need to learn programming again for.. its so sad that i can't remember how i was thinking a year ago.

I can't even write pseudo code properly where i'm at, you see, the way of thought i have does not corresopond with the syntax of the programming language, that is something that you need to learn all over again, how the program thinks.

Because this is all that i have come up with so far doing pseudo code writing.

void setup() {
// put your setup code here, to run once:

}

void loop() {attach interrupt) pin 2
Write the milliseconds since last interrupt to int RPM
Restart the millisecond timer
(

check if sparkTime since last interrupt
has reached its value for the current RPM in the array
If it has, then send spark output high.
Else restart loop
// put your main code here, to run repeatedly:

}

6

The point about pseudo code is that you can use your own system - it is just a way of writing down the logical steps that are required to happen. I find it a very effective way of clarifying my own thoughts.

If you are familiar with Python you can use that style for your pseudo code.

...R

7

Even experienced programmers fall back to pseudocode!
When thatsimple series if calculations runs out the wrong result, the code is broken out on paper into meaningful words... then re-evaluated sequentially in the devs mind to rewrite the code.
Perhaps 2-3 lines of simple, readable logic to replace one 'clever' mistake!

8

Hey again! Guys! i have made some serious progress with my project, i am almost done with a working code, the only thing i haven't implemented is an array with the different sparkTimes for different RPM, but i have written everything down on paper, what i want it to look like :smiley:

BUT! I have a come across a big problem, something is wrong with the code and i can not figure out what it is, in my mind everything should work out fine! But when i put a button as my input on pin 2 and press it, i get absolutely no output from pin 13, i think... Atleast it is not visible output for me.

As i am new to troubleshooting code, could someone give me a hint of where to start? Should i try to grab some info through serial print of the values i get from different parts of the code? if so could someone point to where i should start looking so that i get in the right direction here?

I am totally lost after spending all afternoon and night on developing this code.

This is how far i've come:

volatile int sparkPin = 13;      // the number of the Spark pin
volatile int pickupPin = 2;       //the number of the input Pickup signal pin
volatile int sparkState = LOW;             // sparkState used to fire the spark
volatile int sparkDuration = 100;          /*Sparkduration used to limit the spark duration to 0,05 milliseconds
                                  (how do i avoid float when i need a decimal of milliseconds?)
                                  oh wait... i know.. 1/20=0,05*/
volatile long sparkPrevious = 0;      //will store last time pickup signal was recieved or sparkState was high
volatile long revPrevious = 0;        // will store last time pickup signal was high
volatile long currentMillis = millis();
volatile int revTime = 120;
volatile float sparkTime = 0.9861*revTime;

void setup() 
{
  // set the digital pin as output:
  pinMode(sparkPin, OUTPUT);     
  pinMode(pickupPin,INPUT); 
  attachInterrupt(2, newRev, RISING);    //i read something about returned millis cannot increment, do i need to worry?
}

void loop()
{
    // check to see if it's time to change the state of the sparkPin
  if((sparkState == HIGH) && (currentMillis - sparkPrevious >= sparkDuration))
  {
    sparkState = LOW;  // Turn it off
    digitalWrite(sparkPin, sparkState);  // Update the actual spark
  }
  if ((sparkState == LOW) && (currentMillis - sparkPrevious >= sparkTime))
  {
    sparkState = HIGH;  // turn it on
    sparkPrevious = currentMillis;   // Remember the time
    digitalWrite(sparkPin, sparkState);    // Update the actual spark
  }
}
void newRev() {
    revTime = currentMillis - revPrevious;
    revPrevious = currentMillis;
    sparkPrevious = currentMillis;    
  }
9

Now i got the majority of the code to work! It was that currentMillis did not count inside Loop because i had not declared currentMillis = Millis() Inside loop.

My problem now is that my interrupt does not work!

How can i solve this?

volatile int sparkPin = 13;      // the number of the Spark pin
volatile int pickupPin = 2;       //the number of the input Pickup signal pin
volatile int sparkState = LOW;             // sparkState used to fire the spark
volatile int sparkDuration = 10;          /*Sparkduration used to limit the spark duration to 0,05 milliseconds
                                  (how do i avoid float when i need a decimal of milliseconds?)
                                  oh wait... i know.. 1/20=0,05*/
volatile long sparkPrevious = 5000;      //will store last time pickup signal was recieved or sparkState was high
volatile long revPrevious = 0;        // will store last time pickup signal was high
long currentMillis = millis();
volatile long revTime = 120;
volatile float sparkTime = 0.9861*revTime;

void setup() 
{
  // set the digital pin as output:
  pinMode(sparkPin, OUTPUT);     
  pinMode(pickupPin , INPUT); 
  attachInterrupt(pickupPin, newRev, RISING);    //i read something about returned millis cannot increment, do i need to worry?
  Serial.begin(9600);
}

void loop()
{ 
 int val = digitalRead(pickupPin);
  Serial.println(val);
  currentMillis = millis();
    // check to see if it's time to change the state of the sparkPin
  if((sparkState == HIGH) && (currentMillis - sparkPrevious >= sparkDuration))
  {
    sparkState = LOW;  // Turn it off
    digitalWrite(sparkPin, sparkState);  // Update the actual spark
  }
  if ((sparkState == LOW) && (currentMillis - sparkPrevious >= sparkTime))
  {
    sparkState = HIGH;  // turn it on
    sparkPrevious = currentMillis;   // Remember the time
    digitalWrite(sparkPin, sparkState);    // Update the actual spark
  }
}
void newRev()
 {
  revTime = currentMillis - revPrevious;
  Serial.println(revTime);
  revPrevious = currentMillis;
  sparkPrevious = currentMillis;
}
10

Don't try to Serial.println() inside of the ISR. It won't work/

11

All variables associated with millis() should be defined as unsigned long.

You should have a working variable that can copy the value from the ISR so that the ISR can change it again without screwing up further calculations. For example

void loop()
{
 int val = digitalRead(pickupPin);
 noInterrupts();
   unsignedLong wkgSparkPrevious = sparkPrevious;
 interrupts();
 // etc - and use wkgSparkPrevious everywhere outside the ISR

It is essential to turn interrupts off briefly so that the value of sparkPrevious does not change while you are copying it.

It might also be a good idea to include a flag variable in the ISR such as

newPulse = true;

and only read the value from the ISR when there is a new value.

You seem to have revPrevious and sparkPrevious in the ISR both with the same value. That seems just a waste of CPU cycles.

You might like to look at how the ISR is dealt with in the .ino code in this Link

...R
PS I missed the Serial.print - that must be taken out of the ISR.

12

Robin2:
All variables associated with millis() should be defined as unsigned long.

I want to have a negative value rather than positive 4000000000 when comparing

if ((sparkState == LOW) && (currentMillis - sparkPrevious >= sparkTime)

What i mean is that with "unsigned i get "currentMillis - sparkPrevious = 4000000000 something"
But with "long" i get "currentMillis - sparkPrevious = -5000"

"currentMillis - sparkPrevious" needs to be less or equal to "sparkTime", what is your opinion on that? Is there another way to make it work like this?

You should have a working variable that can copy the value from the ISR so that the ISR can change it again without screwing up further calculations.

For example

void loop()

{
int val = digitalRead(pickupPin);
noInterrupts();
  unsignedLong wkgSparkPrevious = sparkPrevious;
interrupts();
// etc - and use wkgSparkPrevious everywhere outside the ISR



It is essential to turn interrupts off briefly so that the value of sparkPrevious does not change while you are copying it.

Aha! That seems to what have been screwing some things up for me! I have not yet completed the implementation of your proposed ISR system, i do get the interrupts just fine, but the other part of the code seems to not do what it is supposed to with the new variables. I will have to dig into that, could you guide me to an optimal way to troubleshoot my code with serialprints? Where is the best places to put serial print in my code?

noInterrupts();

unsignedLong wkgSparkPrevious = sparkPrevious;
interrupts();




It is essential to turn interrupts off briefly so that the value of sparkPrevious does not change while you are copying it.

So this so called "noInterrupts" and "Interrupts" are supposed to mean like, "detachInterrupt" and then "AttachInterrupt" is that what you ment here?

It might also be a good idea to include a flag variable in the ISR such as

newPulse = true;

and only read the value from the ISR when there is a new value.

What is the point of this? Will it make performance benefits? I am right now trying to replace my old interrupt with the one you provided in the link.

this is now the beginning of my loop, i even got rid of the serial.print in the ISR :slight_smile:

void loop()
{ 
  if(! newISR) return;

   
   Serial.println(revTime);
    revTime = ISRrev;
    revPrevious = prevISRmillis;
    sparkPrevious = prevISRmillis;
    newISR = false;

And this is the interrupt Routine with a kind of debounce i found online, what do you think of the debounce, is there a better way of doing it, hardware or software?

void sensorInterrupt()
{
 static unsigned long last_interrupt_time = 0;
 unsigned long interrupt_time = millis();
 // If interrupts come faster than 200ms, assume it's a bounce and ignore
 if (interrupt_time - last_interrupt_time > 10) 
 {
   unsigned long ISRmillis;
    static unsigned long prevISRmillis;
    ISRmillis = millis();
    ISRrev = (millis() - prevISRmillis);
    prevISRmillis = ISRmillis;
    newISR = true;
 }
 last_interrupt_time = interrupt_time;
}

You seem to have revPrevious and sparkPrevious in the ISR both with the same value. That seems just a waste of CPU cycles.

Both of them needs to "remember" every interrupt, but only one of them (sparkPrevious) needs to also remember when sparkState went high. I see what you mean, but i haven't figured out how to make them 2 different values outside of the ISR.

13

ettla:
I want to have a negative value rather than positive 4000000000 when comparing

No you don't

"currentMillis - sparkPrevious" needs to be less or equal to "sparkTime", what is your opinion on that? Is there another way to make it work like this?

Less than or greater than depends on the logic of your system. Do you want to do something very frequently before the time expires or do you just want to do it once after the time expires?

Where is the best places to put serial print in my code?

Anywhere it is useful, but not in the ISR.

So this so called "noInterrupts" and "Interrupts" are supposed to mean like, "detachInterrupt" and then "AttachInterrupt" is that what you ment here?

noInterrupts() pauses all interrupts without upsetting anything else. detachInterrupt() disconnects the interrupt from the ISR and stops that interrupt from being asserted.

What is the point of this? Will it make performance benefits?

Possibly - it stops wasting time when there is no new data. It may also have logical benefits.

It's late where I am and I'm tired so I haven't studied the rest of your Post.

Maybe just ask one or two questions at a time in future.

...R

14

noInterrupts() pauses all interrupts without upsetting anything else. detachInterrupt() disconnects the interrupt from the ISR and stops that interrupt from being asserted.

Ok, i did not know that. Thanks for teaching me!

Less than or greater than depends on the logic of your system. Do you want to do something very frequently before the time expires or do you just want to do it once after the time expires?

I want to do something once after the time has expired!

I am so frustrated at how my project is stuck right where i thought i would be done when i started the whole thing...

But now i have a working prototype!
A Fan with a glued magnet and a hall sensor attached to the fan connected to the interrupt.. Now i just have to troubleshoot the code until it works as expected.

I only have one input and one output:

INPUT:The Hall effect as input every revolution

OUTPUT: Pin 13 LED to represent the spark.

15

Can someone help me to make the code do what it's supposed to?

I want the code to work like this:

1.A falling edge interrupt should be necessary for the output to go high at any time.

2.The output should go high only once for every interrupt (revolution of the engine)

3.The output should go high a specified count of milliseconds after the last interrupt given by the variable sparkTime.

4.The output should go high only for a maximum duration set by the variable sparkDuration, and then go back to low and wait for the next interrupt and sparkTime.

right now the code can go through the loop and set the output high even without a signal from the sensor interrupt.

Thank you in advance!

16

ettla:
right now the code

Post the latest version of your code so we can keep up with you.

...R

17

Robin2:
Post the latest version of your code so we can keep up with you.

...R

Thank you so much Robin! My name is also Robin Btw, just a coincidence. Only thing is that one Robin is good at posting, and the other is not :wink:

Also, i would like to know if i have missed out on something equally important as everything else!

const int sparkPin = 13;      // the number of the Spark pin
const int pickupPin = 3;       //the number of the input Pickup signal pin
volatile int sparkState = LOW;             // sparkState used to fire the spark
const int sparkDuration = 50;          /*Sparkduration used to limit the spark duration to 0,05 milliseconds
                                  (how do i avoid float when i need a decimal of milliseconds?)
                                  oh wait... i know.. 1/20=0,05*/
volatile long sparkPrevious = 5000;      //will store last time pickup signal was recieved or sparkState was high
volatile long revPrevious = 0;        // will store last time pickup signal was high
long currentMillis = millis();
volatile long revTime = 120;
volatile float sparkTime = 0.9861*revTime;

volatile unsigned long ISRrev;
volatile bool newISR = false;
static unsigned long prevISRmillis;

void setup() 
{
  // set the digital pin as output:
  pinMode(sparkPin, OUTPUT);     
  attachInterrupt(1, sensorInterrupt, FALLING);    //i read something about returned millis cannot increment, do i need to worry?
  Serial.begin(9600);
}

void loop()
{ 
  currentMillis = millis();
  if(! newISR) return;

   
   
    revTime = ISRrev;
    Serial.println("milliseconds per revoulution:");
    Serial.println(revTime);
    revPrevious = prevISRmillis;
    sparkPrevious = prevISRmillis;
    newISR = false;
    
    // check to see if it's time to change the state of the sparkPin
  if((sparkState == HIGH) && (currentMillis - sparkPrevious >= sparkDuration))
  {
    sparkState = LOW;  // Turn it off
    digitalWrite(sparkPin, sparkState);  // Update the actual spark
  }
  if ((sparkState == LOW) && (currentMillis - sparkPrevious >= sparkTime))
  {
    sparkState = HIGH;  // turn it on
    sparkPrevious = currentMillis;   // Remember the time
    digitalWrite(sparkPin, sparkState);    // Update the actual spark
  }
}
void sensorInterrupt()
{
 static unsigned long last_interrupt_time = 0;
 unsigned long interrupt_time = millis();
 // If interrupts come faster than 200ms, assume it's a bounce and ignore
 if (interrupt_time - last_interrupt_time > 10) 
 {
   unsigned long ISRmillis;
    static unsigned long prevISRmillis;
    ISRmillis = millis();
    ISRrev = (millis() - prevISRmillis);
    prevISRmillis = ISRmillis;
    newISR = true;
 }
 last_interrupt_time = interrupt_time;
}

CDI_advance_NewISR_Debounce.ino (2.33 KB)

18

OT: if it's worth any karma points, I was named after Christopher Robin ... :wink:

19

I have had a look at your program alongside your comments in Reply #14

I have a few questions that will help me to understand your requirement.

I'm not sure why your ISR does anything other than record the time and signal that it has occurred. I think all thee rest of the stuff can be done outside the ISR.

Have you an actual problem with bouncing? Have you any idea what causes it? Have you tried capturing 100 succesive times into an array and when they have all been captured printing the array so you can see how the interrupt is behaving?

Why does the HIGH have to last longer than a few microseconds? Isn't it intended to trigger a spark at a precise time?

Why do you need a variable HIGH time?

What is the maximum RPM that you must deal with?

...R

20

I had a problem with debouncing when i had a button instead of a hall switch connected to the interrupt, but i dont know now if i have a problem with debouncing. I will probably try to do what you just mentioned to see if i have a problem.

I haven't accually played around with arrays before so that is one thing i'll have to look into as it will come in handy. Is it relatively easy to capture values into an array, comared to other stuff i have accomplished in this project?

The duration of the HIGH state should be played with back and forth when testing to be able to find the best result, a single spark can ignite the whole fuel mixture but as the capacitor again charges up within the same "spark duration" another spark can be unleached within 100 microseconds and MAYBE in theory further accelerate the flamefront. (usually there can be multiple sparks within the same spark)

http://spdispark.com/pages/about-spdi-spark
Aside from the marketing it is good information about CDI systems.

None of my applications will exceed 15k RPM for sure, maybe not even over 12k but maybe for one machine i will need 11k RPM.