From 7b790d36f60746bb6f62bb31fccf1539202ce5c2 Mon Sep 17 00:00:00 2001 From: Andrey Date: Mon, 15 Nov 2021 20:22:05 -0500 Subject: [PATCH] random refactoring: trigger central encapsulation --- firmware/console/status_loop.cpp | 2 +- firmware/controllers/algo/engine.cpp | 2 +- firmware/controllers/algo/engine.h | 7 +----- .../engine_cycle/main_trigger_callback.cpp | 2 +- .../controllers/trigger/trigger_central.cpp | 22 ++++++++++--------- .../controllers/trigger/trigger_central.h | 17 ++++++++++---- unit_tests/engine_test_helper.cpp | 4 ++-- 7 files changed, 31 insertions(+), 25 deletions(-) diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index e0ace2b441..375e883672 100644 --- a/firmware/console/status_loop.cpp +++ b/firmware/console/status_loop.cpp @@ -332,7 +332,7 @@ static void initStatusLeds() { static bool isTriggerErrorNow() { #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT bool justHadError = (getTimeNowNt() - engine->triggerCentral.triggerState.lastDecodingErrorTime) < MS2NT(200); - return justHadError || isTriggerDecoderError(); + return justHadError || engine->triggerCentral.isTriggerDecoderError(); #else return false; #endif /* EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT */ diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index c4fc57a983..7b550e047f 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -448,7 +448,7 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized) { engine->triggerErrorDetection.add(isDecodingError); - if (isTriggerDecoderError(PASS_ENGINE_PARAMETER_SIGNATURE)) { + if (triggerCentral.isTriggerDecoderError(PASS_ENGINE_PARAMETER_SIGNATURE)) { warning(CUSTOM_OBD_TRG_DECODING, "trigger decoding issue. expected %d/%d/%d got %d/%d/%d", TRIGGER_WAVEFORM(getExpectedEventCount(0)), TRIGGER_WAVEFORM(getExpectedEventCount(1)), diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index dde6c9f3e9..43afcec4bc 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -157,12 +157,7 @@ public: bool hwTriggerInputEnabled = true; int getGlobalConfigurationVersion(void) const; - /** - * true if a recent configuration change has changed any of the trigger settings which - * we have not adjusted for yet - */ - bool isTriggerConfigChanged = false; - LocalVersionHolder triggerVersion; + // a pointer with interface type would make this code nicer but would carry extra runtime // cost to resolve pointer, we use instances as a micro optimization diff --git a/firmware/controllers/engine_cycle/main_trigger_callback.cpp b/firmware/controllers/engine_cycle/main_trigger_callback.cpp index 815084c58d..ebfd23e8b4 100644 --- a/firmware/controllers/engine_cycle/main_trigger_callback.cpp +++ b/firmware/controllers/engine_cycle/main_trigger_callback.cpp @@ -426,7 +426,7 @@ void mainTriggerCallback(uint32_t trgEventIndex, efitick_t edgeTimestamp DECLARE engine->triggerCentral.validateCamVvtCounters(); } - if (checkIfTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) { + if (engine->triggerCentral.checkIfTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) { engine->ignitionEvents.isReady = false; // we need to rebuild complete ignition schedule engine->injectionEvents.isReady = false; // moved 'triggerIndexByAngle' into trigger initialization (why was it invoked from here if it's only about trigger shape & optimization?) diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 8fdfc55dee..cea276a0a1 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -733,8 +733,10 @@ void triggerInfo(void) { efiPrintf("synchronizationNeeded=%s/isError=%s/total errors=%d ord_err=%d/total revolutions=%d/self=%s", boolToString(ts->isSynchronizationNeeded), - boolToString(isTriggerDecoderError()), engine->triggerCentral.triggerState.totalTriggerErrorCounter, - engine->triggerCentral.triggerState.orderingErrorCounter, engine->triggerCentral.triggerState.getTotalRevolutionCounter(), + boolToString(engine->triggerCentral.isTriggerDecoderError()), + engine->triggerCentral.triggerState.totalTriggerErrorCounter, + engine->triggerCentral.triggerState.orderingErrorCounter, + engine->triggerCentral.triggerState.getTotalRevolutionCounter(), boolToString(engine->directSelfStimulation)); if (TRIGGER_WAVEFORM(isSynchronizationNeeded)) { @@ -835,24 +837,24 @@ void onConfigurationChangeTriggerCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) { #endif } #if EFI_DEFAILED_LOGGING - efiPrintf("isTriggerConfigChanged=%d", engine->isTriggerConfigChanged); + efiPrintf("isTriggerConfigChanged=%d", triggerConfigChanged); #endif /* EFI_DEFAILED_LOGGING */ // we do not want to miss two updates in a row - engine->isTriggerConfigChanged = engine->isTriggerConfigChanged || changed; + engine->triggerCentral.triggerConfigChanged = engine->triggerCentral.triggerConfigChanged || changed; } /** * @returns true if configuration just changed, and if that change has affected trigger */ -bool checkIfTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE) { - bool result = engine->triggerVersion.isOld(engine->getGlobalConfigurationVersion()) && engine->isTriggerConfigChanged; - engine->isTriggerConfigChanged = false; // whoever has called the method is supposed to react to changes +bool TriggerCentral::checkIfTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE) { + bool result = triggerVersion.isOld(engine->getGlobalConfigurationVersion()) && triggerConfigChanged; + triggerConfigChanged = false; // whoever has called the method is supposed to react to changes return result; } -bool isTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE) { - return engine->isTriggerConfigChanged; +bool TriggerCentral::isTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE) { + return triggerConfigChanged; } void validateTriggerInputs(DECLARE_ENGINE_PARAMETER_SIGNATURE) { @@ -887,7 +889,7 @@ void initTriggerCentral() { /** * @return TRUE is something is wrong with trigger decoding */ -bool isTriggerDecoderError(DECLARE_ENGINE_PARAMETER_SIGNATURE) { +bool TriggerCentral::isTriggerDecoderError(DECLARE_ENGINE_PARAMETER_SIGNATURE) { return engine->triggerErrorDetection.sum(6) > 4; } diff --git a/firmware/controllers/trigger/trigger_central.h b/firmware/controllers/trigger/trigger_central.h index 7374c04ab0..8f6f228eb0 100644 --- a/firmware/controllers/trigger/trigger_central.h +++ b/firmware/controllers/trigger/trigger_central.h @@ -13,6 +13,7 @@ #include "trigger_central_generated.h" #include "timer.h" #include "pin_repository.h" +#include "local_version_holder.h" class Engine; typedef void (*ShaftPositionListener)(trigger_event_e signal, uint32_t index, efitick_t edgeTimestamp DECLARE_ENGINE_PARAMETER_SUFFIX); @@ -45,6 +46,18 @@ public: void resetCounters(); void validateCamVvtCounters(); + /** + * true if a recent configuration change has changed any of the trigger settings which + * we have not adjusted for yet + */ + bool triggerConfigChanged = false; + LocalVersionHolder triggerVersion; + + bool checkIfTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE); + bool isTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE); + + bool isTriggerDecoderError(DECLARE_ENGINE_PARAMETER_SIGNATURE); + expected getCurrentEnginePhase(efitick_t nowNt) const; float getTimeSinceTriggerEvent(efitick_t nowNt) const { @@ -104,10 +117,6 @@ void initTriggerCentral(); int isSignalDecoderError(void); void onConfigurationChangeTriggerCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE); -bool checkIfTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE); -bool isTriggerConfigChanged(DECLARE_ENGINE_PARAMETER_SIGNATURE); - -bool isTriggerDecoderError(DECLARE_ENGINE_PARAMETER_SIGNATURE); #define SYMMETRICAL_CRANK_SENSOR_DIVIDER 4 #define SYMMETRICAL_THREE_TIMES_CRANK_SENSOR_DIVIDER 6 diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 9973fe82a5..bfa0e4d76b 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -374,14 +374,14 @@ void setupSimpleTestEngineWithMaf(EngineTestHelper *eth, injection_mode_e inject // this is needed to update injectorLag engine->updateSlowSensors(PASS_ENGINE_PARAMETER_SIGNATURE); - ASSERT_EQ( 0, isTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) << "trigger #1"; + ASSERT_EQ( 0, engine->triggerCentral.isTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) << "trigger #1"; eth->setTriggerType(trigger PASS_ENGINE_PARAMETER_SUFFIX); } void EngineTestHelper::setTriggerType(trigger_type_e trigger DECLARE_ENGINE_PARAMETER_SUFFIX) { engineConfiguration->trigger.type = trigger; incrementGlobalConfigurationVersion(PASS_ENGINE_PARAMETER_SIGNATURE); - ASSERT_EQ( 1, isTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) << "trigger #2"; + ASSERT_EQ( 1, engine->triggerCentral.isTriggerConfigChanged(PASS_ENGINE_PARAMETER_SIGNATURE)) << "trigger #2"; applyTriggerWaveform(); }