diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index f08260d80f..71ea488bbe 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -458,6 +458,7 @@ void TriggerState::decodeTriggerEvent(trigger_event_e const signal, efitime_t no #endif bool isSynchronizationPoint; + bool wasSynchronized = shaft_is_synchronized; if (triggerShape->isSynchronizationNeeded) { // this is getting a little out of hand, any ideas? @@ -563,23 +564,29 @@ void TriggerState::decodeTriggerEvent(trigger_event_e const signal, efitime_t no if (isSynchronizationPoint) { - /** - * We can check if things are fine by comparing the number of events in a cycle with the expected number of event. - */ - bool isDecodingError = validateEventCounters(PASS_ENGINE_PARAMETER_SIGNATURE); + // We only care about trigger shape once we have synchronized trigger. Anything could happen + // during first revolution and it's fine + if (wasSynchronized) { - enginePins.triggerDecoderErrorPin.setValue(isDecodingError); - if (isDecodingError && !engine->isInitializingTrigger) { - handleTriggerError(PASS_ENGINE_PARAMETER_SIGNATURE); - } + /** + * We can check if things are fine by comparing the number of events in a cycle with the expected number of event. + */ + bool isDecodingError = validateEventCounters(PASS_ENGINE_PARAMETER_SIGNATURE); - errorDetection.add(isDecodingError); + enginePins.triggerDecoderErrorPin.setValue(isDecodingError); - if (isTriggerDecoderError()) { - warning(CUSTOM_OBD_TRG_DECODING, "trigger decoding issue. expected %d/%d/%d got %d/%d/%d", - TRIGGER_SHAPE(expectedEventCount[0]), TRIGGER_SHAPE(expectedEventCount[1]), - TRIGGER_SHAPE(expectedEventCount[2]), currentCycle.eventCount[0], currentCycle.eventCount[1], - currentCycle.eventCount[2]); + if (isDecodingError && !engine->isInitializingTrigger) { + handleTriggerError(PASS_ENGINE_PARAMETER_SIGNATURE); + } + + errorDetection.add(isDecodingError); + + if (isTriggerDecoderError()) { + warning(CUSTOM_OBD_TRG_DECODING, "trigger decoding issue. expected %d/%d/%d got %d/%d/%d", + TRIGGER_SHAPE(expectedEventCount[0]), TRIGGER_SHAPE(expectedEventCount[1]), + TRIGGER_SHAPE(expectedEventCount[2]), currentCycle.eventCount[0], currentCycle.eventCount[1], + currentCycle.eventCount[2]); + } } onShaftSynchronization(nowNt, triggerWheel PASS_ENGINE_PARAMETER_SUFFIX); diff --git a/unit_tests/main.cpp b/unit_tests/main.cpp index 8e5519295d..e241dd0c82 100644 --- a/unit_tests/main.cpp +++ b/unit_tests/main.cpp @@ -46,7 +46,7 @@ GTEST_API_ int main(int argc, char **argv) { // printTriggerDebug = true; // resizeMap(); - printf("Success 20190420\r\n"); + printf("Success 20190510\r\n"); printAllTriggers(); // printConvertedTable(); testing::InitGoogleTest(&argc, argv); diff --git a/unit_tests/tests/test_cam_vtt_input.cpp b/unit_tests/tests/test_cam_vtt_input.cpp index 0ed36f69ed..a24ec7e679 100644 --- a/unit_tests/tests/test_cam_vtt_input.cpp +++ b/unit_tests/tests/test_cam_vtt_input.cpp @@ -6,6 +6,38 @@ */ #include "engine_test_helper.h" +extern WarningCodeState unitTestWarningCodeState; + +TEST(sensors, testNoStartUpWarningsNoSyncronizationTrigger) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + // one tooth does not need synchronization it just counts tooth + eth.setTriggerType(TT_ONE PASS_ENGINE_PARAMETER_SUFFIX); + ASSERT_EQ( 0, GET_RPM()) << "testNoStartUpWarnings RPM"; + + eth.fireTriggerEvents2(/*count*/10, /*duration*/50); + ASSERT_EQ(1200, GET_RPM()) << "testNoStartUpWarnings RPM"; + ASSERT_EQ( 0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testNoStartUpWarningsNoSyncronizationTrigger"; +} + +TEST(sensors, testNoStartUpWarnings) { + WITH_ENGINE_TEST_HELPER(TEST_ENGINE); + eth.setTriggerType(TT_ONE PASS_ENGINE_PARAMETER_SUFFIX); + ASSERT_EQ( 0, GET_RPM()) << "testNoStartUpWarnings RPM"; + + // for this test we need a trigger with isSynchronizationNeeded=true + + eth.fireTriggerEvents2(/*count*/10, /*duration*/50); + ASSERT_EQ(1200, GET_RPM()) << "testNoStartUpWarnings RPM"; + ASSERT_EQ( 0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testNoStartUpWarnings"; + // now let's post invalid shape + eth.fireRise(50); + eth.fireRise(50); + eth.fireRise(50); + eth.fireRise(50); + ASSERT_EQ( 0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testNoStartUpWarnings CUSTOM_SYNC_COUNT_MISMATCH expected"; +// ASSERT_EQ(CUSTOM_SYNC_COUNT_MISMATCH, unitTestWarningCodeState.recentWarnings.get(0)); +} + TEST(sensors, testCamInput) { // setting some weird engine @@ -21,12 +53,19 @@ TEST(sensors, testCamInput) { ASSERT_EQ( 0, GET_RPM()) << "testCamInput RPM"; eth.firePrimaryTriggerRise(); + eth.firePrimaryTriggerFall(); eth.firePrimaryTriggerRise(); + eth.firePrimaryTriggerFall(); eth.firePrimaryTriggerRise(); + eth.firePrimaryTriggerFall(); eth.firePrimaryTriggerRise(); - // error condition since two events happened too quick + eth.firePrimaryTriggerFall(); + // error condition since events happened too quick while time does not move ASSERT_EQ(NOISY_RPM, GET_RPM()) << "testCamInput RPM should be noisy"; + // todo: open question what are these warnings about? + ASSERT_EQ( 2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testCamInput"; + unitTestWarningCodeState.recentWarnings.clear(); eth.fireRise(50); eth.fireRise(50); @@ -34,4 +73,9 @@ TEST(sensors, testCamInput) { eth.fireRise(50); eth.fireRise(50); ASSERT_EQ(1200, GET_RPM()) << "testCamInput RPM"; + ASSERT_EQ(2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testCamInput"; + + for (int i = 0; i < 100;i++) + eth.fireRise(50); + } diff --git a/unit_tests/tests/test_trigger_decoder.cpp b/unit_tests/tests/test_trigger_decoder.cpp index 642b81b51e..516684fe68 100644 --- a/unit_tests/tests/test_trigger_decoder.cpp +++ b/unit_tests/tests/test_trigger_decoder.cpp @@ -933,9 +933,8 @@ void doTestFuelSchedulerBug299smallAndMedium(int startUpDelayMs) { mockMapValue = 0; testMafValue = 0; - ASSERT_EQ( 2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testFuelSchedulerBug299smallAndMedium"; - ASSERT_EQ(CUSTOM_SYNC_COUNT_MISMATCH, unitTestWarningCodeState.recentWarnings.get(0)); - ASSERT_EQ(CUSTOM_OBD_SKIPPED_FUEL, unitTestWarningCodeState.recentWarnings.get(1)); + ASSERT_EQ( 1, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testFuelSchedulerBug299smallAndMedium"; + ASSERT_EQ(CUSTOM_OBD_SKIPPED_FUEL, unitTestWarningCodeState.recentWarnings.get(0)); } static void setInjectionMode(int value DECLARE_ENGINE_PARAMETER_SUFFIX) { @@ -976,7 +975,7 @@ TEST(big, testDifferentInjectionModes) { setInjectionMode((int)IM_SINGLE_POINT PASS_ENGINE_PARAMETER_SUFFIX); engine->periodicFastCallback(PASS_ENGINE_PARAMETER_SIGNATURE); ASSERT_EQ( 40, engine->injectionDuration) << "injection while IM_SINGLE_POINT"; - ASSERT_EQ( 1, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testDifferentInjectionModes"; + ASSERT_EQ( 0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testDifferentInjectionModes"; } TEST(big, testFuelSchedulerBug299smallAndLarge) { @@ -1086,9 +1085,8 @@ TEST(big, testFuelSchedulerBug299smallAndLarge) { eth.moveTimeForwardUs(MS2US(20)); eth.executeActions(); - ASSERT_EQ( 2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testFuelSchedulerBug299smallAndLarge"; - ASSERT_EQ(CUSTOM_SYNC_COUNT_MISMATCH, unitTestWarningCodeState.recentWarnings.get(0)); - ASSERT_EQ(CUSTOM_OBD_SKIPPED_FUEL, unitTestWarningCodeState.recentWarnings.get(1)); + ASSERT_EQ( 1, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#testFuelSchedulerBug299smallAndLarge"; + ASSERT_EQ(CUSTOM_OBD_SKIPPED_FUEL, unitTestWarningCodeState.recentWarnings.get(0)); } TEST(big, testSparkReverseOrderBug319) { @@ -1188,10 +1186,9 @@ TEST(big, testSparkReverseOrderBug319) { eth.fireFall(20); eth.executeActions(); ASSERT_EQ( 0, enginePins.coils[3].outOfOrder) << "out-of-order #8"; - ASSERT_EQ( 3, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#SparkReverseOrderBug319"; - ASSERT_EQ(CUSTOM_SYNC_COUNT_MISMATCH, unitTestWarningCodeState.recentWarnings.get(0)); - ASSERT_EQ(CUSTOM_DWELL_TOO_LONG, unitTestWarningCodeState.recentWarnings.get(1)); - ASSERT_EQ(CUSTOM_OUT_OF_ORDER_COIL, unitTestWarningCodeState.recentWarnings.get(2)); + ASSERT_EQ( 2, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#SparkReverseOrderBug319"; + ASSERT_EQ(CUSTOM_DWELL_TOO_LONG, unitTestWarningCodeState.recentWarnings.get(0)) << "warning @0"; + ASSERT_EQ(CUSTOM_OUT_OF_ORDER_COIL, unitTestWarningCodeState.recentWarnings.get(1)); } TEST(big, testMissedSpark299) { @@ -1274,6 +1271,5 @@ TEST(big, testMissedSpark299) { eth.fireFall(20); eth.executeActions(); - ASSERT_EQ( 1, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#1"; - assertEqualsM("warningCounter code", CUSTOM_SYNC_COUNT_MISMATCH, unitTestWarningCodeState.recentWarnings.get(0)); + ASSERT_EQ( 0, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#1"; }