making progres on angle-based scheduling

firmware/controllers/algo/engine.cpp

@@ -392,3 +392,12 @@ void action_s::execute() {
 	efiAssertVoid(CUSTOM_ERR_ASSERT, callback != NULL, "callback==null1");
 	callback(param);
 }
+
+schfunc_t action_s::getCallback() const {
+	return callback;
+}
+
+void * action_s::getArgument() const {
+	return param;
+}
+

firmware/controllers/algo/engine.h

@@ -104,7 +104,7 @@ public:
 	/**
 	 * That's the linked list of pending spark firing events
 	 */
-	IgnitionEvent *ignitionEventsHead = nullptr;
+	AngleBasedEvent *ignitionEventsHead = nullptr;
 	/**
 	 * this is based on isEngineChartEnabled and engineSnifferRpmThreshold settings
 	 */

firmware/controllers/algo/event_registry.h

@@ -74,6 +74,10 @@ public:
 	scheduling_s scheduling;
 	event_trigger_position_s position;
 	action_s action;
+	/**
+	 * Trigger-based scheduler maintains a linked list of all pending tooth-based events.
+	 */
+	AngleBasedEvent *nextToothEvent = nullptr;
 };
 
 #define MAX_OUTPUTS_FOR_IGNITION 2
@@ -94,10 +98,6 @@ public:
 	 */
 	uint32_t actualStartOfDwellNt;
 	event_trigger_position_s dwellPosition;
-	/**
-	 * Ignition scheduler maintains a linked list of all pending ignition events.
-	 */
-	IgnitionEvent *nextIgnitionEvent = nullptr;
 	/**
 	 * Sequential number of currently processed spark event
 	 * @see globalSparkIdCounter

firmware/controllers/scheduling/event_queue.cpp

@@ -171,7 +171,7 @@ int EventQueue::executeAll(efitime_t now) {
 		uint32_t howFarOff = now - current->momentX;
 		maxSchedulingPrecisionLoss = maxI(maxSchedulingPrecisionLoss, howFarOff);
 #if EFI_UNIT_TEST
-		printf("QUEUE: execute current=%d param=%d\r\n", (long)current, (long)current->action.param);
+		printf("QUEUE: execute current=%d param=%d\r\n", (long)current, (long)current->action.getArgument());
 #endif
 
 		{

firmware/controllers/scheduling/scheduler.h

@@ -14,10 +14,10 @@ class action_s {
 public:
 	void setAction(schfunc_t callback, void *param);
 	void execute();
+	schfunc_t getCallback() const;
+	void * getArgument() const;
 
-#if EFI_PROD_CODE
 private:
-#endif /* EFI_PROD_CODE */
 	schfunc_t callback = nullptr;
 	void *param = nullptr;
 };

firmware/controllers/scheduling/signal_executor_sleep.cpp

