Knock retard (#3396)

* output gauge

* knock controller

* don't need that

* inject engine ref

* test knock

Co-authored-by: Matthew Kennedy <makenne@microsoft.com>

firmware/console/binary/tunerstudio_outputs.h

@@ -107,7 +107,8 @@ struct TunerStudioOutputChannels {
 	scaled_pressure baroPressure; // 32
 
 	scaled_lambda lambda; // 34
-	uint16_t unused36; // 36
+	scaled_channel<uint8_t, 10> knockRetard; // 36
+	uint8_t unused37;
 
 	// misc sensors
 	scaled_voltage vBatt; // 38

firmware/console/status_loop.cpp

@@ -634,6 +634,8 @@ static void updateIgnition(int rpm) {
 	tsOutputChannels.sparkDwell = ENGINE(engineState.sparkDwell);
 
 	tsOutputChannels.coilDutyCycle = getCoilDutyCycle(rpm PASS_ENGINE_PARAMETER_SUFFIX);
+
+	tsOutputChannels.knockRetard = ENGINE(knockController).getKnockRetard();
 }
 
 static void updateFlags() {

firmware/controllers/algo/advance_map.cpp

@@ -101,9 +101,7 @@ angle_t getAdvanceCorrections(int rpm DECLARE_ENGINE_PARAMETER_SUFFIX) {
 
 	return iatCorrection
 		+ engine->engineState.cltTimingCorrection
-		+ pidTimingCorrection
-		// todo: uncomment once we get usable knock   - engine->knockCount
-		;
+		+ pidTimingCorrection;
 }
 
 /**

firmware/controllers/algo/engine.cpp

@@ -465,6 +465,7 @@ void Engine::injectEngineReferences() {
 		INJECT_ENGINE_REFERENCE(&vvtTriggerConfiguration[camIndex]);
 	}
 	INJECT_ENGINE_REFERENCE(&limpManager);
+	INJECT_ENGINE_REFERENCE(&knockController);
 
 	primaryTriggerConfiguration.update();
 	for (int camIndex = 0;camIndex < CAMS_PER_BANK;camIndex++) {
@@ -634,6 +635,8 @@ void Engine::periodicFastCallback(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
 
 	engineState.periodicFastCallback(PASS_ENGINE_PARAMETER_SIGNATURE);
 
+	knockController.periodicFastCallback();
+
 	tachSignalCallback(PASS_ENGINE_PARAMETER_SIGNATURE);
 }
 

firmware/controllers/algo/engine.h

@@ -21,6 +21,7 @@
 #include "limp_manager.h"
 #include "pin_repository.h"
 #include "ac_control.h"
+#include "knock_logic.h"
 #include "idle_state_generated.h"
 
 #if EFI_SIGNAL_EXECUTOR_ONE_TIMER
@@ -380,8 +381,7 @@ public:
 
 	void onSparkFireKnockSense(uint8_t cylinderIndex, efitick_t nowNt);
 
-	// onKnockSenseCompleted is the callback from the knock sense driver to report a sensed knock level
-	bool onKnockSenseCompleted(uint8_t cylinderIndex, float levelDbv, efitick_t lastKnockTime);
+	KnockController knockController;
 
 	AirmassModelBase* mockAirmassModel = nullptr;
 

firmware/controllers/engine_cycle/knock_logic.cpp

@@ -8,7 +8,6 @@
 #include "pch.h"
 #include "knock_logic.h"
 #include "os_access.h"
-#include "peak_detect.h"
 
 #include "hip9011.h"
 
@@ -46,11 +45,7 @@ int getCylinderKnockBank(uint8_t cylinderIndex) {
 	}
 }
 
-using PD = PeakDetect<float, MS2NT(100)>;
-static PD peakDetectors[12];
-static PD allCylinderPeakDetector;
-
-bool Engine::onKnockSenseCompleted(uint8_t cylinderIndex, float dbv, efitick_t lastKnockTime) {
+bool KnockController::onKnockSenseCompleted(uint8_t cylinderIndex, float dbv, efitick_t lastKnockTime) {
 	bool isKnock = dbv > ENGINE(engineState).knockThreshold;
 
 #if EFI_TUNER_STUDIO
@@ -68,10 +63,49 @@ bool Engine::onKnockSenseCompleted(uint8_t cylinderIndex, float dbv, efitick_t l
 #endif // EFI_TUNER_STUDIO
 
 	// TODO: retard timing, then put it back!
+	if (isKnock) {
+		auto baseTiming = ENGINE(engineState).timingAdvance;
+
+		// TODO: 20 configurable? Better explanation why 20?
+		auto distToMinimum = baseTiming - (-20);
+
+		// 0.1% per unit -> multiply by 0.001
+		auto retardFraction = CONFIG(knockRetardAggression) * 0.001f;
+		auto retardAmount = distToMinimum * retardFraction;
+
+		{
+			// Adjust knock retard under lock
+			chibios_rt::CriticalSectionLocker csl;
+			auto newRetard = m_knockRetard + retardAmount;
+			m_knockRetard = clampF(0, newRetard, CONFIG(knockRetardMaximum));
+		}
+	}
 
 	return isKnock;
 }
 
+float KnockController::getKnockRetard() const {
+	return m_knockRetard;
+}
+
+void KnockController::periodicFastCallback() {
+	constexpr auto callbackPeriodSeconds = FAST_CALLBACK_PERIOD_MS / 1000.0f;
+
+	// stored in units of 0.1 deg/sec
+	auto applyRate = CONFIG(knockRetardReapplyRate) * 0.1f;
+	auto applyAmount = applyRate * callbackPeriodSeconds;
+
+	{
+		// Adjust knock retard under lock
+		chibios_rt::CriticalSectionLocker csl;
+
+		float newRetard = m_knockRetard - applyAmount;
+
+		// don't allow retard to go negative
+		m_knockRetard = maxF(0, newRetard);
+	}
+}
+
 // This callback is to be implemented by the knock sense driver
 __attribute__((weak)) void onStartKnockSampling(uint8_t cylinderIndex, float samplingTimeSeconds, uint8_t channelIdx) {
 	UNUSED(cylinderIndex);

firmware/controllers/engine_cycle/knock_logic.h

@@ -7,4 +7,27 @@
 
 #pragma once
 
+#include "peak_detect.h"
+
 int getCylinderKnockBank(uint8_t cylinderIndex);
+
+class KnockController {
+public:
+	DECLARE_ENGINE_PTR;
+
+	// onKnockSenseCompleted is the callback from the knock sense driver to report a sensed knock level
+	bool onKnockSenseCompleted(uint8_t cylinderIndex, float dbv, efitick_t lastKnockTime);
+	void periodicFastCallback();
+
+	float getKnockRetard() const;
+
+private:
+	// Degrees retarded: larger number = more retard
+	float m_knockRetard = 0;
+
+	using PD = PeakDetect<float, MS2NT(100)>;
+	PD peakDetectors[12];
+	PD allCylinderPeakDetector;
+
+
+};

firmware/controllers/engine_cycle/spark_logic.cpp

@@ -85,7 +85,7 @@ static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_
 	assertAngleRange(ignitionPositionWithinEngineCycle, "aPWEC", CUSTOM_ERR_6566);
 	// this correction is usually zero (not used)
 	float perCylinderCorrection = CONFIG(timing_offset_cylinder[event->cylinderIndex]);
-	const angle_t sparkAngle = -ENGINE(engineState.timingAdvance) + ignitionPositionWithinEngineCycle + perCylinderCorrection;
+	const angle_t sparkAngle = -ENGINE(engineState.timingAdvance) + ENGINE(knockController).getKnockRetard() + ignitionPositionWithinEngineCycle + perCylinderCorrection;
 	efiAssertVoid(CUSTOM_SPARK_ANGLE_9, !cisnan(sparkAngle), "findAngle#9");
 
 	efiAssertVoid(CUSTOM_SPARK_ANGLE_1, !cisnan(sparkAngle), "sparkAngle#1");

firmware/controllers/sensors/software_knock.cpp

@@ -201,7 +201,7 @@ void processLastKnockEvent() {
 	// clamp to reasonable range
 	db = clampF(-100, db, 100);
 
-	engine->onKnockSenseCompleted(currentCylinderIndex, db, lastKnockTime);
+	engine->knockController.onKnockSenseCompleted(currentCylinderIndex, db, lastKnockTime);
 }
 
 void KnockThread::ThreadTask() {

firmware/hw_layer/sensors/hip9011.cpp

@@ -501,7 +501,7 @@ static msg_t hipThread(void *arg) {
 			/* Check for correct cylinder/input */
 			if (correctCylinder) {
 				// TODO: convert knock level to dBv
-				engine->onKnockSenseCompleted(instance.cylinderNumber, knockVolts, instance.knockSampleTimestamp);
+				engine->knockController.onKnockSenseCompleted(instance.cylinderNumber, knockVolts, instance.knockSampleTimestamp);
 
 				#if EFI_HIP_9011_DEBUG
 					/* debug */

firmware/tunerstudio/rusefi.input

@@ -301,6 +301,7 @@ enable2ndByteCanID = false
 ; Knock
       knockLevel      = scalar,  F32,    108,  "Volts",      1,         0
 	  knockCount      = scalar,  U16,    306,  "count",      1,         0
+	  knockRetard     = scalar,  U08,    36,   "deg",      0.1,         0
 
 ; Mode, firmware, protocol, run time
 ; TS requires 'seconds' name since it has special internal meaning
@@ -1145,6 +1146,7 @@ gaugeCategory = Knock
    knock10Gauge = knock10, "Knock Cyl 10", "dBv", -60, 10, -60, -60, 10, 10, 0, 0
    knock11Gauge = knock11, "Knock Cyl 11", "dBv", -60, 10, -60, -60, 10, 10, 0, 0
    knock12Gauge = knock12, "Knock Cyl 12", "dBv", -60, 10, -60, -60, 10, 10, 0, 0
+   knockRetardGauge = knockRetard, "Knock Retard", "deg", 0, 10, 0, 0, 10, 10, 0, 1, 0
 
 gaugeCategory = DynoView
 	accelGauge = VssAcceleration, "Vehicle acceleration",    "m/s2",       -10,   10,      -6,     -4,      4,    6,   2,   2

unit_tests/tests/test_knock.cpp

@@ -0,0 +1,82 @@
+#include "pch.h"
+
+#include "knock_logic.h"
+
+TEST(Knock, Retards) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	// Knock threshold of 20dBv
+	ENGINE(engineState).knockThreshold = 20;
+	// Aggression of 10%
+	CONFIG(knockRetardAggression) = 100;
+	// Maximum 8 degrees retarded
+	CONFIG(knockRetardMaximum) = 8;
+
+	KnockController dut;
+	INJECT_ENGINE_REFERENCE(&dut);
+
+	// No retard unless we knock
+	ASSERT_FLOAT_EQ(dut.getKnockRetard(), 0);
+
+	// Send some weak knocks, should yield no response
+	for (size_t i = 0; i < 10; i++) {
+		dut.onKnockSenseCompleted(0, 10, 0);
+	}
+
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 0);
+
+	// Send a strong knock!
+	dut.onKnockSenseCompleted(0, 30, 0);
+
+	// Should retard 10% of the distance between current timing and "maximum"
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 2);
+
+	// Send tons of strong knocks, make sure we don't go over the configured limit
+	for (size_t i = 0; i < 100; i++) {
+		dut.onKnockSenseCompleted(0, 30, 0);
+	}
+
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 8);
+}
+
+TEST(Knock, Reapply) {
+	WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
+
+	KnockController dut;
+	INJECT_ENGINE_REFERENCE(&dut);
+
+	// Knock threshold of 20dBv
+	ENGINE(engineState).knockThreshold = 20;
+	// Aggression of 10%
+	CONFIG(knockRetardAggression) = 100;
+	// Maximum 8 degrees retarded
+	CONFIG(knockRetardMaximum) = 8;
+	// Apply 1 degree/second
+	CONFIG(knockRetardReapplyRate) = 10;
+
+	// Send a strong knock!
+	dut.onKnockSenseCompleted(0, 30, 0);
+
+	// Should retard 10% of the distance between current timing and "maximum"
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 2);
+
+	constexpr auto fastPeriodSec = FAST_CALLBACK_PERIOD_MS / 1000.0f;
+
+	// call the fast callback, should reapply 1 degree * callback period
+	dut.periodicFastCallback();
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 2 - 1.0f * fastPeriodSec);
+
+	// 10 updates total
+	for (size_t i = 0; i < 9; i++) {
+		dut.periodicFastCallback();
+	}
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 2 - 10 * 1.0f * fastPeriodSec);
+
+	// Spend a long time without knock
+	for (size_t i = 0; i < 1000; i++) {
+		dut.periodicFastCallback();
+	}
+
+	// Should have no knock retard
+	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 0);
+}

unit_tests/tests/test_util.cpp

@@ -20,7 +20,6 @@
 #include "lcd_menu_tree.h"
 #include "crc.h"
 #include "fl_stack.h"
-#include "peak_detect.h"
 
 TEST(util, negativeZero) {
 	ASSERT_TRUE(IS_NEGATIVE_ZERO(-0.0));

unit_tests/tests/tests.mk

@@ -61,6 +61,7 @@ TESTS_SRC_CPP = \
 	tests/test_tacho.cpp \
 	tests/test_gpiochip.cpp \
 	tests/test_deadband.cpp \
+	tests/test_knock.cpp \
 	tests/sensor/basic_sensor.cpp \
 	tests/sensor/func_sensor.cpp \
 	tests/sensor/function_pointer_sensor.cpp \