From 8650686ab752ba77d3e06fc821fc92b39add85f9 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Mon, 18 Apr 2022 05:03:16 -0700 Subject: [PATCH] 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 | 3 ++- firmware/config/engines/dodge_neon.cpp | 12 ---------- firmware/console/status_loop.cpp | 3 ++- firmware/controllers/algo/engine.cpp | 2 -- firmware/controllers/algo/engine.h | 3 +-- firmware/controllers/algo/engine2.cpp | 3 --- .../controllers/algo/engine_configuration.cpp | 9 +------- firmware/controllers/algo/engine_state.h | 2 -- firmware/controllers/can/can_verbose.cpp | 2 +- .../controllers/engine_cycle/knock_logic.cpp | 20 +++++++++++++--- .../controllers/engine_cycle/knock_logic.h | 16 +++++++++---- .../controllers/engine_cycle/spark_logic.cpp | 2 +- .../controllers/sensors/software_knock.cpp | 2 +- firmware/hw_layer/sensors/hip9011.cpp | 2 +- firmware/integration/rusefi_config.txt | 7 +++--- firmware/integration/rusefi_template.xml | 10 -------- firmware/tunerstudio/rusefi.input | 17 ++++---------- unit_tests/tests/test_knock.cpp | 23 +++++++++++-------- 18 files changed, 61 insertions(+), 77 deletions(-) diff --git a/firmware/CHANGELOG.md b/firmware/CHANGELOG.md index c759e7021e..9e06f3d2f9 100644 --- a/firmware/CHANGELOG.md +++ b/firmware/CHANGELOG.md @@ -26,7 +26,8 @@ Release template (copy/paste this for new release): All notable user-facing or behavior-altering changes will be documented in this file. ### Fixed - - An attempt to make 'Trigger' dialog a bit less confusing #4021 + - 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 diff --git a/firmware/config/engines/dodge_neon.cpp b/firmware/config/engines/dodge_neon.cpp index 28e9d0938a..5cf40f3c3e 100644 --- a/firmware/config/engines/dodge_neon.cpp +++ b/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; diff --git a/firmware/console/status_loop.cpp b/firmware/console/status_loop.cpp index 2118239364..b7464624a4 100644 --- a/firmware/console/status_loop.cpp +++ b/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()->getKnockCount(); + engine->outputChannels.knockRetard = engine->module()->getKnockRetard(); } static void updateFlags() { diff --git a/firmware/controllers/algo/engine.cpp b/firmware/controllers/algo/engine.cpp index ff5c102353..8f468db18a 100644 --- a/firmware/controllers/algo/engine.cpp +++ b/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(); }); diff --git a/firmware/controllers/algo/engine.h b/firmware/controllers/algo/engine.h index 3bd1d5c689..a88cf193ce 100644 --- a/firmware/controllers/algo/engine.h +++ b/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; diff --git a/firmware/controllers/algo/engine2.cpp b/firmware/controllers/algo/engine2.cpp index 9c435ec0f6..65d00b9b11 100644 --- a/firmware/controllers/algo/engine2.cpp +++ b/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); diff --git a/firmware/controllers/algo/engine_configuration.cpp b/firmware/controllers/algo/engine_configuration.cpp index b2e3d78803..a52d32e263 100644 --- a/firmware/controllers/algo/engine_configuration.cpp +++ b/firmware/controllers/algo/engine_configuration.cpp @@ -404,14 +404,7 @@ static void setDefaultEngineNoiseTable() { engineConfiguration->knockSamplingDuration = 45; - engineConfiguration->knockNoise[0] = 2; // 800 - 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 + setArrayValues(engineConfiguration->knockBaseNoise, -20); } static void setHip9011FrankensoPinout() { diff --git a/firmware/controllers/algo/engine_state.h b/firmware/controllers/algo/engine_state.h index 3288239df4..0554ff33da 100644 --- a/firmware/controllers/algo/engine_state.h +++ b/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; diff --git a/firmware/controllers/can/can_verbose.cpp b/firmware/controllers/can/can_verbose.cpp index 0c786c0072..f9c7737f27 100644 --- a/firmware/controllers/can/can_verbose.cpp +++ b/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()->getKnockCount(); } struct Fueling2 { diff --git a/firmware/controllers/engine_cycle/knock_logic.cpp b/firmware/controllers/engine_cycle/knock_logic.cpp index e3be5fb107..fccce593da 100644 --- a/firmware/controllers/engine_cycle/knock_logic.cpp +++ b/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); diff --git a/firmware/controllers/engine_cycle/knock_logic.h b/firmware/controllers/engine_cycle/knock_logic.h index 8fbec2089f..0e58aec978 100644 --- a/firmware/controllers/engine_cycle/knock_logic.h +++ b/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; PD peakDetectors[12]; PD allCylinderPeakDetector; - - }; diff --git a/firmware/controllers/engine_cycle/spark_logic.cpp b/firmware/controllers/engine_cycle/spark_logic.cpp index 10157f8cd4..4c1d1561ef 100644 --- a/firmware/controllers/engine_cycle/spark_logic.cpp +++ b/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()->getKnockRetard(); efiAssertVoid(CUSTOM_SPARK_ANGLE_1, !cisnan(sparkAngle), "sparkAngle#1"); const int index = engine->ignitionPin[event->cylinderIndex]; diff --git a/firmware/controllers/sensors/software_knock.cpp b/firmware/controllers/sensors/software_knock.cpp index 7bc92f954b..dde9f9e1f3 100644 --- a/firmware/controllers/sensors/software_knock.cpp +++ b/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()->onKnockSenseCompleted(currentCylinderNumber, db, lastKnockTime); } void KnockThread::ThreadTask() { diff --git a/firmware/hw_layer/sensors/hip9011.cpp b/firmware/hw_layer/sensors/hip9011.cpp index d1b6a28881..e38e412d9d 100644 --- a/firmware/hw_layer/sensors/hip9011.cpp +++ b/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()->onKnockSenseCompleted(instance.cylinderNumber, knockVolts, instance.knockSampleTimestamp); #if EFI_HIP_9011_DEBUG /* debug */ diff --git a/firmware/integration/rusefi_config.txt b/firmware/integration/rusefi_config.txt index 7d282c8f7e..e8522953f2 100644 --- a/firmware/integration/rusefi_config.txt +++ b/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 diff --git a/firmware/integration/rusefi_template.xml b/firmware/integration/rusefi_template.xml index e25a59125d..20c521ac05 100644 --- a/firmware/integration/rusefi_template.xml +++ b/firmware/integration/rusefi_template.xml @@ -200,16 +200,6 @@ - - - -
- -
- - hip9011 Settings dialog = hipFunction, "HIP9011 Settings (knock decoder)" diff --git a/unit_tests/tests/test_knock.cpp b/unit_tests/tests/test_knock.cpp index 9ab406c149..053300dbe7 100644 --- a/unit_tests/tests/test_knock.cpp +++ b/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