Trigger rabbit hole (#682)

fix #681

* this did not work right

* progress: spinning is detected sooner now

* and now things are much better?

* and now reducing RAM usage
This commit is contained in:
rusefi 2019-01-24 21:12:55 -05:00 committed by GitHub
parent d2164a51a0
commit ae88cbf7eb
5 changed files with 63 additions and 29 deletions

View File

@ -297,7 +297,10 @@ static void periodicSlowCallback(Engine *engine) {
/** /**
* Update engine RPM state if needed (check timeouts). * Update engine RPM state if needed (check timeouts).
*/ */
engine->rpmCalculator.checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); bool isSpinning = engine->rpmCalculator.checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE);
if (!isSpinning) {
engine->rpmCalculator.setStopSpinning(PASS_ENGINE_PARAMETER_SIGNATURE);
}
if (engine->rpmCalculator.isStopped(PASS_ENGINE_PARAMETER_SIGNATURE)) { if (engine->rpmCalculator.isStopped(PASS_ENGINE_PARAMETER_SIGNATURE)) {
#if EFI_INTERNAL_FLASH || defined(__DOXYGEN__) #if EFI_INTERNAL_FLASH || defined(__DOXYGEN__)
@ -750,7 +753,7 @@ void initEngineContoller(Logging *sharedLogger DECLARE_ENGINE_PARAMETER_SUFFIX)
// help to notice when RAM usage goes up - if a code change adds to RAM usage these variables would fail // help to notice when RAM usage goes up - if a code change adds to RAM usage these variables would fail
// linking process which is the way to raise the alarm // linking process which is the way to raise the alarm
static char UNUSED_RAM_SIZE[10200]; static char UNUSED_RAM_SIZE[10200];
static char UNUSED_CCM_SIZE[7100] CCM_OPTIONAL; static char UNUSED_CCM_SIZE[7000] CCM_OPTIONAL;
/** /**
* See also VCS_VERSION * See also VCS_VERSION
@ -765,5 +768,5 @@ int getRusEfiVersion(void) {
if (initBootloader() != 0) if (initBootloader() != 0)
return 123; return 123;
#endif /* EFI_BOOTLOADER_INCLUDE_CODE */ #endif /* EFI_BOOTLOADER_INCLUDE_CODE */
return 20190120; return 20190124;
} }

View File

