From c0d8cbfef4cbceca5cd026a73a64737fb67789cc Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Tue, 31 May 2022 21:55:34 -0700 Subject: [PATCH] Trigger cleanup init (#4222) * extra parameter * name decoders in constructor * s --- firmware/controllers/algo/engine.cpp | 16 +++------------- .../trigger/decoders/trigger_structure.cpp | 3 ++- .../trigger/decoders/trigger_structure.h | 3 +-- .../controllers/trigger/trigger_central.cpp | 3 ++- firmware/controllers/trigger/trigger_central.h | 18 +++++++++++++++++- .../controllers/trigger/trigger_decoder.cpp | 16 ++++++++++------ firmware/controllers/trigger/trigger_decoder.h | 11 +++++++---- .../tests/trigger/test_trigger_decoder.cpp | 4 ++-- .../tests/trigger/test_trigger_decoder_2.cpp | 4 +++- 9 files changed, 47 insertions(+), 31 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 78175bd6bd..b13ca4e322 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -107,15 +107,13 @@ static operation_mode_e lookupOperationMode() { } static void initVvtShape(TriggerWaveform& shape, const TriggerConfiguration& config, TriggerDecoderBase &initState) { - shape.initializeTriggerWaveform( - lookupOperationMode(), - engineConfiguration->vvtCamSensorUseRise, config); + shape.initializeTriggerWaveform(FOUR_STROKE_CAM_SENSOR, config); shape.initializeSyncPoint(initState, config); } void Engine::updateTriggerWaveform() { - static TriggerDecoderBase initState; + static TriggerDecoderBase initState("init"); // Re-read config in case it's changed primaryTriggerConfiguration.update(); @@ -127,9 +125,7 @@ void Engine::updateTriggerWaveform() { // we have a confusing threading model so some synchronization would not hurt chibios_rt::CriticalSectionLocker csl; - TRIGGER_WAVEFORM(initializeTriggerWaveform( - lookupOperationMode(), - engineConfiguration->useOnlyRisingEdgeForTrigger, primaryTriggerConfiguration)); + TRIGGER_WAVEFORM(initializeTriggerWaveform(lookupOperationMode(), primaryTriggerConfiguration)); /** * this is only useful while troubleshooting a new trigger shape in the field @@ -160,15 +156,9 @@ void Engine::updateTriggerWaveform() { calculateTriggerSynchPoint(engine->triggerCentral.triggerShape, initState); - engine->triggerCentral.triggerState.name = "TRG"; engine->engineCycleEventCount = TRIGGER_WAVEFORM(getLength()); } - engine->triggerCentral.vvtState[0][0].name = "VVT B1 Int"; - engine->triggerCentral.vvtState[0][1].name = "VVT B1 Exh"; - engine->triggerCentral.vvtState[1][0].name = "VVT B2 Int"; - engine->triggerCentral.vvtState[1][1].name = "VVT B2 Exh"; - for (int camIndex = 0; camIndex < CAMS_PER_BANK; camIndex++) { // todo: should 'vvtWithRealDecoder' be used here? if (engineConfiguration->vvtMode[camIndex] != VVT_INACTIVE) { diff --git a/firmware/controllers/trigger/decoders/trigger_structure.cpp b/firmware/controllers/trigger/decoders/trigger_structure.cpp index 7f0cd4a0de..36357569c9 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.cpp +++ b/firmware/controllers/trigger/decoders/trigger_structure.cpp @@ -446,7 +446,7 @@ void TriggerWaveform::setThirdTriggerSynchronizationGap(float syncRatio) { /** * External logger is needed because at this point our logger is not yet initialized */ -void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperationMode, bool useOnlyRisingEdgeForTrigger, const TriggerConfiguration& triggerConfig) { +void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperationMode, const TriggerConfiguration& triggerConfig) { #if EFI_PROD_CODE efiAssertVoid(CUSTOM_ERR_6641, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "init t"); @@ -455,6 +455,7 @@ void TriggerWaveform::initializeTriggerWaveform(operation_mode_e triggerOperatio shapeDefinitionError = false; + bool useOnlyRisingEdgeForTrigger = triggerConfig.UseOnlyRisingEdgeForTrigger; this->useOnlyRisingEdgeForTriggerTemp = useOnlyRisingEdgeForTrigger; switch (triggerConfig.TriggerType.type) { diff --git a/firmware/controllers/trigger/decoders/trigger_structure.h b/firmware/controllers/trigger/decoders/trigger_structure.h index 09472b9e42..9ae4a8b20b 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.h +++ b/firmware/controllers/trigger/decoders/trigger_structure.h @@ -73,8 +73,7 @@ class TriggerConfiguration; class TriggerWaveform { public: TriggerWaveform(); - void initializeTriggerWaveform(operation_mode_e triggerOperationMode, - bool useOnlyRisingEdgeForTrigger, const TriggerConfiguration& triggerConfig); + void initializeTriggerWaveform(operation_mode_e triggerOperationMode, const TriggerConfiguration& triggerConfig); void setShapeDefinitionError(bool value); /** diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 174c3f326d..18f4aee228 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -43,7 +43,8 @@ static scheduling_s debugToggleScheduling; TriggerCentral::TriggerCentral() : vvtEventRiseCounter(), vvtEventFallCounter(), - vvtPosition() + vvtPosition(), + triggerState("TRG") { memset(&hwEventCounters, 0, sizeof(hwEventCounters)); triggerState.resetTriggerState(); diff --git a/firmware/controllers/trigger/trigger_central.h b/firmware/controllers/trigger/trigger_central.h index fb885a657c..39fefdcfc3 100644 --- a/firmware/controllers/trigger/trigger_central.h +++ b/firmware/controllers/trigger/trigger_central.h @@ -113,7 +113,23 @@ public: TriggerWaveform triggerShape; - VvtTriggerDecoder vvtState[BANKS_COUNT][CAMS_PER_BANK]; + VvtTriggerDecoder vvtState[BANKS_COUNT][CAMS_PER_BANK] = { + { + "VVT B1 Int", +#if CAMS_PER_BANK >= 2 + "VVT B1 Exh" +#endif + }, +#if BANKS_COUNT >= 2 + { + "VVT B2 Int", +#if CAMS_PER_BANK >= 2 + "VVT B1 Exh" +#endif + } +#endif + }; + TriggerWaveform vvtShape[CAMS_PER_BANK]; TriggerFormDetails triggerFormDetails; diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index 23c35ab212..728f3bf8f9 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -38,7 +38,9 @@ #include "sensor_chart.h" #endif -TriggerDecoderBase::TriggerDecoderBase() { +TriggerDecoderBase::TriggerDecoderBase(const char* name) + : name(name) +{ resetTriggerState(); } @@ -90,10 +92,12 @@ void TriggerDecoderBase::resetCurrentCycleState() { #if EFI_SHAFT_POSITION_INPUT -PrimaryTriggerDecoder::PrimaryTriggerDecoder() : - //https://en.cppreference.com/w/cpp/language/zero_initialization - timeOfLastEvent(), instantRpmValue() - { +PrimaryTriggerDecoder::PrimaryTriggerDecoder(const char* name) + : TriggerDecoderBase(name) + //https://en.cppreference.com/w/cpp/language/zero_initialization + , timeOfLastEvent() + , instantRpmValue() +{ } #if ! EFI_PROD_CODE @@ -343,7 +347,7 @@ static trigger_value_e eventType[6] = { TV_FALL, TV_RISE, TV_FALL, TV_RISE, TV_F #define nextTriggerEvent() \ { \ - if (triggerConfiguration.UseOnlyRisingEdgeForTrigger) {currentCycle.current_index++;} \ + if (useOnlyRisingEdgeForTrigger) {currentCycle.current_index++;} \ currentCycle.current_index++; \ PRINT_INC_INDEX; \ } diff --git a/firmware/controllers/trigger/trigger_decoder.h b/firmware/controllers/trigger/trigger_decoder.h index 67bd954890..ce5b0bd144 100644 --- a/firmware/controllers/trigger/trigger_decoder.h +++ b/firmware/controllers/trigger/trigger_decoder.h @@ -62,7 +62,7 @@ struct TriggerDecodeResult { */ class TriggerDecoderBase : public trigger_state_s { public: - TriggerDecoderBase(); + TriggerDecoderBase(const char* name); /** * current trigger processing index, between zero and #size */ @@ -108,7 +108,7 @@ public: efitick_t toothed_previous_time; current_cycle_state_s currentCycle; - const char *name = nullptr; + const char* const name; /** * how many times since ECU reboot we had unexpected number of teeth in trigger cycle @@ -165,7 +165,7 @@ private: */ class PrimaryTriggerDecoder : public TriggerDecoderBase { public: - PrimaryTriggerDecoder(); + PrimaryTriggerDecoder(const char* name); void resetTriggerState() override; angle_t syncEnginePhase(int divider, int remainder, angle_t engineCycle); @@ -223,7 +223,10 @@ private: bool m_hasSynchronizedPhase = false; }; -class VvtTriggerDecoder : public TriggerDecoderBase { }; +class VvtTriggerDecoder : public TriggerDecoderBase { +public: + VvtTriggerDecoder(const char* name) : TriggerDecoderBase(name) { } +}; angle_t getEngineCycle(operation_mode_e operationMode); diff --git a/unit_tests/tests/trigger/test_trigger_decoder.cpp b/unit_tests/tests/trigger/test_trigger_decoder.cpp index 786ca27b80..e20900a9ba 100644 --- a/unit_tests/tests/trigger/test_trigger_decoder.cpp +++ b/unit_tests/tests/trigger/test_trigger_decoder.cpp @@ -64,7 +64,7 @@ static void testDodgeNeonDecoder() { TriggerWaveform * shape = ð.engine.triggerCentral.triggerShape; ASSERT_EQ(8, shape->getTriggerWaveformSynchPointIndex()); - TriggerDecoderBase state; + TriggerDecoderBase state("test"); ASSERT_FALSE(state.getShaftSynchronized()) << "1 shaft_is_synchronized"; @@ -111,7 +111,7 @@ static void assertTriggerPosition(event_trigger_position_s *position, int eventI TEST(trigger, testSomethingWeird) { EngineTestHelper eth(FORD_INLINE_6_1995); - TriggerDecoderBase state_; + TriggerDecoderBase state_("test"); TriggerDecoderBase *sta = &state_; const auto& triggerConfiguration = engine->primaryTriggerConfiguration; diff --git a/unit_tests/tests/trigger/test_trigger_decoder_2.cpp b/unit_tests/tests/trigger/test_trigger_decoder_2.cpp index 1a90652998..b6a30cbca1 100644 --- a/unit_tests/tests/trigger/test_trigger_decoder_2.cpp +++ b/unit_tests/tests/trigger/test_trigger_decoder_2.cpp @@ -29,12 +29,14 @@ private: }; struct MockTriggerDecoder : public TriggerDecoderBase { + MockTriggerDecoder() : TriggerDecoderBase("mock") { } + MOCK_METHOD(void, onTriggerError, (), (override)); }; static auto makeTriggerShape(operation_mode_e mode, const TriggerConfiguration& config) { TriggerWaveform shape; - shape.initializeTriggerWaveform(mode, true, config); + shape.initializeTriggerWaveform(mode, config); return shape; }