From e09ed790ebe38740fa200c53fb8a08e13c8ed0d9 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Thu, 12 Oct 2023 18:00:33 -0700 Subject: [PATCH] extract injection refactoring from #215 --- firmware/console/binary/live_data.cpp | 2 +- firmware/console/status_loop.cpp | 2 +- firmware/controllers/engine_controller.cpp | 2 +- .../engine_cycle/fuel_schedule.cpp | 34 ++++++++++--------- .../controllers/engine_cycle/fuel_schedule.h | 34 ++++++++++++------- .../engine_cycle/main_trigger_callback.cpp | 4 +-- .../test_fuel_wall_wetting.cpp | 8 ++--- 7 files changed, 49 insertions(+), 37 deletions(-) diff --git a/firmware/console/binary/live_data.cpp b/firmware/console/binary/live_data.cpp index 010512fc1f..392dee37f5 100644 --- a/firmware/console/binary/live_data.cpp +++ b/firmware/console/binary/live_data.cpp @@ -156,7 +156,7 @@ const trigger_state_primary_s* getLiveData(size_t) { template<> const wall_fuel_state_s* getLiveData(size_t) { - return &engine->injectionEvents.elements[0].wallFuel; + return &engine->injectionEvents.elements[0].getWallFuel(); } template<> diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index d072d650a6..fe2ad09d48 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -541,7 +541,7 @@ static void updateFuelInfo() { updateFuelCorrections(); updateFuelResults(); - const auto& wallFuel = engine->injectionEvents.elements[0].wallFuel; + const auto& wallFuel = engine->injectionEvents.elements[0].getWallFuel(); engine->outputChannels.wallFuelAmount = wallFuel.getWallFuel() * 1000; // Convert grams to mg engine->outputChannels.wallFuelCorrectionValue = wallFuel.wallFuelCorrection * 1000; // Convert grams to mg diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 1b86ae0148..ffc2a92b2b 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -159,7 +159,7 @@ static void resetAccel() { for (size_t i = 0; i < efi::size(engine->injectionEvents.elements); i++) { - engine->injectionEvents.elements[i].wallFuel.resetWF(); + engine->injectionEvents.elements[i].getWallFuel().resetWF(); } } diff --git a/firmware/controllers/engine_cycle/fuel_schedule.cpp b/firmware/controllers/engine_cycle/fuel_schedule.cpp index 6824bafdca..e64e241525 100644 --- a/firmware/controllers/engine_cycle/fuel_schedule.cpp +++ b/firmware/controllers/engine_cycle/fuel_schedule.cpp @@ -21,10 +21,14 @@ void turnInjectionPinHigh(InjectionEvent *event) { FuelSchedule::FuelSchedule() { for (int cylinderIndex = 0; cylinderIndex < MAX_CYLINDER_COUNT; cylinderIndex++) { - elements[cylinderIndex].ownIndex = cylinderIndex; + elements[cylinderIndex].setIndex(cylinderIndex); } } +WallFuel& InjectionEvent::getWallFuel() { + return wallFuel; +} + void FuelSchedule::invalidate() { isReady = false; } @@ -64,7 +68,7 @@ InjectionEvent::InjectionEvent() { // Returns the start angle of this injector in engine coordinates (0-720 for a 4 stroke), // or unexpected if unable to calculate the start angle due to missing information. -expected InjectionEvent::computeInjectionAngle(int cylinderIndex) const { +expected InjectionEvent::computeInjectionAngle() const { floatus_t oneDegreeUs = getEngineRotationState()->getOneDegreeUs(); if (cisnan(oneDegreeUs)) { // in order to have fuel schedule we need to have current RPM @@ -86,7 +90,7 @@ expected InjectionEvent::computeInjectionAngle(int cylinderIndex) const { assertAngleRange(openingAngle, "openingAngle_r", ObdCode::CUSTOM_ERR_6554); // Convert from cylinder-relative to cylinder-1-relative - openingAngle += getCylinderAngle(cylinderIndex, cylinderNumber); + openingAngle += getCylinderAngle(ownIndex, cylinderNumber); efiAssert(ObdCode::CUSTOM_ERR_ASSERT, !cisnan(openingAngle), "findAngle#3", false); assertAngleRange(openingAngle, "findAngle#a33", ObdCode::CUSTOM_ERR_6544); @@ -100,8 +104,8 @@ expected InjectionEvent::computeInjectionAngle(int cylinderIndex) const { return openingAngle; } -bool InjectionEvent::updateInjectionAngle(int cylinderIndex) { - auto result = computeInjectionAngle(cylinderIndex); +bool InjectionEvent::updateInjectionAngle() { + auto result = computeInjectionAngle(); if (result) { // If injector duty cycle is high, lock injection SOI so that we @@ -119,10 +123,8 @@ bool InjectionEvent::updateInjectionAngle(int cylinderIndex) { /** * @returns false in case of error, true if success */ -bool FuelSchedule::addFuelEventsForCylinder(int i) { - InjectionEvent *ev = &elements[i]; - - bool updatedAngle = ev->updateInjectionAngle(i); +bool InjectionEvent::update() { + bool updatedAngle = updateInjectionAngle(); if (!updatedAngle) { return false; @@ -133,7 +135,7 @@ bool FuelSchedule::addFuelEventsForCylinder(int i) { // Map order index -> cylinder index (firing order) // Single point only uses injector 1 (index 0) - int injectorIndex = mode == IM_SINGLE_POINT ? 0 : ID2INDEX(getCylinderId(i)); + int injectorIndex = mode == IM_SINGLE_POINT ? 0 : ID2INDEX(getCylinderId(ownIndex)); InjectorOutputPin *secondOutput = nullptr; @@ -144,23 +146,23 @@ bool FuelSchedule::addFuelEventsForCylinder(int i) { // Compute the position of this cylinder's twin in the firing order // Each injector gets fired as a primary (the same as sequential), but also // fires the injector 360 degrees later in the firing order. - int secondOrder = (i + (engineConfiguration->cylindersCount / 2)) % engineConfiguration->cylindersCount; + int secondOrder = (ownIndex + (engineConfiguration->cylindersCount / 2)) % engineConfiguration->cylindersCount; int secondIndex = ID2INDEX(getCylinderId(secondOrder)); secondOutput = &enginePins.injectors[secondIndex]; } - ev->outputs[0] = &enginePins.injectors[injectorIndex]; - ev->outputs[1] = secondOutput; - ev->isSimultaneous = mode == IM_SIMULTANEOUS; + outputs[0] = &enginePins.injectors[injectorIndex]; + outputs[1] = secondOutput; + isSimultaneous = mode == IM_SIMULTANEOUS; // Stash the cylinder number so we can select the correct fueling bank later - ev->cylinderNumber = injectorIndex; + cylinderNumber = injectorIndex; return true; } void FuelSchedule::addFuelEvents() { for (size_t cylinderIndex = 0; cylinderIndex < engineConfiguration->cylindersCount; cylinderIndex++) { - bool result = addFuelEventsForCylinder(cylinderIndex); + bool result = elements[cylinderIndex].update(); if (!result) { invalidate(); diff --git a/firmware/controllers/engine_cycle/fuel_schedule.h b/firmware/controllers/engine_cycle/fuel_schedule.h index 0767618f1c..6112f2d253 100644 --- a/firmware/controllers/engine_cycle/fuel_schedule.h +++ b/firmware/controllers/engine_cycle/fuel_schedule.h @@ -18,26 +18,33 @@ class InjectionEvent { public: InjectionEvent(); - // Update the injection start angle - bool updateInjectionAngle(int cylinderIndex); - - // Compute the injection start angle, compensating for injection duration and injection phase settings. - expected computeInjectionAngle(int cylinderIndex) const; + bool update(); // Call this every decoded trigger tooth. It will schedule any relevant events for this injector. void onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase, float nextPhase); + WallFuel& getWallFuel(); + + void setIndex(uint8_t index) { + ownIndex = index; + } + +private: + // Update the injection start angle + bool updateInjectionAngle(); + + // Compute the injection start angle, compensating for injection duration and injection phase settings. + expected computeInjectionAngle() const; + /** * This is a performance optimization for IM_SIMULTANEOUS fuel strategy. * It's more efficient to handle all injectors together if that's the case */ bool isSimultaneous = false; - InjectorOutputPin *outputs[MAX_WIRES_COUNT]; uint8_t ownIndex = 0; uint8_t cylinderNumber = 0; - float injectionStartAngle = 0; - +public: /** * we need atomic flag so that we do not schedule a new pair of up/down before previous down was executed. * @@ -47,7 +54,13 @@ public: */ bool isScheduled = false; +private: WallFuel wallFuel; + +public: + // TODO: this should be private + InjectorOutputPin *outputs[MAX_WIRES_COUNT]; + float injectionStartAngle = 0; }; void turnInjectionPinHigh(InjectionEvent *event); @@ -66,11 +79,8 @@ public: // Call this every trigger tooth. It will schedule all required injector events. void onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase, float nextPhase); - /** - * this method schedules all fuel events for an engine cycle - */ + // Calculate injector opening angle, pins, and mode for all injectors void addFuelEvents(); - bool addFuelEventsForCylinder(int cylinderIndex); void resetOverlapping(); diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index e7b76ccba7..7819e916fd 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -49,7 +49,7 @@ void endSimultaneousInjection(InjectionEvent *event) { event->isScheduled = false; endSimultaneousInjectionOnlyTogglePins(); - getFuelSchedule()->addFuelEventsForCylinder(event->ownIndex); + event->update(); } void turnInjectionPinLow(InjectionEvent *event) { @@ -62,7 +62,7 @@ void turnInjectionPinLow(InjectionEvent *event) { output->close(nowNt); } } - getFuelSchedule()->addFuelEventsForCylinder(event->ownIndex); + event->update(); } void InjectionEvent::onTriggerTooth(int rpm, efitick_t nowNt, float currentPhase, float nextPhase) { diff --git a/unit_tests/tests/ignition_injection/test_fuel_wall_wetting.cpp b/unit_tests/tests/ignition_injection/test_fuel_wall_wetting.cpp index 858d4c4370..dd51492fe3 100644 --- a/unit_tests/tests/ignition_injection/test_fuel_wall_wetting.cpp +++ b/unit_tests/tests/ignition_injection/test_fuel_wall_wetting.cpp @@ -76,11 +76,11 @@ TEST(fuel, testWallWettingEnrichmentScheduling) { int expectedInvocationCounter = 1; for (int i = 0; i < 4; i++) { - ASSERT_EQ(expectedInvocationCounter, engine->injectionEvents.elements[i].wallFuel.invocationCounter); + ASSERT_EQ(expectedInvocationCounter, engine->injectionEvents.elements[i].getWallFuel().invocationCounter); } // Cylinder 5 doesn't exist - shouldn't have been called! - ASSERT_EQ(0, engine->injectionEvents.elements[5].wallFuel.invocationCounter); + ASSERT_EQ(0, engine->injectionEvents.elements[5].getWallFuel().invocationCounter); eth.engine.periodicFastCallback(); eth.engine.periodicFastCallback(); @@ -88,9 +88,9 @@ TEST(fuel, testWallWettingEnrichmentScheduling) { // still same 1 per cylinder - wall wetting is NOT invoked from 'periodicFastCallback' for (int i = 0; i < 4; i++) { - ASSERT_EQ(expectedInvocationCounter, engine->injectionEvents.elements[i].wallFuel.invocationCounter); + ASSERT_EQ(expectedInvocationCounter, engine->injectionEvents.elements[i].getWallFuel().invocationCounter); } // Cylinder 5 doesn't exist - shouldn't have been called! - ASSERT_EQ(0, engine->injectionEvents.elements[5].wallFuel.invocationCounter); + ASSERT_EQ(0, engine->injectionEvents.elements[5].getWallFuel().invocationCounter); }