Go Down

Topic: Missing pulse detector misses pulse! (Read 2270 times) previous topic - next topic

derryuk

I wrote previously here about wanting to build a small ignition unit for my motorbike. The key element with these units is generating a method of knowing where the crankshaft is so as to be able to generate a spark at the appropriate time.

One method used by car manufacturers is to have a toothed wheel on the crankshaft in the form of 36-1. That is the wheel has 36 teeth with one tooth missing so generates a gap. Using a suitable sensor then generates a square wave where each cycle=10 deg of crankshaft rotation.

I already had a simulator to generate a 36-1 waveform so wrote a short piece of code to capture the gaps and check on the Arduino's performance. Sadly before I reached my required max rpm figure (9000) it was missing gaps. The software generates a pulse on seeing a gap, this makes it easy to compare with the original on my oscilloscope.

One though was that the simulator I had was perhaps troublesome. So I bought another Arduino and used that to generate the 36-1 waveform.

Following further tests I have discovered that the system is stable up to 4100 rpm which equates to a pulse width of 204uS. At 4150 rpm 201uS it starts missing gaps. This would mean in the real world that the board wouldn't generate all the rquired ignition pulses.

The board will detect up to 9000 rpm but increasing misses more gaps.

The software below writes an asterisk to the serial port when a gap is missed. Interestingly there is a cycle of initially 12 seconds to the missed gaps ie several asterisks will appear and then no more for about 12 seconds. As the rpm is increased this delay reduces so by about 4500 rpm the delay is 6 seconds. By about 5000 rpm there is no delay and continuous but irregularly spaced asterisks are generated.

So it looks like I have hit a limitation with the Arduino software and need to go to assembler to regain performance required.

The software I used is:
/*  This routine takes a 36-1 signal in pin7
and puts a pulse out on pin 13 when the gap passes by
*/

int inpin=7, outpin=13;
unsigned long old,new1,oldtime,newtime,savedtime;

void setup()
{
 pinMode(inpin,INPUT);
 pinMode(outpin,OUTPUT);
 old=1;                                     //size of last gap
 new1=1;  //size of current gap
 oldtime=millis();                     //get time since program start
 newtime,savedtime=1;
 Serial.begin(9600);
}

void loop()
{
  new1=pulseIn(inpin,LOW);                  //get pulsewidth in usec
  if (new1 != 0)                            //if no signal within 1 sec counter timed out so zero
  {
   if (new1 > 2.9*old && new1 < 3.1*old)  //if within 10% of 3xoldgap we must be at large gap
   {
     digitalWrite(outpin,HIGH);           //write an out pulse
     new1=pulseIn(inpin,LOW);             //wait til next pulse passes - this defines outpulse width
     digitalWrite(outpin,LOW);
     newtime=millis()-oldtime;
     if (newtime > 1.5*savedtime) {
     Serial.print("*");}
     oldtime=millis();
     savedtime=newtime;
   }
   else
   {
     old=new1;                     //else just save the gap size - this allows for a change in gap size
   }                                      //during accel/deaccel
 }
 else
 {
   old=1;                                 //if no signal reset old to prevent random activity
 }
}

mem

#1
Jun 09, 2008, 06:06 pm Last Edit: Jun 09, 2008, 06:07 pm by mem Reason: 1
You probably don't need to use assembler to get the performance you need.  You can reduce a lot of unnecessary cycles by making your math routines more efficient. Avoiding the floating point calculation should bring significant performance improvements, try:

  unsigned long  new1L = new1 * 10;
  if ( new1L  > (29 * old)  &&  new1L  < (31L * old))  
instead of
 if (new1 > 2.9*old && new1 < 3.1*old)  //if within 10% of 3xoldgap we must be at large gap

