Refactor Trigger System #635

injecting callback via parameters instead of nasty "bool isInitializingTrigger" field
This commit is contained in:
rusefi 2019-12-06 01:57:11 -05:00
parent 717abd6b67
commit d6471a84bc
7 changed files with 59 additions and 28 deletions

View File

@ -227,6 +227,19 @@ void Engine::preCalculate(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
#endif
}
void Engine::OnTriggerStateDecodingError() {
Engine *engine = this;
EXPAND_Engine;
triggerCentral.triggerState.handleTriggerError(PASS_ENGINE_PARAMETER_SIGNATURE);
}
void Engine::OnTriggerStateProperState(efitick_t nowNt) {
Engine *engine = this;
EXPAND_Engine;
rpmCalculator.setSpinningUp(nowNt PASS_ENGINE_PARAMETER_SUFFIX);
}
void Engine::setConfig(persistent_config_s *config) {
this->config = config;
engineConfigurationPtr = &config->engineConfiguration;

View File

@ -47,10 +47,14 @@ class RpmCalculator;
#define CYCLE_ALTERNATION 2
class Engine {
class Engine : public TriggerStateListener {
public:
explicit Engine(persistent_config_s *config);
Engine();
void OnTriggerStateDecodingError() override;
void OnTriggerStateProperState(efitick_t nowNt) override;
void setConfig(persistent_config_s *config);
injection_mode_e getCurrentInjectionMode(DECLARE_ENGINE_PARAMETER_SIGNATURE);
@ -140,9 +144,6 @@ public:
// timestamp of most recent time RPM hard limit was triggered
efitime_t rpmHardLimitTimestamp = 0;
// todo: should be a field on some other class, not Engine?
bool isInitializingTrigger = false;
/**
* This flag indicated a big enough problem that engine control would be
* prohibited if this flag is set to true.

View File

@ -364,7 +364,7 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal DECLARE_ENGINE_PAR
/**
* This invocation changes the state of triggerState
*/
triggerState.decodeTriggerEvent(nullptr, signal, nowNt PASS_ENGINE_PARAMETER_SUFFIX);
triggerState.decodeTriggerEvent(nullptr, engine, signal, nowNt 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

View File

@ -343,7 +343,7 @@ bool TriggerState::validateEventCounters(DECLARE_ENGINE_PARAMETER_SIGNATURE) con
|| currentCycle.eventCount[2] != TRIGGER_SHAPE(expectedEventCount[2]);
#if EFI_UNIT_TEST
printf("sync point: isDecodingError=%d isInit=%d\r\n", isDecodingError, engine->isInitializingTrigger);
printf("sync point: isDecodingError=%d\r\n", isDecodingError);
if (isDecodingError) {
printf("count: cur=%d exp=%d\r\n", currentCycle.eventCount[0], TRIGGER_SHAPE(expectedEventCount[0]));
printf("count: cur=%d exp=%d\r\n", currentCycle.eventCount[1], TRIGGER_SHAPE(expectedEventCount[1]));
@ -417,6 +417,7 @@ void TriggerState::onShaftSynchronization(const TriggerStateCallback triggerCycl
* @param nowNt current time
*/
void TriggerState::decodeTriggerEvent(const TriggerStateCallback triggerCycleCallback,
TriggerStateListener * triggerStateListener,
trigger_event_e const signal, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) {
ScopePerf perf(PE::DecodeTriggerEvent, static_cast<uint8_t>(signal));
@ -449,6 +450,7 @@ void TriggerState::decodeTriggerEvent(const TriggerStateCallback triggerCycleCal
toothDurations[0] =
currentDurationLong > 10 * US2NT(US_PER_SECOND_LL) ? 10 * US2NT(US_PER_SECOND_LL) : currentDurationLong;
bool haveListener = triggerStateListener != NULL;
bool isPrimary = triggerWheel == T_PRIMARY;
if (needToSkipFall(type) || needToSkipRise(type) || (!considerEventForGap())) {
@ -647,8 +649,10 @@ void TriggerState::decodeTriggerEvent(const TriggerStateCallback triggerCycleCal
enginePins.triggerDecoderErrorPin.setValue(isDecodingError);
if (isDecodingError && !engine->isInitializingTrigger) {
handleTriggerError(PASS_ENGINE_PARAMETER_SIGNATURE);
// 'haveListener' means we are running a real engine and now just preparing trigger shape
// that's a bit of a hack, a sweet OOP solution would be a real callback or at least 'needDecodingErrorLogic' method?
if (isDecodingError && haveListener) {
triggerStateListener->OnTriggerStateDecodingError();
}
errorDetection.add(isDecodingError);
@ -674,7 +678,7 @@ void TriggerState::decodeTriggerEvent(const TriggerStateCallback triggerCycleCal
toothed_previous_time = nowNt;
}
if (!isValidIndex(PASS_ENGINE_PARAMETER_SIGNATURE) && !engine->isInitializingTrigger) {
if (!isValidIndex(PASS_ENGINE_PARAMETER_SIGNATURE) && haveListener) {
// let's not show a warning if we are just starting to spin
if (GET_RPM_VALUE != 0) {
warning(CUSTOM_SYNC_ERROR, "sync error: index #%d above total size %d", currentCycle.current_index, getTriggerSize());
@ -691,8 +695,8 @@ void TriggerState::decodeTriggerEvent(const TriggerStateCallback triggerCycleCal
runtimeStatistics(nowNt PASS_ENGINE_PARAMETER_SUFFIX);
// Needed for early instant-RPM detection
if (!engine->isInitializingTrigger) {
engine->rpmCalculator.setSpinningUp(nowNt PASS_ENGINE_PARAMETER_SUFFIX);
if (haveListener) {
triggerStateListener->OnTriggerStateProperState(nowNt);
}
}
@ -724,14 +728,12 @@ uint32_t findTriggerZeroEventIndex(TriggerState *state, TriggerShape * shape,
return 0;
}
engine->isInitializingTrigger = true;
// todo: should this variable be declared 'static' to reduce stack usage?
TriggerStimulatorHelper helper;
uint32_t syncIndex = helper.findTriggerSyncPoint(shape, state PASS_ENGINE_PARAMETER_SUFFIX);
if (syncIndex == EFI_ERROR_CODE) {
engine->isInitializingTrigger = false;
return syncIndex;
}
efiAssert(CUSTOM_ERR_ASSERT, state->getTotalRevolutionCounter() == 1, "findZero_revCounter", EFI_ERROR_CODE);
@ -751,7 +753,6 @@ uint32_t findTriggerZeroEventIndex(TriggerState *state, TriggerShape * shape,
helper.assertSyncPositionAndSetDutyCycle(onFindIndexCallback, syncIndex, state, shape PASS_ENGINE_PARAMETER_SUFFIX);
engine->isInitializingTrigger = false;
return syncIndex % shape->getSize();
}

View File

@ -14,6 +14,12 @@
class TriggerState;
class TriggerStateListener {
public:
virtual void OnTriggerStateDecodingError() = 0;
virtual void OnTriggerStateProperState(efitick_t nowNt) = 0;
};
typedef void (*TriggerStateCallback)(TriggerState *);
typedef struct {
@ -60,6 +66,7 @@ public:
efitime_t getTotalEventCounter() const;
void decodeTriggerEvent(const TriggerStateCallback triggerCycleCallback,
TriggerStateListener * triggerStateListener,
trigger_event_e const signal, efitime_t nowUs DECLARE_ENGINE_PARAMETER_SUFFIX);
bool validateEventCounters(DECLARE_ENGINE_PARAMETER_SIGNATURE) const;

View File

@ -63,22 +63,31 @@ void TriggerStimulatorHelper::feedSimulatedEvent(const TriggerStateCallback trig
if (needEvent(stateIndex, size, multiChannelStateSequence, 0)) {
pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/0, stateIndex);
trigger_event_e s = currentValue ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING;
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX))
state->decodeTriggerEvent(triggerCycleCallback, s, time PASS_ENGINE_PARAMETER_SUFFIX);
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX)) {
state->decodeTriggerEvent(triggerCycleCallback,
/* override */ nullptr,
s, time PASS_ENGINE_PARAMETER_SUFFIX);
}
}
if (needEvent(stateIndex, size, multiChannelStateSequence, 1)) {
pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/1, stateIndex);
trigger_event_e s = currentValue ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING;
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX))
state->decodeTriggerEvent(triggerCycleCallback, s, time PASS_ENGINE_PARAMETER_SUFFIX);
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX)) {
state->decodeTriggerEvent(triggerCycleCallback,
/* override */ nullptr,
s, time PASS_ENGINE_PARAMETER_SUFFIX);
}
}
if (needEvent(stateIndex, size, multiChannelStateSequence, 2)) {
pin_state_t currentValue = multiChannelStateSequence->getChannelState(/*phaseIndex*/2, stateIndex);
trigger_event_e s = currentValue ? SHAFT_3RD_RISING : SHAFT_3RD_FALLING;
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX))
state->decodeTriggerEvent(triggerCycleCallback, s, time PASS_ENGINE_PARAMETER_SUFFIX);
if (isUsefulSignal(s PASS_ENGINE_PARAMETER_SUFFIX)) {
state->decodeTriggerEvent(triggerCycleCallback,
/* override */ nullptr,
s, time PASS_ENGINE_PARAMETER_SUFFIX);
}
}
}

View File

@ -118,28 +118,28 @@ TEST(misc, testSomethingWeird) {
ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized";
int r = 10;
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_FALLING, r PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_FALLING, r PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_FALSE(sta->shaft_is_synchronized) << "shaft_is_synchronized"; // still no synchronization
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_RISING, ++r PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_RISING, ++r PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_TRUE(sta->shaft_is_synchronized); // first signal rise synchronize
ASSERT_EQ(0, sta->getCurrentIndex());
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_EQ(1, sta->getCurrentIndex());
for (int i = 2; i < 10;) {
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
assertEqualsM("even", i++, sta->getCurrentIndex());
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
assertEqualsM("odd", i++, sta->getCurrentIndex());
}
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_EQ(10, sta->getCurrentIndex());
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_FALLING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_EQ(11, sta->getCurrentIndex());
sta->decodeTriggerEvent(nullptr, SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
sta->decodeTriggerEvent(nullptr, /* override */ nullptr,SHAFT_PRIMARY_RISING, r++ PASS_ENGINE_PARAMETER_SUFFIX);
ASSERT_EQ(0, sta->getCurrentIndex()); // new revolution
}