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 <makenne@microsoft.com>

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

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
+	float timeSinceZeroAngle = engine->rpmCalculator.lastTdcTimer.getElapsedSeconds(timeNt);
-			- engine->rpmCalculator.lastRpmEventTimeNt;
 
-	/**
-	 * 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) {

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

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);
 

firmware/controllers/trigger/trigger_central.h

@@ -44,8 +44,8 @@ public:
 	void resetCounters();
 	void validateCamVvtCounters();
 
-	float getTimeSinceTriggerEvent() const {
+	float getTimeSinceTriggerEvent(efitick_t nowNt) const {
-		return m_lastEventTimer.getElapsedSeconds();
+		return m_lastEventTimer.getElapsedSeconds(nowNt);
 	}
 
 	TriggerNoiseFilter noiseFilter;

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;
 }

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;
 };

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));