From cf51533f454ffd404a6b8daaac58121f6f0da1fa Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sun, 29 May 2022 10:49:00 -0700 Subject: [PATCH] More trigger encapsulation (#4207) * trigger decoder returns a result * TriggerFormDetails * s * don't reach out and touch the engine --- firmware/controllers/math/engine_math.cpp | 2 +- .../trigger/decoders/trigger_structure.cpp | 4 ++-- .../trigger/decoders/trigger_structure.h | 6 ++++-- firmware/controllers/trigger/trigger_central.cpp | 4 ++-- firmware/controllers/trigger/trigger_decoder.cpp | 15 +++++++-------- unit_tests/engine_test_helper.cpp | 2 +- 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/firmware/controllers/math/engine_math.cpp b/firmware/controllers/math/engine_math.cpp index 5dc2ca5751..7c3c716699 100644 --- a/firmware/controllers/math/engine_math.cpp +++ b/firmware/controllers/math/engine_math.cpp @@ -443,7 +443,7 @@ void prepareOutputSignals() { prepareIgnitionPinIndices(engineConfiguration->ignitionMode); - TRIGGER_WAVEFORM(prepareShape(&engine->triggerCentral.triggerFormDetails)); + TRIGGER_WAVEFORM(prepareShape(engine->triggerCentral.triggerFormDetails)); // Fuel schedule may now be completely wrong, force a reset engine->injectionEvents.invalidate(); diff --git a/firmware/controllers/trigger/decoders/trigger_structure.cpp b/firmware/controllers/trigger/decoders/trigger_structure.cpp index 631a7b9346..e344465852 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.cpp +++ b/firmware/controllers/trigger/decoders/trigger_structure.cpp @@ -417,14 +417,14 @@ void findTriggerPosition(TriggerWaveform *triggerShape, } } -void TriggerWaveform::prepareShape(TriggerFormDetails *details) { +void TriggerWaveform::prepareShape(TriggerFormDetails& details) { #if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT if (shapeDefinitionError) { // Nothing to do here if there's a problem with the trigger shape return; } - prepareEventAngles(this, details); + details.prepareEventAngles(this); #endif } diff --git a/firmware/controllers/trigger/decoders/trigger_structure.h b/firmware/controllers/trigger/decoders/trigger_structure.h index c68f4bde62..d6616de6fa 100644 --- a/firmware/controllers/trigger/decoders/trigger_structure.h +++ b/firmware/controllers/trigger/decoders/trigger_structure.h @@ -194,7 +194,7 @@ public: * TODO this should be migrated to CRANKshaft revolution, this would go together * this variable is public for performance reasons (I want to avoid costs of method if it's not inlined) * but name is supposed to hint at the fact that decoders should not be assigning to it - * Please use "getTriggerSize()" macro or "getSize()" method to read this value + * Please use "getSize()" function to read this value */ MultiChannelStateSequenceWithData wave; @@ -247,7 +247,7 @@ public: size_t getSize() const; int getTriggerWaveformSynchPointIndex() const; - void prepareShape(TriggerFormDetails *details); + void prepareShape(TriggerFormDetails& details); /** * This private method should only be used to prepare the array of pre-calculated values @@ -297,6 +297,8 @@ private: */ class TriggerFormDetails { public: + void prepareEventAngles(TriggerWaveform *shape); + /** * These angles are in event coordinates - with synchronization point located at angle zero. * These values are pre-calculated for performance reasons. diff --git a/firmware/controllers/trigger/trigger_central.cpp b/firmware/controllers/trigger/trigger_central.cpp index 112cd37739..b4db0e39bd 100644 --- a/firmware/controllers/trigger/trigger_central.cpp +++ b/firmware/controllers/trigger/trigger_central.cpp @@ -690,9 +690,9 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta * If we only have a crank position sensor with four stroke, here we are extending crank revolutions with a 360 degree * cycle into a four stroke, 720 degrees cycle. */ - int crankDivider = getCrankDivider(engine->getOperationMode()); + int crankDivider = getCrankDivider(triggerShape.getOperationMode()); int crankInternalIndex = triggerState.getTotalRevolutionCounter() % crankDivider; - int triggerIndexForListeners = decodeResult.Value.CurrentIndex + (crankInternalIndex * getTriggerSize()); + int triggerIndexForListeners = decodeResult.Value.CurrentIndex + (crankInternalIndex * triggerShape.getSize()); if (triggerIndexForListeners == 0) { m_syncPointTimer.reset(timestamp); } diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index 13da947e29..1f0f85b58a 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -147,8 +147,7 @@ void calculateTriggerSynchPoint( } } -void prepareEventAngles(TriggerWaveform *shape, - TriggerFormDetails *details) { +void TriggerFormDetails::prepareEventAngles(TriggerWaveform *shape) { int triggerShapeSynchPointIndex = shape->triggerShapeSynchPointIndex; if (triggerShapeSynchPointIndex == EFI_ERROR_CODE) { return; @@ -160,7 +159,7 @@ void prepareEventAngles(TriggerWaveform *shape, size_t length = shape->getLength(); - memset(details->eventAngles, 0, sizeof(details->eventAngles)); + memset(eventAngles, 0, sizeof(eventAngles)); // this may be getSize(); @@ -171,9 +170,9 @@ void prepareEventAngles(TriggerWaveform *shape, for (size_t eventIndex = 0; eventIndex < length; eventIndex++) { if (eventIndex == 0) { // explicit check for zero to avoid issues where logical zero is not exactly zero due to float nature - details->eventAngles[0] = 0; + eventAngles[0] = 0; // this value would be used in case of front-only - details->eventAngles[1] = 0; + eventAngles[1] = 0; } else { // Rotate the trigger around so that the sync point is at position 0 auto wrappedIndex = (shape->triggerShapeSynchPointIndex + eventIndex) % length; @@ -196,11 +195,11 @@ void prepareEventAngles(TriggerWaveform *shape, // In case this is a rising event, replace the following fall event with the rising as well if (shape->isRiseEvent[triggerDefinitionIndex]) { riseOnlyIndex += 2; - details->eventAngles[riseOnlyIndex] = angle; - details->eventAngles[riseOnlyIndex + 1] = angle; + eventAngles[riseOnlyIndex] = angle; + eventAngles[riseOnlyIndex + 1] = angle; } } else { - details->eventAngles[eventIndex] = angle; + eventAngles[eventIndex] = angle; } } } diff --git a/unit_tests/engine_test_helper.cpp b/unit_tests/engine_test_helper.cpp index 35402f2d15..c5e162a31d 100644 --- a/unit_tests/engine_test_helper.cpp +++ b/unit_tests/engine_test_helper.cpp @@ -390,8 +390,8 @@ void EngineTestHelper::executeUntil(int timeUs) { } void setupSimpleTestEngineWithMafAndTT_ONE_trigger(EngineTestHelper *eth, injection_mode_e injectionMode) { - setupSimpleTestEngineWithMaf(eth, injectionMode, TT_ONE); setCamOperationMode(); + setupSimpleTestEngineWithMaf(eth, injectionMode, TT_ONE); } void setVerboseTrigger(bool isEnabled) {