move logic in to trigger decoder (#4162)

* move logic in to trigger decoder

* status loop

* minor cleanup

* s
This commit is contained in:
Matthew Kennedy 2022-05-10 13:55:28 -07:00 committed by GitHub
parent 7597b2c0eb
commit 170d574a5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 70 additions and 55 deletions

View File

@ -344,7 +344,7 @@ static void initStatusLeds() {
static bool isTriggerErrorNow() {
#if EFI_ENGINE_CONTROL && EFI_SHAFT_POSITION_INPUT
bool justHadError = (getTimeNowNt() - engine->triggerCentral.triggerState.lastDecodingErrorTime) < MS2NT(200);
bool justHadError = engine->triggerCentral.triggerState.someSortOfTriggerError();
return justHadError || engine->triggerCentral.isTriggerDecoderError();
#else
return false;

View File

@ -387,11 +387,8 @@ void Engine::OnTriggerStateDecodingError() {
TRIGGER_WAVEFORM(getExpectedEventCount(0)),
TRIGGER_WAVEFORM(getExpectedEventCount(1)),
TRIGGER_WAVEFORM(getExpectedEventCount(2)));
triggerCentral.triggerState.setTriggerErrorState();
triggerCentral.triggerState.totalTriggerErrorCounter++;
if (engineConfiguration->verboseTriggerSynchDetails || (triggerCentral.triggerState.someSortOfTriggerError && !engineConfiguration->silentTriggerError)) {
if (engineConfiguration->verboseTriggerSynchDetails || (triggerCentral.triggerState.someSortOfTriggerError() && !engineConfiguration->silentTriggerError)) {
#if EFI_PROD_CODE
efiPrintf("error: synchronizationPoint @ index %d expected %d/%d/%d got %d/%d/%d",
triggerCentral.triggerState.currentCycle.current_index,
@ -423,23 +420,12 @@ void Engine::OnTriggerSynchronizationLost() {
}
}
void Engine::OnTriggerInvalidIndex(int currentIndex) {
// let's not show a warning if we are just starting to spin
if (Sensor::getOrZero(SensorType::Rpm) != 0) {
warning(CUSTOM_SYNC_ERROR, "sync error: index #%d above total size %d", currentIndex, triggerCentral.triggerShape.getSize());
triggerCentral.triggerState.setTriggerErrorState();
}
}
void Engine::OnTriggerSyncronization(bool wasSynchronized, bool isDecodingError) {
// TODO: this logic probably shouldn't be part of engine.cpp
void Engine::OnTriggerSyncronization(bool wasSynchronized) {
// We only care about trigger shape once we have synchronized trigger. Anything could happen
// during first revolution and it's fine
if (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);
enginePins.triggerDecoderErrorPin.setValue(isDecodingError);
// 'triggerStateListener is not null' means we are running a real engine and now just preparing trigger shape

View File

@ -241,8 +241,7 @@ public:
#if EFI_SHAFT_POSITION_INPUT
void OnTriggerStateDecodingError();
void OnTriggerStateProperState(efitick_t nowNt) override;
void OnTriggerSyncronization(bool wasSynchronized) override;
void OnTriggerInvalidIndex(int currentIndex) override;
void OnTriggerSyncronization(bool wasSynchronized, bool isDecodingError) override;
void OnTriggerSynchronizationLost() override;
#endif

View File

@ -285,9 +285,9 @@ void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt, int index) {
return;
}
if (isVvtWithRealDecoder) {
TriggerDecoderBase& vvtDecoder = tc->vvtState[bankIndex][camIndex];
TriggerDecoderBase& vvtDecoder = tc->vvtState[bankIndex][camIndex];
if (isVvtWithRealDecoder) {
vvtDecoder.decodeTriggerEvent(
"vvt",
tc->vvtShape[camIndex],
@ -320,7 +320,7 @@ void hwHandleVvtCamSignal(trigger_value_e front, efitick_t nowNt, int index) {
tc->triggerState.vvtCurrentPosition = currentPosition;
if (isVvtWithRealDecoder && tc->vvtState[bankIndex][camIndex].currentCycle.current_index != 0) {
if (isVvtWithRealDecoder && vvtDecoder.currentCycle.current_index != 0) {
// this is not sync tooth - exiting
return;
}
@ -627,10 +627,6 @@ void TriggerCentral::handleShaftSignal(trigger_event_e signal, efitick_t timesta
engine->primaryTriggerConfiguration,
signal, timestamp);
triggerState.triggerStateIndex = triggerState.currentCycle.current_index;
/**
* 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.

View File

@ -68,10 +68,8 @@ void TriggerDecoderBase::resetTriggerState() {
totalRevolutionCounter = 0;
totalTriggerErrorCounter = 0;
orderingErrorCounter = 0;
lastDecodingErrorTime = US2NT(-10000000LL);
someSortOfTriggerError = false;
m_timeSinceDecodeError.init();
curSignal = SHAFT_PRIMARY_FALLING;
prevSignal = SHAFT_PRIMARY_FALLING;
startOfCycleNt = 0;
@ -83,8 +81,7 @@ void TriggerDecoderBase::resetTriggerState() {
}
void TriggerDecoderBase::setTriggerErrorState() {
lastDecodingErrorTime = getTimeNowNt();
someSortOfTriggerError = true;
m_timeSinceDecodeError.reset();
}
void TriggerDecoderBase::resetCurrentCycleState() {
@ -418,7 +415,14 @@ void TriggerDecoderBase::incrementTotalEventCounter() {
totalRevolutionCounter++;
}
void PrimaryTriggerDecoder::onTriggerError() {
// On trigger error, we've lost full sync
m_hasSynchronizedPhase = false;
}
bool TriggerDecoderBase::validateEventCounters(const TriggerWaveform& triggerShape) const {
// We can check if things are fine by comparing the number of events in a cycle with the expected number of event.
bool isDecodingError = false;
for (int i = 0;i < PWM_PHASE_MAX_WAVE_PER_PWM;i++) {
isDecodingError |= (currentCycle.eventCount[i] != triggerShape.getExpectedEventCount(i));
@ -505,12 +509,12 @@ void TriggerDecoderBase::decodeTriggerEvent(
trigger_wheel_e triggerWheel = eventIndex[signal];
trigger_value_e type = eventType[signal];
if (!useOnlyRisingEdgeForTrigger && curSignal == prevSignal) {
// Check that we didn't get the same edge twice in a row - that should be impossible
if (!useOnlyRisingEdgeForTrigger && prevSignal == signal) {
orderingErrorCounter++;
}
prevSignal = curSignal;
curSignal = signal;
prevSignal = signal;
currentCycle.eventCount[triggerWheel]++;
@ -581,7 +585,7 @@ void TriggerDecoderBase::decodeTriggerEvent(
#endif /* EFI_UNIT_TEST */
#if EFI_PROD_CODE || EFI_SIMULATOR
if (triggerConfiguration.VerboseTriggerSynchDetails || (someSortOfTriggerError && !silentTriggerError)) {
if (triggerConfiguration.VerboseTriggerSynchDetails || (someSortOfTriggerError() && !silentTriggerError)) {
int rpm = Sensor::getOrZero(SensorType::Rpm);
floatms_t engineCycleDuration = getEngineCycleDuration(rpm);
@ -615,7 +619,7 @@ void TriggerDecoderBase::decodeTriggerEvent(
gap,
ratioFrom,
triggerShape.syncronizationRatioTo[i],
boolToString(someSortOfTriggerError));
boolToString(someSortOfTriggerError()));
}
}
}
@ -630,7 +634,7 @@ void TriggerDecoderBase::decodeTriggerEvent(
gap,
triggerShape.syncronizationRatioFrom[i],
triggerShape.syncronizationRatioTo[i],
boolToString(someSortOfTriggerError));
boolToString(someSortOfTriggerError()));
}
}
#endif /* EFI_PROD_CODE */
@ -667,7 +671,18 @@ void TriggerDecoderBase::decodeTriggerEvent(
if (isSynchronizationPoint) {
if (triggerStateListener) {
triggerStateListener->OnTriggerSyncronization(wasSynchronized);
bool isDecodingError = validateEventCounters(triggerShape);
triggerStateListener->OnTriggerSyncronization(wasSynchronized, isDecodingError);
// If we got a sync point, but the wrong number of events since the last sync point
if (wasSynchronized && isDecodingError) {
setTriggerErrorState();
totalTriggerErrorCounter++;
// This is a decoding error
onTriggerError();
}
}
setShaftSynchronized(true);
@ -676,7 +691,6 @@ void TriggerDecoderBase::decodeTriggerEvent(
;
onShaftSynchronization(triggerCycleCallback, wasSynchronized, nowNt, triggerShape);
} else { /* if (!isSynchronizationPoint) */
nextTriggerEvent()
;
@ -688,21 +702,31 @@ void TriggerDecoderBase::decodeTriggerEvent(
toothed_previous_time = nowNt;
}
// TODO: should we include triggerStateListener here? That seems vestigial from when it called a listener, but it changes the behavior to remove it.
if (getShaftSynchronized() && !isValidIndex(triggerShape) && triggerStateListener) {
triggerStateListener->OnTriggerInvalidIndex(currentCycle.current_index);
// We've had too many events since the last sync point, we should have seen a sync point by now.
// This is a trigger error.
// let's not show a warning if we are just starting to spin
if (Sensor::getOrZero(SensorType::Rpm) != 0) {
warning(CUSTOM_SYNC_ERROR, "sync error for %s: index #%d above total size %d", name, currentCycle.current_index, triggerShape.getSize());
setTriggerErrorState();
// TODO: should we increment totalTriggerErrorCounter here too?
}
onTriggerError();
return;
}
if (someSortOfTriggerError) {
if (getTimeNowNt() - lastDecodingErrorTime > NT_PER_SECOND) {
someSortOfTriggerError = false;
}
}
// Needed for early instant-RPM detection
if (triggerStateListener) {
triggerStateListener->OnTriggerStateProperState(nowNt);
}
triggerStateIndex = currentCycle.current_index;
}
bool TriggerDecoderBase::isSyncPoint(const TriggerWaveform& triggerShape, trigger_type_e triggerType) const {

View File

@ -18,8 +18,7 @@ class TriggerDecoderBase;
struct TriggerStateListener {
#if EFI_SHAFT_POSITION_INPUT
virtual void OnTriggerStateProperState(efitick_t nowNt) = 0;
virtual void OnTriggerSyncronization(bool wasSynchronized) = 0;
virtual void OnTriggerInvalidIndex(int currentIndex) = 0;
virtual void OnTriggerSyncronization(bool wasSynchronized, bool isDecodingError) = 0;
virtual void OnTriggerSynchronizationLost() = 0;
#endif // EFI_SHAFT_POSITION_INPUT
};
@ -98,7 +97,6 @@ public:
const trigger_event_e signal,
const efitime_t nowUs);
bool validateEventCounters(const TriggerWaveform& triggerShape) const;
void onShaftSynchronization(
const TriggerStateCallback triggerCycleCallback,
bool wasSynchronized,
@ -117,10 +115,6 @@ public:
void setTriggerErrorState();
efitick_t lastDecodingErrorTime;
// the boolean flag is a performance optimization so that complex comparison is avoided if no error
bool someSortOfTriggerError;
/**
* current duration at index zero and previous durations are following
*/
@ -155,15 +149,29 @@ public:
const trigger_config_s& triggerConfig
);
bool someSortOfTriggerError() const {
return m_timeSinceDecodeError.getElapsedSeconds(1);
}
protected:
// Called when some problem is detected with trigger decoding.
// That means either:
// - Too many events without a sync point
// - Saw a sync point but the wrong number of events in the cycle
virtual void onTriggerError() { }
private:
void resetCurrentCycleState();
bool isSyncPoint(const TriggerWaveform& triggerShape, trigger_type_e triggerType) const;
trigger_event_e curSignal;
bool validateEventCounters(const TriggerWaveform& triggerShape) const;
trigger_event_e prevSignal;
int64_t totalEventCountBase;
bool isFirstEvent;
Timer m_timeSinceDecodeError;
};
// we only need 90 degrees of events so /4 or maybe even /8 should work?
@ -220,6 +228,8 @@ public:
return m_hasSynchronizedPhase;
}
void onTriggerError() override;
private:
float calculateInstantRpm(
TriggerWaveform const & triggerShape, TriggerFormDetails *triggerFormDetails,