From aa0cffcfdb2820b0805d67bd2ca56ea993f778ff Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 14 Sep 2022 02:06:52 -0400 Subject: [PATCH] deep rabbit holes are the best ones! refactoring: encapsulation --- firmware/controllers/algo/engine.cpp | 4 ++ firmware/controllers/algo/engine.h | 9 --- firmware/controllers/engine_controller.cpp | 2 +- firmware/controllers/settings.cpp | 4 +- .../controllers/trigger/trigger_central.cpp | 56 ++++++++++--------- .../controllers/trigger/trigger_central.h | 11 ++++ .../trigger/trigger_emulator_algo.cpp | 8 +-- 7 files changed, 52 insertions(+), 42 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index cacf620753..b64ce3c4c8 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -605,3 +605,7 @@ TunerStudioOutputChannels *getTunerStudioOutputChannels() { ExecutorInterface *getExecutorInterface() { return &engine->executor; } + +TriggerCentral * getTriggerCentral() { + return &engine->triggerCentral; +} diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 809d3ae266..a287393ae9 100644 --- a/firmware/controllers/algo/engine.h +++ b/firmware/controllers/algo/engine.h @@ -197,10 +197,6 @@ public: #endif /* EFI_UNIT_TEST */ - // this is useful at least for real hardware integration testing - maybe a proper solution would be to simply - // GND input pins instead of leaving them floating - bool hwTriggerInputEnabled = true; - int getGlobalConfigurationVersion(void) const; @@ -283,11 +279,6 @@ public: */ bool isFunctionalTestMode = false; - /** - * See also triggerSimulatorFrequency - */ - bool directSelfStimulation = false; - void resetEngineSnifferIfInTestMode(); /** diff --git a/firmware/controllers/engine_controller.cpp b/firmware/controllers/engine_controller.cpp index 59bf5c048b..0b15a97809 100644 --- a/firmware/controllers/engine_controller.cpp +++ b/firmware/controllers/engine_controller.cpp @@ -180,7 +180,7 @@ static void doPeriodicSlowCallback() { engine->rpmCalculator.onSlowCallback(); - if (engine->directSelfStimulation || engine->rpmCalculator.isStopped()) { + if (engine->triggerCentral.directSelfStimulation || engine->rpmCalculator.isStopped()) { /** * rusEfi usually runs on hardware which halts execution while writing to internal flash, so we * postpone writes to until engine is stopped. Writes in case of self-stimulation are fine. diff --git a/firmware/controllers/settings.cpp b/firmware/controllers/settings.cpp index 4bc278a317..a3a9f9861a 100644 --- a/firmware/controllers/settings.cpp +++ b/firmware/controllers/settings.cpp @@ -692,7 +692,7 @@ static void setSpiMode(int index, bool mode) { static void enableOrDisable(const char *param, bool isEnabled) { if (strEqualCaseInsensitive(param, CMD_TRIGGER_HW_INPUT)) { - engine->hwTriggerInputEnabled = isEnabled; + getTriggerCentral()->hwTriggerInputEnabled = isEnabled; } else if (strEqualCaseInsensitive(param, "useTLE8888_cranking_hack")) { engineConfiguration->useTLE8888_cranking_hack = isEnabled; } else if (strEqualCaseInsensitive(param, "verboseTLE8888")) { @@ -918,7 +918,7 @@ static void getValue(const char *paramStr) { } else if (strEqualCaseInsensitive(paramStr, "global_trigger_offset_angle")) { efiPrintf("global_trigger_offset=%.2f", engineConfiguration->globalTriggerAngleOffset); } else if (strEqualCaseInsensitive(paramStr, "trigger_hw_input")) { - efiPrintf("trigger_hw_input=%s", boolToString(engine->hwTriggerInputEnabled)); + efiPrintf("trigger_hw_input=%s", boolToString(getTriggerCentral()->hwTriggerInputEnabled)); } else if (strEqualCaseInsensitive(paramStr, "is_enabled_spi_1")) { efiPrintf("is_enabled_spi_1=%s", boolToString(engineConfiguration->is_enabled_spi_1)); } else if (strEqualCaseInsensitive(paramStr, "is_enabled_spi_2")) { diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 6a033e051c..7db84b5d92 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -154,7 +154,7 @@ static angle_t adjustCrankPhase(int camIndex) { return 0; } - TriggerCentral *tc = &engine->triggerCentral; + TriggerCentral *tc = getTriggerCentral(); operation_mode_e operationMode = getEngineRotationState()->getOperationMode(); vvt_mode_e vvtMode = engineConfiguration->vvtMode[camIndex]; @@ -235,14 +235,14 @@ static void logFront(bool isImportantFront, efitick_t nowNt, int index) { } void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) { - if (engine->directSelfStimulation || !engine->hwTriggerInputEnabled) { + TriggerCentral *tc = getTriggerCentral(); + if (tc->directSelfStimulation || !tc->hwTriggerInputEnabled) { // sensor noise + self-stim = loss of trigger sync return; } int bankIndex = index / CAMS_PER_BANK; int camIndex = index % CAMS_PER_BANK; - TriggerCentral *tc = &engine->triggerCentral; if (front == TriggerValue::RISE) { tc->vvtEventRiseCounter[index]++; } else { @@ -301,7 +301,7 @@ void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) { "vvt", tc->vvtShape[camIndex], nullptr, - engine->triggerCentral.vvtTriggerConfiguration[camIndex], + tc->vvtTriggerConfiguration[camIndex], front == TriggerValue::RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt); // yes we log data from all VVT channels into same fields for now tc->triggerState.vvtSyncGapRatio = vvtDecoder.triggerSyncGapRatio; @@ -402,6 +402,7 @@ uint32_t triggerMaxDuration = 0; * - Trigger replay from CSV (unit tests) */ void hwHandleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) { + TriggerCentral *tc = getTriggerCentral(); ScopePerf perf(PE::HandleShaftSignal); #if VR_HW_CHECK_MODE // some boards do not have hardware VR input LEDs which makes such boards harder to validate @@ -422,7 +423,7 @@ void hwHandleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) { palWritePad(criticalErrorLedPort, criticalErrorLedPin, 0); #endif // VR_HW_CHECK_MODE - if (engine->directSelfStimulation || !engine->hwTriggerInputEnabled) { + if (tc->directSelfStimulation || !tc->hwTriggerInputEnabled) { // sensor noise + self-stim = loss of trigger sync return; } @@ -471,7 +472,7 @@ void handleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) { // for effective noise filtering, we need both signal edges, // so we pass them to handleShaftSignal() and defer this test if (!engineConfiguration->useNoiselessTriggerDecoder) { - if (!isUsefulSignal(signal, engine->triggerCentral.primaryTriggerConfiguration)) { + if (!isUsefulSignal(signal, getTriggerCentral()->primaryTriggerConfiguration)) { /** * no need to process VR falls further */ @@ -483,7 +484,7 @@ void handleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) { #if EFI_PROD_CODE writePad("trigger debug", engineConfiguration->triggerInputDebugPins[signalIndex], 1); #endif /* EFI_PROD_CODE */ - engine->executor.scheduleByTimestampNt("dbg_off", &debugToggleScheduling, timestamp + DEBUG_PIN_DELAY, &turnOffAllDebugFields); + getExecutorInterface()->scheduleByTimestampNt("dbg_off", &debugToggleScheduling, timestamp + DEBUG_PIN_DELAY, &turnOffAllDebugFields); } #if EFI_TOOTH_LOGGER @@ -504,7 +505,7 @@ void handleShaftSignal(int signalIndex, bool isRising, efitick_t timestamp) { maxTriggerReentrant = triggerReentrant; triggerReentrant++; - engine->triggerCentral.handleShaftSignal(signal, timestamp); + getTriggerCentral()->handleShaftSignal(signal, timestamp); triggerReentrant--; triggerDuration = getTimeNowLowerNt() - triggerHandlerEntryTime; @@ -619,7 +620,7 @@ void TriggerCentral::decodeMapCam(efitick_t timestamp, float currentPhase) { if (diff > 0) { mapVvt_map_peak++; - int revolutionCounter = engine->triggerCentral.triggerState.getCrankSynchronizationCounter(); + int revolutionCounter = getTriggerCentral()->triggerState.getCrankSynchronizationCounter(); mapVvt_MAP_AT_CYCLE_COUNT = revolutionCounter - prevChangeAtCycle; prevChangeAtCycle = revolutionCounter; @@ -696,7 +697,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta reportEventToWaveChart(signal, triggerIndexForListeners); // Look up this tooth's angle from the sync point. If this tooth is the sync point, we'll get 0 here. - auto currentPhaseFromSyncPoint = engine->triggerCentral.triggerFormDetails.eventAngles[triggerIndexForListeners]; + auto currentPhaseFromSyncPoint = getTriggerCentral()->triggerFormDetails.eventAngles[triggerIndexForListeners]; // Adjust so currentPhase is in engine-space angle, not trigger-space angle auto currentPhase = wrapAngleMethod(currentPhaseFromSyncPoint - tdcPosition(), "currentEnginePhase", CUSTOM_ERR_6555); @@ -740,14 +741,14 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta do { // I don't love this. nextToothIndex = (nextToothIndex + 1) % engine->engineCycleEventCount; - nextPhase = engine->triggerCentral.triggerFormDetails.eventAngles[nextToothIndex] - tdcPosition(); + nextPhase = getTriggerCentral()->triggerFormDetails.eventAngles[nextToothIndex] - tdcPosition(); wrapAngle(nextPhase, "nextEnginePhase", CUSTOM_ERR_6555); } while (nextPhase == currentPhase); #if EFI_CDM_INTEGRATION if (trgEventIndex == 0 && isBrainPinValid(engineConfiguration->cdmInputPin)) { - int cdmKnockValue = getCurrentCdmValue(engine->triggerCentral.triggerState.getCrankSynchronizationCounter()); + int cdmKnockValue = getCurrentCdmValue(getTriggerCentral()->triggerState.getCrankSynchronizationCounter()); engine->knockLogic(cdmKnockValue); } #endif /* EFI_CDM_INTEGRATION */ @@ -765,8 +766,8 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta static void triggerShapeInfo() { #if EFI_PROD_CODE || EFI_SIMULATOR - TriggerWaveform *shape = &engine->triggerCentral.triggerShape; - TriggerFormDetails *triggerFormDetails = &engine->triggerCentral.triggerFormDetails; + TriggerWaveform *shape = &getTriggerCentral()->triggerShape; + TriggerFormDetails *triggerFormDetails = &getTriggerCentral()->triggerFormDetails; efiPrintf("useRise=%s", boolToString(TRIGGER_WAVEFORM(useRiseEdge))); efiPrintf("gap from %.2f to %.2f", TRIGGER_WAVEFORM(syncronizationRatioFrom[0]), TRIGGER_WAVEFORM(syncronizationRatioTo[0])); @@ -783,11 +784,12 @@ extern PwmConfig triggerSignal; void triggerInfo(void) { #if EFI_PROD_CODE || EFI_SIMULATOR - TriggerWaveform *ts = &engine->triggerCentral.triggerShape; + TriggerCentral *tc = getTriggerCentral(); + TriggerWaveform *ts = &tc->triggerShape; #if (HAL_TRIGGER_USE_PAL == TRUE) && (PAL_USE_CALLBACKS == TRUE) - efiPrintf("trigger PAL mode %d", engine->hwTriggerInputEnabled); + efiPrintf("trigger PAL mode %d", tc->hwTriggerInputEnabled); #else #endif /* HAL_TRIGGER_USE_PAL */ @@ -803,12 +805,13 @@ void triggerInfo(void) { engineConfiguration->trigger.customSkippedToothCount); } - efiPrintf("trigger#1 event counters up=%d/down=%d", engine->triggerCentral.getHwEventCounter(0), - engine->triggerCentral.getHwEventCounter(1)); + + efiPrintf("trigger#1 event counters up=%d/down=%d", tc->getHwEventCounter(0), + tc->getHwEventCounter(1)); if (ts->needSecondTriggerInput) { - efiPrintf("trigger#2 event counters up=%d/down=%d", engine->triggerCentral.getHwEventCounter(2), - engine->triggerCentral.getHwEventCounter(3)); + efiPrintf("trigger#2 event counters up=%d/down=%d", tc->getHwEventCounter(2), + tc->getHwEventCounter(3)); } efiPrintf("expected cycle events %d/%d", TRIGGER_WAVEFORM(getExpectedEventCount(TriggerWheel::T_PRIMARY)), @@ -817,13 +820,14 @@ void triggerInfo(void) { efiPrintf("trigger type=%d/need2ndChannel=%s", engineConfiguration->trigger.type, boolToString(TRIGGER_WAVEFORM(needSecondTriggerInput))); + efiPrintf("synchronizationNeeded=%s/isError=%s/total errors=%d ord_err=%d/total revolutions=%d/self=%s", boolToString(ts->isSynchronizationNeeded), - boolToString(engine->triggerCentral.isTriggerDecoderError()), - engine->triggerCentral.triggerState.totalTriggerErrorCounter, - engine->triggerCentral.triggerState.orderingErrorCounter, - engine->triggerCentral.triggerState.getCrankSynchronizationCounter(), - boolToString(engine->directSelfStimulation)); + boolToString(tc->isTriggerDecoderError()), + tc->triggerState.totalTriggerErrorCounter, + tc->triggerState.orderingErrorCounter, + tc->triggerState.getCrankSynchronizationCounter(), + boolToString(tc->directSelfStimulation)); if (TRIGGER_WAVEFORM(isSynchronizationNeeded)) { efiPrintf("gap from %.2f to %.2f", TRIGGER_WAVEFORM(syncronizationRatioFrom[0]), TRIGGER_WAVEFORM(syncronizationRatioTo[0])); @@ -856,7 +860,7 @@ void triggerInfo(void) { getVvt_mode_e(engineConfiguration->vvtMode[camLogicalIndex])); efiPrintf("VVT %d event counters: %d/%d", camInputIndex, - engine->triggerCentral.vvtEventRiseCounter[camInputIndex], engine->triggerCentral.vvtEventFallCounter[camInputIndex]); + tc->vvtEventRiseCounter[camInputIndex], tc->vvtEventFallCounter[camInputIndex]); } } diff --git a/firmware/controllers/trigger/trigger_central.h b/firmware/controllers/trigger/trigger_central.h index db2845917e..689547fe56 100644 --- a/firmware/controllers/trigger/trigger_central.h +++ b/firmware/controllers/trigger/trigger_central.h @@ -52,6 +52,15 @@ public: void resetCounters(); void validateCamVvtCounters(); + // this is useful at least for real hardware integration testing - maybe a proper solution would be to simply + // GND input pins instead of leaving them floating + bool hwTriggerInputEnabled = true; + + /** + * See also triggerSimulatorFrequency + */ + bool directSelfStimulation = false; + PrimaryTriggerConfiguration primaryTriggerConfiguration; #if CAMS_PER_BANK == 1 VvtTriggerConfiguration vvtTriggerConfiguration[CAMS_PER_BANK] = {{"VVT1 ", 0}}; @@ -169,3 +178,5 @@ void onConfigurationChangeTriggerCallback(); #define SYMMETRICAL_CRANK_SENSOR_DIVIDER 4 #define SYMMETRICAL_THREE_TIMES_CRANK_SENSOR_DIVIDER 6 #define SYMMETRICAL_TWELVE_TIMES_CRANK_SENSOR_DIVIDER 24 + +TriggerCentral * getTriggerCentral(); diff --git a/firmware/controllers/trigger/trigger_emulator_algo.cpp b/firmware/controllers/trigger/trigger_emulator_algo.cpp index 97c2d95ead..07ce427fb5 100644 --- a/firmware/controllers/trigger/trigger_emulator_algo.cpp +++ b/firmware/controllers/trigger/trigger_emulator_algo.cpp @@ -123,7 +123,7 @@ static bool hasInitTriggerEmulator = false; # if !EFI_UNIT_TEST static void emulatorApplyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ { - if (engine->directSelfStimulation) { + if (engine->triggerCentral.directSelfStimulation) { /** * this callback would invoke the input signal handlers directly */ @@ -158,16 +158,16 @@ static void initTriggerPwm() { void enableTriggerStimulator() { initTriggerPwm(); - engine->directSelfStimulation = true; + engine->triggerCentral.directSelfStimulation = true; } void enableExternalTriggerStimulator() { initTriggerPwm(); - engine->directSelfStimulation = false; + engine->triggerCentral.directSelfStimulation = false; } void disableTriggerStimulator() { - engine->directSelfStimulation = false; + engine->triggerCentral.directSelfStimulation = false; triggerSignal.stop(); hasInitTriggerEmulator = false; }