@ -88,7 +88,6 @@ bool RpmCalculator::isRunning(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) { bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
efitick_t nowNt = getTimeNowNt(); efitick_t nowNt = getTimeNowNt();
if (ENGINE(needToStopEngine(nowNt))) { if (ENGINE(needToStopEngine(nowNt))) {
setStopped(PASS_ENGINE_PARAMETER_SIGNATURE);
return false; return false;
} }
@ -102,7 +101,6 @@ bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
*/ */
bool noTriggerEventsForTooLong = nowNt - engine->triggerCentral.previousShaftEventTimeNt >= US2NT(US_PER_SECOND_LL); bool noTriggerEventsForTooLong = nowNt - engine->triggerCentral.previousShaftEventTimeNt >= US2NT(US_PER_SECOND_LL);
if (noRpmEventsForTooLong || noTriggerEventsForTooLong) { if (noRpmEventsForTooLong || noTriggerEventsForTooLong) {
setStopSpinning(PASS_ENGINE_PARAMETER_SIGNATURE);
return false; return false;
} }
@ -194,6 +192,7 @@ void RpmCalculator::setSpinningUp(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFI
// Only a completely stopped and non-spinning engine can enter the spinning-up state. // Only a completely stopped and non-spinning engine can enter the spinning-up state.
if (isStopped(PASS_ENGINE_PARAMETER_SIGNATURE) && !isSpinning) { if (isStopped(PASS_ENGINE_PARAMETER_SIGNATURE) && !isSpinning) {
state = SPINNING_UP; state = SPINNING_UP;
engine->triggerCentral.triggerState.spinningEventIndex = 0;
isSpinning = true; isSpinning = true;
} }
// update variables needed by early instant RPM calc. // update variables needed by early instant RPM calc.
@ -238,8 +237,6 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
if (index == 0) { if (index == 0) {
ENGINE(m.beforeRpmCb) = GET_TIMESTAMP(); ENGINE(m.beforeRpmCb) = GET_TIMESTAMP();
// WAT? we are here in case of trigger sync. why are we checking (incorrectly)
// if engine is spinning and STOPPING it (incorrectly since 'lastRpmEventTimeNt' was not assigned yet?)
bool hadRpmRecently = rpmState->checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE); bool hadRpmRecently = rpmState->checkIfSpinning(PASS_ENGINE_PARAMETER_SIGNATURE);
if (hadRpmRecently) { if (hadRpmRecently) {
@ -275,8 +272,11 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
} }
#endif #endif
// Replace 'normal' RPM with instant RPM for the initial spin-up period
if (rpmState->isSpinningUp(PASS_ENGINE_PARAMETER_SIGNATURE)) { if (rpmState->isSpinningUp(PASS_ENGINE_PARAMETER_SIGNATURE)) {
// we are here only once trigger is synchronized for the first time
// while transitioning from 'spinning' to 'running'
// Replace 'normal' RPM with instant RPM for the initial spin-up period
engine->triggerCentral.triggerState.movePreSynchTimestamps(PASS_ENGINE_PARAMETER_SIGNATURE);
int prevIndex; int prevIndex;
int iRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&prevIndex, nowNt PASS_ENGINE_PARAMETER_SUFFIX); int iRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&prevIndex, nowNt PASS_ENGINE_PARAMETER_SUFFIX);
// validate instant RPM - we shouldn't skip the cranking state // validate instant RPM - we shouldn't skip the cranking state

View File

@ -126,17 +126,21 @@ int TriggerState::getTotalRevolutionCounter() const {
TriggerStateWithRunningStatistics::TriggerStateWithRunningStatistics() : TriggerStateWithRunningStatistics::TriggerStateWithRunningStatistics() :
//https://en.cppreference.com/w/cpp/language/zero_initialization //https://en.cppreference.com/w/cpp/language/zero_initialization
instantRpmValue() timeOfLastEvent(), instantRpmValue()
{ {
// avoid ill-defined instant RPM when the data is not gathered yet }
efitime_t nowNt = getTimeNowNt();
for (int i = 0; i < PWM_PHASE_MAX_COUNT; i++) { void TriggerStateWithRunningStatistics::movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
timeOfLastEvent[i] = nowNt; // here we take timestamps of events which happened prior to synchronization and place them
// at appropriate locations
for (int i = 0; i < spinningEventIndex;i++) {
timeOfLastEvent[getTriggerSize() - i] = spinningEvents[i];
} }
} }
float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) {
int current_index = currentCycle.current_index; // local copy so that noone changes the value on us int current_index = currentCycle.current_index; // local copy so that noone changes the value on us
timeOfLastEvent[current_index] = nowNt;
/** /**
* Here we calculate RPM based on last 90 degrees * Here we calculate RPM based on last 90 degrees
*/ */
@ -149,7 +153,13 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi
// now let's get precise angle for that event // now let's get precise angle for that event
angle_t prevIndexAngle = TRIGGER_SHAPE(eventAngles[*prevIndex]); angle_t prevIndexAngle = TRIGGER_SHAPE(eventAngles[*prevIndex]);
uint32_t time = nowNt - timeOfLastEvent[*prevIndex]; efitick_t time90ago = timeOfLastEvent[*prevIndex];
if (time90ago == 0) {
return prevInstantRpmValue;
}
// we are OK to subtract 32 bit value from more precise 64 bit since the result would 32 bit which is
// OK for small time differences like this one
uint32_t time = nowNt - time90ago;
angle_t angleDiff = currentAngle - prevIndexAngle; angle_t angleDiff = currentAngle - prevIndexAngle;
// todo: angle diff should be pre-calculated // todo: angle diff should be pre-calculated
fixAngle(angleDiff, "angleDiff", CUSTOM_ERR_6561); fixAngle(angleDiff, "angleDiff", CUSTOM_ERR_6561);
@ -160,7 +170,6 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi
float instantRpm = (60000000.0 / 360 * US_TO_NT_MULTIPLIER) * angleDiff / time; float instantRpm = (60000000.0 / 360 * US_TO_NT_MULTIPLIER) * angleDiff / time;
instantRpmValue[current_index] = instantRpm; instantRpmValue[current_index] = instantRpm;
timeOfLastEvent[current_index] = nowNt;
// This fixes early RPM instability based on incomplete data // This fixes early RPM instability based on incomplete data
if (instantRpm < RPM_LOW_THRESHOLD) if (instantRpm < RPM_LOW_THRESHOLD)
@ -171,7 +180,17 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi
} }
void TriggerStateWithRunningStatistics::setLastEventTimeForInstantRpm(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { void TriggerStateWithRunningStatistics::setLastEventTimeForInstantRpm(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) {
timeOfLastEvent[currentCycle.current_index] = nowNt; if (shaft_is_synchronized) {
return;
}
// here we remember tooth timestamps which happen prior to synchronization
if (spinningEventIndex >= PRE_SYNC_EVENTS) {
// too many events while trying to find synchronization point
// todo: better implementation would be to shift here or use cyclic buffer so that we keep last
// 'PRE_SYNC_EVENTS' events
return;
}
spinningEvents[spinningEventIndex++] = nowNt;
} }
void TriggerStateWithRunningStatistics::runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) { void TriggerStateWithRunningStatistics::runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX) {

View File

@ -118,6 +118,9 @@ private:
}; };
// we only need 90 degrees of events so /4 or maybe even /8 should work?
#define PRE_SYNC_EVENTS (PWM_PHASE_MAX_COUNT / 4)
/** /**
* the reason for sub-class is simply to save RAM but not having statisics in the trigger initialization instance * the reason for sub-class is simply to save RAM but not having statisics in the trigger initialization instance
@ -130,6 +133,10 @@ public:
* timestamp of each trigger wheel tooth * timestamp of each trigger wheel tooth
*/ */
uint32_t timeOfLastEvent[PWM_PHASE_MAX_COUNT]; uint32_t timeOfLastEvent[PWM_PHASE_MAX_COUNT];
int spinningEventIndex = 0;
// todo: change the implementation to reuse 'timeOfLastEvent'
uint32_t spinningEvents[PRE_SYNC_EVENTS];
/** /**
* instant RPM calculated at this trigger wheel tooth * instant RPM calculated at this trigger wheel tooth
*/ */
@ -138,6 +145,7 @@ public:
* Stores last non-zero instant RPM value to fix early instability * Stores last non-zero instant RPM value to fix early instability
*/ */
float prevInstantRpmValue = 0; float prevInstantRpmValue = 0;
void movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE);
float calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX); float calculateInstantRpm(int *prevIndex, efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX);
virtual void runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX); virtual void runtimeStatistics(efitime_t nowNt DECLARE_ENGINE_PARAMETER_SUFFIX);
/** /**

View File

@ -14,9 +14,6 @@ TEST(cranking, testFasterEngineSpinningUp) {
// turn on FasterEngineSpinUp mode // turn on FasterEngineSpinUp mode
engineConfiguration->bc.isFasterEngineSpinUpEnabled = true; engineConfiguration->bc.isFasterEngineSpinUpEnabled = true;
eth.moveTimeForwardMs(1000 /*ms*/);
eth.firePrimaryTriggerRise();
// set ignition mode // set ignition mode
engineConfiguration->ignitionMode = IM_INDIVIDUAL_COILS; engineConfiguration->ignitionMode = IM_INDIVIDUAL_COILS;
// set cranking threshold (used below) // set cranking threshold (used below)
@ -24,11 +21,16 @@ TEST(cranking, testFasterEngineSpinningUp) {
// set sequential injection mode to test auto-change to simultaneous when spinning-up // set sequential injection mode to test auto-change to simultaneous when spinning-up
setupSimpleTestEngineWithMafAndTT_ONE_trigger(&eth, IM_SEQUENTIAL); setupSimpleTestEngineWithMafAndTT_ONE_trigger(&eth, IM_SEQUENTIAL);
ASSERT_EQ(IM_INDIVIDUAL_COILS, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE));
eth.moveTimeForwardMs(1000 /*ms*/);
eth.firePrimaryTriggerRise();
// check if it's true // check if it's true
ASSERT_EQ(IM_SEQUENTIAL, engine->getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); ASSERT_EQ(IM_SEQUENTIAL, engine->getCurrentInjectionMode(PASS_ENGINE_PARAMETER_SIGNATURE));
ASSERT_EQ(IM_INDIVIDUAL_COILS, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE)); ASSERT_EQ(IM_WASTED_SPARK, getCurrentIgnitionMode(PASS_ENGINE_PARAMETER_SIGNATURE));
// check if the engine has the right state // check if the engine has the right state
ASSERT_EQ(STOPPED, engine->rpmCalculator.getState()); ASSERT_EQ(SPINNING_UP, engine->rpmCalculator.getState());
// check RPM // check RPM
ASSERT_EQ( 0, GET_RPM()) << "RPM=0"; ASSERT_EQ( 0, GET_RPM()) << "RPM=0";
// the queue should be empty, no trigger events yet // the queue should be empty, no trigger events yet
@ -98,7 +100,7 @@ TEST(cranking, testFasterEngineSpinningUp) {
eth.assertEvent5(&engine->executor, "inj end#3", 1, (void*)seTurnPinLow, timeStartUs, MS2US(60) + 27974 + 3000); eth.assertEvent5(&engine->executor, "inj end#3", 1, (void*)seTurnPinLow, timeStartUs, MS2US(60) + 27974 + 3000);
} }
static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int expectedRpm) { static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int rpm1, int expectedRpm) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE); WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
// turn on FasterEngineSpinUp mode // turn on FasterEngineSpinUp mode
engineConfiguration->bc.isFasterEngineSpinUpEnabled = true; engineConfiguration->bc.isFasterEngineSpinUpEnabled = true;
@ -106,15 +108,17 @@ static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int expectedRpm
setupSimpleTestEngineWithMaf(&eth, IM_SEQUENTIAL, TT_TOOTHED_WHEEL_60_2); setupSimpleTestEngineWithMaf(&eth, IM_SEQUENTIAL, TT_TOOTHED_WHEEL_60_2);
eth.moveTimeForwardMs(startUpDelayMs); eth.moveTimeForwardMs(startUpDelayMs);
// fire 30 tooth rise/fall signals
eth.fireTriggerEvents2(30 /* count */, 1 /*ms*/); eth.fireTriggerEvents2(30 /* count */, 1 /*ms*/);
eth.fireTriggerEvents2(1, 4); // now fire missed tooth rise/fall
EXPECT_EQ(expectedRpm, GET_RPM()) << "test RPM with " + std::to_string(startUpDelayMs) + " startUpDelayMs"; eth.fireRise(4 /*ms*/);
EXPECT_EQ(rpm1, GET_RPM()) << "test RPM: After rise " + std::to_string(startUpDelayMs);
eth.fireFall(4 /*ms*/);
EXPECT_EQ(expectedRpm, GET_RPM()) << "test RPM: with " + std::to_string(startUpDelayMs) + " startUpDelayMs";
} }
TEST(cranking, testFasterEngineSpinningUp60_2) { TEST(cranking, testFasterEngineSpinningUp60_2) {
// I do not get it. Startup delay is affecting instance RPM? doTestFasterEngineSpinningUp60_2(0, 288, 263);
// todo: is this feature implementation issue or test framework issue? doTestFasterEngineSpinningUp60_2(100, 288, 263);
doTestFasterEngineSpinningUp60_2(0, 220); doTestFasterEngineSpinningUp60_2(1000, 288, 263);
doTestFasterEngineSpinningUp60_2(100, 89);
doTestFasterEngineSpinningUp60_2(1000, 0);
} }