Advisory for Chrysler Returnless Fuel Supply Systems

[KY Metro]

OK, I read it closer, you're working off the MAP sensor.

Could the same code be converted to work with a fuel pressure sensor instead?

[t vago]

Originally Posted by KY Metro

OK, I read it closer, you're working off the MAP sensor.

Could the same code be converted to work with a fuel pressure sensor instead?

Can't see why not. You could also use a MAP sensor and a fuel pressure sensor, to more accurately read the gauge pressure across the fuel injectors. This would assume that both the MAP sensor and the fuel pressure sensor would read absolute pressure. If the fuel pressure sensor reads gauge pressure, then that would add a bit of complexity to the algorithm.

[t vago]

% shave	size	notes
.	17160	original v0.86
89.8	15410	consolidate setup(), mainloop() into main.
89.4	15340	Folded enableXButton procedures into main()
89.0	15272	Got rid of events[] feature.
88.6	15202	pushed display names firmly into flash.
88.3	15144	transformed getstr() into LCD::flashPrint(). Rewrote hardware initialization code to be more understandable.
98.2	16846	converted to hardware timer-based delay scheme, modified delay2(), modified mainloop cpu utilization reporting, modified timeout feature, fixed wake-on-button bug, added button debouncing, got rid of microseconds(), got rid of millis2()
92.2	15820	shifted tmptrip measurement transfers completely to timer 2 overflow interrupt, re-wrote calculations, got rid of tmptrip, got rid of instantmpg() and instantmph() and instantgph(). Renamed class functions to remove references to gallons and miles.
90.0	15436	Added idleFuelRate, EOCSpeed functions. Consolidated speed, distance, fuel consumed, fuel rate functions. Keypress examination now done with value checking, and not doing bit conditions.
87.4	15006	Converted display routines to use indexed character strings instead of conditional us/metric strings.

^ Notes on what I did today to my MPGuino code.

Also experimented in Excel about what sort of square root algorithm to use.

[t vago]

% shave	size	notes
.	17160	original v0.86
91.6	15718	Re-wrote editparms and editGuino to be more user-friendly. Added ability to turn off serial transmitter via setup menu. Bit-wise parameters (injector trigger, metric, and serial transmitter on/off) no longer require a 32-bit number for EEPROM storage. VSS pulses/mile, contrast, and rpm factor turned into 8-bit values.
93.6	16062	Re-wrote display routines to reduce processing overhead. Got rid of displayTripCombo() and tDisplay() and dispv().
95.2	16332	Added clock display screen. Added time "hhmmss" numeric formatting function. Added "long button press" functionality. Moved tank clear and current trip clear to long button presses. Added ability to call up cpu utilization for any screen via long center button.

^ What I have done in the past two days. Probably going to make my own "version 2" thread.

