knock threshold table fix (#4077)

* fix knock threshold table

* defaults

* knock controller is engine module

* testable

* test builds

* knock tests work

* s

* format

* everyone loves changelogs

firmware/CHANGELOG.md

@@ -27,6 +27,7 @@ All notable user-facing or behavior-altering changes will be documented in this
 
 ### Fixed
  - An attempt to make 'Trigger' dialog a bit less confusing #4021
+ - Fixed knock threshold table, improved knock sensing status gauges
 
 ### Added
  - Mitsubishi 36-2-1-1 trigger wheel

firmware/config/engines/dodge_neon.cpp

@@ -256,18 +256,6 @@ void setDodgeNeonNGCEngineConfiguration() {
 
 	engineConfiguration->hip9011Gain = 0.3;
 
-	float t = 0.5;
-
-	engineConfiguration->knockNoise[0] = 2.1 + t; // 800
-	engineConfiguration->knockNoise[1] = 2.1 + t; // 1700
-	engineConfiguration->knockNoise[2] = 2.2 + t; // 2600
-	engineConfiguration->knockNoise[3] = 2.2 + t; // 3400
-	engineConfiguration->knockNoise[4] = 2.3 + t; // 4300
-	engineConfiguration->knockNoise[5] = 2.7 + t; // 5200
-	engineConfiguration->knockNoise[6] = 3.1 + t; // 6100
-	engineConfiguration->knockNoise[7] = 3.3 + t; // 7000
-
-
 	engineConfiguration->cylinderBore = 87.5;
 
 	engineConfiguration->clutchDownPin = GPIOC_12;

firmware/console/status_loop.cpp

@@ -666,7 +666,8 @@ static void updateIgnition(int rpm) {
 
 	engine->outputChannels.coilDutyCycle = getCoilDutyCycle(rpm);
 
-	engine->outputChannels.knockRetard = engine->knockController.getKnockRetard();
+	engine->outputChannels.knockCount = engine->module<KnockController>()->getKnockCount();
+	engine->outputChannels.knockRetard = engine->module<KnockController>()->getKnockRetard();
 }
 
 static void updateFlags() {

firmware/controllers/algo/engine.cpp

@@ -642,8 +642,6 @@ void Engine::periodicFastCallback() {
 
 	engineState.periodicFastCallback();
 
-	knockController.periodicFastCallback();
-
 	tachSignalCallback();
 
 	engine->engineModules.apply_all([](auto & m) { m.onFastCallback(); });

firmware/controllers/algo/engine.h

@@ -191,6 +191,7 @@ public:
 #if EFI_VEHICLE_SPEED
 		GearDetector,
 #endif // EFI_VEHICLE_SPEED
+		KnockController,
 		EngineModule // dummy placeholder so the previous entries can all have commas
 		> engineModules;
 
@@ -442,8 +443,6 @@ public:
 
 	void onSparkFireKnockSense(uint8_t cylinderIndex, efitick_t nowNt);
 
-	KnockController knockController;
-
 	AirmassModelBase* mockAirmassModel = nullptr;
 
 	LimpManager limpManager;

firmware/controllers/algo/engine2.cpp

@@ -125,9 +125,6 @@ void EngineState::periodicFastCallback() {
 
 	cltTimingCorrection = getCltTimingCorrection();
 
-	knockThreshold = interpolate2d(rpm, engineConfiguration->knockNoiseRpmBins,
-					engineConfiguration->knockNoise);
-
 	baroCorrection = getBaroCorrection();
 
 	auto tps = Sensor::get(SensorType::Tps1);

firmware/controllers/algo/engine_configuration.cpp

@@ -404,14 +404,7 @@ static void setDefaultEngineNoiseTable() {
 
 	engineConfiguration->knockSamplingDuration = 45;
 
-	engineConfiguration->knockNoise[0] = 2; // 800
+	setArrayValues(engineConfiguration->knockBaseNoise, -20);
-	engineConfiguration->knockNoise[1] = 2; // 1700
-	engineConfiguration->knockNoise[2] = 2; // 2600
-	engineConfiguration->knockNoise[3] = 2; // 3400
-	engineConfiguration->knockNoise[4] = 2; // 4300
-	engineConfiguration->knockNoise[5] = 2; // 5200
-	engineConfiguration->knockNoise[6] = 2; // 6100
-	engineConfiguration->knockNoise[7] = 2; // 7000
 }
 
 static void setHip9011FrankensoPinout() {

firmware/controllers/algo/engine_state.h

@@ -38,8 +38,6 @@ public:
 	// Estimated airflow based on whatever airmass model is active
 	float airflowEstimate = 0;
 
-	float knockThreshold = 0;
-
 	float auxValveStart = 0;
 	float auxValveEnd = 0;
 

firmware/controllers/can/can_verbose.cpp

@@ -129,7 +129,7 @@ static void populateFrame(Fueling& msg) {
 	msg.cylAirmass = engine->engineState.sd.airMassInOneCylinder;
 	msg.estAirflow = engine->engineState.airflowEstimate;
 	msg.fuel_pulse = engine->actualLastInjection[0];
-	msg.knockCount = engine->outputChannels.knockCount;
+	msg.knockCount = engine->module<KnockController>()->getKnockCount();
 }
 
 struct Fueling2 {

firmware/controllers/engine_cycle/knock_logic.cpp

@@ -46,7 +46,7 @@ int getCylinderKnockBank(uint8_t cylinderNumber) {
 }
 
 bool KnockController::onKnockSenseCompleted(uint8_t cylinderNumber, float dbv, efitick_t lastKnockTime) {
-	bool isKnock = dbv > engine->engineState.knockThreshold;
+	bool isKnock = dbv > m_knockThreshold;
 
 #if EFI_TUNER_STUDIO
 	// Pass through per-cylinder peak detector
@@ -58,7 +58,7 @@ bool KnockController::onKnockSenseCompleted(uint8_t cylinderNumber, float dbv, e
 
 	// If this was a knock, count it!
 	if (isKnock) {
-		engine->outputChannels.knockCount++;
+		m_knockCount++;
 	}
 #endif // EFI_TUNER_STUDIO
 
@@ -88,7 +88,13 @@ float KnockController::getKnockRetard() const {
 	return m_knockRetard;
 }
 
-void KnockController::periodicFastCallback() {
+uint32_t KnockController::getKnockCount() const {
+	return m_knockCount;
+}
+
+void KnockController::onFastCallback() {
+	m_knockThreshold = getKnockThreshold();
+
 	constexpr auto callbackPeriodSeconds = FAST_CALLBACK_PERIOD_MS / 1000.0f;
 
 	// stored in units of 0.1 deg/sec
@@ -106,6 +112,14 @@ void KnockController::periodicFastCallback() {
 	}
 }
 
+float KnockController::getKnockThreshold() const {
+	return interpolate2d(
+		Sensor::getOrZero(SensorType::Rpm),
+		engineConfiguration->knockNoiseRpmBins,
+		engineConfiguration->knockBaseNoise
+	);
+}
+
 // This callback is to be implemented by the knock sense driver
 __attribute__((weak)) void onStartKnockSampling(uint8_t cylinderNumber, float samplingTimeSeconds, uint8_t channelIdx) {
 	UNUSED(cylinderNumber);

firmware/controllers/engine_cycle/knock_logic.h

@@ -11,21 +11,29 @@
 
 int getCylinderKnockBank(uint8_t cylinderNumber);
 
-class KnockController {
+class KnockController : public EngineModule {
 public:
+	// EngineModule implementation
+	void onFastCallback() override;
+
 	// onKnockSenseCompleted is the callback from the knock sense driver to report a sensed knock level
 	bool onKnockSenseCompleted(uint8_t cylinderNumber, float dbv, efitick_t lastKnockTime);
-	void periodicFastCallback();
 
 	float getKnockRetard() const;
+	uint32_t getKnockCount() const;
+
+	virtual float getKnockThreshold() const;
 
 private:
+	// start with threshold higher than any possible knock to avoid recording spurious knocks
+	float m_knockThreshold = 100;
+
 	// Degrees retarded: larger number = more retard
 	float m_knockRetard = 0;
 
+	uint32_t m_knockCount = 0;
+
 	using PD = PeakDetect<float, MS2NT(100)>;
 	PD peakDetectors[12];
 	PD allCylinderPeakDetector;
-
-
 };

firmware/controllers/engine_cycle/spark_logic.cpp

@@ -79,7 +79,7 @@ static void prepareCylinderIgnitionSchedule(angle_t dwellAngleDuration, floatms_
 		// Offset by this cylinder's position in the cycle
 		+ getCylinderAngle(event->cylinderIndex, event->cylinderNumber)
 		// Pull any extra timing for knock retard
-		+ engine->knockController.getKnockRetard();
+		+ engine->module<KnockController>()->getKnockRetard();
 
 	efiAssertVoid(CUSTOM_SPARK_ANGLE_1, !cisnan(sparkAngle), "sparkAngle#1");
 	const int index = engine->ignitionPin[event->cylinderIndex];

firmware/controllers/sensors/software_knock.cpp

@@ -201,7 +201,7 @@ void processLastKnockEvent() {
 	// clamp to reasonable range
 	db = clampF(-100, db, 100);
 
-	engine->knockController.onKnockSenseCompleted(currentCylinderNumber, db, lastKnockTime);
+	engine->module<KnockController>()->onKnockSenseCompleted(currentCylinderNumber, 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->knockController.onKnockSenseCompleted(instance.cylinderNumber, knockVolts, instance.knockSampleTimestamp);
+				engine->module<KnockController>()->onKnockSenseCompleted(instance.cylinderNumber, knockVolts, instance.knockSampleTimestamp);
 
 				#if EFI_HIP_9011_DEBUG
 					/* debug */

firmware/integration/rusefi_config.txt

@@ -163,7 +163,7 @@ struct_no_prefix engine_configuration_s
 #define IDLE_ADVANCE_CURVE_SIZE 8
 #define CRANKING_ADVANCE_CURVE_SIZE 4
 
-#define ENGINE_NOISE_CURVE_SIZE 8
+#define ENGINE_NOISE_CURVE_SIZE 16
 #define CLT_TIMING_CURVE_SIZE 8
 #define IDLE_VE_SIZE 4
 
@@ -446,9 +446,10 @@ uint8_t autoscale engineSnifferRpmThreshold;Engine sniffer would be disabled abo
 	uint8_t maxAcTps;+Above this TPS, disable AC. Set to 0 to disable check.;"%", 1, 0, 0, 100, 0
 	uint8_t maxAcClt;+Above this CLT, disable AC to prevent overheating the engine. Set to 0 to disable check.;"deg C", 1, 0, 0, 150, 0
 
-	float[ENGINE_NOISE_CURVE_SIZE] knockNoise;Knock sensor output knock detection threshold depending on current RPM;"v", 1, 0, 0, 10, 2
 	uint8_t[ENGINE_NOISE_CURVE_SIZE] autoscale knockNoiseRpmBins;;"RPM", @@RPM_1_BYTE_PACKING_MULT@@, 0, 0, 18000, 2
 
+	uint8_t[24] unused28;;"", 1, 0, 0, 0, 0
+
 	uint8_t multisparkMaxSparkingAngle;+This parameter sets the latest that the last multispark can occur after the main ignition event. For example, if the ignition timing is 30 degrees BTDC, and this parameter is set to 45, no multispark will ever be fired after 15 degrees ATDC.;"deg", 1, 0, 0, 60, 0
 	uint8_t multisparkMaxExtraSparkCount;+Configures the maximum number of extra sparks to fire (does not include main spark);"count", 1, 0, 1, 5, 0
 
@@ -1482,7 +1483,7 @@ tChargeMode_e tChargeMode;
 
 	pin_output_mode_e[TCU_SOLENOID_COUNT iterate] tcu_solenoid_mode;
 
-	int8_t[IGN_RPM_COUNT] knockBaseNoise;;"dB", 1, 0, -30, 0, 2
+	int8_t[ENGINE_NOISE_CURVE_SIZE] autoscale knockBaseNoise;Knock sensor output knock detection threshold depending on current RPM.;"dB", 0.5, 0, -50, 10, 1
 
 	float[GAP_TRACKING_LENGTH iterate] triggerGapOverrideFrom;;"from", 1, 0, 0, 20, 2
 

firmware/integration/rusefi_template.xml

@@ -200,16 +200,6 @@
             </table>
         </table>
 
-
-        <table type="2D" name="Engine Noise" storageaddress="@@knockNoise_offset_hex@@"
-               sizex="@@ENGINE_NOISE_CURVE_SIZE@@" storagetype="float" endian="big">
-            <scaling units="Volts" expression="x" to_byte="x" format="0.00"
-                     fineincrement=".01" coarseincrement="0.1"/>
-            <table type="X Axis" storageaddress="@@knockNoiseRpmBins_offset_hex@@" storagetype="float" endian="big">
-                <scaling units="RPM" expression="x" to_byte="x" format="0.00" fineincrement=".1" coarseincrement="1"/>
-            </table>
-        </table>
-
         <table type="2D" name="Injector Lag" storageaddress="@@injector_battLagCorr_offset_hex@@"
                sizex="@@VBAT_INJECTOR_CURVE_SIZE@@" storagetype="float" endian="big">
             <scaling units="ms" expression="x" to_byte="x" format="0.00"

firmware/tunerstudio/rusefi.input

@@ -307,20 +307,11 @@ enable2ndByteCanID = false
 ;	xAxis       =  leftValue, rightValue, step
 ;	yAxis       =  bottomValue, topValue, step
 
-
 	curve = knockThresholdCurve, "Engine knock threshold RPM based"
       columnLabel = "RPM", "Threshold"
       xAxis       =  0, 8000, 9
       yAxis       =  0,  8, 10
       xBins       = knockNoiseRpmBins, RPMValue
-      yBins       = knockNoise
-      gauge       = RPMGauge
-
-	curve = swKnockThresholdCurve, "Engine knock threshold RPM based"
-      columnLabel = "RPM", "Threshold dB"
-      xAxis       =  0, 8000, 9
-      yAxis       =  -30,  0, 10
-      xBins       = ignitionRpmBins,  RPMValue
       yBins       = knockBaseNoise
       gauge       = RPMGauge
 
@@ -1046,8 +1037,6 @@ gaugeCategory = Sensors - Extra 2
    egt7Gauge         = egt7, "EGT#7", "C", 0, 2000
    egt8Gauge         = egt8, "EGT#8", "C", 0, 2000
    rpmAccelerationGa = rpmAcceleration,     "rpm delta",        "RPM/s",        -2000,      2000,       -2000,      2000,       -2000,     2000,   0,   0
-   knockLevelGauge   = knockLevel,"Knock level",           "V",        0,      7,      10,     10,     100,   100,   1,   2
-   knockCountGauge   = knockCount, "Knock count",          "count",        0, 10000, 0, 0, 10000, 10000, 0, 0
    fuelTankLevelGauge   = fuelTankLevel,"Fuel level",           "%",        0,      100,      10,     20,     100,   100,   1,   1
    wastegatePosGauge = wastegatePositionSensor, @@GAUGE_NAME_WG_POSITION@@,  "%",        0,    100,       0,      0,     100,   100,   1,   1
    idlePosSensGauge  = idlePositionSensor,      @@GAUGE_NAME_IDLE_POSITION@@,       "%",        0,    100,       0,      0,     100,   100,   1,   1
@@ -1210,7 +1199,9 @@ 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
+   knockLevelGauge = knockLevel,"Knock level", "dBv", -60, 10, -60, -60, 10, 10, 0, 0
+   knockRetardGauge = knockRetard, "Knock retard", "deg", 0, 10, 0, 0, 10, 10, 1, 1
+   knockCountGauge = knockCount, "Knock count", "count", 0, 10000, 0, 0, 10000, 10000, 0, 0
 
 gaugeCategory = DynoView
 	accelGauge = VssAcceleration, "Vehicle acceleration",    "m/s2",       -10,   10,      -6,     -4,      4,    6,   2,   2
@@ -2953,7 +2944,7 @@ cmd_set_engine_type_default					= "@@TS_IO_TEST_COMMAND_char@@\x00\x31\x00\x00"
 
 	dialog = softwareKnock, "Software Knock", border
 		panel = softwareKnockCfg, West
-		panel = swKnockThresholdCurve, Center
+		panel = knockThresholdCurve, Center
 
 ;			Engine->hip9011 Settings
 	dialog = hipFunction, "HIP9011 Settings (knock decoder)"

unit_tests/tests/test_knock.cpp

@@ -2,17 +2,23 @@
 
 #include "knock_logic.h"
 
+struct MockKnockController : public KnockController {
+	float getKnockThreshold() const override {
+		// Knock threshold of 20dBv
+		return 20;
+	}
+};
+
 TEST(Knock, Retards) {
 	EngineTestHelper eth(TEST_ENGINE);
 
-	// Knock threshold of 20dBv
-	engine->engineState.knockThreshold = 20;
 	// Aggression of 10%
 	engineConfiguration->knockRetardAggression = 100;
 	// Maximum 8 degrees retarded
 	engineConfiguration->knockRetardMaximum = 8;
 
-	KnockController dut;
+	MockKnockController dut;
+	dut.onFastCallback();
 
 	// No retard unless we knock
 	ASSERT_FLOAT_EQ(dut.getKnockRetard(), 0);
@@ -41,10 +47,9 @@ TEST(Knock, Retards) {
 TEST(Knock, Reapply) {
 	EngineTestHelper eth(TEST_ENGINE);
 
-	KnockController dut;
+	MockKnockController dut;
+	dut.onFastCallback();
 
-	// Knock threshold of 20dBv
-	engine->engineState.knockThreshold = 20;
 	// Aggression of 10%
 	engineConfiguration->knockRetardAggression = 100;
 	// Maximum 8 degrees retarded
@@ -61,18 +66,18 @@ TEST(Knock, Reapply) {
 	constexpr auto fastPeriodSec = FAST_CALLBACK_PERIOD_MS / 1000.0f;
 
 	// call the fast callback, should reapply 1 degree * callback period
-	dut.periodicFastCallback();
+	dut.onFastCallback();
 	EXPECT_FLOAT_EQ(dut.getKnockRetard(), 2 - 1.0f * fastPeriodSec);
 
 	// 10 updates total
 	for (size_t i = 0; i < 9; i++) {
-		dut.periodicFastCallback();
+		dut.onFastCallback();
 	}
 	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();
+		dut.onFastCallback();
 	}
 
 	// Should have no knock retard