From 5925a5f4bd64ec69793c9780c95352bf8606a21d Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 5 Oct 2020 11:22:59 -0700 Subject: [PATCH] const ref-ify trigger (#1856) * const ref-ify * tests --- firmware/controllers/algo/engine.cpp | 12 +- .../trigger/decoders/trigger_structure.h | 8 +- .../controllers/trigger/trigger_central.cpp | 17 ++- .../controllers/trigger/trigger_decoder.cpp | 116 +++++++++--------- .../controllers/trigger/trigger_decoder.h | 25 ++-- .../trigger/trigger_emulator_algo.cpp | 9 +- .../trigger/trigger_emulator_algo.h | 2 +- .../controllers/trigger/trigger_simulator.cpp | 79 ++++++------ .../controllers/trigger/trigger_simulator.h | 25 ++-- .../tests/trigger/test_trigger_decoder.cpp | 29 ++--- 10 files changed, 168 insertions(+), 154 deletions(-) diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index 03103029b9..f19cf086d3 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/firmware/controllers/algo/engine.cpp @@ -109,8 +109,8 @@ void Engine::initializeTriggerWaveform(Logging *logger DECLARE_ENGINE_PARAMETER_ * 'initState' instance of TriggerState is used only to initialize 'this' TriggerWaveform instance * #192 BUG real hardware trigger events could be coming even while we are initializing trigger */ - calculateTriggerSynchPoint(&ENGINE(triggerCentral.triggerShape), - &initState PASS_ENGINE_PARAMETER_SUFFIX); + calculateTriggerSynchPoint(ENGINE(triggerCentral.triggerShape), + initState PASS_ENGINE_PARAMETER_SUFFIX); engine->engineCycleEventCount = TRIGGER_WAVEFORM(getLength()); } @@ -124,9 +124,9 @@ void Engine::initializeTriggerWaveform(Logging *logger DECLARE_ENGINE_PARAMETER_ engineConfiguration->ambiguousOperationMode, engine->engineConfigurationPtr->vvtCamSensorUseRise, &config); - ENGINE(triggerCentral).vvtShape.initializeSyncPoint(&initState, - &engine->vvtTriggerConfiguration, - &config); + ENGINE(triggerCentral).vvtShape.initializeSyncPoint(initState, + engine->vvtTriggerConfiguration, + config); } if (!alreadyLocked) { @@ -370,7 +370,7 @@ void Engine::OnTriggerSyncronization(bool wasSynchronized) { /** * We can check if things are fine by comparing the number of events in a cycle with the expected number of event. */ - bool isDecodingError = triggerCentral.triggerState.validateEventCounters(&triggerCentral.triggerShape); + bool isDecodingError = triggerCentral.triggerState.validateEventCounters(triggerCentral.triggerShape); enginePins.triggerDecoderErrorPin.setValue(isDecodingError); diff --git a/firmware/controllers/trigger/decoders/trigger_structure.h b/firmware/controllers/trigger/decoders/trigger_structure.h index 8017202484..5fd05cc99c 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.h +++ b/firmware/controllers/trigger/decoders/trigger_structure.h @@ -256,9 +256,11 @@ public: */ int triggerShapeSynchPointIndex; - void initializeSyncPoint(TriggerState *state, - const TriggerConfiguration * triggerConfiguration, - trigger_config_s const*triggerConfig); + void initializeSyncPoint( + TriggerState& state, + const TriggerConfiguration& triggerConfiguration, + const trigger_config_s& triggerConfig + ); private: trigger_shape_helper h; diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index c9b9cd5416..7272ce9ab1 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -157,10 +157,10 @@ void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt DECLARE_ENGINE_ } ENGINE(triggerCentral).vvtState.decodeTriggerEvent( - &ENGINE(triggerCentral).vvtShape, + ENGINE(triggerCentral).vvtShape, nullptr, nullptr, - &engine->vvtTriggerConfiguration, + engine->vvtTriggerConfiguration, front == TV_RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt); @@ -292,8 +292,7 @@ void hwHandleShaftSignal(trigger_event_e signal, efitick_t timestamp) { // for effective noise filtering, we need both signal edges, // so we pass them to handleShaftSignal() and defer this test if (!CONFIG(useNoiselessTriggerDecoder)) { - const TriggerConfiguration * triggerConfiguration = &ENGINE(primaryTriggerConfiguration); - if (!isUsefulSignal(signal, triggerConfiguration)) { + if (!isUsefulSignal(signal, ENGINE(primaryTriggerConfiguration))) { /** * no need to process VR falls further */ @@ -434,9 +433,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta if (!noiseFilter.noiseFilter(timestamp, &triggerState, signal PASS_ENGINE_PARAMETER_SUFFIX)) { return; } - const TriggerConfiguration* triggerConfiguration = &ENGINE(primaryTriggerConfiguration); - - if (!isUsefulSignal(signal, triggerConfiguration)) { + if (!isUsefulSignal(signal, ENGINE(primaryTriggerConfiguration))) { return; } } @@ -451,10 +448,10 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta /** * This invocation changes the state of triggerState */ - triggerState.decodeTriggerEvent(&triggerShape, + triggerState.decodeTriggerEvent(triggerShape, nullptr, engine, - &engine->primaryTriggerConfiguration, + engine->primaryTriggerConfiguration, signal, timestamp); /** @@ -483,7 +480,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta return; } - if (triggerState.isValidIndex(&ENGINE(triggerCentral.triggerShape))) { + if (triggerState.isValidIndex(ENGINE(triggerCentral.triggerShape))) { ScopePerf perf(PE::ShaftPositionListeners); #if TRIGGER_EXTREME_LOGGING diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index fe11025126..507f4df43b 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -114,38 +114,41 @@ float actualSynchGap; static Logging * logger = nullptr; -void TriggerWaveform::initializeSyncPoint(TriggerState *state, - const TriggerConfiguration * triggerConfiguration, - trigger_config_s const*triggerConfig) { - triggerShapeSynchPointIndex = state->findTriggerZeroEventIndex(this, +void TriggerWaveform::initializeSyncPoint(TriggerState& state, + const TriggerConfiguration& triggerConfiguration, + const trigger_config_s& triggerConfig) { + triggerShapeSynchPointIndex = state.findTriggerZeroEventIndex(*this, triggerConfiguration, triggerConfig); } /** * Calculate 'shape.triggerShapeSynchPointIndex' value using 'TriggerState *state' */ -void calculateTriggerSynchPoint(TriggerWaveform *shape, - TriggerState *state DECLARE_ENGINE_PARAMETER_SUFFIX) { - state->resetTriggerState(); +void calculateTriggerSynchPoint( + TriggerWaveform& shape, + TriggerState& state + DECLARE_ENGINE_PARAMETER_SUFFIX) { + state.resetTriggerState(); + #if EFI_PROD_CODE efiAssertVoid(CUSTOM_TRIGGER_STACK, getCurrentRemainingStack() > EXPECTED_REMAINING_STACK, "calc s"); #endif - trigger_config_s const*triggerConfig = &engineConfiguration->trigger; - engine->triggerErrorDetection.clear(); - shape->initializeSyncPoint(state, - &engine->primaryTriggerConfiguration, - triggerConfig); + shape.initializeSyncPoint(state, + engine->primaryTriggerConfiguration, + CONFIG(trigger)); - int length = shape->getLength(); + int length = shape.getLength(); engine->engineCycleEventCount = length; + efiAssertVoid(CUSTOM_SHAPE_LEN_ZERO, length > 0, "shapeLength=0"); if (length >= PWM_PHASE_MAX_COUNT) { firmwareError(CUSTOM_ERR_TRIGGER_WAVEFORM_TOO_LONG, "Trigger length above maximum: %d", length); - shape->setShapeDefinitionError(true); + shape.setShapeDefinitionError(true); return; } - if (shape->getSize() == 0) { + + if (shape.getSize() == 0) { firmwareError(CUSTOM_ERR_TRIGGER_ZERO, "triggerShape size is zero"); } } @@ -286,8 +289,8 @@ void TriggerStateWithRunningStatistics::runtimeStatistics(TriggerFormDetails *tr } } -bool TriggerState::isValidIndex(const TriggerWaveform *triggerShape) const { - return currentCycle.current_index < triggerShape->getSize(); +bool TriggerState::isValidIndex(const TriggerWaveform& triggerShape) const { + return currentCycle.current_index < triggerShape.getSize(); } static trigger_wheel_e eventIndex[6] = { T_PRIMARY, T_PRIMARY, T_SECONDARY, T_SECONDARY, T_CHANNEL_3, T_CHANNEL_3 }; @@ -315,15 +318,15 @@ static trigger_value_e eventType[6] = { TV_FALL, TV_RISE, TV_FALL, TV_RISE, TV_F /* odd event - start accumulation */ \ currentCycle.timeOfPreviousEventNt[triggerWheel] = nowNt; \ } \ - if (triggerConfiguration->isUseOnlyRisingEdgeForTrigger()) {currentCycle.current_index++;} \ + if (triggerConfiguration.isUseOnlyRisingEdgeForTrigger()) {currentCycle.current_index++;} \ currentCycle.current_index++; \ PRINT_INC_INDEX; \ } -#define considerEventForGap() (!triggerShape->useOnlyPrimaryForSync || isPrimary) +#define considerEventForGap() (!triggerShape.useOnlyPrimaryForSync || isPrimary) -#define needToSkipFall(type) ((!triggerShape->gapBothDirections) && (( triggerShape->useRiseEdge) && (type != TV_RISE))) -#define needToSkipRise(type) ((!triggerShape->gapBothDirections) && ((!triggerShape->useRiseEdge) && (type != TV_FALL))) +#define needToSkipFall(type) ((!triggerShape.gapBothDirections) && (( triggerShape.useRiseEdge) && (type != TV_RISE))) +#define needToSkipRise(type) ((!triggerShape.gapBothDirections) && ((!triggerShape.useRiseEdge) && (type != TV_FALL))) int TriggerState::getCurrentIndex() const { return currentCycle.current_index; @@ -348,10 +351,10 @@ bool TriggerState::isEvenRevolution() const { return totalRevolutionCounter & 1; } -bool TriggerState::validateEventCounters(TriggerWaveform *triggerShape) const { +bool TriggerState::validateEventCounters(const TriggerWaveform& triggerShape) const { bool isDecodingError = false; for (int i = 0;i < PWM_PHASE_MAX_WAVE_PER_PWM;i++) { - isDecodingError |= (currentCycle.eventCount[i] != triggerShape->expectedEventCount[i]); + isDecodingError |= (currentCycle.eventCount[i] != triggerShape.expectedEventCount[i]); } @@ -359,7 +362,7 @@ bool TriggerState::validateEventCounters(TriggerWaveform *triggerShape) const { printf("sync point: isDecodingError=%d\r\n", isDecodingError); if (isDecodingError) { for (int i = 0;i < PWM_PHASE_MAX_WAVE_PER_PWM;i++) { - printf("count: cur=%d exp=%d\r\n", currentCycle.eventCount[i], triggerShape->expectedEventCount[i]); + printf("count: cur=%d exp=%d\r\n", currentCycle.eventCount[i], triggerShape.expectedEventCount[i]); } } #endif /* EFI_UNIT_TEST */ @@ -370,7 +373,7 @@ bool TriggerState::validateEventCounters(TriggerWaveform *triggerShape) const { void TriggerState::onShaftSynchronization( const TriggerStateCallback triggerCycleCallback, const efitick_t nowNt, - const TriggerWaveform *triggerShape) { + const TriggerWaveform& triggerShape) { if (triggerCycleCallback) { @@ -380,7 +383,7 @@ void TriggerState::onShaftSynchronization( startOfCycleNt = nowNt; resetCurrentCycleState(); incrementTotalEventCounter(); - totalEventCountBase += triggerShape->getSize(); + totalEventCountBase += triggerShape.getSize(); #if EFI_UNIT_TEST if (printTriggerDebug) { @@ -400,10 +403,10 @@ void TriggerState::onShaftSynchronization( * @param nowNt current time */ void TriggerState::decodeTriggerEvent( - const TriggerWaveform* const triggerShape, + const TriggerWaveform& triggerShape, const TriggerStateCallback triggerCycleCallback, TriggerStateListener* triggerStateListener, - const TriggerConfiguration* const triggerConfiguration, + const TriggerConfiguration& triggerConfiguration, const trigger_event_e signal, const efitick_t nowNt) { ScopePerf perf(PE::DecodeTriggerEvent); @@ -421,7 +424,7 @@ void TriggerState::decodeTriggerEvent( previousShaftEventTimeNt = nowNt; - bool useOnlyRisingEdgeForTrigger = triggerConfiguration->isUseOnlyRisingEdgeForTrigger(); + bool useOnlyRisingEdgeForTrigger = triggerConfiguration.isUseOnlyRisingEdgeForTrigger(); efiAssertVoid(CUSTOM_TRIGGER_UNEXPECTED, signal <= SHAFT_3RD_RISING, "unexpected signal"); @@ -455,7 +458,7 @@ void TriggerState::decodeTriggerEvent( #if EFI_UNIT_TEST if (printTriggerTrace) { printf("%s isLessImportant %s now=%d index=%d\r\n", - getTrigger_type_e(triggerConfiguration->getType()), + getTrigger_type_e(triggerConfiguration.getType()), getTrigger_event_e(signal), (int)nowNt, currentCycle.current_index); @@ -471,7 +474,7 @@ void TriggerState::decodeTriggerEvent( #if !EFI_PROD_CODE if (printTriggerTrace) { printf("%s event %s %d\r\n", - getTrigger_type_e(triggerConfiguration->getType()), + getTrigger_type_e(triggerConfiguration.getType()), getTrigger_event_e(signal), nowNt); printf("decodeTriggerEvent ratio %.2f: current=%d previous=%d\r\n", 1.0 * toothDurations[0] / toothDurations[1], @@ -509,7 +512,7 @@ void TriggerState::decodeTriggerEvent( DISPLAY(DISPLAY_FIELD(vvtEventFallCounter)); DISPLAY(DISPLAY_FIELD(vvtCamCounter)); - if (triggerShape->isSynchronizationNeeded) { + if (triggerShape.isSynchronizationNeeded) { currentGap = 1.0 * toothDurations[0] / toothDurations[1]; if (CONFIG(debugMode) == DBG_TRIGGER_COUNTERS) { @@ -520,9 +523,9 @@ void TriggerState::decodeTriggerEvent( } bool isSync = true; - for (int i = 0; i < triggerShape->gapTrackingLength; i++) { - bool isGapCondition = cisnan(triggerShape->syncronizationRatioFrom[i]) || (toothDurations[i] > toothDurations[i + 1] * triggerShape->syncronizationRatioFrom[i] - && toothDurations[i] < toothDurations[i + 1] * triggerShape->syncronizationRatioTo[i]); + for (int i = 0; i < triggerShape.gapTrackingLength; i++) { + bool isGapCondition = cisnan(triggerShape.syncronizationRatioFrom[i]) || (toothDurations[i] > toothDurations[i + 1] * triggerShape.syncronizationRatioFrom[i] + && toothDurations[i] < toothDurations[i + 1] * triggerShape.syncronizationRatioTo[i]); isSync &= isGapCondition; } @@ -535,18 +538,18 @@ void TriggerState::decodeTriggerEvent( /** * todo: technically we can afford detailed logging even with 60/2 as long as low RPM * todo: figure out exact threshold as a function of RPM and tooth count? - * Open question what is 'triggerShape->getSize()' for 60/2 is it 58 or 58*2 or 58*4? + * Open question what is 'triggerShape.getSize()' for 60/2 is it 58 or 58*2 or 58*4? */ - bool silentTriggerError = triggerShape->getSize() > 40 && CONFIG(silentTriggerError); + bool silentTriggerError = triggerShape.getSize() > 40 && CONFIG(silentTriggerError); #if EFI_UNIT_TEST actualSynchGap = 1.0 * toothDurations[0] / toothDurations[1]; #endif /* EFI_UNIT_TEST */ #if EFI_PROD_CODE || EFI_SIMULATOR - if (triggerConfiguration->isVerboseTriggerSynchDetails() || (someSortOfTriggerError && !silentTriggerError)) { - for (int i = 0;igapTrackingLength;i++) { - float ratioFrom = triggerShape->syncronizationRatioFrom[i]; + if (triggerConfiguration.isVerboseTriggerSynchDetails() || (someSortOfTriggerError && !silentTriggerError)) { + for (int i = 0;igetPrintPrefix(), + triggerConfiguration.getPrintPrefix(), GET_RPM(), /* cast is needed to make sure we do not put 64 bit value to stack*/ (int)getTimeNowSeconds(), i, gap, ratioFrom, - triggerShape->syncronizationRatioTo[i], + triggerShape.syncronizationRatioTo[i], boolToString(someSortOfTriggerError)); } } @@ -572,13 +575,13 @@ void TriggerState::decodeTriggerEvent( #else if (printTriggerTrace) { float gap = 1.0 * toothDurations[0] / toothDurations[1]; - for (int i = 0;igapTrackingLength;i++) { + for (int i = 0;isyncronizationRatioFrom[i], - triggerShape->syncronizationRatioTo[i], + triggerShape.syncronizationRatioFrom[i], + triggerShape.syncronizationRatioTo[i], boolToString(someSortOfTriggerError)); } } @@ -591,7 +594,7 @@ void TriggerState::decodeTriggerEvent( * in case of noise the counter could be above the expected number of events, that's why 'more or equals' and not just 'equals' */ - unsigned int endOfCycleIndex = triggerShape->getSize() - (triggerConfiguration->isUseOnlyRisingEdgeForTrigger() ? 2 : 1); + unsigned int endOfCycleIndex = triggerShape.getSize() - (triggerConfiguration.isUseOnlyRisingEdgeForTrigger() ? 2 : 1); isSynchronizationPoint = !shaft_is_synchronized || (currentCycle.current_index >= endOfCycleIndex); @@ -601,14 +604,14 @@ void TriggerState::decodeTriggerEvent( shaft_is_synchronized, isSynchronizationPoint, currentCycle.current_index, - triggerShape->getSize()); + triggerShape.getSize()); } #endif /* EFI_UNIT_TEST */ } #if EFI_UNIT_TEST if (printTriggerTrace) { printf("decodeTriggerEvent %s isSynchronizationPoint=%d index=%d %s\r\n", - getTrigger_type_e(triggerConfiguration->getType()), + getTrigger_type_e(triggerConfiguration.getType()), isSynchronizationPoint, currentCycle.current_index, getTrigger_event_e(signal)); } @@ -631,7 +634,7 @@ void TriggerState::decodeTriggerEvent( ; } - for (int i = triggerShape->gapTrackingLength; i > 0; i--) { + for (int i = triggerShape.gapTrackingLength; i > 0; i--) { toothDurations[i] = toothDurations[i - 1]; } @@ -667,9 +670,10 @@ static void onFindIndexCallback(TriggerState *state) { * * This function finds the index of synchronization event within TriggerWaveform */ -uint32_t TriggerState::findTriggerZeroEventIndex(TriggerWaveform * shape, - const TriggerConfiguration * triggerConfiguration, - trigger_config_s const*triggerConfig) { +uint32_t TriggerState::findTriggerZeroEventIndex( + TriggerWaveform& shape, + const TriggerConfiguration& triggerConfiguration, + const trigger_config_s& triggerConfig) { UNUSED(triggerConfig); #if EFI_PROD_CODE efiAssert(CUSTOM_ERR_ASSERT, getCurrentRemainingStack() > 128, "findPos", -1); @@ -678,7 +682,7 @@ uint32_t TriggerState::findTriggerZeroEventIndex(TriggerWaveform * shape, resetTriggerState(); - if (shape->shapeDefinitionError) { + if (shape.shapeDefinitionError) { return 0; } @@ -688,7 +692,7 @@ uint32_t TriggerState::findTriggerZeroEventIndex(TriggerWaveform * shape, uint32_t syncIndex = helper.findTriggerSyncPoint(shape, triggerConfiguration, - this); + *this); if (syncIndex == EFI_ERROR_CODE) { return syncIndex; } @@ -708,9 +712,9 @@ uint32_t TriggerState::findTriggerZeroEventIndex(TriggerWaveform * shape, */ helper.assertSyncPositionAndSetDutyCycle(onFindIndexCallback, triggerConfiguration, - syncIndex, this, shape); + syncIndex, *this, shape); - return syncIndex % shape->getSize(); + return syncIndex % shape.getSize(); } void initTriggerDecoderLogger(Logging *sharedLogger) { diff --git a/firmware/controllers/trigger/trigger_decoder.h b/firmware/controllers/trigger/trigger_decoder.h index 7ea26cd207..29241213a3 100644 --- a/firmware/controllers/trigger/trigger_decoder.h +++ b/firmware/controllers/trigger/trigger_decoder.h @@ -81,20 +81,20 @@ public: efitime_t getTotalEventCounter() const; void decodeTriggerEvent( - const TriggerWaveform *triggerShape, + const TriggerWaveform& triggerShape, const TriggerStateCallback triggerCycleCallback, - TriggerStateListener * triggerStateListener, - const TriggerConfiguration * triggerConfiguration, + TriggerStateListener* triggerStateListener, + const TriggerConfiguration& triggerConfiguration, const trigger_event_e signal, const efitime_t nowUs); - bool validateEventCounters(TriggerWaveform *triggerShape) const; + bool validateEventCounters(const TriggerWaveform& triggerShape) const; void onShaftSynchronization( const TriggerStateCallback triggerCycleCallback, const efitick_t nowNt, - const TriggerWaveform *triggerShape); + const TriggerWaveform& triggerShape); - bool isValidIndex(const TriggerWaveform *triggerShape) const; + bool isValidIndex(const TriggerWaveform& triggerShape) const; /** * TRUE if we know where we are @@ -135,9 +135,10 @@ public: */ efitick_t startOfCycleNt; - uint32_t findTriggerZeroEventIndex(TriggerWaveform * shape, - const TriggerConfiguration * triggerConfiguration, - trigger_config_s const*triggerConfig + uint32_t findTriggerZeroEventIndex( + TriggerWaveform& shape, + const TriggerConfiguration& triggerConfiguration, + const trigger_config_s& triggerConfig ); private: @@ -195,7 +196,9 @@ class Engine; void initTriggerDecoderLogger(Logging *sharedLogger); -void calculateTriggerSynchPoint(TriggerWaveform *shape, - TriggerState *state DECLARE_ENGINE_PARAMETER_SUFFIX); +void calculateTriggerSynchPoint( + TriggerWaveform& shape, + TriggerState& state + DECLARE_ENGINE_PARAMETER_SUFFIX); void prepareEventAngles(TriggerWaveform *shape, TriggerFormDetails *details DECLARE_ENGINE_PARAMETER_SUFFIX); diff --git a/firmware/controllers/trigger/trigger_emulator_algo.cpp b/firmware/controllers/trigger/trigger_emulator_algo.cpp index b5b528950c..6da5d96709 100644 --- a/firmware/controllers/trigger/trigger_emulator_algo.cpp +++ b/firmware/controllers/trigger/trigger_emulator_algo.cpp @@ -21,10 +21,11 @@ int getPreviousIndex(const int currentIndex, const int size) { return (currentIndex + size - 1) % size; } -bool needEvent(const int currentIndex, const int size, MultiChannelStateSequence *multiChannelStateSequence, int channelIndex) { +bool needEvent(const int currentIndex, const int size, const MultiChannelStateSequence& mcss, int channelIndex) { int prevIndex = getPreviousIndex(currentIndex, size); - pin_state_t previousValue = multiChannelStateSequence->getChannelState(channelIndex, /*phaseIndex*/prevIndex); - pin_state_t currentValue = multiChannelStateSequence->getChannelState(channelIndex, /*phaseIndex*/currentIndex); + pin_state_t previousValue = mcss.getChannelState(channelIndex, /*phaseIndex*/prevIndex); + pin_state_t currentValue = mcss.getChannelState(channelIndex, /*phaseIndex*/currentIndex); + return previousValue != currentValue; } @@ -53,7 +54,7 @@ void TriggerEmulatorHelper::handleEmulatorCallback(PwmConfig *state, int stateIn for (size_t i = 0; i < efi::size(emulatorOutputs); i++) { - if (needEvent(stateIndex, state->phaseCount, &state->multiChannelStateSequence, i)) { + if (needEvent(stateIndex, state->phaseCount, state->multiChannelStateSequence, i)) { pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/i, stateIndex); constexpr trigger_event_e riseEvents[] = { SHAFT_PRIMARY_RISING, SHAFT_SECONDARY_RISING, SHAFT_3RD_RISING }; diff --git a/firmware/controllers/trigger/trigger_emulator_algo.h b/firmware/controllers/trigger/trigger_emulator_algo.h index cd5c89c07f..6c22182314 100644 --- a/firmware/controllers/trigger/trigger_emulator_algo.h +++ b/firmware/controllers/trigger/trigger_emulator_algo.h @@ -27,5 +27,5 @@ public: void initTriggerEmulatorLogic(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX); int getPreviousIndex(const int currentIndex, const int size); -bool needEvent(const int currentIndex, const int size, MultiChannelStateSequence *multiChannelStateSequence, int channelIndex); +bool needEvent(const int currentIndex, const int size, const MultiChannelStateSequence& mcss, int channelIndex); diff --git a/firmware/controllers/trigger/trigger_simulator.cpp b/firmware/controllers/trigger/trigger_simulator.cpp index ac1cac540c..04c8217ec5 100644 --- a/firmware/controllers/trigger/trigger_simulator.cpp +++ b/firmware/controllers/trigger/trigger_simulator.cpp @@ -21,36 +21,39 @@ static const bool isRisingEdge[HW_EVENT_TYPES] = { false, true, false, true, fal * todo: should this method be invoked somewhere deeper? at the moment we have too many usages too high * @return true if front should be decoded further, false if we are not interested */ -bool isUsefulSignal(trigger_event_e signal, const TriggerConfiguration * triggerConfiguration) { - return !triggerConfiguration->isUseOnlyRisingEdgeForTrigger() || isRisingEdge[(int) signal]; +bool isUsefulSignal(trigger_event_e signal, const TriggerConfiguration& triggerConfiguration) { + return !triggerConfiguration.isUseOnlyRisingEdgeForTrigger() || isRisingEdge[(int) signal]; } #if EFI_UNIT_TEST extern bool printTriggerDebug; #endif /* ! EFI_UNIT_TEST */ -void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback triggerCycleCallback, - const TriggerConfiguration * triggerConfiguration, - TriggerState *state, TriggerWaveform * shape, int i +void TriggerStimulatorHelper::feedSimulatedEvent( + const TriggerStateCallback triggerCycleCallback, + const TriggerConfiguration& triggerConfiguration, + TriggerState& state, + const TriggerWaveform& shape, + int i ) { - efiAssertVoid(CUSTOM_ERR_6593, shape->getSize() > 0, "size not zero"); - int stateIndex = i % shape->getSize(); - int size = shape->getSize(); + efiAssertVoid(CUSTOM_ERR_6593, shape.getSize() > 0, "size not zero"); + int stateIndex = i % shape.getSize(); + int size = shape.getSize(); - int loopIndex = i / shape->getSize(); + int loopIndex = i / shape.getSize(); - int time = (int) (SIMULATION_CYCLE_PERIOD * (loopIndex + shape->wave.getSwitchTime(stateIndex))); + int time = (int) (SIMULATION_CYCLE_PERIOD * (loopIndex + shape.wave.getSwitchTime(stateIndex))); - MultiChannelStateSequence *multiChannelStateSequence = &shape->wave; + const MultiChannelStateSequence& multiChannelStateSequence = shape.wave; #if EFI_UNIT_TEST - int prevIndex = getPreviousIndex(stateIndex, shape->getSize()); + int prevIndex = getPreviousIndex(stateIndex, shape.getSize()); - pin_state_t primaryWheelState = multiChannelStateSequence->getChannelState(0, prevIndex); - pin_state_t newPrimaryWheelState = multiChannelStateSequence->getChannelState(0, stateIndex); + pin_state_t primaryWheelState = multiChannelStateSequence.getChannelState(0, prevIndex); + pin_state_t newPrimaryWheelState = multiChannelStateSequence.getChannelState(0, stateIndex); - pin_state_t secondaryWheelState = multiChannelStateSequence->getChannelState(1, prevIndex); - pin_state_t newSecondaryWheelState = multiChannelStateSequence->getChannelState(1, stateIndex); + pin_state_t secondaryWheelState = multiChannelStateSequence.getChannelState(1, prevIndex); + pin_state_t newSecondaryWheelState = multiChannelStateSequence.getChannelState(1, stateIndex); // pin_state_t thirdWheelState = multiChannelStateSequence->getChannelState(2, prevIndex); // pin_state_t new3rdWheelState = multiChannelStateSequence->getChannelState(2, stateIndex); @@ -65,10 +68,10 @@ void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback trig // todo: code duplication with TriggerEmulatorHelper::handleEmulatorCallback? if (needEvent(stateIndex, size, multiChannelStateSequence, 0)) { - pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/0, stateIndex); + pin_state_t currentValue = multiChannelStateSequence.getChannelState(/*phaseIndex*/0, stateIndex); trigger_event_e event = currentValue ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING; if (isUsefulSignal(event, triggerConfiguration)) { - state->decodeTriggerEvent(shape, + state.decodeTriggerEvent(shape, triggerCycleCallback, /* override */ nullptr, triggerConfiguration, @@ -77,10 +80,10 @@ void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback trig } if (needEvent(stateIndex, size, multiChannelStateSequence, 1)) { - pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/1, stateIndex); + pin_state_t currentValue = multiChannelStateSequence.getChannelState(/*phaseIndex*/1, stateIndex); trigger_event_e event = currentValue ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING; if (isUsefulSignal(event, triggerConfiguration)) { - state->decodeTriggerEvent(shape, + state.decodeTriggerEvent(shape, triggerCycleCallback, /* override */ nullptr, triggerConfiguration, @@ -89,10 +92,10 @@ void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback trig } if (needEvent(stateIndex, size, multiChannelStateSequence, 2)) { - pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/2, stateIndex); + pin_state_t currentValue = multiChannelStateSequence.getChannelState(/*phaseIndex*/2, stateIndex); trigger_event_e event = currentValue ? SHAFT_3RD_RISING : SHAFT_3RD_FALLING; if (isUsefulSignal(event, triggerConfiguration)) { - state->decodeTriggerEvent(shape, + state.decodeTriggerEvent(shape, triggerCycleCallback, /* override */ nullptr, triggerConfiguration, @@ -101,48 +104,52 @@ void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback trig } } -void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle(const TriggerStateCallback triggerCycleCallback, - const TriggerConfiguration * triggerConfiguration, - const uint32_t syncIndex, TriggerState *state, TriggerWaveform * shape +void TriggerStimulatorHelper::assertSyncPositionAndSetDutyCycle( + const TriggerStateCallback triggerCycleCallback, + const TriggerConfiguration& triggerConfiguration, + const uint32_t syncIndex, + TriggerState& state, + TriggerWaveform& shape ) { /** * let's feed two more cycles to validate shape definition */ - for (uint32_t i = syncIndex + 1; i <= syncIndex + GAP_TRACKING_LENGTH * shape->getSize(); i++) { + for (uint32_t i = syncIndex + 1; i <= syncIndex + GAP_TRACKING_LENGTH * shape.getSize(); i++) { feedSimulatedEvent(triggerCycleCallback, triggerConfiguration, state, shape, i); } - int revolutionCounter = state->getTotalRevolutionCounter(); + int revolutionCounter = state.getTotalRevolutionCounter(); if (revolutionCounter != GAP_TRACKING_LENGTH + 1) { - warning(CUSTOM_OBD_TRIGGER_WAVEFORM, "sync failed/wrong gap parameters trigger=%s rc=%d", getTrigger_type_e(triggerConfiguration->getType()), revolutionCounter); - shape->setShapeDefinitionError(true); + warning(CUSTOM_OBD_TRIGGER_WAVEFORM, "sync failed/wrong gap parameters trigger=%s rc=%d", getTrigger_type_e(triggerConfiguration.getType()), revolutionCounter); + shape.setShapeDefinitionError(true); return; } - shape->shapeDefinitionError = false; + shape.shapeDefinitionError = false; for (int i = 0; i < PWM_PHASE_MAX_WAVE_PER_PWM; i++) { - shape->expectedDutyCycle[i] = 1.0 * state->expectedTotalTime[i] / SIMULATION_CYCLE_PERIOD; + shape.expectedDutyCycle[i] = 1.0 * state.expectedTotalTime[i] / SIMULATION_CYCLE_PERIOD; } } /** * @return trigger synchronization point index, or error code if not found */ -uint32_t TriggerStimulatorHelper::findTriggerSyncPoint(TriggerWaveform * shape, - const TriggerConfiguration * triggerConfiguration, - TriggerState *state) { +uint32_t TriggerStimulatorHelper::findTriggerSyncPoint( + TriggerWaveform& shape, + const TriggerConfiguration& triggerConfiguration, + TriggerState& state) { for (int i = 0; i < 4 * PWM_PHASE_MAX_COUNT; i++) { feedSimulatedEvent(nullptr, triggerConfiguration, state, shape, i); - if (state->shaft_is_synchronized) { + if (state.shaft_is_synchronized) { return i; } } - shape->setShapeDefinitionError(true); + shape.setShapeDefinitionError(true); warning(CUSTOM_ERR_TRIGGER_SYNC, "findTriggerZeroEventIndex() failed"); return EFI_ERROR_CODE; } diff --git a/firmware/controllers/trigger/trigger_simulator.h b/firmware/controllers/trigger/trigger_simulator.h index 4f89da1008..8952f5db2f 100644 --- a/firmware/controllers/trigger/trigger_simulator.h +++ b/firmware/controllers/trigger/trigger_simulator.h @@ -13,21 +13,26 @@ class TriggerStimulatorHelper { public: - uint32_t findTriggerSyncPoint(TriggerWaveform * shape, - const TriggerConfiguration * triggerConfiguration, - TriggerState *state); + uint32_t findTriggerSyncPoint( + TriggerWaveform& shape, + const TriggerConfiguration& triggerConfiguration, + TriggerState& state); - void assertSyncPositionAndSetDutyCycle(const TriggerStateCallback triggerCycleCallback, - const TriggerConfiguration * triggerConfiguration, - const uint32_t index, TriggerState *state, TriggerWaveform * shape + void assertSyncPositionAndSetDutyCycle( + const TriggerStateCallback triggerCycleCallback, + const TriggerConfiguration& triggerConfiguration, + const uint32_t index, + TriggerState& state, + TriggerWaveform& shape ); private: // send next event so that we can see how state reacts void feedSimulatedEvent(const TriggerStateCallback triggerCycleCallback, - const TriggerConfiguration * triggerConfiguration, - TriggerState *state, - TriggerWaveform * shape, int i); + const TriggerConfiguration& triggerConfiguration, + TriggerState& state, + const TriggerWaveform& shape, + int i); }; -bool isUsefulSignal(trigger_event_e signal, const TriggerConfiguration * triggerConfiguration); +bool isUsefulSignal(trigger_event_e signal, const TriggerConfiguration& triggerConfiguration); diff --git a/unit_tests/tests/trigger/test_trigger_decoder.cpp b/unit_tests/tests/trigger/test_trigger_decoder.cpp index deed5c98fc..28da911a84 100644 --- a/unit_tests/tests/trigger/test_trigger_decoder.cpp +++ b/unit_tests/tests/trigger/test_trigger_decoder.cpp @@ -42,11 +42,11 @@ static int getTriggerZeroEventIndex(engine_type_e engineType) { initDataStructures(PASS_ENGINE_PARAMETER_SIGNATURE); - const TriggerConfiguration * triggerConfiguration = &engine->primaryTriggerConfiguration; + const auto& triggerConfiguration = engine->primaryTriggerConfiguration; - TriggerWaveform * shape = ð.engine.triggerCentral.triggerShape; + TriggerWaveform& shape = eth.engine.triggerCentral.triggerShape; return eth.engine.triggerCentral.triggerState.findTriggerZeroEventIndex(shape, triggerConfiguration, - &engineConfiguration->trigger); + engineConfiguration->trigger); } TEST(misc, testSkipped2_0) { @@ -113,47 +113,42 @@ static void assertTriggerPosition(event_trigger_position_s *position, int eventI } TEST(misc, testSomethingWeird) { - printf("*************************************************** testSomethingWeird\r\n"); - WITH_ENGINE_TEST_HELPER(FORD_INLINE_6_1995); TriggerState state_; TriggerState *sta = &state_; - const TriggerConfiguration * triggerConfiguration = &engine->primaryTriggerConfiguration; + const auto& triggerConfiguration = engine->primaryTriggerConfiguration; ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized"; int r = 10; - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r); ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized"; // still no synchronization - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, ++r); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, ++r); ASSERT_TRUE(sta->shaft_is_synchronized); // first signal rise synchronize ASSERT_EQ(0, sta->getCurrentIndex()); - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); ASSERT_EQ(1, sta->getCurrentIndex()); for (int i = 2; i < 10;) { - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); assertEqualsM("even", i++, sta->getCurrentIndex()); - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); assertEqualsM("odd", i++, sta->getCurrentIndex()); } - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); ASSERT_EQ(10, sta->getCurrentIndex()); - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_FALLING, r++); ASSERT_EQ(11, sta->getCurrentIndex()); - sta->decodeTriggerEvent(&ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); + sta->decodeTriggerEvent(ENGINE(triggerCentral.triggerShape), nullptr, /* override */ nullptr, triggerConfiguration, SHAFT_PRIMARY_RISING, r++); ASSERT_EQ(0, sta->getCurrentIndex()); // new revolution } TEST(misc, test1995FordInline6TriggerDecoder) { - - printf("*************************************************** test1995FordInline6TriggerDecoder\r\n"); - ASSERT_EQ( 0, getTriggerZeroEventIndex(FORD_INLINE_6_1995)) << "triggerIndex "; WITH_ENGINE_TEST_HELPER(FORD_INLINE_6_1995);