From ffae0cd33552d13a82f3bcbfaa09e450c9a100a9 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sat, 5 Dec 2020 23:11:57 -0600 Subject: [PATCH] round rpm instead of truncating (#2023) * round rpm instead of truncating * efiround is expensive * Revert "efiround is expensive" This reverts commit e5690f89e1b1988aacf5ced1f024d576465a7cd6. * round is better than rintf * testing * it works now?! * comment Co-authored-by: Matthew Kennedy --- firmware/controllers/engine_cycle/rpm_calculator.cpp | 10 ++++++---- firmware/util/efilib.cpp | 2 +- .../test_dwell_corner_case_issue_796.cpp | 6 +++--- .../test_miata_na6_real_cranking.cpp | 2 +- unit_tests/tests/trigger/test_symmetrical_crank.cpp | 6 +++--- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index b7677b012d..5c4a6c3c37 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/firmware/controllers/engine_cycle/rpm_calculator.cpp @@ -132,8 +132,10 @@ bool RpmCalculator::checkIfSpinning(efitick_t nowNt) const { void RpmCalculator::assignRpmValue(float floatRpmValue) { previousRpmValue = rpmValue; - // we still persist integer RPM! todo: figure out the next steps - rpmValue = floatRpmValue; + + // Round to the nearest integer RPM - some other parts of the ECU expect integer, so that's what we hand out. + // TODO: RPM should eventually switch to floating point across the ECU + rpmValue = efiRound(floatRpmValue, 1); if (rpmValue <= 0) { oneDegreeUs = NAN; @@ -286,10 +288,10 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType, // Replace 'normal' RPM with instant RPM for the initial spin-up period engine->triggerCentral.triggerState.movePreSynchTimestamps(PASS_ENGINE_PARAMETER_SIGNATURE); int prevIndex; - int instantRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&engine->triggerCentral.triggerFormDetails, + float instantRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&engine->triggerCentral.triggerFormDetails, &prevIndex, nowNt PASS_ENGINE_PARAMETER_SUFFIX); // validate instant RPM - we shouldn't skip the cranking state - instantRpm = minI(instantRpm, CONFIG(cranking.rpm) - 1); + instantRpm = minF(instantRpm, CONFIG(cranking.rpm) - 1); rpmState->assignRpmValue(instantRpm); #if 0 scheduleMsg(logger, "** RPM: idx=%d sig=%d iRPM=%d", index, ckpSignalType, instantRpm); diff --git a/firmware/util/efilib.cpp b/firmware/util/efilib.cpp index dd5f27ee03..91fd4b1cf2 100644 --- a/firmware/util/efilib.cpp +++ b/firmware/util/efilib.cpp @@ -35,7 +35,7 @@ float efiFloor(float value, float precision) { */ float efiRound(float value, float precision) { efiAssert(CUSTOM_ERR_ASSERT, precision != 0, "zero precision", NAN); - float a = rintf (value / precision); + float a = round(value / precision); return fixNegativeZero(a * precision); } diff --git a/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp b/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp index 089b81b4f6..f8d43d06a0 100644 --- a/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp +++ b/unit_tests/tests/ignition_injection/test_dwell_corner_case_issue_796.cpp @@ -18,12 +18,12 @@ TEST(scheduler, dwellIssue796) { ASSERT_EQ(CRANKING, engine->rpmCalculator.getState()); // due to isFasterEngineSpinUp=true, we should have already detected RPM! ASSERT_EQ( 100, GET_RPM()) << "spinning-RPM#1"; - ASSERT_EQ(300000, ENGINE(rpmCalculator.oneDegreeUs) * 180); + ASSERT_NEAR(300000.0f, ENGINE(rpmCalculator.oneDegreeUs) * 180, 1); // with just a bit much time between events integer RPM goes down one full percent eth.smartFireRise(601); eth.smartFireFall(600); - ASSERT_NEAR( 99, GET_RPM(), EPS3D) << "spinning-RPM#2"; + ASSERT_NEAR( 100, GET_RPM(), EPS3D) << "spinning-RPM#2"; // while integer RPM value is 1% away from rpm=100, below oneDegreeUs is much closer to RPM=100 value - ASSERT_EQ(300250, (int)(ENGINE(rpmCalculator.oneDegreeUs) * 180)); + ASSERT_NEAR(300250, (int)(ENGINE(rpmCalculator.oneDegreeUs) * 180), 1); } diff --git a/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp b/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp index 070f430394..1272e3c3f3 100644 --- a/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp +++ b/unit_tests/tests/ignition_injection/test_miata_na6_real_cranking.cpp @@ -69,7 +69,7 @@ TEST(miataNA6, realCranking) { // second synch /* 22 */ EVENT(/* timestamp*/1.45352675, /*index*/1, /*value*/true); ASSERT_EQ( 1, unitTestWarningCodeState.recentWarnings.getCount()) << "warningCounter#realCranking"; - ASSERT_EQ( 463, GET_RPM()) << "RPM at the 22"; + ASSERT_EQ( 464, GET_RPM()) << "RPM at the 22"; /* 23 */ EVENT(/* timestamp*/1.46291525, /*index*/0, /*value*/false); /* 25 */ EVENT(/* timestamp*/1.49939025, /*index*/1, /*value*/false); /* 27 */ EVENT(/* timestamp*/1.511785, /*index*/0, /*value*/true); diff --git a/unit_tests/tests/trigger/test_symmetrical_crank.cpp b/unit_tests/tests/trigger/test_symmetrical_crank.cpp index e0f726929f..0da58a95b1 100644 --- a/unit_tests/tests/trigger/test_symmetrical_crank.cpp +++ b/unit_tests/tests/trigger/test_symmetrical_crank.cpp @@ -49,11 +49,11 @@ TEST(engine, testSymmetricalCrank) { mult = 0.1; postFourEvents(ð, mult); - ASSERT_EQ( 1041, GET_RPM()) << "RPM#11"; + ASSERT_EQ( 1042, GET_RPM()) << "RPM#11"; postFourEvents(ð, mult); - ASSERT_EQ( 1041, GET_RPM()) << "RPM#11"; + ASSERT_EQ( 1042, GET_RPM()) << "RPM#11"; postFourEvents(ð, mult); - ASSERT_EQ( 1041, GET_RPM()) << "RPM#11"; + ASSERT_EQ( 1042, GET_RPM()) << "RPM#11"; }