(still haven't decided on a suitable square root function yet...)

[t vago]

Testing my updated code revealed that rather interesting fuel consumption runaway bug that was first documented in this post by DCB. It is always interesting to see my tank fuel economy just drop from 23.6 MPG to 1.0 MPG.

Re-examined the code and did some more searching around this sub-forum, and theorized that when my car goes into DFCO, it shuts off the fuel injectors, but it still sends little pulses, like in this post by rmccomiskie. I can reliably see the above fuel consumption runaway happen on every commute so far. Hopefully, tonight, when I load in this code, it will squash this bug.

ISR(INT0_vect) { // fuel injector open event handler

	injOpenStart = timer2_overflow_count | TCNT2; // calculate current cycle count
	// if timer overflow condition occurred, recalculate current cycle count and fold in overflow
	if (TIFR2 & 0b00000001) injOpenStart = (timer2_overflow_count + 256) | TCNT2;

	dirty |= dirtyInjTick; // tell close event handler that fuel injector has in fact opened

}

ISR(INT1_vect) { // fuel injector close event handler

	injOpenStop = timer2_overflow_count | TCNT2; // calculate current cycle count
	// if timer overflow condition occurred, recalculate current cycle count and fold in overflow
	if (TIFR2 & 0b00000001) injOpenStop = (timer2_overflow_count + 256) | TCNT2;

	if (dirty & dirtyInjTick) { // must have read in a injector start event first

		// calculate fuel injector pulse length
		if (injOpenStop < injOpenStart) cycleLength = 4294967295ul - injOpenStop + injOpenStart + 1;
		else cycleLength = injOpenStop - injOpenStart;

		if (cycleLength > injSettleCycles) { // if the pulse length is greater than the settle time, it's valid

			cycleTemp = injOpenCycles[1] + cycleLength - injSettleCycles; // add to injector cycle accumulator
			if (injOpenCycles[1] > cycleTemp) injOpenCycles[0]++; // handle any possible overflow
			injOpenCycles[1] = cycleTemp;
			injPulseCount++; // update pulse count
			timerStatus |= timerWakeUp; // tell timer to wake up main program
			dirty |= dirtyInj; // signal that a valid fuel injector pulse has just been read

		}

		dirty &= ~dirtyInjTick; // ignore any more close events until another open event is received

	}

}

Astute readers will be able to make out the remnants of functions microSeconds() and elapsedMicroseconds().

[nickdigger]

Originally Posted by t vago

VSS pulses/mile, contrast, and rpm factor turned into 8-bit values.

I can't think of a car that could use an 8-bit value for VSS/mile.

One thing i did with contrast, is built-in the brightness setting into it. I.e, take the LSB 2 bits of Contrast to determine which brightness index to use. So 44 would be brightness[0], 45 is [1], 46 is [2], 47 is [3], 48 is [0]. I found that I was never ever changing the brightness, so I re-assigned the middle button to other things, and just make it dim-out a couple minutes after engine-off.

I would like to see how you cleaned up the timers & buttons. I like what you did in the Inj ISRs, by directly grabbing TCNT2 and overflow_count instead of calling microseconds().

I only recently discovered that recursive microseconds bug & got rid of it after i couldn't understand why it should ever do that. I still have an older bug that's hanging my 'guino up, right near the time when the timer2 value rolls over past 0xFFFF FFFF. Unfortunately, it takes 5-7 days for the bug to trigger, so I'm still waiting to catch it.

[nickdigger]

One thing about your injOpenStop = timer2_overflow_count | TCNT2; code: Are you using the same prescale factor=64 (TCCR2B= 1<<CS22;); If so, I believe each tick is worth 4usec, so your calculation should be multiplied by 4. (I think. I've only just started looking into timers.)

[t vago]

Originally Posted by nickdigger

I can't think of a car that could use an 8-bit value for VSS/mile.

Ooops! I meant to say that the VSS debounce factor got turned into an 8-bit value. The VSS pulses/mile factor is still a 32-bit value (for now).

volatile uint8_t lastPINCstate = PINC;
uint8_t VSSCounts = 0;

...

ISR( PCINT1_vect ) {

	static uint8_t p;
	static uint8_t q;

	p = PINC; // read current pin C state
	q = p ^ lastPINCstate; // detect any changes from the last time this ISR is called

	timerStatus |= timerWakeUp; // tell system timer to wake up the main program

	if (q & vssBit) { // if a VSS pulse is received

		if (vssPause == 0) { // if there is no VSS pulse delay defined

			vssPulseCount++; // immediately update the VSS pulse count
			dirty |= dirtyVSS; // tell system timer that raw readings changed, and that the VSS reading itself has changed

		} else VSSCounts = vssPause; // otherwise, set VSS debounce count and let system timer handle the debouncing

	}

	if (q & buttonsUp) { // if a button press is detected

		keyCounts = keyDelay; // set keypress debounce count, and let system timer handle the debouncing

	}

	lastPINCstate = p; // remember the current pin C state for the next time this ISR gets called

}

This above is the current keypress/VSS interrupt handler. It, of course, fires any time a VSS pulse or a keypress is detected. It is important to note that this ISR will get called more than once each time a button is pressed, and that the debounce is handled by the use of a counter that is updated by the system timer. Once the counter winds down to zero, the new keypress has been stable for that amount of time, and the system timer then passes the keypress off to the main program. This is also how I created "long" button press detection.

ISR(TIMER2_OVF_vect) { // system timer interrupt handler

(...)

	if (VSSCounts != 0) { // if there is a VSS debounce countdown in progress

		VSSCounts--; // bump down the VSS count
		if (VSSCounts == 0) { // if count has reached zero,

			vssPulseCount++; // update the VSS pulse count
			dirty |= dirtyVSS; // tell system timer that raw readings changed, and that the VSS reading itself has changed

		}

	}

	if (keyCounts) { // if there is a button press debounce countdown in progress

		keyCounts--; // bump down the button press count by one

		if (keyCounts == 0) { // if the count has reached zero

			lastKeyPressed |= vssBit; // use vssBit to signal that a "long" keypress has been detected
			buttonState = lastKeyPressed; // pass off the remembered keypress status to the main program
			timerStatus &= ~timerKeyWait; // signal main program that a key press was detected

		}

		if (keyCounts == keyShortDelay) { // if button debounce countdown reaches this point

			tempFlags = buttonsUp & lastPINCstate; // figure out what buttons are being pressed

			if ((tempFlags != buttonsUp) || (lastKeyPressed & vssBit)) { // if any buttons are pressed, or there is a long press status

				lastKeyPressed = tempFlags; // remember the keypress status for later

			} else { // all buttons have been released, and there is no long keypress status detected

				buttonState = lastKeyPressed; // pass off the remembered keypress status to the main program
				timerStatus &= ~timerKeyWait; // signal main program that a key press was detected 
				keyCounts = 0; // reset button press debounce countdown to zero

			}

		}

	}

(...)

}

This code is in the system timer ISR. It enables a couple of things. First, if the user presses a button, then releases it, it will release just that keypress. If the user presses two buttons together, and releases them, it will release this combination. However, if the user presses the two buttons, and there is a slight delay between, say, pressing the center button and the left button, then the code will wait for the buttons to stabilize. Finally, if the user holds down the button combination for more than one second, the code will pass that along. The difference there is that the short keypresses require that the user release the buttons first, while the long keypresses only require that the buttons have been pressed for a second.

Originally Posted by nickdigger

One thing about your injOpenStop = timer2_overflow_count | TCNT2; code: Are you using the same prescale factor=64 (TCCR2B= 1<<CS22;); If so, I believe each tick is worth 4usec, so your calculation should be multiplied by 4. (I think. I've only just started looking into timers.)

Heh. It's good that you noticed that.

After going through the code, I decided I just didn't need that any more. For one thing, it wasn't really returning values in true microseconds. For 16 MHz systems, it'd return values in units of (16 000 000 / 64), or 4 us, but for 20 MHz values, it't return values in units of (20 000 000 / 64), or 3.2 us. Besides that, doing the conversion was just another arithmetic operation that could have been folded into the mileage/distance/economy/time calculations. So, fuel injector pulse times, idle times, and even the system time are now measured in timer2 cycles

Also, to optimize calculations involving timer2_overflow_count, I decided to have the timer2 overflow handler bump timer2_overflow_count by 256 instead of by 1. That got rid of constantly having to multiply timer2_overflow_count by 256 whenever it was desired to read the time.

The timer2 overflow has been vastly expanded, to get rid of microseconds() altogether. The main loop uses timer2 to do its loop delays now, instead of having to track the time by itself. I'm thinking that this may have gotten rid of that freeze-up bug altogether. Also, function delay2() now "calls" the overflow timer to do its delays, but also I'm thinking to get rid of that, in favor of having timer2 drive the LCD output directly by using an output buffer.

	injOpenStop = timer2_overflow_count + TCNT2; // read current TCNT2
	if (TIFR2 & (1 << TOV2)) injOpenStop = timer2_overflow_count + 256 + TCNT2; // if overflow occurred, re-read TCNT2 and adjust for overflow

That above is a slightly modified version that actually manages to save about 18 bytes or so in compilation. Who knew that logical OR-ing would take up more space than just adding? Heh.

[nickdigger]

Originally Posted by t vago

Also, to optimize calculations involving timer2_overflow_count, I decided to have the timer2 overflow handler bump timer2_overflow_count by 256 instead of by 1. That got rid of constantly having to multiply timer2_overflow_count by 256 whenever it was desired to read the time. Also, function delay2() now "calls" the overflow timer to do its delays, but also I'm thinking to get rid of that, in favor of having timer2 drive the LCD output directly by using an output buffer.

injOpenStop = timer2_overflow_count + TCNT2;
if (TIFR2 & (1 << TOV2)) injOpenStop = timer2_overflow_count + 256 + TCNT2;

That above is a slightly modified version that actually manages to save about 18 bytes or so in compilation. Who knew that logical OR-ing would take up more space than just adding? Heh.

The OR was probably (stupidly) ORing 0x00 to each of the 3 high bytes as well. I've been using unions, as below, to eliminate some of that nonsense. With the add-256 method, the compiler knew the high bytes would just carry-the-one from the lower add, hence the "better" compile.
Here's how I optimized my microseconds():

typedef union { ulong  ul;
                struct {uint i0; uint i1;};
                struct {byte b0; byte b1; byte b2; byte b3;}; } union32;

ulong microSeconds (void)
{
  union32 tmp;
  const byte *t2ocptr = (byte *)&timer2_overflow_count;

  cli();
  //tmp.ul = timer2_overflow_count<<8;
  //tmp_tcnt2 = TCNT2;
  tmp.b3 = *(t2ocptr+2);
  tmp.b2 = *(t2ocptr+1);
  tmp.b1 = *(t2ocptr);
  tmp.b0 = TCNT2;
  sei();
  //return (tmp + tmp_tcnt2) * 4;
  return tmp.ul * 4;

//each TCNT2 == 4us
//each timer2_overflow == 1024us == 256us*4
//return (timer2_overflow*256*4 + TCNT2*4)
//ie, return (timer2_overflow*256 + TCNT2) * 4

It's pretty tight. Only 38 bytes compiled, no stack usage, and only 5 instructions between cli & sei. Now that I look at it, 14 of those 38 are the mult-by-4 op.

The timer2 overflow has been vastly expanded, to get rid of microseconds() altogether. The main loop uses timer2 to do its loop delays now, instead of having to track the time by itself. I'm thinking that this may have gotten rid of that freeze-up bug altogether.

I didn't know others were freezing as well. My "blank" screen after the 7min timeout now displays a running clock, and the timer_overflow values. Hopefully this will help me catch the bug in the act.

[t vago]

I noticed that when I started to perform code optimization of the 0.86 code, it would start to just freeze up. I haven't yet figured out why, though, as I had already progressed on more-or-less re-writing the 0.86 code. That code is nice and rock-solid stable.

Originally Posted by nickdigger

Here's how I optimized my microseconds():[/code]

Hey, that's pretty cool. I might try that, too, at some point.

Are you calling your microSeconds() function in your ISRs? If you are, you might want to slightly modify your interrupt disabling mechanism a bit. Something more like:

ulong microSeconds (void)
{
  union32 tmp;
  const byte *t2ocptr = (byte *)&timer2_overflow_count;

  byte oldSREG = SREG; // save interrupt flag status (in case an ISR called here)
  cli(); // now, we disable interrupts
  //tmp.ul = timer2_overflow_count<<8;
  //tmp_tcnt2 = TCNT2;
  tmp.b3 = *(t2ocptr+2);
  tmp.b2 = *(t2ocptr+1);
  tmp.b1 = *(t2ocptr);
  tmp.b0 = TCNT2;
  SREG = oldSREG; // restore previous interrupt flag state
//  sei();
  //return (tmp + tmp_tcnt2) * 4;
  return tmp.ul * 4;

This way, you're not inadvertently re-enabling interrupts while servicing an interrupt. Nested interrupts a a total PITA at best.

Remember also that if you happen to read TCNT2, it might roll over and overflow right before you read it, thus giving a false lower value. This is why it's generally better to read the value, check for overflow condition, then read again and adjust for the overflow if the overflow condition is found to exist. All this, of course, is done while interrupts are disabled. I strongly suspect this is why the original microSeconds() function had a recursive call, to try to get around this quirky aspect of timers.