From 851c6041b1d078c423838f678e9e0fabfc9a5cbe Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Thu, 9 Jan 2020 10:19:11 -0800 Subject: [PATCH] Inject timestamps in to hwHandleShaftSignal (#1090) * injection * injectors --- .../controllers/trigger/trigger_central.cpp | 26 ++++++++----------- .../controllers/trigger/trigger_central.h | 8 +++--- .../trigger/trigger_emulator_algo.cpp | 8 +++--- firmware/hw_layer/trigger_input_comp.cpp | 12 ++++++--- firmware/hw_layer/trigger_input_exti.cpp | 13 +++++++--- firmware/hw_layer/trigger_input_icu.cpp | 18 +++++++------ unit_tests/engine_test_helper.cpp | 4 +-- unit_tests/tests/test_cam_vtt_input.cpp | 2 +- .../tests/test_miata_na6_real_cranking.cpp | 2 +- 9 files changed, 51 insertions(+), 42 deletions(-) diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 4e31290a51..60af428d30 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -84,7 +84,7 @@ void addTriggerEventListener(ShaftPositionListener listener, const char *name, E engine->triggerCentral.addEventListener(listener, name, engine); } -void hwHandleVvtCamSignal(trigger_value_e front DECLARE_ENGINE_PARAMETER_SUFFIX) { +void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { TriggerCentral *tc = &engine->triggerCentral; if (front == TV_RISE) { tc->vvtEventRiseCounter++; @@ -108,8 +108,6 @@ void hwHandleVvtCamSignal(trigger_value_e front DECLARE_ENGINE_PARAMETER_SUFFIX) tc->vvtCamCounter++; - efitick_t nowNt = getTimeNowNt(); - if (engineConfiguration->vvtMode == MIATA_NB2) { uint32_t currentDuration = nowNt - tc->previousVvtCamTime; float ratio = ((float) currentDuration) / tc->previousVvtCamDuration; @@ -193,7 +191,7 @@ uint32_t triggerMaxDuration = 0; static bool isInsideTriggerHandler = false; -void hwHandleShaftSignal(trigger_event_e signal) { +void hwHandleShaftSignal(trigger_event_e signal, efitick_t timestamp) { ScopePerf perf(PE::HandleShaftSignal, static_cast(signal)); #if EFI_TOOTH_LOGGER @@ -216,7 +214,7 @@ void hwHandleShaftSignal(trigger_event_e signal) { maxTriggerReentraint = triggerReentraint; triggerReentraint++; efiAssertVoid(CUSTOM_ERR_6636, getCurrentRemainingStack() > 128, "lowstck#8"); - engine->triggerCentral.handleShaftSignal(signal PASS_ENGINE_PARAMETER_SUFFIX); + engine->triggerCentral.handleShaftSignal(signal, timestamp PASS_ENGINE_PARAMETER_SUFFIX); triggerReentraint--; triggerDuration = getTimeNowLowerNt() - triggerHandlerEntryTime; isInsideTriggerHandler = false; @@ -314,7 +312,7 @@ bool TriggerCentral::noiseFilter(efitick_t nowNt, trigger_event_e signal DECLARE return false; } -void TriggerCentral::handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PARAMETER_SUFFIX) { +void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timestamp DECLARE_ENGINE_PARAMETER_SUFFIX) { efiAssertVoid(CUSTOM_CONF_NULL, engine!=NULL, "configuration"); if (triggerShape.shapeDefinitionError) { @@ -325,11 +323,9 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PAR return; } - nowNt = getTimeNowNt(); - // This code gathers some statistics on signals and compares accumulated periods to filter interference if (CONFIG(useNoiselessTriggerDecoder)) { - if (!noiseFilter(nowNt, signal PASS_ENGINE_PARAMETER_SUFFIX)) { + if (!noiseFilter(timestamp, signal PASS_ENGINE_PARAMETER_SUFFIX)) { return; } // moved here from hwHandleShaftSignal() @@ -338,25 +334,25 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PAR } } - engine->onTriggerSignalEvent(nowNt); + engine->onTriggerSignalEvent(timestamp); int eventIndex = (int) signal; efiAssertVoid(CUSTOM_ERR_6638, eventIndex >= 0 && eventIndex < HW_EVENT_TYPES, "signal type"); hwEventCounters[eventIndex]++; - if (nowNt - previousShaftEventTimeNt > US2NT(US_PER_SECOND_LL)) { + if (timestamp - previousShaftEventTimeNt > US2NT(US_PER_SECOND_LL)) { /** - * We are here if there is a time gap between now and previous shaft event - that means the engine is not runnig. + * We are here if there is a time gap between now and previous shaft event - that means the engine is not running. * That means we have lost synchronization since the engine is not running :) */ triggerState.onSynchronizationLost(PASS_ENGINE_PARAMETER_SIGNATURE); } - previousShaftEventTimeNt = nowNt; + previousShaftEventTimeNt = timestamp; /** * This invocation changes the state of triggerState */ - triggerState.decodeTriggerEvent(nullptr, engine, signal, nowNt PASS_ENGINE_PARAMETER_SUFFIX); + triggerState.decodeTriggerEvent(nullptr, engine, signal, timestamp PASS_ENGINE_PARAMETER_SUFFIX); /** * If we only have a crank position sensor with four stroke, here we are extending crank revolutions with a 360 degree @@ -376,7 +372,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PAR triggerIndexForListeners = triggerState.getCurrentIndex() + (crankInternalIndex * getTriggerSize()); } if (triggerIndexForListeners == 0) { - timeAtVirtualZeroNt = nowNt; + timeAtVirtualZeroNt = timestamp; } reportEventToWaveChart(signal, triggerIndexForListeners PASS_ENGINE_PARAMETER_SUFFIX); diff --git a/firmware/controllers/trigger/trigger_central.h b/firmware/controllers/trigger/trigger_central.h index 12f0c3857b..2ce33db5b0 100644 --- a/firmware/controllers/trigger/trigger_central.h +++ b/firmware/controllers/trigger/trigger_central.h @@ -26,14 +26,14 @@ class TriggerCentral : public trigger_central_s { public: TriggerCentral(); void addEventListener(ShaftPositionListener handler, const char *name, Engine *engine); - void handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PARAMETER_SUFFIX); + void handleShaftSignal(trigger_event_e signal, efitick_t timestamp DECLARE_ENGINE_PARAMETER_SUFFIX); int getHwEventCounter(int index) const; void resetCounters(); void resetAccumSignalData(); bool noiseFilter(efitick_t nowNt, trigger_event_e signal DECLARE_ENGINE_PARAMETER_SUFFIX); void validateCamVvtCounters(); TriggerStateWithRunningStatistics triggerState; - efitick_t nowNt = 0; + angle_t vvtPosition = 0; /** * this is similar to TriggerState#startOfCycleNt but with the crank-only sensor magic @@ -56,8 +56,8 @@ private: }; void triggerInfo(void); -void hwHandleShaftSignal(trigger_event_e signal); -void hwHandleVvtCamSignal(trigger_value_e front DECLARE_ENGINE_PARAMETER_SUFFIX); +void hwHandleShaftSignal(trigger_event_e signal, efitick_t timestamp); +void hwHandleVvtCamSignal(trigger_value_e front, efitick_t timestamp DECLARE_ENGINE_PARAMETER_SUFFIX); void initTriggerCentral(Logging *sharedLogger); void printAllTriggers(); diff --git a/firmware/controllers/trigger/trigger_emulator_algo.cpp b/firmware/controllers/trigger/trigger_emulator_algo.cpp index ab2d8e0e4f..be923786d1 100644 --- a/firmware/controllers/trigger/trigger_emulator_algo.cpp +++ b/firmware/controllers/trigger/trigger_emulator_algo.cpp @@ -45,22 +45,24 @@ EXTERN_ENGINE ; void TriggerEmulatorHelper::handleEmulatorCallback(PwmConfig *state, int stateIndex) { + efitick_t stamp = getTimeNowNt(); + // todo: code duplication with TriggerStimulatorHelper::feedSimulatedEvent? MultiChannelStateSequence *multiChannelStateSequence = &state->multiChannelStateSequence; if (needEvent(stateIndex, state->phaseCount, &state->multiChannelStateSequence, 0)) { pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/0, stateIndex); - hwHandleShaftSignal(currentValue ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING); + hwHandleShaftSignal(currentValue ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, stamp); } if (needEvent(stateIndex, state->phaseCount, &state->multiChannelStateSequence, 1)) { pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/1, stateIndex); - hwHandleShaftSignal(currentValue ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING); + hwHandleShaftSignal(currentValue ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING, stamp); } if (needEvent(stateIndex, state->phaseCount, &state->multiChannelStateSequence, 2)) { pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/2, stateIndex); - hwHandleShaftSignal(currentValue ? SHAFT_3RD_RISING : SHAFT_3RD_FALLING); + hwHandleShaftSignal(currentValue ? SHAFT_3RD_RISING : SHAFT_3RD_FALLING, stamp); } // print("hello %d\r\n", chTimeNow()); diff --git a/firmware/hw_layer/trigger_input_comp.cpp b/firmware/hw_layer/trigger_input_comp.cpp index 9b42efe066..e3d345c033 100644 --- a/firmware/hw_layer/trigger_input_comp.cpp +++ b/firmware/hw_layer/trigger_input_comp.cpp @@ -55,6 +55,8 @@ static void setHysteresis(COMPDriver *comp, int sign) { } static void comp_shaft_callback(COMPDriver *comp) { + efitick_t stamp = getTimeNowNt(); + uint32_t status = comp_lld_get_status(comp); int isPrimary = (comp == EFI_COMP_PRIMARY_DEVICE); if (!isPrimary && !TRIGGER_WAVEFORM(needSecondTriggerInput)) { @@ -64,7 +66,7 @@ static void comp_shaft_callback(COMPDriver *comp) { if (status & COMP_IRQ_RISING) { signal = isPrimary ? (engineConfiguration->invertPrimaryTriggerSignal ? SHAFT_PRIMARY_FALLING : SHAFT_PRIMARY_RISING) : (engineConfiguration->invertSecondaryTriggerSignal ? SHAFT_SECONDARY_FALLING : SHAFT_SECONDARY_RISING); - hwHandleShaftSignal(signal); + hwHandleShaftSignal(signal, stamp); // shift the threshold down a little bit to avoid false-triggering (threshold hysteresis) setHysteresis(comp, -1); } @@ -72,7 +74,7 @@ static void comp_shaft_callback(COMPDriver *comp) { if (status & COMP_IRQ_FALLING) { signal = isPrimary ? (engineConfiguration->invertPrimaryTriggerSignal ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING) : (engineConfiguration->invertSecondaryTriggerSignal ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING); - hwHandleShaftSignal(signal); + hwHandleShaftSignal(signal, stamp); // shift the threshold up a little bit to avoid false-triggering (threshold hysteresis) setHysteresis(comp, 1); } @@ -81,10 +83,12 @@ static void comp_shaft_callback(COMPDriver *comp) { // todo: add cam support? #if 0 static void comp_cam_callback(COMPDriver *comp) { + efitick_t stamp = getTimeNowNt(); + if (isRising) { - hwHandleVvtCamSignal(TV_RISE); + hwHandleVvtCamSignal(TV_RISE, stamp); } else { - hwHandleVvtCamSignal(TV_FALL); + hwHandleVvtCamSignal(TV_FALL, stamp); } } #endif diff --git a/firmware/hw_layer/trigger_input_exti.cpp b/firmware/hw_layer/trigger_input_exti.cpp index 9662392647..d4512f0652 100644 --- a/firmware/hw_layer/trigger_input_exti.cpp +++ b/firmware/hw_layer/trigger_input_exti.cpp @@ -29,7 +29,11 @@ EXTERN_ENGINE; static ioline_t primary_line; static void shaft_callback(void *arg) { + // do the time sensitive things as early as possible! + efitick_t stamp = getTimeNowNt(); ioline_t pal_line = (ioline_t)arg; + bool rise = (palReadLine(pal_line) == PAL_HIGH); + // todo: support for 3rd trigger input channel // todo: start using real event time from HW event, not just software timer? if (hasFirmwareErrorFlag) @@ -40,7 +44,6 @@ static void shaft_callback(void *arg) { return; } - bool rise = (palReadLine(pal_line) == PAL_HIGH); trigger_event_e signal; // todo: add support for 3rd channel if (rise) { @@ -53,18 +56,20 @@ static void shaft_callback(void *arg) { (engineConfiguration->invertSecondaryTriggerSignal ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING); } - hwHandleShaftSignal(signal); + hwHandleShaftSignal(signal, stamp); } static void cam_callback(void *arg) { + efitick_t stamp = getTimeNowNt(); + ioline_t pal_line = (ioline_t)arg; bool rise = (palReadLine(pal_line) == PAL_HIGH); if (rise) { - hwHandleVvtCamSignal(TV_RISE); + hwHandleVvtCamSignal(TV_RISE, stamp); } else { - hwHandleVvtCamSignal(TV_FALL); + hwHandleVvtCamSignal(TV_FALL, stamp); } } diff --git a/firmware/hw_layer/trigger_input_icu.cpp b/firmware/hw_layer/trigger_input_icu.cpp index b3fb6320c3..4ac3f146ba 100644 --- a/firmware/hw_layer/trigger_input_icu.cpp +++ b/firmware/hw_layer/trigger_input_icu.cpp @@ -28,14 +28,12 @@ extern bool hasFirmwareErrorFlag; static Logging *logger; -static void vvtWidthCallback(void *arg) { - (void)arg; - hwHandleVvtCamSignal(TV_RISE); +static void vvtWidthCallback(void *) { + hwHandleVvtCamSignal(TV_RISE, getTimeNowNt()); } -static void vvtPeriodCallback(void *arg) { - (void)arg; - hwHandleVvtCamSignal(TV_FALL); +static void vvtPeriodCallback(void *) { + hwHandleVvtCamSignal(TV_FALL, getTimeNowNt()); } /** @@ -43,6 +41,8 @@ static void vvtPeriodCallback(void *arg) { * 'width' events happens before the 'period' event */ static void shaftWidthCallback(bool isPrimary) { + efitick_t stamp = getTimeNowNt(); + if (!engine->hwTriggerInputEnabled) { return; } @@ -58,10 +58,12 @@ static void shaftWidthCallback(bool isPrimary) { // todo: add support for 3rd channel trigger_event_e signal = isPrimary ? (engineConfiguration->invertPrimaryTriggerSignal ? SHAFT_PRIMARY_FALLING : SHAFT_PRIMARY_RISING) : (engineConfiguration->invertSecondaryTriggerSignal ? SHAFT_SECONDARY_FALLING : SHAFT_SECONDARY_RISING); - hwHandleShaftSignal(signal); + hwHandleShaftSignal(signal, stamp); } static void shaftPeriodCallback(bool isPrimary) { + efitick_t stamp = getTimeNowNt(); + if (!engine->hwTriggerInputEnabled) { return; } @@ -76,7 +78,7 @@ static void shaftPeriodCallback(bool isPrimary) { // icucnt_t last_period = icuGetPeriod(icup); so far we are fine with system time trigger_event_e signal = isPrimary ? (engineConfiguration->invertPrimaryTriggerSignal ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING) : (engineConfiguration->invertSecondaryTriggerSignal ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING); - hwHandleShaftSignal(signal); + hwHandleShaftSignal(signal, stamp); } /*==========================================================================*/ diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 24cc2e4ee4..2e2a4691e5 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -90,7 +90,7 @@ void EngineTestHelper::fireRise(float delayMs) { * fire single RISE front event */ void EngineTestHelper::firePrimaryTriggerRise() { - engine.triggerCentral.handleShaftSignal(SHAFT_PRIMARY_RISING, &engine, engine.engineConfigurationPtr, &persistentConfig); + engine.triggerCentral.handleShaftSignal(SHAFT_PRIMARY_RISING, getTimeNowNt(), &engine, engine.engineConfigurationPtr, &persistentConfig); } void EngineTestHelper::fireFall(float delayMs) { @@ -99,7 +99,7 @@ void EngineTestHelper::fireFall(float delayMs) { } void EngineTestHelper::firePrimaryTriggerFall() { - engine.triggerCentral.handleShaftSignal(SHAFT_PRIMARY_FALLING, &engine, engine.engineConfigurationPtr, &persistentConfig); + engine.triggerCentral.handleShaftSignal(SHAFT_PRIMARY_FALLING, getTimeNowNt(), &engine, engine.engineConfigurationPtr, &persistentConfig); } void EngineTestHelper::fireTriggerEventsWithDuration(float durationMs) { diff --git a/unit_tests/tests/test_cam_vtt_input.cpp b/unit_tests/tests/test_cam_vtt_input.cpp index e50a8f4d40..222810e024 100644 --- a/unit_tests/tests/test_cam_vtt_input.cpp +++ b/unit_tests/tests/test_cam_vtt_input.cpp @@ -105,7 +105,7 @@ TEST(sensors, testCamInput) { for (int i = 0; i < 600;i++) { eth.fireRise(50); - hwHandleVvtCamSignal(TV_FALL PASS_ENGINE_PARAMETER_SUFFIX); + hwHandleVvtCamSignal(TV_FALL, getTimeNowNt() PASS_ENGINE_PARAMETER_SUFFIX); } ASSERT_EQ(0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testCamInput #3"; diff --git a/unit_tests/tests/test_miata_na6_real_cranking.cpp b/unit_tests/tests/test_miata_na6_real_cranking.cpp index 20c68bbbab..070f430394 100644 --- a/unit_tests/tests/test_miata_na6_real_cranking.cpp +++ b/unit_tests/tests/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, engine, engine->engineConfigurationPtr, ð->persistentConfig); + engine->triggerCentral.handleShaftSignal(event, getTimeNowNt(), engine, engine->engineConfigurationPtr, ð->persistentConfig); } TEST(miataNA6, realCranking) {