Merge pull request #1606 from mck1117/reset-overlap

reset overlap counter on fuel reschedule

firmware/controllers/algo/event_registry.h

@@ -60,6 +60,8 @@ public:
 	void addFuelEvents(DECLARE_ENGINE_PARAMETER_SIGNATURE);
 	bool addFuelEventsForCylinder(int cylinderIndex DECLARE_ENGINE_PARAMETER_SUFFIX);
 
+	void resetOverlapping();
+
 	/**
 	 * injection events, per cylinder
 	 */

firmware/controllers/engine_cycle/main_trigger_callback.cpp

@@ -162,6 +162,11 @@ void InjectorOutputPin::close(efitick_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) {
 #endif // EFI_TOOTH_LOGGER
 		setLow();
 	}
+
+	// Don't allow negative overlap count
+	if (overlappingCounter < 0) {
+		overlappingCounter = 0;
+	}
 }
 
 void turnInjectionPinLow(InjectionEvent *event) {

firmware/controllers/engine_cycle/rpm_calculator.cpp

@@ -165,7 +165,14 @@ void RpmCalculator::setRpmValue(float value DECLARE_ENGINE_PARAMETER_SUFFIX) {
 #if EFI_ENGINE_CONTROL
 	// This presumably fixes injection mode change for cranking-to-running transition.
 	// 'isSimultanious' flag should be updated for events if injection modes differ for cranking and running.
-	if (state != oldState) {
+	if (state != oldState && CONFIG(crankingInjectionMode) != CONFIG(injectionMode)) {
+		// Reset the state of all injectors: when we change fueling modes, we could
+		// immediately reschedule an injection that's currently underway.  That will cause
+		// the injector's overlappingCounter to get out of sync with reality.  As the fix,
+		// every injector's state is forcibly reset just before we could cause that to happen.
+		engine->injectionEvents.resetOverlapping();
+
+		// reschedule all injection events now that we've reset them
 		engine->injectionEvents.addFuelEvents(PASS_ENGINE_PARAMETER_SIGNATURE);
 	}
 #endif

firmware/controllers/math/engine_math.cpp

@@ -117,6 +117,12 @@ void FuelSchedule::clear() {
 	isReady = false;
 }
 
+void FuelSchedule::resetOverlapping() {
+	for (size_t i = 0; i < efi::size(enginePins.injectors); i++) {
+		enginePins.injectors[i].reset();
+	}
+}
+
 /**
  * @returns false in case of error, true if success
  */

firmware/controllers/system/efi_gpio.cpp

@@ -277,7 +277,12 @@ bool NamedOutputPin::stop() {
 }
 
 void InjectorOutputPin::reset() {
-	overlappingCounter = 0;
+	// If this injector was open, close it and reset state
+	if (overlappingCounter != 0) {
+		overlappingCounter = 0;
+		setValue(0);
+	}
+
 	// todo: this could be refactored by calling some super-reset method
 	currentLogicValue = INITIAL_PIN_STATE;
 }

firmware/controllers/system/efi_gpio.h

@@ -103,7 +103,7 @@ public:
 	const char *shortName = NULL;
 };
 
-class InjectorOutputPin : public NamedOutputPin {
+class InjectorOutputPin final : public NamedOutputPin {
 public:
 	InjectorOutputPin();
 	void reset();

unit_tests/tests/ignition_injection/injection_mode_transition.cpp

@@ -84,14 +84,9 @@ TEST(fuelControl, transitionIssue1592) {
 	}
 
 	// Check that no injectors are stuck open
-	// Only injector 1 should currently be open
+	// Injectors 1/3 should be open
 	EXPECT_EQ(enginePins.injectors[0].getOverlappingCounter(), 1);
 	EXPECT_EQ(enginePins.injectors[1].getOverlappingCounter(), 0);
-
-	// !!!!!!!!! BUG !!!!!!!!!!!!!!!
-	// Injector #3 gets stuck open!
-	EXPECT_EQ(enginePins.injectors[2].getOverlappingCounter(), 2);
-	// !!!!!!!!! BUG !!!!!!!!!!!!!!!
-
+	EXPECT_EQ(enginePins.injectors[2].getOverlappingCounter(), 1);
 	EXPECT_EQ(enginePins.injectors[3].getOverlappingCounter(), 0);
 }