From 0b10f7dca81603924d6cff9a143a7aba8c8d8cf5 Mon Sep 17 00:00:00 2001 From: Matthew Kennedy Date: Sat, 2 Jul 2022 02:19:02 -0700 Subject: [PATCH] hellen board ID detect improvements (#4307) * move call * board detections work correctly * improve detector * s * break instead of return * tests Co-authored-by: Matthew Kennedy --- .../hellen154hyundai/board_configuration.cpp | 2 +- .../hellen/hellen72/board_configuration.cpp | 2 +- .../config/boards/hellen/hellen_board_id.cpp | 100 ++++++++++-------- .../controllers/algo/engine_configuration.cpp | 2 - firmware/rusefi.cpp | 2 + unit_tests/tests/test_hellen_board_id.cpp | 16 +-- 6 files changed, 68 insertions(+), 56 deletions(-) diff --git a/firmware/config/boards/hellen/hellen154hyundai/board_configuration.cpp b/firmware/config/boards/hellen/hellen154hyundai/board_configuration.cpp index 2b1eb66a08..a1d76db4fc 100644 --- a/firmware/config/boards/hellen/hellen154hyundai/board_configuration.cpp +++ b/firmware/config/boards/hellen/hellen154hyundai/board_configuration.cpp @@ -105,7 +105,7 @@ void setBoardConfigOverrides() { engineConfiguration->clt.config.bias_resistor = 4700; engineConfiguration->iat.config.bias_resistor = 4700; - if (engine->engineState.hellenBoardId == 0) { + if (engine->engineState.hellenBoardId == -1) { // first revision of did not have Hellen Board ID // https://github.com/rusefi/hellen154hyundai/issues/55 engineConfiguration->etbIo[1].directionPin1 = Gpio::Unassigned; diff --git a/firmware/config/boards/hellen/hellen72/board_configuration.cpp b/firmware/config/boards/hellen/hellen72/board_configuration.cpp index b6c7d45509..63fe9618af 100644 --- a/firmware/config/boards/hellen/hellen72/board_configuration.cpp +++ b/firmware/config/boards/hellen/hellen72/board_configuration.cpp @@ -93,7 +93,7 @@ void setBoardConfigOverrides() { setHellen176LedPins(); setupVbatt(); - if (engine->engineState.hellenBoardId == 0) { + if (engine->engineState.hellenBoardId == -1) { // Rev a-d use SPI3 for SD card setHellenSdCardSpi3(); } else { diff --git a/firmware/config/boards/hellen/hellen_board_id.cpp b/firmware/config/boards/hellen/hellen_board_id.cpp index cf9487b10c..3c648c271d 100644 --- a/firmware/config/boards/hellen/hellen_board_id.cpp +++ b/firmware/config/boards/hellen/hellen_board_id.cpp @@ -61,23 +61,16 @@ //#define HELLEN_BOARD_ID_DEBUG #if EFI_PROD_CODE -#if STM32_GPT_USE_TIM6 -#define HELLEN_BOARD_ID_GPTDEVICE GPTD6 -#else -#error "STM32_GPT_USE_TIM6 is required for Hellen Board-ID detector!" -#endif /* STM32_GPT_USE_TIM6 */ static void hellenBoardIdInputCallback(void *arg, efitick_t nowNt) { UNUSED(arg); - chibios_rt::CriticalSectionLocker csl; - HellenBoardIdFinderState *state = (HellenBoardIdFinderState *)arg; - // Now start discharging immediately! This should be the first command in the interrupt handler. palClearPad(state->rOutputPinPort, state->rOutputPinIdx); state->timeChargeNt = nowNt; + chibios_rt::CriticalSectionLocker csl; chSemSignalI(&state->boardId_wake); // no need to call chSchRescheduleS() because we're inside the ISR } @@ -97,20 +90,22 @@ float HellenBoardIdSolver::solve(float Tc1, float Tc2, float x0, float y, float float Xcur, Xnext; Xnext = x0; - int safetyLimit = 5000; // since we had https://github.com/rusefi/rusefi/issues/4084 let's add paranoia check + // since we had https://github.com/rusefi/rusefi/issues/4084 let's add paranoia check + // All real cases seem to converge in <= 5 iterations, so we don't need to try more than 20. + int safetyLimit = 20; do { - if (safetyLimit-- < 0) { - firmwareError(OBD_PCM_Processor_Fault, "hellen boardID is broken"); - return Xnext; - } + if (safetyLimit-- < 0) { + firmwareError(OBD_PCM_Processor_Fault, "hellen boardID is broken"); + break; + } Xcur = Xnext; Xnext = Xcur - fx(Xcur) / dfx(Xcur); #ifdef HELLEN_BOARD_ID_DEBUG efiPrintf ("* %f", Xnext); -#endif /* HELLEN_BOARD_ID_DEBUG */ +#endif /* HELLEN_BOARD_ID_DEBUG */ } while (absF(Xnext - Xcur) > deltaX); - + return Xnext; } @@ -133,7 +128,8 @@ float HellenBoardIdFinderBase::findClosestResistor(float R, bool testOnlyMajorSe *rIdx = -1; float minDelta = 1.e6f; for (size_t i = 0; i < rValueSize; i++) { - float delta = absF(R - rAllValues[i]); + // Find the nearest resistor by least ratio error + float delta = absF(1 - (R / rAllValues[i])); if (delta < minDelta) { minDelta = delta; *rIdx = i; @@ -163,9 +159,10 @@ float HellenBoardIdFinderBase::calc(float Tc1_us, float Tc2_us, float Rest, floa // solve the equation for R (1 Ohm precision is more than enough) *Rmeasured = rSolver.solve(Tc1_us, Tc2_us, Rest, C, 1.0f); - // add 30 Ohms for pin's internal resistance + // add 22 Ohms for pin's internal resistance // (according to the STM32 datasheets, the voltage drop on an output pin can be up to 0.4V for 8 mA current) - constexpr float Rinternal = 30.0f; + // Actual measured value was is in the low-20s on most chips. + constexpr float Rinternal = 22.0f; float R = findClosestResistor(*Rmeasured - Rinternal, testOnlyMajorSeries, rIdx); // Find the 'real' capacitance value and use it for the next resistor iteration (gives more precision) @@ -220,34 +217,39 @@ bool HellenBoardIdFinder::measureChargingTimes(int i, float & Tc1_us, f return false; } + // Timestamps: + // t1 = Starts charging from 0v + // t2 = Threshold reached, starts discharging + // t3 = Random voltage reached, starts charging again + // t4 = Threshold reached again, process finished. + // 2. Start charging until the input pin triggers (V01 threshold is reached) state.timeChargeNt = 0; - efitick_t nowNt1 = getTimeNowNt(); + efitick_t t1 = getTimeNowNt(); palSetPad(state.rOutputPinPort, state.rOutputPinIdx); chSemWaitTimeout(&state.boardId_wake, TIME_US2I(Tf_us)); + efitick_t t2 = state.timeChargeNt; // 3. At the moment, the discharging has already been started! // Meanwhile we need to do some checks - until some pre-selected voltage is presumably reached. - // if voltage didn't change on the input pin, then the charging didn't start, + // if voltage didn't change on the input pin (or changed impossibly fast), then the charging didn't start, // meaning there's no capacitor and/or resistors on these pins. - if (state.timeChargeNt <= nowNt1) { + if (t2 - t1 < US2NT(100)) { efiPrintf("* Hellen Board ID circuitry wasn't detected! Aborting!"); return false; } // 4. calculate the first charging time - Tc1_us = NT2USF(state.timeChargeNt - nowNt1); + efitick_t Tc1_nt = t2 - t1; + Tc1_us = NT2USF(Tc1_nt); // We use the same 'charging time' to discharge the capacitor to some random voltage below the threshold voltage. - float Td_us = Tc1_us; - - // we can make a tiny delay adjustments to compensate for the code execution overhead (every usec matters!) - efitick_t nowNt2 = getTimeNowNt(); - float TdAdj_us = NT2USF(nowNt2 - state.timeChargeNt); + efitick_t Td_nt = Tc1_nt; // 5. And now just wait for the rest of the discharge process... - // We cannot use chThdSleepMicroseconds() here because we need more precise delay - gptPolledDelay(&HELLEN_BOARD_ID_GPTDEVICE, Td_us - TdAdj_us); + // Spin wait since chThdSleepMicroseconds() lacks the resolution we need + efitick_t t3 = t2 + Td_nt; + while (getTimeNowNt() < t3) ; // the input pin state should be low when the capacitor is discharged to Vl pinState = palReadPad(state.rInputPinPort, state.rInputPinIdx); @@ -257,28 +259,29 @@ bool HellenBoardIdFinder::measureChargingTimes(int i, float & Tc1_us, f palSetPad(state.rOutputPinPort, state.rOutputPinIdx); // Wait for the charging completion - efitick_t nowNt3 = getTimeNowNt(); chSemReset(&state.boardId_wake, 0); chSemWaitTimeout(&state.boardId_wake, TIME_US2I(Tf_us)); + efitick_t t4 = state.timeChargeNt; // 7. calculate the second charge time - Tc2_us = NT2USF(state.timeChargeNt - nowNt3); + Tc2_us = NT2USF(t4 - t3); #ifdef HELLEN_BOARD_ID_DEBUG efitick_t nowNt4 = getTimeNowNt(); - efiPrintf("* dTime21 = %d", (int)(nowNt2 - nowNt1)); - efiPrintf("* dTime32 = %d", (int)(nowNt3 - nowNt2)); - efiPrintf("* dTime43 = %d", (int)(nowNt4 - nowNt3)); - efiPrintf("* Tc1 = %f, Tc2 = %f, Td = %f, TdAdj = %f", Tc1_us, Tc2_us, Td_us, TdAdj_us); + efiPrintf("* dTime2-1 = %d", (int)(t2 - t1)); + efiPrintf("* dTime3-2 = %d", (int)(t3 - t2)); + efiPrintf("* dTime4-3 = %d", (int)(t4 - t3)); + efiPrintf("* Tc1 = %f, Tc2 = %f, Td = %f", Tc1_us, Tc2_us, Td_us); #endif /* HELLEN_BOARD_ID_DEBUG */ // sanity checks if (pinState != 0) { - efiPrintf("* Board detection error! (Td=%f is too small)", Td_us); - return false; + float Td_us = NT2USF(Td_nt); + efiPrintf("* Board detection error! (Td=%f is too small)", Td_us); + return false; } - if (state.timeChargeNt <= nowNt3) { + if (t4 <= t3) { efiPrintf("* Estimates are out of limit! Something went wrong. Aborting!"); return false; } @@ -312,7 +315,7 @@ bool HellenBoardIdFinder::measureChargingTimesAveraged(int i, float & T int detectHellenBoardId() { - int boardId = 0; + int boardId = -1; #if EFI_PROD_CODE efiPrintf("Starting Hellen Board ID detection..."); efitick_t beginNt = getTimeNowNt(); @@ -332,8 +335,6 @@ int detectHellenBoardId() { // init some ChibiOs objects chSemObjectInit(&finder.state.boardId_wake, 0); - static constexpr GPTConfig gptCfg = { 1000000 /* 1 MHz timer clock.*/, NULL, 0, 0 }; - gptStart(&HELLEN_BOARD_ID_GPTDEVICE, &gptCfg); // R1 is the first, R2 is the second for (int i = 0; i < numPins; i++) { @@ -375,13 +376,22 @@ int detectHellenBoardId() { palSetPadMode(getBrainPinPort(rPins[k]), getBrainPinIndex(rPins[k]), PAL_MODE_RESET); } - gptStop(&HELLEN_BOARD_ID_GPTDEVICE); - efitick_t endNt = getTimeNowNt(); int elapsed_Ms = US2MS(NT2US(endNt - beginNt)); - // '+1' so that we can distinguish between identification not invoked and identification invoked - boardId = 1 + HELLEN_GET_BOARD_ID(rIdx[0], rIdx[1]); + // Check that all resistors were actually detected + bool allRValid = true; + for (size_t i = 0; i < numPins; i++) { + allRValid &= R[i] != 0; + } + + // Decode board ID only if all resistors could be decoded, otherwise we return -1 + if (allRValid) { + boardId = HELLEN_GET_BOARD_ID(rIdx[0], rIdx[1]); + } else { + boardId = -1; + } + efiPrintf("* RESULT: BoardId = %d, R1 = %.0f, R2 = %.0f (Elapsed time: %d ms)", boardId, R[0], R[1], elapsed_Ms); #endif /* EFI_PROD_CODE */ return boardId; diff --git a/firmware/controllers/algo/engine_configuration.cpp b/firmware/controllers/algo/engine_configuration.cpp index aee42d80dd..4eda17f1b3 100644 --- a/firmware/controllers/algo/engine_configuration.cpp +++ b/firmware/controllers/algo/engine_configuration.cpp @@ -775,8 +775,6 @@ void loadConfiguration() { resetConfigurationExt(engineConfiguration->engineType); #endif /* EFI_INTERNAL_FLASH */ - detectBoardType(); - // Force any board configuration options that humans shouldn't be able to change setBoardConfigOverrides(); } diff --git a/firmware/rusefi.cpp b/firmware/rusefi.cpp index 2039831b3c..4c7d09ddb1 100644 --- a/firmware/rusefi.cpp +++ b/firmware/rusefi.cpp @@ -202,6 +202,8 @@ void runRusEfi() { // Perform hardware initialization that doesn't need configuration initHardwareNoConfig(); + detectBoardType(); + #if EFI_ETHERNET startEthernetConsole(); #endif diff --git a/unit_tests/tests/test_hellen_board_id.cpp b/unit_tests/tests/test_hellen_board_id.cpp index f1427d0741..61120d6974 100644 --- a/unit_tests/tests/test_hellen_board_id.cpp +++ b/unit_tests/tests/test_hellen_board_id.cpp @@ -27,13 +27,15 @@ TEST(hellen_board_id, testClosestResistor) { int rIdx; // use only major series EXPECT_FLOAT_EQ(1000, finder.findClosestResistor(876, true, &rIdx)); - EXPECT_FLOAT_EQ(1000, finder.findClosestResistor(1100, true, &rIdx)); - EXPECT_FLOAT_EQ(1200, finder.findClosestResistor(1100+1, true, &rIdx)); + + // break point between 1000 and 1200 = ~1091 + EXPECT_FLOAT_EQ(1000, finder.findClosestResistor(1090, true, &rIdx)); + EXPECT_FLOAT_EQ(1200, finder.findClosestResistor(1091, true, &rIdx)); // use full series - EXPECT_FLOAT_EQ(1000, finder.findClosestResistor(1050, false, &rIdx)); - EXPECT_FLOAT_EQ(1100, finder.findClosestResistor(1050+1, false, &rIdx)); - EXPECT_FLOAT_EQ(1100, finder.findClosestResistor(1149, false, &rIdx)); - EXPECT_FLOAT_EQ(1200, finder.findClosestResistor(1150, false, &rIdx)); + EXPECT_FLOAT_EQ(1000, finder.findClosestResistor(1047, false, &rIdx)); + EXPECT_FLOAT_EQ(1100, finder.findClosestResistor(1049, false, &rIdx)); + EXPECT_FLOAT_EQ(1100, finder.findClosestResistor(1147, false, &rIdx)); + EXPECT_FLOAT_EQ(1200, finder.findClosestResistor(1149, false, &rIdx)); EXPECT_FLOAT_EQ(510, finder.findClosestResistor(0, true, &rIdx)); ASSERT_EQ(0, rIdx); @@ -51,6 +53,6 @@ TEST(hellen_board_id, testCalc) { float R = finder.calc(1024.714f, 724.639555f, 1099.0f, 1.0f, false, &Rmeasured, &newC, &rIdx); EXPECT_NEAR(1100, R, 0.001); EXPECT_NEAR(1099.998779, Rmeasured, 0.001); - EXPECT_NEAR(0.973396897, newC, 0.001); + EXPECT_NEAR(0.980316, newC, 0.001); ASSERT_EQ(19, rIdx); }