From 8569f028d607af1b82348398c9d08b7d10dfd83f Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Thu, 1 Jul 2021 15:58:28 -0700 Subject: [PATCH] Fix pre sync timestamp copy (#2892) * fix tests * fix pre sync copy * this fixes the issue --- .../controllers/trigger/trigger_decoder.cpp | 23 +++++++++++++++---- .../injection_mode_transition.cpp | 2 +- .../trigger/test_fasterEngineSpinningUp.cpp | 6 ++--- .../trigger/test_real_cranking_miata_NA.cpp | 9 +++----- .../trigger/test_real_cranking_miata_na6.cpp | 6 ++--- 5 files changed, 28 insertions(+), 18 deletions(-) diff --git a/firmware/controllers/trigger/trigger_decoder.cpp b/firmware/controllers/trigger/trigger_decoder.cpp index 80ddb7c032..2e13a9df92 100644 --- a/firmware/controllers/trigger/trigger_decoder.cpp +++ b/firmware/controllers/trigger/trigger_decoder.cpp @@ -203,11 +203,24 @@ int TriggerState::getTotalRevolutionCounter() const { void TriggerStateWithRunningStatistics::movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE) { // here we take timestamps of events which happened prior to synchronization and place them // at appropriate locations - for (int i = 0; i < spinningEventIndex;i++) { - int newIndex = getTriggerSize() - i; - assertIsInBounds(newIndex, timeOfLastEvent, "move timeOfLastEvent"); - timeOfLastEvent[newIndex] = spinningEvents[i]; + auto triggerSize = getTriggerSize(); + + int eventsToCopy = minI(spinningEventIndex, triggerSize); + + size_t firstSrc; + size_t firstDst; + + if (eventsToCopy >= triggerSize) { + // Only copy one trigger length worth of events, filling the whole buffer + firstSrc = spinningEventIndex - triggerSize; + firstDst = 0; + } else { + // There is less than one full cycle, copy to the end of the buffer + firstSrc = 0; + firstDst = triggerSize - spinningEventIndex; } + + memcpy(timeOfLastEvent + firstDst, spinningEvents + firstSrc, eventsToCopy * sizeof(timeOfLastEvent[0])); } float TriggerStateWithRunningStatistics::calculateInstantRpm(TriggerFormDetails *triggerFormDetails, efitick_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { @@ -650,7 +663,7 @@ void TriggerState::decodeTriggerEvent( toothed_previous_time = nowNt; } - if (!isValidIndex(triggerShape) && triggerStateListener) { + if (shaft_is_synchronized && !isValidIndex(triggerShape) && triggerStateListener) { triggerStateListener->OnTriggerInvalidIndex(currentCycle.current_index); return; } diff --git a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp index 11146189e6..e73c3698c2 100644 --- a/unit_tests/tests/ignition_injection/injection_mode_transition.cpp +++ b/unit_tests/tests/ignition_injection/injection_mode_transition.cpp @@ -32,7 +32,7 @@ TEST(fuelControl, transitionIssue1592) { ENGINE(tdcMarkEnabled) = false; setupSimpleTestEngineWithMafAndTT_ONE_trigger(ð, IM_SEQUENTIAL); - EXPECT_CALL(eth.mockAirmass, getAirmass(400)) + EXPECT_CALL(eth.mockAirmass, getAirmass(499)) .WillRepeatedly(Return(AirmassResult{0.1008f, 50.0f})); // This is easiest to trip on a wheel that requires sync diff --git a/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp b/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp index 2b1b693261..42eee22733 100644 --- a/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp +++ b/unit_tests/tests/trigger/test_fasterEngineSpinningUp.cpp @@ -115,7 +115,7 @@ static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int rpm1, int e } TEST(cranking, testFasterEngineSpinningUp60_2) { - doTestFasterEngineSpinningUp60_2(0, 288, 263); - doTestFasterEngineSpinningUp60_2(100, 288, 263); - doTestFasterEngineSpinningUp60_2(1000, 288, 263); + doTestFasterEngineSpinningUp60_2(0, 549, 549); + doTestFasterEngineSpinningUp60_2(100, 549, 549); + doTestFasterEngineSpinningUp60_2(1000, 549, 549); } diff --git a/unit_tests/tests/trigger/test_real_cranking_miata_NA.cpp b/unit_tests/tests/trigger/test_real_cranking_miata_NA.cpp index 299b75052d..59dd447577 100644 --- a/unit_tests/tests/trigger/test_real_cranking_miata_NA.cpp +++ b/unit_tests/tests/trigger/test_real_cranking_miata_NA.cpp @@ -22,10 +22,9 @@ TEST(cranking, realCrankingFromFile) { reader.readLine(ð); } - ASSERT_EQ( 42, GET_RPM())<< reader.lineIndex() << " @ 0"; + ASSERT_EQ( 229, GET_RPM())<< reader.lineIndex() << " @ 0"; ASSERT_EQ( 0, eth.recentWarnings()->getCount())<< "warningCounter#got synch"; - ASSERT_EQ(0, engine->tdcScheduler[1].momentX); reader.readLine(ð); ASSERT_EQ( 213, GET_RPM())<< reader.lineIndex() << " @ 1"; @@ -45,8 +44,6 @@ TEST(cranking, realCrankingFromFile) { reader.processLine(ð); } - // TODO: we should avoid this warning - // See https://github.com/rusefi/rusefi/issues/2889 - ASSERT_EQ(1, eth.recentWarnings()->getCount())<< "warningCounter#realCranking"; - ASSERT_EQ(560, GET_RPM()) << reader.lineIndex(); + ASSERT_EQ(0, eth.recentWarnings()->getCount())<< "warningCounter#realCranking"; + ASSERT_EQ(560, GET_RPM())<< reader.lineIndex(); } diff --git a/unit_tests/tests/trigger/test_real_cranking_miata_na6.cpp b/unit_tests/tests/trigger/test_real_cranking_miata_na6.cpp index a40b68be57..f2ab47ee84 100644 --- a/unit_tests/tests/trigger/test_real_cranking_miata_na6.cpp +++ b/unit_tests/tests/trigger/test_real_cranking_miata_na6.cpp @@ -56,11 +56,11 @@ TEST(cranking, hardcodedRealCranking) { EXPECT_EQ(0, GET_RPM()); /* 14 */ EVENT(/* timestamp*/1.194742, T_PRIMARY, /*value*/true); // first synch & fast spinning RPM - EXPECT_EQ( 31, GET_RPM()); + EXPECT_EQ(227, GET_RPM()); /* 15 */ EVENT(/* timestamp*/1.20417975, T_SECONDARY, /*value*/false); - EXPECT_EQ(36, GET_RPM()); + EXPECT_EQ(233, GET_RPM()); /* 16 */ EVENT(/* timestamp*/1.25380075, T_SECONDARY, /*value*/true); - EXPECT_EQ(53, GET_RPM()); + EXPECT_EQ(234, GET_RPM()); /* 17 */ EVENT(/* timestamp*/1.30114225, T_PRIMARY, /*value*/true); EXPECT_EQ(219, GET_RPM()); /* 18 */ EVENT(/* timestamp*/1.3341915, T_SECONDARY, /*value*/false);