and
 if (newtime > (savedtime +( savedtime / 2)) {
instead of
 if (newtime > 1.5*savedtime) {


also, not sure how long your loop takes to repeat but you may want to increase the baud rate so that you don't overflow the hardware serial port buffer.

Quote

 if (newtime > (savedtime +( savedtime / 2))

Maybe also replace this with

if (newtime > (savedtime + (savedtime >> 1)))

I'm not sure if the compiler is smart enough to detect division by a power of two, but I never like to take the chance.  If for some reason it's not, division takes a whole lot longer than bit-shifting, which takes a single instruction cycle to perform on a byte.

Also, if possible, work with unsigned integers (or even unsigned chars) rather than longs (they're half the size and hence the mathematical operations involving them are much faster).  For example, you are expecting your pulses to be only a few hundred microseconds, so you should be able to safely cast the result of the pulseIn() call to an unsigned int.

- Ben

mem

#3
Jun 09, 2008, 07:55 pm Last Edit: Jun 09, 2008, 07:56 pm by mem Reason: 1
Quote
Quote

 if (newtime > (savedtime +( savedtime / 2))

Maybe also replace this with

if (newtime > (savedtime + (savedtime >> 1)))

I'm not sure if the compiler is smart enough to detect division by a power of two, but I never like to take the chance.  If for some reason it's not, division takes a whole lot longer than bit-shifting, which takes a single instruction cycle to perform on a byte.

Also, if possible, work with unsigned integers (or even unsigned chars) rather than longs (they're half the size and hence the mathematical operations involving them are much faster).  For example, you are expecting your pulses to be only a few hundred microseconds, so you should be able to safely cast the result of the pulseIn() call to an unsigned int.

- Ben


Hi Ben, the compiler used by the Arduino IDE is smart enough to use shifts when multiplying or dividing by a power of two  ;)

In the app in question, the pulses will fit in an unsigned int (for speeds of at least 15pm or so)  but will overflow doing the multiplication to check the range. Perhaps it could be speeded up using unsigned ints if another method was used to check if the range is within 10% of 3 times the oldgap. But lets see if just eliminating floating point is enough

neuron_upheaval

Make sure the digital input pin is HIGH before calling pulseIn(pin, LOW).

Otherwise it may not measure the pulse width correctly.

melka

If it still not works, could he try with a faster crystal ? (the answer to this question is for me too ^^)
http://melka.one.free.fr/blog/
http://www.flickr.com/photos/melkaone/

mem

#6
Jun 13, 2008, 04:20 pm Last Edit: Jun 13, 2008, 04:22 pm by mem Reason: 1
Quote
Make sure the digital input pin is HIGH before calling pulseIn(pin, LOW).

Otherwise it may not measure the pulse width correctly.


The pulseIn routine does that for you.
It checks and waits for an already started pulse to finish before starting to measure.

The source code is in  wiring_pulse.c if you want to see how it does it.

neuron_upheaval

Quote

The pulseIn routine does that for you.
It checks and waits for an already started pulse to finish before starting to measure.

The source code is in  wiring_pulse.c if you want to see how it does it.


That's true, I didn't know that! Thanks.
But there's something I couldn't figure out: the pulseIn() function expects three arguments--the digital pin #, the state, and the timeout.
What happens if you don't specify the timeout value? What value does pulseIn() assume it to be?

Quote
What happens if you don't specify the timeout value?

You get a compiler error?  I haven't tried pulseIn(), but looking at the code in wiring.h and wiring_pulse.c, it looks like you don't have the option of calling it pulseIn() with only two arguments...

- Ben

mellis


I take back my previous statement, then.  Where is the two-argument version of pulseIn() defined?  I can't find it in the wiring files...

- Ben

mem

#11
Jun 15, 2008, 08:32 am Last Edit: Jun 15, 2008, 08:33 am by mem Reason: 1
the two-argument version of pulseIn() is defined in WProgram.h

unsigned long pulseIn(uint8_t pin, uint8_t state, unsigned long timeout = 1000000L);

Ah, thanks.  I was seeing the pulseIn() prototype in wiring.h and it definitely wasn't giving a default value for timeout!

- Ben

Go Up