MPGuino release one workspace

[rmccomiskie]

I think I've identified the source of the erroneous GPH readings. To recap, I'm running a Fundamental Logic MPGuino in my 97 Audi A6. I tapped into an injector for the fuel flow signal. The MPGuino seems to run normally except when coasting. In the instant display, when coasting down a hill, the GPH reading goes to a very large value and the MPG does the same.

Looking at the injector signal when coasting shows that the pulses become very narrow. The ECU appears to be shutting off the injectors. That makes sense for fuel economy reasons. I assumed that the MPGuino was not handling the lack of pulses well.

I observed the behavior of the code by inserting debug variables and displaying them on the MPGuino. Backtracking from the instantgph() routine, I found that the origin of the very large GPH number was in the external interrupt routine "processInjClosed()". That routine executes on the trailing edge of every injector pulse.

The processInjClosed() routine calculates the width of the injector pulse and subtracts the injector settling time. It then updates trip and instant variables with the pulse time and count. It's the pulse width calculation that causes the problem.

Here's the calculation in v0.75:

void processInjClosed(void){      
  long t =  microSeconds();
  long x = elapsedMicroseconds(injHiStart, t)- parms[injectorSettleTimeIdx];       
  if(x >0)
    tmpTrip.injHius += x;       
  tmpTrip.injPulses++;      

  if (tmpInstInjStart != nil) {
    if(x >0)
      tmpInstInjTot += x;     
    tmpInstInjCount++;
  } else {
    tmpInstInjStart = t;
  }
  
  tmpInstInjEnd = t;
}

The variable "t" is the time right now when the injector closes. Var "x" is the calculated elapsed time of the pulse. "injHiStart" is the beginning time of the pulse. The parameter "injectorSettleTimeIdx" is the settling time. The settling time represents the opening of the injector valve before any fuel starts to spray. In my case, the settling time is set to 500uS. injHiStart is subtracted from t to get the raw pulse width and then the injectorSettleTime is subtracted from that to get the effective pulse width, i.e. the part of the pulse when fuel is actually flowing.

This calculation is where the problem occurs. If the raw pulse width is less than the settling time, a negative number is the result. For example, if the raw pulse width is 300uS then the calculation is 300uS - 500uS = -200uS. Okay, not a big deal but all of the variables here are declared as unsigned long integers. They cannot represent a negative number. So, what happens? The unsigned integer represents the negative number as a very large positive integer.

How can this be solved? I assumed that the raw pulse width can never be negative. I don't know how to deal with negative time anyway! It has to be zero or greater.

So, I modified v0.75 like this:

void processInjClosed(void){      
  long t =  microSeconds();
//  long x = elapsedMicroseconds(injHiStart, t)- parms[injectorSettleTimeIdx]; 
  unsigned long s = parms[injectorSettleTimeIdx];	
  unsigned long x = t - injHiStart;
  if (x < s) {  //if pulse width < settling time
    x = 0;      //pulse width must be zero
  }else{
    x -= s;     //otherwise subtract settling time from the pulse width
  }
//  if(x >0)    //not needed because x is always zero or greater
    tmpTrip.injHius += x;       
  tmpTrip.injPulses++;      

  if (tmpInstInjStart != nil) {
    if(x >0)
      tmpInstInjTot += x;     
    tmpInstInjCount++;
  } else {
    tmpInstInjStart = t;
  }
  
  tmpInstInjEnd = t;
}

I removed the call to elapsedMicroseconds() just for simplicity and did the calculation directly in this routine. If the raw pulse width (x) is less than the settling time (s) then the pulse width is set to zero. If the raw pulse width is equal to or greater than the settling time then the settling time is subtracted from the pulse width. The result will always be a positive number. I removed the test for x > 0 because it's no longer needed. x is guaranteed to be zero or greater.

I have tested this modification on the road and I no longer see the high GPH values when coasting.

[dcb]

