From 39884f89343b304e645fb4f4ac7ec43db9e4eecb Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 16 Feb 2021 13:58:54 -0800 Subject: [PATCH] use DECLARE_ENGINE_PTR for Engine class (#2365) * engine cleanup * do the cleanup * one last spot * expand_engine noop for firmware * simulator --- firmware/controllers/algo/engine.cpp | 42 +++---------------- firmware/controllers/algo/engine.h | 19 ++------- firmware/controllers/core/engine_ptr.h | 12 +++--- firmware/controllers/core/fsio_impl.cpp | 2 +- .../engine_cycle/main_trigger_callback.cpp | 4 +- firmware/hw_layer/adc/adc_inputs.h | 1 - firmware/hw_layer/hardware.cpp | 1 - firmware/hw_layer/sensors/hip9011.cpp | 2 +- firmware/rusefi.cpp | 2 +- simulator/simulator/rusEfiFunctionalTest.cpp | 2 +- unit_tests/engine_test_helper.cpp | 9 ++-- .../ignition_injection/test_fuel_map.cpp | 4 +- .../test_miata_na6_real_cranking.cpp | 2 +- .../tests/trigger/test_all_triggers.cpp | 8 ++-- .../tests/trigger/test_trigger_decoder.cpp | 11 ++--- .../tests/trigger/test_trigger_noiseless.cpp | 2 +- 16 files changed, 37 insertions(+), 86 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index e18a3fc570..5b34d5fc76 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -113,7 +113,7 @@ static void initVvtShape(Logging *logger, int index, TriggerState &initState DEC shape->initializeTriggerWaveform(logger, engineConfiguration->ambiguousOperationMode, - engine->engineConfigurationPtr->vvtCamSensorUseRise, &config); + CONFIG(vvtCamSensorUseRise), &config); shape->initializeSyncPoint(initState, engine->vvtTriggerConfiguration[index], @@ -332,11 +332,6 @@ Engine::Engine() { reset(); } -Engine::Engine(persistent_config_s *config) { - setConfig(config); - reset(); -} - /** * @see scheduleStopEngine() * @return true if there is a reason to stop engine @@ -368,29 +363,19 @@ void Engine::reset() { * so that we can prepare some helper structures */ void Engine::preCalculate(DECLARE_ENGINE_PARAMETER_SIGNATURE) { -// todo: start using this 'adcToVoltageInputDividerCoefficient' micro-optimization or... throw it away? -#if HAL_USE_ADC - adcToVoltageInputDividerCoefficient = adcToVolts(1) * engineConfiguration->analogInputDividerCoefficient; -#else - adcToVoltageInputDividerCoefficient = engineConfigurationPtr->analogInputDividerCoefficient; -#endif - #if EFI_TUNER_STUDIO // we take 2 bytes of crc32, no idea if it's right to call it crc16 or not // we have a hack here - we rely on the fact that engineMake is the first of three relevant fields tsOutputChannels.engineMakeCodeNameCrc16 = crc32(engineConfiguration->engineMake, 3 * VEHICLE_INFO_SIZE); // we need and can empty warning message for CRC purposes - memset(engine->config->warning_message, 0, sizeof(error_message_t)); - tsOutputChannels.tuneCrc16 = crc32(engine->config, sizeof(persistent_config_s)); + memset(config->warning_message, 0, sizeof(error_message_t)); + tsOutputChannels.tuneCrc16 = crc32(config, sizeof(persistent_config_s)); #endif /* EFI_TUNER_STUDIO */ } #if EFI_SHAFT_POSITION_INPUT void Engine::OnTriggerStateDecodingError() { - Engine *engine = this; - EXPAND_Engine; - warning(CUSTOM_SYNC_COUNT_MISMATCH, "trigger not happy current %d/%d/%d expected %d/%d/%d", triggerCentral.triggerState.currentCycle.eventCount[0], triggerCentral.triggerState.currentCycle.eventCount[1], @@ -418,23 +403,15 @@ void Engine::OnTriggerStateDecodingError() { } void Engine::OnTriggerStateProperState(efitick_t nowNt) { - Engine *engine = this; - EXPAND_Engine; - rpmCalculator.setSpinningUp(nowNt); } void Engine::OnTriggerSynchronizationLost() { - Engine *engine = this; - EXPAND_Engine; - // Needed for early instant-RPM detection - engine->rpmCalculator.setStopSpinning(); + rpmCalculator.setStopSpinning(); } void Engine::OnTriggerInvalidIndex(int currentIndex) { - Engine *engine = this; - EXPAND_Engine; // let's not show a warning if we are just starting to spin if (GET_RPM() != 0) { warning(CUSTOM_SYNC_ERROR, "sync error: index #%d above total size %d", currentIndex, triggerCentral.triggerShape.getSize()); @@ -446,9 +423,6 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized) { // We only care about trigger shape once we have synchronized trigger. Anything could happen // during first revolution and it's fine if (wasSynchronized) { - Engine *engine = this; - EXPAND_Engine; - /** * We can check if things are fine by comparing the number of events in a cycle with the expected number of event. */ @@ -478,9 +452,6 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized) { #endif void Engine::injectEngineReferences() { - Engine *engine = this; - EXPAND_Engine; - INJECT_ENGINE_REFERENCE(&primaryTriggerConfiguration); for (int camIndex = 0;camIndex < CAMS_PER_BANK;camIndex++) { INJECT_ENGINE_REFERENCE(&vvtTriggerConfiguration[camIndex]); @@ -494,9 +465,8 @@ void Engine::injectEngineReferences() { triggerCentral.init(PASS_ENGINE_PARAMETER_SIGNATURE); } -void Engine::setConfig(persistent_config_s *config) { - this->config = config; - engineConfigurationPtr = &config->engineConfiguration; +void Engine::setConfig(DECLARE_ENGINE_PARAMETER_SIGNATURE) { + INJECT_ENGINE_REFERENCE(this); memset(config, 0, sizeof(persistent_config_s)); injectEngineReferences(); diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 38f8ca4a1a..33c97074a8 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -81,7 +81,8 @@ protected: class Engine final : public TriggerStateListener { public: - explicit Engine(persistent_config_s *config); + DECLARE_ENGINE_PTR; + Engine(); bool isPwmEnabled = true; int triggerActivitySecond = 0; @@ -107,7 +108,7 @@ public: void OnTriggerSynchronizationLost() override; #endif - void setConfig(persistent_config_s *config); + void setConfig(DECLARE_ENGINE_PARAMETER_SIGNATURE); injection_mode_e getCurrentInjectionMode(DECLARE_ENGINE_PARAMETER_SIGNATURE); LocalVersionHolder versionForConfigurationListeners; @@ -189,7 +190,6 @@ public: */ bool isAlternatorControlEnabled = false; - bool isCltBroken = false; bool slowCallBackWasInvoked = false; /** @@ -199,12 +199,6 @@ public: efitimems64_t callFromPitStopEndTime = 0; RpmCalculator rpmCalculator; - persistent_config_s *config = nullptr; - /** - * we use funny unique name to make sure that compiler is not confused between global variable and class member - * todo: this variable is probably a sign of some problem, should we even have it? - */ - engine_configuration_s *engineConfigurationPtr = nullptr; /** * this is about 'stopengine' command @@ -325,13 +319,6 @@ public: efitick_t lastTriggerToothEventTimeNt = 0; efitick_t mainRelayBenchStartNt = 0; - - /** - * This coefficient translates ADC value directly into voltage adjusted according to - * voltage divider configuration with just one multiplication. This is a future (?) performance optimization. - */ - float adcToVoltageInputDividerCoefficient = NAN; - /** * This field is true if we are in 'cylinder cleanup' state right now * see isCylinderCleanupEnabled diff --git a/firmware/controllers/core/engine_ptr.h b/firmware/controllers/core/engine_ptr.h index a6332b3583..e36547993f 100644 --- a/firmware/controllers/core/engine_ptr.h +++ b/firmware/controllers/core/engine_ptr.h @@ -55,6 +55,11 @@ struct persistent_config_s; #define DEFINE_CONFIG_PARAM(x, y) , x y #define PASS_CONFIG_PARAM(x) , x + #define EXPAND_Engine \ + engine_configuration_s *engineConfiguration = engine->engineConfiguration; \ + persistent_config_s *config = engine->config; \ + (void)engineConfiguration; \ + (void)config; #else // EFI_UNIT_TEST // These are the non-unit-test (AKA real firmware) noop versions @@ -119,10 +124,5 @@ struct persistent_config_s; #define CONFIG_PARAM(x) CONFIG(x) #define PASS_CONFIG_PARAM(x) + #define EXPAND_Engine #endif // EFI_UNIT_TEST - -#define EXPAND_Engine \ - engine_configuration_s *engineConfiguration = engine->engineConfigurationPtr; \ - persistent_config_s *config = engine->config; \ - (void)engineConfiguration; \ - (void)config; diff --git a/firmware/controllers/core/fsio_impl.cpp b/firmware/controllers/core/fsio_impl.cpp index b450993d35..0ad1dc3a06 100644 --- a/firmware/controllers/core/fsio_impl.cpp +++ b/firmware/controllers/core/fsio_impl.cpp @@ -781,7 +781,7 @@ void runHardcodedFsio(DECLARE_ENGINE_PARAMETER_SIGNATURE) { if (isBrainPinValid(CONFIG(fanPin))) { auto clt = Sensor::get(SensorType::Clt); enginePins.fanRelay.setValue(!clt.Valid || (enginePins.fanRelay.getLogicValue() && (clt.Value > engineConfiguration->fanOffTemperature)) || - (clt.Value > engineConfiguration->fanOnTemperature) || engine->isCltBroken); + (clt.Value > engineConfiguration->fanOnTemperature) || !clt.Valid); } // see AC_RELAY_LOGIC if (isBrainPinValid(CONFIG(acRelayPin))) { diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index bcabcd0997..17aecc410a 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -78,7 +78,7 @@ void startSimultaniousInjection(Engine *engine) { EXPAND_Engine; #endif // EFI_UNIT_TEST efitick_t nowNt = getTimeNowNt(); - for (int i = 0; i < engine->engineConfigurationPtr->specs.cylindersCount; i++) { + for (int i = 0; i < CONFIG(specs.cylindersCount); i++) { enginePins.injectors[i].open(nowNt PASS_ENGINE_PARAMETER_SUFFIX); } } @@ -88,7 +88,7 @@ static void endSimultaniousInjectionOnlyTogglePins(Engine *engine) { EXPAND_Engine; #endif efitick_t nowNt = getTimeNowNt(); - for (int i = 0; i < engine->engineConfigurationPtr->specs.cylindersCount; i++) { + for (int i = 0; i < CONFIG(specs.cylindersCount); i++) { enginePins.injectors[i].close(nowNt PASS_ENGINE_PARAMETER_SUFFIX); } } diff --git a/firmware/hw_layer/adc/adc_inputs.h b/firmware/hw_layer/adc/adc_inputs.h index 3d3aee2bd1..ec0e171e9a 100644 --- a/firmware/hw_layer/adc/adc_inputs.h +++ b/firmware/hw_layer/adc/adc_inputs.h @@ -71,7 +71,6 @@ void removeChannel(const char *name, adc_channel_e setting); #define getAdcValue(msg, hwChannel) getInternalAdcValue(msg, hwChannel) -// todo: migrate to adcToVoltageInputDividerCoefficient #define adcToVoltsDivided(adc) (adcToVolts(adc) * engineConfiguration->analogInputDividerCoefficient) #else diff --git a/firmware/hw_layer/hardware.cpp b/firmware/hw_layer/hardware.cpp index bffbfa0b0a..8dfe7715eb 100644 --- a/firmware/hw_layer/hardware.cpp +++ b/firmware/hw_layer/hardware.cpp @@ -467,7 +467,6 @@ void showBor(void) { void initHardware(Logging *l) { efiAssertVoid(CUSTOM_IH_STACK, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "init h"); sharedLogger = l; - engine_configuration_s *engineConfiguration = engine->engineConfigurationPtr; efiAssertVoid(CUSTOM_EC_NULL, engineConfiguration!=NULL, "engineConfiguration"); diff --git a/firmware/hw_layer/sensors/hip9011.cpp b/firmware/hw_layer/sensors/hip9011.cpp index 8c47428206..47cab762be 100644 --- a/firmware/hw_layer/sensors/hip9011.cpp +++ b/firmware/hw_layer/sensors/hip9011.cpp @@ -305,7 +305,7 @@ void hipAdcCallback(adcsample_t adcValue) { if (instance.state == WAITING_FOR_ADC_TO_SKIP) { instance.state = WAITING_FOR_RESULT_ADC; } else if (instance.state == WAITING_FOR_RESULT_ADC) { - engine->knockVolts = adcValue * engine->adcToVoltageInputDividerCoefficient; + engine->knockVolts = adcValue * adcToVolts(1) * CONFIG(analogInputDividerCoefficient); hipValueMax = maxF(engine->knockVolts, hipValueMax); engine->knockLogic(engine->knockVolts); diff --git a/firmware/rusefi.cpp b/firmware/rusefi.cpp index 701543277d..9d3146dbbc 100644 --- a/firmware/rusefi.cpp +++ b/firmware/rusefi.cpp @@ -159,7 +159,7 @@ static void scheduleReboot(void) { void runRusEfi(void) { efiAssertVoid(CUSTOM_RM_STACK_1, getCurrentRemainingStack() > 512, "init s"); assertEngineReference(); - engine->setConfig(config); + engine->setConfig(); addConsoleAction(CMD_REBOOT, scheduleReboot); addConsoleAction(CMD_REBOOT_DFU, jump_to_bootloader); diff --git a/simulator/simulator/rusEfiFunctionalTest.cpp b/simulator/simulator/rusEfiFunctionalTest.cpp index fa3d87adea..c9e5221fbf 100644 --- a/simulator/simulator/rusEfiFunctionalTest.cpp +++ b/simulator/simulator/rusEfiFunctionalTest.cpp @@ -103,7 +103,7 @@ void rusEfiFunctionalTest(void) { initTriggerDecoderLogger(&sharedLogger); #endif /* EFI_SHAFT_POSITION_INPUT */ - engine->setConfig(config); + engine->setConfig(); initializeConsole(&sharedLogger); diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 3c4231080a..8c41d9fadb 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -53,10 +53,9 @@ EngineTestHelper::EngineTestHelper(engine_type_e engineType, configuration_callb enginePins.reset(); enginePins.unregisterPins(); - persistent_config_s *config = &persistentConfig; Engine *engine = &this->engine; - engine->setConfig(config); - engine_configuration_s *engineConfiguration = engine->engineConfigurationPtr; + engine->setConfig(engine, &persistentConfig.engineConfiguration, &persistentConfig); + EXPAND_Engine; setCurveValue(config->cltFuelCorrBins, config->cltFuelCorr, CLT_CURVE_SIZE, -40, 1.5); setCurveValue(config->cltFuelCorrBins, config->cltFuelCorr, CLT_CURVE_SIZE, -30, 1.5); @@ -150,7 +149,7 @@ void EngineTestHelper::firePrimaryTriggerRise() { Engine *engine = &this->engine; EXPAND_Engine; LogTriggerTooth(SHAFT_PRIMARY_RISING, nowNt PASS_ENGINE_PARAMETER_SUFFIX); - engine->triggerCentral.handleShaftSignal(SHAFT_PRIMARY_RISING, nowNt, engine, engine->engineConfigurationPtr, &persistentConfig); + engine->triggerCentral.handleShaftSignal(SHAFT_PRIMARY_RISING, nowNt PASS_ENGINE_PARAMETER_SUFFIX); } void EngineTestHelper::firePrimaryTriggerFall() { @@ -158,7 +157,7 @@ void EngineTestHelper::firePrimaryTriggerFall() { Engine *engine = &this->engine; EXPAND_Engine; LogTriggerTooth(SHAFT_PRIMARY_FALLING, nowNt PASS_ENGINE_PARAMETER_SUFFIX); - engine->triggerCentral.handleShaftSignal(SHAFT_PRIMARY_FALLING, nowNt, engine, engine->engineConfigurationPtr, &persistentConfig); + engine->triggerCentral.handleShaftSignal(SHAFT_PRIMARY_FALLING, nowNt PASS_ENGINE_PARAMETER_SUFFIX); } void EngineTestHelper::fireTriggerEventsWithDuration(float durationMs) { diff --git a/unit_tests/tests/ignition_injection/test_fuel_map.cpp b/unit_tests/tests/ignition_injection/test_fuel_map.cpp index ffcee5af32..48b083449a 100644 --- a/unit_tests/tests/ignition_injection/test_fuel_map.cpp +++ b/unit_tests/tests/ignition_injection/test_fuel_map.cpp @@ -23,8 +23,8 @@ TEST(misc, testFuelMap) { WITH_ENGINE_TEST_HELPER(FORD_ASPIRE_1996); for (int i = 0; i < VBAT_INJECTOR_CURVE_SIZE; i++) { - eth.engine.engineConfigurationPtr->injector.battLagCorrBins[i] = i; - eth.engine.engineConfigurationPtr->injector.battLagCorr[i] = 0.5 + 2 * i; + CONFIG(injector.battLagCorrBins[i]) = i; + CONFIG(injector.battLagCorr[i]) = 0.5 + 2 * i; } eth.engine.updateSlowSensors(PASS_ENGINE_PARAMETER_SIGNATURE); diff --git a/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp b/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp index 1272e3c3f3..5981d57fd6 100644 --- a/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp +++ b/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp @@ -29,7 +29,7 @@ static void fireTriggerEvent(EngineTestHelper*eth, double timestampS, int channe EXPAND_Engine; timeNowUs = 1000000 * timestampS; printf("MIATANA: posting time=%d event=%d\r\n", timeNowUs, event); - engine->triggerCentral.handleShaftSignal(event, getTimeNowNt(), engine, engine->engineConfigurationPtr, ð->persistentConfig); + engine->triggerCentral.handleShaftSignal(event, getTimeNowNt() PASS_ENGINE_PARAMETER_SUFFIX); } TEST(miataNA6, realCranking) { diff --git a/unit_tests/tests/trigger/test_all_triggers.cpp b/unit_tests/tests/trigger/test_all_triggers.cpp index a19d1fc644..8191004798 100644 --- a/unit_tests/tests/trigger/test_all_triggers.cpp +++ b/unit_tests/tests/trigger/test_all_triggers.cpp @@ -54,10 +54,10 @@ TEST_P(AllTriggersFixture, TestTrigger) { printf("Exporting %s\r\n", getTrigger_type_e(tt)); persistent_config_s pc; - Engine e(&pc); - Engine *engine = &e; - persistent_config_s *config = &pc; - engine_configuration_s *engineConfiguration = &pc.engineConfiguration; + Engine e; + Engine* engine = &e; + engine->setConfig(engine, &pc.engineConfiguration, &pc); + EXPAND_Engine; engineConfiguration->trigger.type = tt; engineConfiguration->ambiguousOperationMode = FOUR_STROKE_CAM_SENSOR; diff --git a/unit_tests/tests/trigger/test_trigger_decoder.cpp b/unit_tests/tests/trigger/test_trigger_decoder.cpp index 4c033e9b17..5192d58372 100644 --- a/unit_tests/tests/trigger/test_trigger_decoder.cpp +++ b/unit_tests/tests/trigger/test_trigger_decoder.cpp @@ -459,15 +459,12 @@ TEST(misc, testTriggerDecoder) { { persistent_config_s c; - Engine e(&c); + Engine e; + e.setConfig(&e, &c.engineConfiguration, &c); + Engine* engine = &e; + EXPAND_Engine; TriggerWaveform * s = &e.triggerCentral.triggerShape; - - persistent_config_s *config = &c; - Engine *engine = &e; - - engine_configuration_s *engineConfiguration = &c.engineConfiguration; - initializeSkippedToothTriggerWaveformExt(s, 2, 0, FOUR_STROKE_CAM_SENSOR); assertEqualsM("shape size", s->getSize(), 4); ASSERT_EQ(s->wave.switchTimes[0], 0.25); diff --git a/unit_tests/tests/trigger/test_trigger_noiseless.cpp b/unit_tests/tests/trigger/test_trigger_noiseless.cpp index 9fe7de572a..b6edfabfc0 100644 --- a/unit_tests/tests/trigger/test_trigger_noiseless.cpp +++ b/unit_tests/tests/trigger/test_trigger_noiseless.cpp @@ -27,7 +27,7 @@ static void fireEvent(EngineTestHelper *eth, bool isRise) { // but for noise filtering, both edges should be processed, so we fire falling events too if (isRise) eth->firePrimaryTriggerRise(); - else if (eth->engine.engineConfigurationPtr->useNoiselessTriggerDecoder) + else if (eth->engine.engineConfiguration->useNoiselessTriggerDecoder) eth->firePrimaryTriggerFall(); }