From a26ed3eb5a2857413f95339d176b3a13200b6420 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sun, 6 Dec 2020 15:01:45 -0600 Subject: [PATCH] switch rpm calculator to use timer class (#2005) * add api * rpm calculator * fix fsio * fix float * remove wrong comment * fix timer * clang didn't like this Co-authored-by: Matthew Kennedy --- firmware/controllers/core/fsio_impl.cpp | 2 +- .../engine_cycle/rpm_calculator.cpp | 33 +++++++++---------- .../controllers/engine_cycle/rpm_calculator.h | 4 ++- firmware/controllers/math/engine_math.h | 5 --- .../controllers/trigger/trigger_central.h | 4 +-- firmware/util/timer.cpp | 16 +++++++-- firmware/util/timer.h | 8 ++++- unit_tests/tests/sensor/test_sensor_init.cpp | 2 +- 8 files changed, 44 insertions(+), 30 deletions(-) diff --git a/firmware/controllers/core/fsio_impl.cpp b/firmware/controllers/core/fsio_impl.cpp index 6b9b193ac7..35e8146dbc 100644 --- a/firmware/controllers/core/fsio_impl.cpp +++ b/firmware/controllers/core/fsio_impl.cpp @@ -141,7 +141,7 @@ FsioValue getEngineValue(le_action_e action DECLARE_ENGINE_PARAMETER_SUFFIX) { return engine->triggerCentral.getVVTPosition(); #endif case LE_METHOD_TIME_SINCE_TRIGGER_EVENT: - return engine->triggerCentral.getTimeSinceTriggerEvent(); + return engine->triggerCentral.getTimeSinceTriggerEvent(getTimeNowNt()); case LE_METHOD_TIME_SINCE_BOOT: #if EFI_MAIN_RELAY_CONTROL // in main relay control mode, we return the number of seconds since the ignition is turned on diff --git a/firmware/controllers/engine_cycle/rpm_calculator.cpp b/firmware/controllers/engine_cycle/rpm_calculator.cpp index 5c4a6c3c37..f32670681e 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.cpp +++ b/firmware/controllers/engine_cycle/rpm_calculator.cpp @@ -94,9 +94,6 @@ RpmCalculator::RpmCalculator() : #endif /* EFI_PROD_CODE */ // todo: reuse assignRpmValue() method which needs PASS_ENGINE_PARAMETER_SUFFIX // which we cannot provide inside this parameter-less constructor. need a solution for this minor mess - - // we need this initial to have not_running at first invocation - lastRpmEventTimeNt = (efitick_t) DEEP_IN_THE_PAST_SECONDS * NT_PER_SECOND; } /** @@ -118,11 +115,15 @@ bool RpmCalculator::checkIfSpinning(efitick_t nowNt) const { * note that the result of this subtraction could be negative, that would happen if * we have a trigger event between the time we've invoked 'getTimeNow' and here */ - bool noRpmEventsForTooLong = nowNt - lastRpmEventTimeNt >= NT_PER_SECOND * NO_RPM_EVENTS_TIMEOUT_SECS; // Anything below 60 rpm is not running + + // Anything below 60 rpm is not running + bool noRpmEventsForTooLong = lastTdcTimer.getElapsedSeconds(nowNt) > NO_RPM_EVENTS_TIMEOUT_SECS; + /** * Also check if there were no trigger events */ - bool noTriggerEventsForTooLong = nowNt - engine->triggerCentral.triggerState.previousShaftEventTimeNt >= NT_PER_SECOND; + bool noTriggerEventsForTooLong = engine->triggerCentral.getTimeSinceTriggerEvent(nowNt) >= 1; + if (noRpmEventsForTooLong || noTriggerEventsForTooLong) { return false; } @@ -249,8 +250,9 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType, if (index == 0) { bool hadRpmRecently = rpmState->checkIfSpinning(nowNt); + float periodSeconds = engine->rpmCalculator.lastTdcTimer.getElapsedSecondsAndReset(nowNt); + if (hadRpmRecently) { - efitick_t diffNt = nowNt - rpmState->lastRpmEventTimeNt; /** * Four stroke cycle is two crankshaft revolutions * @@ -258,16 +260,16 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType, * and each revolution of crankshaft consists of two engine cycles revolutions * */ - if (diffNt == 0) { + if (periodSeconds == 0) { rpmState->setRpmValue(NOISY_RPM); } else { int mult = (int)getEngineCycle(engine->getOperationMode(PASS_ENGINE_PARAMETER_SIGNATURE)) / 360; - float rpm = 60.0 * NT_PER_SECOND * mult / diffNt; + float rpm = 60 * mult / periodSeconds; rpmState->setRpmValue(rpm > UNREALISTIC_RPM ? NOISY_RPM : rpm); } } + rpmState->onNewEngineCycle(); - rpmState->lastRpmEventTimeNt = nowNt; engine->isRpmHardLimit = GET_RPM() > engine->getRpmHardLimit(PASS_ENGINE_PARAMETER_SIGNATURE); } @@ -350,16 +352,13 @@ void tdcMarkCallback( * @return Current crankshaft angle, 0 to 720 for four-stroke */ float getCrankshaftAngleNt(efitick_t timeNt DECLARE_ENGINE_PARAMETER_SUFFIX) { - efitick_t timeSinceZeroAngleNt = timeNt - - engine->rpmCalculator.lastRpmEventTimeNt; + float timeSinceZeroAngle = engine->rpmCalculator.lastTdcTimer.getElapsedSeconds(timeNt); - /** - * even if we use 'getOneDegreeTimeUs' macros here, it looks like the - * compiler is not smart enough to figure out that "A / ( B / C)" could be optimized into - * "A * C / B" in order to replace a slower division with a faster multiplication. - */ int rpm = GET_RPM(); - return rpm == 0 ? NAN : timeSinceZeroAngleNt / getOneDegreeTimeNt(rpm); + + float oneDegreeSeconds = (60.0f / 360) / rpm; + + return rpm == 0 ? NAN : timeSinceZeroAngle / oneDegreeSeconds; } void initRpmCalculator(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX) { diff --git a/firmware/controllers/engine_cycle/rpm_calculator.h b/firmware/controllers/engine_cycle/rpm_calculator.h index 2552788357..65702be588 100644 --- a/firmware/controllers/engine_cycle/rpm_calculator.h +++ b/firmware/controllers/engine_cycle/rpm_calculator.h @@ -11,6 +11,7 @@ #include "globalaccess.h" #include "scheduler.h" #include "stored_value_sensor.h" +#include "timer.h" // we use this value in case of noise on trigger input lines #define NOISY_RPM -1 @@ -113,7 +114,8 @@ public: * NaN while engine is not spinning */ volatile floatus_t oneDegreeUs = NAN; - volatile efitick_t lastRpmEventTimeNt = 0; + + Timer lastTdcTimer; protected: // Print sensor info - current RPM state diff --git a/firmware/controllers/math/engine_math.h b/firmware/controllers/math/engine_math.h index 625986bddd..262d293b9b 100644 --- a/firmware/controllers/math/engine_math.h +++ b/firmware/controllers/math/engine_math.h @@ -34,11 +34,6 @@ void setFlatInjectorLag(float value DECLARE_CONFIG_PARAMETER_SUFFIX); */ #define getOneDegreeTimeUs(rpm) (1000000.0f * 60 / 360 / (rpm)) -/** - * @return float, time needed to rotate crankshaft by one degree, in native clicks. - */ -#define getOneDegreeTimeNt(rpm) (US2NT(1000000) * 60.0f / 360 / (rpm)) - floatms_t getCrankshaftRevolutionTimeMs(int rpm); floatms_t getEngineCycleDuration(int rpm DECLARE_ENGINE_PARAMETER_SUFFIX); diff --git a/firmware/controllers/trigger/trigger_central.h b/firmware/controllers/trigger/trigger_central.h index 20d2107a6e..1cded0e064 100644 --- a/firmware/controllers/trigger/trigger_central.h +++ b/firmware/controllers/trigger/trigger_central.h @@ -44,8 +44,8 @@ public: void resetCounters(); void validateCamVvtCounters(); - float getTimeSinceTriggerEvent() const { - return m_lastEventTimer.getElapsedSeconds(); + float getTimeSinceTriggerEvent(efitick_t nowNt) const { + return m_lastEventTimer.getElapsedSeconds(nowNt); } TriggerNoiseFilter noiseFilter; diff --git a/firmware/util/timer.cpp b/firmware/util/timer.cpp index 93f6844592..01db33eabb 100644 --- a/firmware/util/timer.cpp +++ b/firmware/util/timer.cpp @@ -31,7 +31,11 @@ bool Timer::hasElapsedUs(float microseconds) const { } float Timer::getElapsedSeconds() const { - auto delta = getTimeNowNt() - m_lastReset; + return getElapsedSeconds(getTimeNowNt()); +} + +float Timer::getElapsedSeconds(efitick_t nowNt) const { + auto delta = nowNt - m_lastReset; if (delta > UINT32_MAX - 1) { delta = UINT32_MAX - 1; @@ -39,5 +43,13 @@ float Timer::getElapsedSeconds() const { auto delta32 = (uint32_t)delta; - return NT2US(delta32); + return 1e-6 * NT2US(delta32); +} + +float Timer::getElapsedSecondsAndReset(efitick_t nowNt) { + float result = getElapsedSeconds(nowNt); + + reset(nowNt); + + return result; } diff --git a/firmware/util/timer.h b/firmware/util/timer.h index 719f71466d..2c08639de1 100644 --- a/firmware/util/timer.h +++ b/firmware/util/timer.h @@ -17,7 +17,13 @@ public: // If the elapsed time is longer than 2^32 timer tick counts, // then a time period representing 2^32 counts will be returned. float getElapsedSeconds() const; + float getElapsedSeconds(efitick_t nowNt) const; + + // Perform an atomic update event based on the passed timestamp, + // returning the delta between the last reset and the provided timestamp + float getElapsedSecondsAndReset(efitick_t nowNt); private: - efitick_t m_lastReset = INT64_MIN; + // Use not-quite-minimum value to avoid overflow + efitick_t m_lastReset = INT64_MIN / 8; }; diff --git a/unit_tests/tests/sensor/test_sensor_init.cpp b/unit_tests/tests/sensor/test_sensor_init.cpp index dc23dd471f..01d01a56ec 100644 --- a/unit_tests/tests/sensor/test_sensor_init.cpp +++ b/unit_tests/tests/sensor/test_sensor_init.cpp @@ -82,7 +82,7 @@ TEST(SensorInit, TpsValuesTooClose) { // With no pin, it should be ok that they are the same // Should succeed, -0.51 volts apart - CONFIG(tps1_1AdcChannel) = ADC_CHANNEL_NONE; + CONFIG(tps1_1AdcChannel) = EFI_ADC_NONE; CONFIG(tpsMin) = 200; // 1.00 volt CONFIG(tpsMax) = 200; // 1.00 volts EXPECT_NO_FATAL_ERROR(initTps(PASS_CONFIG_PARAMETER_SIGNATURE));