it *might* also be a bug in microseconds, I seem to recall some updated register analysis over on ardino.cc. Let me see if I can dig it up.

[Nevyn]

I think you have a typo.

 }else{
    x -= s;     //otherwise subtract settling time from the pulse width
  }

Should be

 }else{
    x - s;     //otherwise subtract settling time from the pulse width
  }

[Nevyn]

Arduino v.013 has native support for the ATMega328 chip, if anyone wants to upgrade.

[dcb]

FYI, I'm not willing to push the 328 till they trim back the bootloader (and until we are happy with 1.0 on the 168).

[2003protege]

Originally Posted by Nevyn

I think you have a typo.

 }else{
    x -= s;     //otherwise subtract settling time from the pulse width
  }

Should be

 }else{
    x - s;     //otherwise subtract settling time from the pulse width
  }

no, it's just the syntax used to store "x-s" as x

[Nevyn]

Agreed, but at least there's no modding of the IDE for 013.

[rmccomiskie]

Originally Posted by Nevyn

I think you have a typo.

 }else{
    x -= s;     //otherwise subtract settling time from the pulse width
  }

Should be

 }else{
    x - s;     //otherwise subtract settling time from the pulse width
  }

Actually, no. "x - s;" is syntactically incorrect. The calculation must be assigned to something. "x -= s;" is equivalent to "x = x - s;" which subtracts s from x and puts the result in x. I'm glad someone cares enough to look it over though. Cheers.

[rmccomiskie]

I found an error in my code mod. The value of x should be determined by a call to elapsedMicroseconds() as shown here:

void processInjClosed(void){      
  long t =  microSeconds();
  unsigned long s = parms[injectorSettleTimeIdx];	
  unsigned long x = elapsedMicroseconds(injHiStart, t); //using elapsedMicroseconds afterall
  if (x < s) {  //if pulse width < settling time
    x = 0;      //pulse width must be zero
  }else{
    x -= s;     //otherwise subtract settling time from the pulse width
  }
//  if(x >0)    //not needed because x is always zero or greater
    tmpTrip.injHius += x;       
  tmpTrip.injPulses++;      

  if (tmpInstInjStart != nil) {
    if(x >0)
      tmpInstInjTot += x;     
    tmpInstInjCount++;
  } else {
    tmpInstInjStart = t;
  }
  
  tmpInstInjEnd = t;
}

By eliminating elapsedMicroseconds(), I disregarded the case where the injHiStart (start of pulse) could be larger than t (end of pulse). That can occur when the unsigned integer returned by Microseconds() rolls off the end of the number range and starts again from zero.

For example, suppose injHiStart is set to 4294967285, i.e. near the end of the number range for an unsigned long. Then the pulse takes 260uS. When processInjClosed() executes on the trailing edge of the pulse, the number returned by microSeconds() will be 250.

To determine the elapsed time a call is made to elapsedMicroseconds() with startMicroSeconds=4294967285 and currentMicroseconds=250 as parameters. From the v0.75 code:

unsigned long elapsedMicroseconds(unsigned long startMicroSeconds, unsigned long currentMicroseconds ){  
  if(currentMicroseconds >= startMicroSeconds) return currentMicroseconds-startMicroSeconds;      
  return 4294967295 - (startMicroSeconds-currentMicroseconds);      
}

Since currentMicroSeconds is not >= startMicroSeconds, this calculation is used:

4294967295 - (4294967285 - 250) = 260

So, every so often, the clock will roll off the end of the number range and the values being compared will be reversed. The elapsedMicroseconds() routine handles the situation correctly and returns the true elapsed time (260uS in this case).

[Nevyn]

Thanks for the explanation. I'm going to update my code with that today - maybe it'll solve my problem of GPH going .44, .44, .44, 5.27, .44, .44...........


Sketch size is 14288 of 14306.

Do we need the "Weight" variable declared at the beginning? I don't see it used for anything.