diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index 90e51f7de5..ccc03da940 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -472,26 +472,11 @@ float IdleController::getClosedLoop(IIdleController::Phase phase, float tpsPos, return iacPosition; } -void IdleController::update() { +void IdleController::onSlowCallback() { float position = getIdlePosition(); applyIACposition(position); } -IdleController idleControllerInstance; - -void updateIdleControl() -{ - idleControllerInstance.update(); -} - -float getIdleTimingAdjustment(int rpm) { - return idleControllerInstance.getIdleTimingAdjustment(rpm); -} - -bool isIdlingOrTaper() { - return idleControllerInstance.isIdlingOrTaper(); -} - static void applyPidSettings() { getIdlePid()->updateFactors(engineConfiguration->idleRpmPid.pFactor, engineConfiguration->idleRpmPid.iFactor, engineConfiguration->idleRpmPid.dFactor); iacPidMultMap.init(engineConfiguration->iacPidMultTable, engineConfiguration->iacPidMultLoadBins, engineConfiguration->iacPidMultRpmBins); @@ -518,13 +503,15 @@ void setDefaultIdleParameters() { engineConfiguration->idlePidRpmUpperLimit = 100; } +void IdleController::onConfigurationChange(engine_configuration_s const * previousConfiguration) { #if ! EFI_UNIT_TEST - -void onConfigurationChangeIdleCallback(engine_configuration_s *previousConfiguration) { engine->idle.shouldResetPid = !getIdlePid()->isSame(&previousConfiguration->idleRpmPid); engine->idle.mustResetPid = engine->idle.shouldResetPid; +#endif } +#if ! EFI_UNIT_TEST + void setTargetIdleRpm(int value) { setTargetRpmCurve(value); efiPrintf("target idle RPM %d", value); @@ -566,9 +553,7 @@ void startIdleBench(void) { #endif /* EFI_UNIT_TEST */ void startIdleThread() { - idleControllerInstance.init(&engineConfiguration->idleTimingPid); - - engine->idleController = &idleControllerInstance; + engine->engineModules.unmock().init(&engineConfiguration->idleTimingPid); getIdlePid()->initPidClass(&engineConfiguration->idleRpmPid); diff --git a/firmware/controllers/actuators/idle_thread.h b/firmware/controllers/actuators/idle_thread.h index 8079ecfdef..19bc8eda43 100644 --- a/firmware/controllers/actuators/idle_thread.h +++ b/firmware/controllers/actuators/idle_thread.h @@ -8,6 +8,7 @@ #pragma once +#include "engine_module.h" #include "engine_ptr.h" #include "rusefi_types.h" #include "periodic_task.h" @@ -31,12 +32,13 @@ struct IIdleController { virtual float getCrankingTaperFraction() const = 0; }; -class IdleController : public IIdleController { +class IdleController : public IIdleController, public EngineModule { public: + typedef IIdleController interface_t; + void init(pid_s* idlePidConfig); float getIdlePosition(); - void update(); // TARGET DETERMINATION int getTargetRpm(float clt) const override; @@ -56,6 +58,9 @@ public: // CLOSED LOOP CORRECTION float getClosedLoop(IIdleController::Phase phase, float tpsPos, int rpm, int targetRpm) override; + void onConfigurationChange(engine_configuration_s const * previousConfig) final; + void onSlowCallback() final; + // Allow querying state from outside bool isIdlingOrTaper() { return m_lastPhase == Phase::Idling || (engineConfiguration->useSeparateIdleTablesForCrankingTaper && m_lastPhase == Phase::CrankToIdleTaper); @@ -72,13 +77,8 @@ private: Pid m_timingPid; }; -void updateIdleControl(); percent_t getIdlePosition(); -float getIdleTimingAdjustment(int rpm); - -bool isIdlingOrTaper(); - void applyIACposition(percent_t position); void setManualIdleValvePosition(int positionPercent); @@ -91,8 +91,6 @@ void setIdleIFactor(float value); void setIdleDFactor(float value); void setIdleMode(idle_mode_e value); void setTargetIdleRpm(int value); -void onConfigurationChangeIdleCallback(engine_configuration_s *previousConfiguration); Pid * getIdlePid(); void startPedalPins(); void stopPedalPins(); - diff --git a/firmware/controllers/algo/advance_map.cpp b/firmware/controllers/algo/advance_map.cpp index 3b3ff1ca4c..326d824599 100644 --- a/firmware/controllers/algo/advance_map.cpp +++ b/firmware/controllers/algo/advance_map.cpp @@ -50,7 +50,8 @@ static angle_t getRunningAdvance(int rpm, float engineLoad) { float advanceAngle = advanceMap.getValue((float) rpm, engineLoad); // get advance from the separate table for Idle - if (engineConfiguration->useSeparateAdvanceForIdle && isIdlingOrTaper()) { + if (engineConfiguration->useSeparateAdvanceForIdle && + engine->engineModules.unmock().isIdlingOrTaper()) { float idleAdvance = interpolate2d(rpm, config->idleAdvanceBins, config->idleAdvance); auto [valid, tps] = Sensor::get(SensorType::DriverThrottleIntent); @@ -88,7 +89,7 @@ angle_t getAdvanceCorrections(int rpm) { iatCorrection = iatAdvanceCorrectionMap.getValue(rpm, iat); } - float pidTimingCorrection = getIdleTimingAdjustment(rpm); + float pidTimingCorrection = ENGINE(engineModules).unmock().getIdleTimingAdjustment(rpm); if (engineConfiguration->debugMode == DBG_IGNITION_TIMING) { #if EFI_TUNER_STUDIO diff --git a/firmware/controllers/algo/airmass/airmass.cpp b/firmware/controllers/algo/airmass/airmass.cpp index ccd6812fc5..238afa6b77 100644 --- a/firmware/controllers/algo/airmass/airmass.cpp +++ b/firmware/controllers/algo/airmass/airmass.cpp @@ -24,7 +24,8 @@ float AirmassVeModelBase::getVe(int rpm, float load) const { auto tps = Sensor::get(SensorType::Tps1); // get VE from the separate table for Idle if idling - if (isIdlingOrTaper() && tps && engineConfiguration->useSeparateVeForIdle) { + if (engine->engineModules.unmock().isIdlingOrTaper() && + tps && engineConfiguration->useSeparateVeForIdle) { float idleVe = interpolate2d(rpm, config->idleVeBins, config->idleVe); // interpolate between idle table and normal (running) table using TPS threshold ve = interpolateClamped(0.0f, idleVe, engineConfiguration->idlePidDeactivationTpsThreshold, ve, tps.Value); diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index f088da8fdc..601a31089a 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -235,7 +235,7 @@ void Engine::periodicSlowCallback() { updateGppwm(); - updateIdleControl(); + ENGINE(engineModules).apply_all([](auto & m) { m.onSlowCallback(); }); #if EFI_BOOST_CONTROL updateBoostControl(); @@ -631,6 +631,8 @@ void Engine::periodicFastCallback() { knockController.periodicFastCallback(); tachSignalCallback(); + + ENGINE(engineModules).apply_all([](auto & m) { m.onFastCallback(); }); } void doScheduleStopEngine() { diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index ef4821e61e..d75c26b72d 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -23,7 +23,10 @@ #include "ac_control.h" #include "knock_logic.h" #include "idle_state_generated.h" +#include "idle_thread.h" +#include "injector_model.h" #include "launch_control.h" +#include "type_list.h" #if EFI_SIGNAL_EXECUTOR_ONE_TIMER // PROD real firmware uses this implementation @@ -59,7 +62,6 @@ struct AirmassModelBase; class IEtbController; struct IFuelComputer; -struct IInjectorModel; struct IIdleController; class PrimaryTriggerConfiguration final : public TriggerConfiguration { @@ -108,8 +110,14 @@ public: IEtbController *etbControllers[ETB_COUNT] = {nullptr}; IFuelComputer *fuelComputer = nullptr; - IInjectorModel *injectorModel = nullptr; - IIdleController* idleController = nullptr; + + type_list< + Mockable, +#if EFI_IDLE_CONTROL + IdleController, +#endif + EngineModule // dummy placeholder so the previous entries can all have commas + > engineModules; cyclic_buffer triggerErrorDetection; diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 3b52e65ddf..72adaff273 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/firmware/controllers/algo/engine2.cpp @@ -155,7 +155,7 @@ void EngineState::periodicFastCallback() { } // Store the pre-wall wetting injection duration for scheduling purposes only, not the actual injection duration - engine->injectionDuration = engine->injectorModel->getInjectionDuration(injectionMass); + engine->injectionDuration = engine->engineModules.get().getInjectionDuration(injectionMass); float fuelLoad = getFuelingLoad(); injectionOffset = getInjectionOffset(rpm, fuelLoad); diff --git a/firmware/controllers/algo/engine_configuration.cpp b/firmware/controllers/algo/engine_configuration.cpp index 079f9fed53..6106a8c555 100644 --- a/firmware/controllers/algo/engine_configuration.cpp +++ b/firmware/controllers/algo/engine_configuration.cpp @@ -186,10 +186,6 @@ void incrementGlobalConfigurationVersion() { onConfigurationChangeElectronicThrottleCallback(&activeConfiguration); #endif /* EFI_ELECTRONIC_THROTTLE_BODY */ -#if EFI_IDLE_CONTROL && ! EFI_UNIT_TEST - onConfigurationChangeIdleCallback(&activeConfiguration); -#endif /* EFI_IDLE_CONTROL */ - #if EFI_SHAFT_POSITION_INPUT onConfigurationChangeTriggerCallback(); #endif /* EFI_SHAFT_POSITION_INPUT */ @@ -200,6 +196,10 @@ void incrementGlobalConfigurationVersion() { #if EFI_FSIO onConfigurationChangeFsioCallback(&activeConfiguration); #endif /* EFI_FSIO */ + + ENGINE(engineModules).apply_all([](auto & m) { + m.onConfigurationChange(&activeConfiguration); + }); rememberCurrentConfiguration(); } diff --git a/firmware/controllers/algo/fuel/injector_model.h b/firmware/controllers/algo/fuel/injector_model.h index b0979bf2d5..c873601168 100644 --- a/firmware/controllers/algo/fuel/injector_model.h +++ b/firmware/controllers/algo/fuel/injector_model.h @@ -2,7 +2,7 @@ #include "expected.h" -struct IInjectorModel { +struct IInjectorModel : public EngineModule { virtual void prepare() = 0; virtual floatms_t getInjectionDuration(float fuelMassGram) const = 0; virtual float getFuelMassForDuration(floatms_t duration) const = 0; @@ -38,4 +38,6 @@ public: // Small pulse correction logic float correctShortPulse(float baseDuration) const override; virtual float correctInjectionPolynomial(float baseDuration) const; + + using interface_t = IInjectorModel; // Mock interface }; diff --git a/firmware/controllers/algo/fuel_math.cpp b/firmware/controllers/algo/fuel_math.cpp index 51155023be..7181d705c0 100644 --- a/firmware/controllers/algo/fuel_math.cpp +++ b/firmware/controllers/algo/fuel_math.cpp @@ -279,7 +279,7 @@ float getInjectionMass(int rpm) { float injectionFuelMass = cycleFuelMass * durationMultiplier; // Prepare injector flow rate & deadtime - engine->injectorModel->prepare(); + engine->engineModules.get().prepare(); floatms_t tpsAccelEnrich = ENGINE(tpsAccelEnrichment.getTpsEnrichment()); efiAssert(CUSTOM_ERR_ASSERT, !cisnan(tpsAccelEnrich), "NaN tpsAccelEnrich", 0); @@ -288,7 +288,7 @@ float getInjectionMass(int rpm) { // For legacy reasons, the TPS accel table is in units of milliseconds, so we have to convert BACK to mass float tpsAccelPerInjection = durationMultiplier * tpsAccelEnrich; - float tpsFuelMass = engine->injectorModel->getFuelMassForDuration(tpsAccelPerInjection); + float tpsFuelMass = engine->engineModules.get().getFuelMassForDuration(tpsAccelPerInjection); return injectionFuelMass + tpsFuelMass; #else @@ -297,7 +297,6 @@ float getInjectionMass(int rpm) { } static FuelComputer fuelComputer(lambdaMap); -static InjectorModel injectorModel; /** * @brief Initialize fuel map data structure @@ -308,7 +307,6 @@ void initFuelMap() { engine->fuelComputer = &fuelComputer; - engine->injectorModel = &injectorModel; mapEstimationTable.init(config->mapEstimateTable, config->mapEstimateTpsBins, config->mapEstimateRpmBins); diff --git a/firmware/controllers/core/engine_module.h b/firmware/controllers/core/engine_module.h new file mode 100644 index 0000000000..16d5073fc4 --- /dev/null +++ b/firmware/controllers/core/engine_module.h @@ -0,0 +1,13 @@ +#pragma once + +class EngineModule { +public: + // Called when the engine_configuration_s part of the tune has changed. + virtual void onConfigurationChange(engine_configuration_s const * /*previousConfig*/) { } + + // Called approx 20Hz + virtual void onSlowCallback() { } + + // Called approx 200Hz + virtual void onFastCallback() { } +}; diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index 160db0886f..9bd6015505 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -172,7 +172,7 @@ void InjectionEvent::onTriggerTooth(size_t trgEventIndex, int rpm, efitick_t now // Perform wall wetting adjustment on fuel mass, not duration, so that // it's correct during fuel pressure (injector flow) or battery voltage (deadtime) transients injectionMassGrams = wallFuel.adjust(injectionMassGrams); - const floatms_t injectionDuration = engine->injectorModel->getInjectionDuration(injectionMassGrams); + const floatms_t injectionDuration = engine->engineModules.get().getInjectionDuration(injectionMassGrams); #if EFI_PRINTF_FUEL_DETAILS if (printFuelDebug) { diff --git a/firmware/util/containers/type_list.h b/firmware/util/containers/type_list.h new file mode 100644 index 0000000000..cd07130150 --- /dev/null +++ b/firmware/util/containers/type_list.h @@ -0,0 +1,194 @@ +#pragma once + +/* + * Indicates that a member of type_list should be able to be replaced in a unit test. + */ +template +struct Mockable; + +/* + * Instantiates each type, allowing you to fetch by type. + * + * This is basically std::tuple, std::get, and std::apply, but with the API oriented more toward + * our needs. + * + * The primary use of this is to provide a list of engine modules in the giant Engine structure. + * + * The main feature is that objects are mockable. If you pass in Mockable instead of T, and + * typedef T::interface_t, then you can call set to change what get returns. + * + * For a normal type T, + * T & get(); + * T & unmock(); + * + * For Mockable, + * T::interface_t & get(); + * T & unmock(); + */ +template +struct type_list { + type_list first; + type_list others; + + static_assert(!decltype(others)::template has(), + "Each type can only be listed once."); + + /* + * Call the given function on the unmocked version of each type. + * + * It is probably best (and maybe only possible?) to call this with a generic lambda, as: + * tl.apply_all([](auto & m) { m.WhateverFuncIWant(); }); + */ + template + void apply_all(func_t const & f) { + first.apply_all(f); + others.apply_all(f); + } + + /* + * Return the object representing the type get_t. May return a mocked get_t::interface_t instead. + * + * get_t should not be Mockable, it should be the actual type. + */ + template + auto get() -> std::enable_if_t(), + decltype(first.template get())> { + return first.template get(); + } + + template + auto get() -> std::enable_if_t(), + decltype(others.template get())> { + return others.template get(); + } + + /* + * Return the regular, unmocked object unmock_t. + * + * unmock_t should not be Mockable, it should be the actual type. + */ + template + auto unmock() -> std::enable_if_t(), unmock_t &> { + return first.template unmock(); + } + + template + auto unmock() -> std::enable_if_t(), unmock_t &> { + return others.template unmock(); + } + +#if EFI_UNIT_TEST + /* + * Change the object returned by get(). + * + * param_t should be a pointer or nullptr. If you pass nullptr, it will be reset to the + * unmock version. + */ + template + auto set(param_t ptr) -> std::enable_if_t()> { + first.template set(ptr); + } + + template + auto set(param_t ptr) -> std::enable_if_t()> { + others.template set(ptr); + } +#endif // EFI_UNIT_TEST + + /* + * Returns whether has_t exists in the type list + * + * has_t should not be Mockable, it should be the actual type. + */ + template + static constexpr bool has() { + return decltype(first)::template has() || + decltype(others)::template has(); + } +}; + +/* + * Specialization of type_list for a single base_t (that is not Mockable<>). + */ +template +struct type_list { +private: + base_t me; + +public: + template + void apply_all(func_t const & f) { + f(me); + } + + template + static constexpr bool has() { + return std::is_same_v; + } + + template()>> + auto & get() { + return me; + } + + template()>> + auto & unmock() { + return me; + } + +}; + +#if !EFI_UNIT_TEST + +/* + * Production specialization of type_list for a single Mockable. + * + * Performs exactly as base_t. + */ +template +struct type_list> : public type_list { +}; + +#else // if EFI_UNIT_TEST: + +/* + * Unit test specialization of type_list for a single Mockable. + */ +template +struct type_list> { +private: + base_t me; + typename base_t::interface_t * cur = &me; + +public: + template + void apply_all(func_t const & f) { + f(me); + } + + template + static constexpr bool has() { + return std::is_same_v; + } + + template()>> + auto & get() { + return *cur; + } + + template()>> + auto & unmock() { + return me; + } + + template + auto set(param_t ptr) -> std::enable_if_t()> { + if (ptr) { + cur = ptr; + } else { + cur = &me; + } + } +}; + +#endif // EFI_UNIT_TEST diff --git a/unit_tests/tests/trigger/test_injection_scheduling.cpp b/unit_tests/tests/trigger/test_injection_scheduling.cpp index 362762d425..e633d01a58 100644 --- a/unit_tests/tests/trigger/test_injection_scheduling.cpp +++ b/unit_tests/tests/trigger/test_injection_scheduling.cpp @@ -22,7 +22,7 @@ TEST(injectionScheduling, NormalDutyCycle) { // Injection duration of 20ms MockInjectorModel2 im; EXPECT_CALL(im, getInjectionDuration(_)).WillOnce(Return(20.0f)); - engine->injectorModel = &im; + engine->engineModules.set(&im); { InSequence is; diff --git a/unit_tests/tests/trigger/test_trigger_decoder.cpp b/unit_tests/tests/trigger/test_trigger_decoder.cpp index 8960d43d82..4685463bf1 100644 --- a/unit_tests/tests/trigger/test_trigger_decoder.cpp +++ b/unit_tests/tests/trigger/test_trigger_decoder.cpp @@ -697,7 +697,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { // Injection duration of 12.5ms MockInjectorModel2 im; EXPECT_CALL(im, getInjectionDuration(_)).WillRepeatedly(Return(12.5f)); - engine->injectorModel = &im; + engine->engineModules.set(&im); assertEqualsM("duty for maf=3", 62.5, getInjectorDutyCycle(GET_RPM())); @@ -857,7 +857,7 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { // Injection duration of 17.5ms MockInjectorModel2 im2; EXPECT_CALL(im2, getInjectionDuration(_)).WillRepeatedly(Return(17.5f)); - engine->injectorModel = &im2; + engine->engineModules.set(&im2); // duty cycle above 75% is a special use-case because 'special' fuel event overlappes the next normal event in batch mode assertEqualsM("duty for maf=3", 87.5, getInjectorDutyCycle(GET_RPM())); @@ -994,7 +994,7 @@ TEST(big, testFuelSchedulerBug299smallAndLarge) { // Injection duration of 17.5ms MockInjectorModel2 im; EXPECT_CALL(im, getInjectionDuration(_)).WillRepeatedly(Return(17.5f)); - engine->injectorModel = &im; + engine->engineModules.set(&im); assertEqualsM("Lduty for maf=3", 87.5, getInjectorDutyCycle(GET_RPM())); @@ -1061,7 +1061,7 @@ TEST(big, testFuelSchedulerBug299smallAndLarge) { engine->injectionDuration = 2.0f; MockInjectorModel2 im2; EXPECT_CALL(im2, getInjectionDuration(_)).WillRepeatedly(Return(2.0f)); - engine->injectorModel = &im2; + engine->engineModules.set(&im2); ASSERT_EQ( 10, getInjectorDutyCycle(GET_RPM())) << "Lduty for maf=3";