fix vvt logging logic (#4747)

* fix vvt logging logic

* put that back

* flip condition to avoid branch on uninitialized value
This commit is contained in:
Matthew Kennedy 2022-11-07 07:52:17 -08:00 committed by GitHub
parent ffbf58e0b6
commit e801ffc979
3 changed files with 33 additions and 41 deletions

View File

@ -212,7 +212,7 @@ static angle_t wrapVvt(angle_t vvtPosition, int period) {
return vvtPosition; return vvtPosition;
} }
static void logFront(bool isImportantFront, efitick_t nowNt, int index) { static void logVvtFront(bool useOnlyRise, bool isImportantFront, TriggerValue front, efitick_t nowNt, int index) {
if (isImportantFront && isBrainPinValid(engineConfiguration->camInputsDebug[index])) { if (isImportantFront && isBrainPinValid(engineConfiguration->camInputsDebug[index])) {
#if EFI_PROD_CODE #if EFI_PROD_CODE
writePad("cam debug", engineConfiguration->camInputsDebug[index], 1); writePad("cam debug", engineConfiguration->camInputsDebug[index], 1);
@ -220,24 +220,23 @@ static void logFront(bool isImportantFront, efitick_t nowNt, int index) {
getExecutorInterface()->scheduleByTimestampNt("dbg_on", &debugToggleScheduling, nowNt + DEBUG_PIN_DELAY, &turnOffAllDebugFields); getExecutorInterface()->scheduleByTimestampNt("dbg_on", &debugToggleScheduling, nowNt + DEBUG_PIN_DELAY, &turnOffAllDebugFields);
} }
if (engineConfiguration->displayLogicLevelsInEngineSniffer && isImportantFront) { if (!useOnlyRise || engineConfiguration->displayLogicLevelsInEngineSniffer) {
if (engineConfiguration->vvtCamSensorUseRise) { // If we care about both edges OR displayLogicLevel is set, log every front exactly as it is
// todo: unify TS composite logger code with console Engine Sniffer addEngineSnifferVvtEvent(index, front == TriggerValue::RISE ? FrontDirection::UP : FrontDirection::DOWN);
// todo: better API to reduce copy/paste?
#if EFI_TOOTH_LOGGER
LogTriggerTooth(SHAFT_SECONDARY_RISING, nowNt);
LogTriggerTooth(SHAFT_SECONDARY_FALLING, nowNt);
#endif /* EFI_TOOTH_LOGGER */
addEngineSnifferVvtEvent(index, FrontDirection::UP);
addEngineSnifferVvtEvent(index, FrontDirection::DOWN);
} else {
#if EFI_TOOTH_LOGGER
LogTriggerTooth(SHAFT_SECONDARY_FALLING, nowNt);
LogTriggerTooth(SHAFT_SECONDARY_RISING, nowNt);
#endif /* EFI_TOOTH_LOGGER */
addEngineSnifferVvtEvent(index, FrontDirection::DOWN); #if EFI_TOOTH_LOGGER
LogTriggerTooth(front == TriggerValue::RISE ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING, nowNt);
#endif /* EFI_TOOTH_LOGGER */
} else {
if (isImportantFront) {
// On the important edge, log a rise+fall pair, and nothing on the real falling edge
addEngineSnifferVvtEvent(index, FrontDirection::UP); addEngineSnifferVvtEvent(index, FrontDirection::UP);
addEngineSnifferVvtEvent(index, FrontDirection::DOWN);
#if EFI_TOOTH_LOGGER
LogTriggerTooth(SHAFT_SECONDARY_RISING, nowNt);
LogTriggerTooth(SHAFT_SECONDARY_FALLING, nowNt);
#endif /* EFI_TOOTH_LOGGER */
} }
} }
} }
@ -266,7 +265,6 @@ void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) {
extern ioportid_t criticalErrorLedPort; extern ioportid_t criticalErrorLedPort;
extern ioportmask_t criticalErrorLedPin; extern ioportmask_t criticalErrorLedPin;
for (int i = 0 ; i < 100 ; i++) { for (int i = 0 ; i < 100 ; i++) {
// turning pin ON and busy-waiting a bit // turning pin ON and busy-waiting a bit
palWritePad(criticalErrorLedPort, criticalErrorLedPin, 1); palWritePad(criticalErrorLedPort, criticalErrorLedPin, 1);
@ -275,27 +273,21 @@ void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) {
palWritePad(criticalErrorLedPort, criticalErrorLedPin, 0); palWritePad(criticalErrorLedPort, criticalErrorLedPin, 0);
#endif // VR_HW_CHECK_MODE #endif // VR_HW_CHECK_MODE
if (!engineConfiguration->displayLogicLevelsInEngineSniffer) { const auto& vvtShape = tc->vvtShape[camIndex];
// todo: migrate injector_pressure_type_e to class enum, maybe merge with FrontDirection?
addEngineSnifferVvtEvent(index, front == TriggerValue::RISE ? FrontDirection::UP : FrontDirection::DOWN);
#if EFI_TOOTH_LOGGER
// todo: we need to start logging different VVT channels differently!!!
trigger_event_e tooth = front == TriggerValue::RISE ? SHAFT_SECONDARY_RISING : SHAFT_SECONDARY_FALLING;
LogTriggerTooth(tooth, nowNt);
#endif /* EFI_TOOTH_LOGGER */
}
bool isImportantFront = (engineConfiguration->vvtCamSensorUseRise ^ (front == TriggerValue::FALL));
bool isVvtWithRealDecoder = vvtWithRealDecoder(engineConfiguration->vvtMode[camIndex]); bool isVvtWithRealDecoder = vvtWithRealDecoder(engineConfiguration->vvtMode[camIndex]);
if (!isVvtWithRealDecoder && !isImportantFront) {
// todo: there should be a way to always use real trigger code for this logic? // Non real decoders only use the rising edge
bool vvtUseOnlyRise = !isVvtWithRealDecoder || vvtShape.useOnlyRisingEdges;
bool isImportantFront = !vvtUseOnlyRise || (front == TriggerValue::RISE);
logVvtFront(vvtUseOnlyRise, isImportantFront, front, nowNt, index);
if (!isImportantFront) {
// This edge is unimportant, ignore it.
return; return;
} }
logFront(isImportantFront, nowNt, index);
// If the main trigger is not synchronized, don't decode VVT yet // If the main trigger is not synchronized, don't decode VVT yet
if (!tc->triggerState.getShaftSynchronized()) { if (!tc->triggerState.getShaftSynchronized()) {
return; return;
@ -306,7 +298,7 @@ void hwHandleVvtCamSignal(TriggerValue front, efitick_t nowNt, int index) {
if (isVvtWithRealDecoder) { if (isVvtWithRealDecoder) {
vvtDecoder.decodeTriggerEvent( vvtDecoder.decodeTriggerEvent(
"vvt", "vvt",
tc->vvtShape[camIndex], vvtShape,
nullptr, nullptr,
tc->vvtTriggerConfiguration[camIndex], tc->vvtTriggerConfiguration[camIndex],
front == TriggerValue::RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt); front == TriggerValue::RISE ? SHAFT_PRIMARY_RISING : SHAFT_PRIMARY_FALLING, nowNt);

View File

@ -110,7 +110,7 @@ TEST(trigger, testCamInput) {
for (int i = 0; i < 600;i++) { for (int i = 0; i < 600;i++) {
eth.moveTimeForwardUs(MS2US(10)); eth.moveTimeForwardUs(MS2US(10));
hwHandleVvtCamSignal(TriggerValue::FALL, getTimeNowNt(), 0); hwHandleVvtCamSignal(TriggerValue::RISE, getTimeNowNt(), 0);
eth.moveTimeForwardUs(MS2US(40)); eth.moveTimeForwardUs(MS2US(40));
eth.firePrimaryTriggerRise(); eth.firePrimaryTriggerRise();
} }
@ -177,5 +177,5 @@ TEST(trigger, testNB2CamInput) {
// actually position based on VVT! // actually position based on VVT!
ASSERT_EQ(totalRevolutionCountBeforeVvtSync + 3, engine->triggerCentral.triggerState.getCrankSynchronizationCounter()); ASSERT_EQ(totalRevolutionCountBeforeVvtSync + 3, engine->triggerCentral.triggerState.getCrankSynchronizationCounter());
EXPECT_EQ(40, waveChart.getSize()); EXPECT_EQ(39, waveChart.getSize());
} }

View File

@ -30,8 +30,8 @@ TEST(realCrankingNB2, normalCranking) {
EXPECT_EQ(3, eth.recentWarnings()->getCount()); EXPECT_EQ(3, eth.recentWarnings()->getCount());
EXPECT_EQ(CUSTOM_OUT_OF_ORDER_COIL, eth.recentWarnings()->get(0).Code); EXPECT_EQ(CUSTOM_OUT_OF_ORDER_COIL, eth.recentWarnings()->get(0).Code);
EXPECT_EQ(CUSTOM_CAM_TOO_MANY_TEETH, eth.recentWarnings()->get(1).Code); EXPECT_EQ(CUSTOM_PRIMARY_NOT_ENOUGH_TEETH, eth.recentWarnings()->get(1).Code);
EXPECT_EQ(CUSTOM_PRIMARY_NOT_ENOUGH_TEETH, eth.recentWarnings()->get(2).Code); EXPECT_EQ(CUSTOM_CAM_TOO_MANY_TEETH, eth.recentWarnings()->get(2).Code);
} }
TEST(realCrankingNB2, crankingMissingInjector) { TEST(realCrankingNB2, crankingMissingInjector) {
@ -52,7 +52,7 @@ TEST(realCrankingNB2, crankingMissingInjector) {
EXPECT_EQ(4, eth.recentWarnings()->getCount()); EXPECT_EQ(4, eth.recentWarnings()->getCount());
EXPECT_EQ(CUSTOM_OUT_OF_ORDER_COIL, eth.recentWarnings()->get(0).Code); EXPECT_EQ(CUSTOM_OUT_OF_ORDER_COIL, eth.recentWarnings()->get(0).Code);
EXPECT_EQ(CUSTOM_CAM_TOO_MANY_TEETH, eth.recentWarnings()->get(1).Code); EXPECT_EQ(CUSTOM_PRIMARY_NOT_ENOUGH_TEETH, eth.recentWarnings()->get(1).Code);
EXPECT_EQ(CUSTOM_PRIMARY_NOT_ENOUGH_TEETH, eth.recentWarnings()->get(2).Code); EXPECT_EQ(CUSTOM_CAM_TOO_MANY_TEETH, eth.recentWarnings()->get(2).Code);
EXPECT_EQ(CUSTOM_PRIMARY_TOO_MANY_TEETH, eth.recentWarnings()->get(3).Code); EXPECT_EQ(CUSTOM_PRIMARY_TOO_MANY_TEETH, eth.recentWarnings()->get(3).Code);
} }