From 98040ad9af5843f60e80239b0542861c9057f4e9 Mon Sep 17 00:00:00 2001 From: Scott Smith Date: Wed, 17 Nov 2021 18:50:00 -0800 Subject: [PATCH] Make the module API more concise. (#3571) engineModules.get(). becomes modules()-> I believe the new API is more conducive to supporting arrays, by doing std::array or std::array, N>, with the support of a helper class. --- .../controllers/actuators/idle_thread.cpp | 2 +- firmware/controllers/algo/advance_map.cpp | 4 +- firmware/controllers/algo/airmass/airmass.cpp | 2 +- firmware/controllers/algo/engine.h | 8 +++ firmware/controllers/algo/engine2.cpp | 2 +- firmware/controllers/algo/fuel_math.cpp | 4 +- .../engine_cycle/main_trigger_callback.cpp | 2 +- firmware/util/containers/type_list.h | 64 +++++++------------ .../trigger/test_injection_scheduling.cpp | 2 +- .../tests/trigger/test_trigger_decoder.cpp | 8 +-- 10 files changed, 45 insertions(+), 53 deletions(-) diff --git a/firmware/controllers/actuators/idle_thread.cpp b/firmware/controllers/actuators/idle_thread.cpp index ccc03da940..f0feb5b0a3 100644 --- a/firmware/controllers/actuators/idle_thread.cpp +++ b/firmware/controllers/actuators/idle_thread.cpp @@ -553,7 +553,7 @@ void startIdleBench(void) { #endif /* EFI_UNIT_TEST */ void startIdleThread() { - engine->engineModules.unmock().init(&engineConfiguration->idleTimingPid); + engine->module().unmock().init(&engineConfiguration->idleTimingPid); getIdlePid()->initPidClass(&engineConfiguration->idleRpmPid); diff --git a/firmware/controllers/algo/advance_map.cpp b/firmware/controllers/algo/advance_map.cpp index 8784defb36..958bd9be44 100644 --- a/firmware/controllers/algo/advance_map.cpp +++ b/firmware/controllers/algo/advance_map.cpp @@ -51,7 +51,7 @@ static angle_t getRunningAdvance(int rpm, float engineLoad) { // get advance from the separate table for Idle if (engineConfiguration->useSeparateAdvanceForIdle && - engine->engineModules.unmock().isIdlingOrTaper()) { + engine->module().unmock().isIdlingOrTaper()) { float idleAdvance = interpolate2d(rpm, config->idleAdvanceBins, config->idleAdvance); auto [valid, tps] = Sensor::get(SensorType::DriverThrottleIntent); @@ -89,7 +89,7 @@ angle_t getAdvanceCorrections(int rpm) { iatCorrection = iatAdvanceCorrectionMap.getValue(rpm, iat); } - float pidTimingCorrection = engine->engineModules.unmock().getIdleTimingAdjustment(rpm); + float pidTimingCorrection = engine->module().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 238afa6b77..6706233faa 100644 --- a/firmware/controllers/algo/airmass/airmass.cpp +++ b/firmware/controllers/algo/airmass/airmass.cpp @@ -24,7 +24,7 @@ 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 (engine->engineModules.unmock().isIdlingOrTaper() && + if (engine->module().unmock().isIdlingOrTaper() && tps && engineConfiguration->useSeparateVeForIdle) { float idleVe = interpolate2d(rpm, config->idleVeBins, config->idleVe); // interpolate between idle table and normal (running) table using TPS threshold diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index b90fcfb791..df0a141b67 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -131,6 +131,14 @@ public: EngineModule // dummy placeholder so the previous entries can all have commas > engineModules; + /** + * Slightly shorter helper function to keep the code looking clean. + */ + template + auto & module() { + return engineModules.get(); + } + cyclic_buffer triggerErrorDetection; GearControllerBase *gearController; diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 72adaff273..58984c6e5e 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->engineModules.get().getInjectionDuration(injectionMass); + engine->injectionDuration = engine->module()->getInjectionDuration(injectionMass); float fuelLoad = getFuelingLoad(); injectionOffset = getInjectionOffset(rpm, fuelLoad); diff --git a/firmware/controllers/algo/fuel_math.cpp b/firmware/controllers/algo/fuel_math.cpp index 70a0c9f838..41cf836fbe 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->engineModules.get().prepare(); + engine->module()->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->engineModules.get().getFuelMassForDuration(tpsAccelPerInjection); + float tpsFuelMass = engine->module()->getFuelMassForDuration(tpsAccelPerInjection); return injectionFuelMass + tpsFuelMass; #else diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index a07fdd8131..4682d2c5aa 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->engineModules.get().getInjectionDuration(injectionMassGrams); + const floatms_t injectionDuration = engine->module()->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 index cd07130150..313acfa63b 100644 --- a/firmware/util/containers/type_list.h +++ b/firmware/util/containers/type_list.h @@ -46,9 +46,13 @@ struct type_list { } /* - * Return the object representing the type get_t. May return a mocked get_t::interface_t instead. + * Return the container object for type get_t. * * get_t should not be Mockable, it should be the actual type. + * The return object will have the methods unmock(), operator->, operator*, and if + * Mockable, set(). + * + * The return type is type_list or type_list> */ template auto get() -> std::enable_if_t(), @@ -62,39 +66,6 @@ struct type_list { 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 * @@ -128,14 +99,20 @@ public: template()>> auto & get() { - return me; + return *this; } - template()>> auto & unmock() { return me; } + base_t * operator->() { + return &me; + } + + base_t & operator*() { + return me; + } }; #if !EFI_UNIT_TEST @@ -173,22 +150,29 @@ public: template()>> auto & get() { - return *cur; + return *this; } - template()>> auto & unmock() { return me; } - template - auto set(param_t ptr) -> std::enable_if_t()> { + void set(typename base_t::interface_t * ptr) { if (ptr) { cur = ptr; } else { cur = &me; } } + + auto * operator->() { + return cur; + } + + auto & operator*() { + return *cur; + } + }; #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 e633d01a58..d199ff1bf6 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->engineModules.set(&im); + engine->module().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 4685463bf1..a49ea8607c 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->engineModules.set(&im); + engine->module().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->engineModules.set(&im2); + engine->module().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->engineModules.set(&im); + engine->module().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->engineModules.set(&im2); + engine->module().set(&im2); ASSERT_EQ( 10, getInjectorDutyCycle(GET_RPM())) << "Lduty for maf=3";