@@ -39,8 +39,8 @@ void SleepExecutor::scheduleByTimestamp(scheduling_s *scheduling, efitimeus_t ti
 
 static void timerCallback(scheduling_s *scheduling) {
 #if EFI_PRINTF_FUEL_DETAILS
-	if (scheduling->action.callback == (schfunc_t)&seTurnPinLow) {
+	if (scheduling->action.getCallback() == (schfunc_t)&seTurnPinLow) {
-		printf("executing cb=seTurnPinLow p=%d sch=%d now=%d\r\n", (int)scheduling->action.param, (int)scheduling,
+		printf("executing cb=seTurnPinLow p=%d sch=%d now=%d\r\n", (int)scheduling->action.getArgument(), (int)scheduling,
 				(int)getTimeNowUs());
 	} else {
 //		printf("exec cb=%d p=%d\r\n", (int)scheduling->callback, (int)scheduling->param);

firmware/controllers/trigger/spark_logic.cpp

@@ -215,8 +215,56 @@ void turnSparkPinHigh(IgnitionEvent *event) {
 	}
 }
 
-static bool assertNotInIgnitionList(IgnitionEvent *head, IgnitionEvent *element) {
+static bool assertNotInIgnitionList(AngleBasedEvent *head, AngleBasedEvent *element) {
-	assertNotInListMethodBody(IgnitionEvent, head, element, nextIgnitionEvent)
+	assertNotInListMethodBody(AngleBasedEvent, head, element, nextToothEvent)
+}
+
+/**
+ * @return true if event corresponds to current tooth and was time-based scheduler
+ *         false if event was put into queue for scheduling at a later tooth
+ */
+static bool scheduleOrQueue(AngleBasedEvent *event, uint32_t trgEventIndex, angle_t advance, schfunc_t callback, void *param DECLARE_ENGINE_PARAMETER_SUFFIX) {
+	TRIGGER_SHAPE(findTriggerPosition(&event->position, advance PASS_CONFIG_PARAM(engineConfiguration->globalTriggerAngleOffset)));
+
+	/**
+	 * todo: extract a "scheduleForAngle" method with best implementation into a separate utility method
+	 *
+	 * Here's the status as of Nov 2018:
+	 * "scheduleForLater" uses time only and for best precision it's best to use "scheduleForLater" only
+	 * once we hit the last trigger tooth prior to needed event. This case we use as much trigger position angle as possible
+	 * and only use less precise RPM-based time calculation for the last portion of the angle, the one between two teeth closest to the
+	 * desired angle moment.
+	 *
+	 * At the moment we only have time-based scheduler. I believe what needs to be added is a trigger-event based scheduler on top of the
+	 * time-based schedule. This case we would be firing events with best possible angle precision.
+	 *
+	 */
+	if (event->position.triggerEventIndex == trgEventIndex) {
+		/**
+		 * Spark should be fired before the next trigger event - time-based delay is best precision possible
+		 */
+		float timeTillIgnitionUs = ENGINE(rpmCalculator.oneDegreeUs) * event->position.angleOffsetFromTriggerEvent;
+
+
+		scheduling_s * sDown = &event->scheduling;
+
+		engine->executor.scheduleForLater(sDown, (int) timeTillIgnitionUs, callback, param);
+		return true;
+	} else {
+		event->action.setAction(callback, param);
+		/**
+		 * Spark should be scheduled in relation to some future trigger event, this way we get better firing precision
+		 */
+		bool isPending = assertNotInIgnitionList(ENGINE(ignitionEventsHead), event);
+		if (isPending) {
+#if SPARK_EXTREME_LOGGING
+			scheduleMsg(logger, "isPending thus nt adding to queue index=%d rev=%d now=%d", trgEventIndex, getRevolutionCounter(), (int)getTimeNowUs());
+#endif /* FUEL_MATH_EXTREME_LOGGING */
+		} else {
+			LL_APPEND2(ENGINE(ignitionEventsHead), event, nextToothEvent);
+		}
+		return false;
+	}
 }
 
 static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventIndex, IgnitionEvent *iEvent,
@@ -277,7 +325,21 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI
 
 	efiAssertVoid(CUSTOM_ERR_6591, !cisnan(advance), "findAngle#4");
 	assertAngleRange(advance, "findAngle#a5", CUSTOM_ERR_6549);
-	TRIGGER_SHAPE(findTriggerPosition(&iEvent->sparkEvent.position, advance PASS_CONFIG_PARAM(engineConfiguration->globalTriggerAngleOffset)));
+
+
+	bool scheduled = scheduleOrQueue(&iEvent->sparkEvent, trgEventIndex, advance, (schfunc_t)fireSparkAndPrepareNextSchedule, iEvent PASS_ENGINE_PARAMETER_SUFFIX);
+
+	if (scheduled) {
+#if SPARK_EXTREME_LOGGING
+		scheduleMsg(logger, "scheduling sparkDown ind=%d %d %s now=%d later id=%d", trgEventIndex, getRevolutionCounter(), iEvent->getOutputForLoggins()->name, (int)getTimeNowUs(), iEvent->sparkId);
+#endif /* FUEL_MATH_EXTREME_LOGGING */
+	} else {
+#if SPARK_EXTREME_LOGGING
+		scheduleMsg(logger, "to queue sparkDown ind=%d %d %s now=%d for id=%d", trgEventIndex, getRevolutionCounter(), iEvent->getOutputForLoggins()->name, (int)getTimeNowUs(), iEvent->sparkEvent.position.triggerEventIndex);
+#endif /* FUEL_MATH_EXTREME_LOGGING */
+	}
+
+
 
 #if EFI_UNIT_TEST
 	if (verboseMode) {
@@ -286,50 +348,6 @@ static ALWAYS_INLINE void handleSparkEvent(bool limitedSpark, uint32_t trgEventI
 			iEvent->sparkId);
 	}
 #endif
-
-	/**
-	 * todo: extract a "scheduleForAngle" method with best implementation into a separate utility method
-	 *
-	 * Here's the status as of Nov 2018:
-	 * "scheduleForLater" uses time only and for best precision it's best to use "scheduleForLater" only
-	 * once we hit the last trigger tooth prior to needed event. This case we use as much trigger position angle as possible
-	 * and only use less precise RPM-based time calculation for the last portion of the angle, the one between two teeth closest to the
-	 * desired angle moment.
-	 *
-	 * At the moment we only have time-based scheduler. I believe what needs to be added is a trigger-event based scheduler on top of the
-	 * time-based schedule. This case we would be firing events with best possible angle precision.
-	 *
-	 */
-	if (iEvent->sparkEvent.position.triggerEventIndex == trgEventIndex) {
-		/**
-		 * Spark should be fired before the next trigger event - time-based delay is best precision possible
-		 */
-		float timeTillIgnitionUs = ENGINE(rpmCalculator.oneDegreeUs) * iEvent->sparkEvent.position.angleOffsetFromTriggerEvent;
-
-#if SPARK_EXTREME_LOGGING
-		scheduleMsg(logger, "scheduling sparkDown ind=%d %d %s now=%d %d later id=%d", trgEventIndex, getRevolutionCounter(), iEvent->getOutputForLoggins()->name, (int)getTimeNowUs(), (int)timeTillIgnitionUs, iEvent->sparkId);
-#endif /* FUEL_MATH_EXTREME_LOGGING */
-
-		scheduling_s * sDown = &iEvent->sparkEvent.scheduling;
-
-		engine->executor.scheduleForLater(sDown, (int) timeTillIgnitionUs, (schfunc_t) &fireSparkAndPrepareNextSchedule, iEvent);
-	} else {
-#if SPARK_EXTREME_LOGGING
-		scheduleMsg(logger, "to queue sparkDown ind=%d %d %s %d for %d", trgEventIndex, getRevolutionCounter(), iEvent->getOutputForLoggins()->name, (int)getTimeNowUs(), iEvent->sparkEvent.position.triggerEventIndex);
-#endif /* FUEL_MATH_EXTREME_LOGGING */
-		/**
-		 * Spark should be scheduled in relation to some future trigger event, this way we get better firing precision
-		 */
-		bool isPending = assertNotInIgnitionList(ENGINE(ignitionEventsHead), iEvent);
-		if (isPending) {
-#if SPARK_EXTREME_LOGGING
-			scheduleMsg(logger, "not adding to queue sparkDown ind=%d %d %s %d", trgEventIndex, getRevolutionCounter(), iEvent->getOutputForLoggins()->name, (int)getTimeNowUs());
-#endif /* FUEL_MATH_EXTREME_LOGGING */
-			return;
-		}
-
-		LL_APPEND2(ENGINE(ignitionEventsHead), iEvent, nextIgnitionEvent);
-	}
 }
 
 void initializeIgnitionActions(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
@@ -386,23 +404,23 @@ static ALWAYS_INLINE void prepareIgnitionSchedule(DECLARE_ENGINE_PARAMETER_SIGNA
 
 
 static void scheduleAllSparkEventsUntilNextTriggerTooth(uint32_t trgEventIndex DECLARE_ENGINE_PARAMETER_SUFFIX) {
-	IgnitionEvent *current, *tmp;
+	AngleBasedEvent *current, *tmp;
 
-	LL_FOREACH_SAFE2(ENGINE(ignitionEventsHead), current, tmp, nextIgnitionEvent)
+	LL_FOREACH_SAFE2(ENGINE(ignitionEventsHead), current, tmp, nextToothEvent)
 	{
-		if (current->sparkEvent.position.triggerEventIndex == trgEventIndex) {
+		if (current->position.triggerEventIndex == trgEventIndex) {
 			// time to fire a spark which was scheduled previously
-			LL_DELETE2(ENGINE(ignitionEventsHead), current, nextIgnitionEvent);
+			LL_DELETE2(ENGINE(ignitionEventsHead), current, nextToothEvent);
 
-			scheduling_s * sDown = &current->sparkEvent.scheduling;
+			scheduling_s * sDown = &current->scheduling;
 
 #if SPARK_EXTREME_LOGGING
-	scheduleMsg(logger, "time to sparkDown ind=%d %d %s %d", trgEventIndex, getRevolutionCounter(), current->getOutputForLoggins()->name, (int)getTimeNowUs());
+	scheduleMsg(logger, "time to invoke ind=%d %d %d", trgEventIndex, getRevolutionCounter(), (int)getTimeNowUs());
 #endif /* FUEL_MATH_EXTREME_LOGGING */
 
 
-			float timeTillIgnitionUs = ENGINE(rpmCalculator.oneDegreeUs) * current->sparkEvent.position.angleOffsetFromTriggerEvent;
+			float timeTillIgnitionUs = ENGINE(rpmCalculator.oneDegreeUs) * current->position.angleOffsetFromTriggerEvent;
-			engine->executor.scheduleForLater(sDown, (int) timeTillIgnitionUs, (schfunc_t) &fireSparkAndPrepareNextSchedule, current);
+			engine->executor.scheduleForLater(sDown, (int) timeTillIgnitionUs, (schfunc_t) current->action.getCallback(), current->action.getArgument());
 		}
 	}
 }

unit_tests/engine_test_helper.cpp

@@ -165,7 +165,7 @@ scheduling_s * EngineTestHelper::assertEvent5(const char *msg, int index, void *
 	TestExecutor *executor = &engine.executor;
 	EXPECT_TRUE(executor->size() > index) << msg;
 	scheduling_s *event = executor->getForUnitTest(index);
-	assertEqualsM4(msg, " up/down", (void*)event->action.callback == (void*) callback, 1);
+	assertEqualsM4(msg, " up/down", (void*)event->action.getCallback() == (void*) callback, 1);
 	efitime_t start = getTimeNowUs();
 	assertEqualsM(msg, expectedTimestamp, event->momentX - start);
 	return event;
@@ -174,7 +174,7 @@ scheduling_s * EngineTestHelper::assertEvent5(const char *msg, int index, void *
 void EngineTestHelper::assertEvent(const char *msg, int index, void *callback, efitime_t momentX, InjectionEvent *expectedEvent) {
 	scheduling_s *event = assertEvent5(msg, index, callback, momentX);
 
-	InjectionEvent *actualEvent = (InjectionEvent *)event->action.param;
+	InjectionEvent *actualEvent = (InjectionEvent *)event->action.getArgument();
 
 	assertEqualsLM(msg, expectedEvent->outputs[0], (long)actualEvent->outputs[0]);
 // but this would not work	assertEqualsLM(msg, expectedPair, (long)eventPair);

unit_tests/tests/test_trigger_decoder.cpp

@@ -359,14 +359,14 @@ TEST(misc, testRpmCalculator) {
 	{
 	scheduling_s *ev0 = engine->executor.getForUnitTest(0);
 
-	assertREqualsM("Call@0", (void*)ev0->action.callback, (void*)turnSparkPinHigh);
+	assertREqualsM("Call@0", (void*)ev0->action.getCallback(), (void*)turnSparkPinHigh);
 	assertEqualsM("ev 0", start + 944, ev0->momentX);
-	assertEqualsLM("coil 0", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev0->action.param)->outputs[0]);
+	assertEqualsLM("coil 0", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev0->action.getArgument())->outputs[0]);
 
 	scheduling_s *ev1 = engine->executor.getForUnitTest(1);
-	assertREqualsM("Call@1", (void*)ev1->action.callback, (void*)fireSparkAndPrepareNextSchedule);
+	assertREqualsM("Call@1", (void*)ev1->action.getCallback(), (void*)fireSparkAndPrepareNextSchedule);
 	assertEqualsM("ev 1", start + 1444, ev1->momentX);
-	assertEqualsLM("coil 1", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev1->action.param)->outputs[0]);
+	assertEqualsLM("coil 1", (long)&enginePins.coils[0], (long)((IgnitionEvent*)ev1->action.getArgument())->outputs[0]);
 
 	}