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 f2c321d37b
commit 14199080b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
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).
*/
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 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
// linking process which is the way to raise the alarm
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
@ -765,5 +768,5 @@ int getRusEfiVersion(void) {
if (initBootloader() != 0)
return 123;
#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) {
efitick_t nowNt = getTimeNowNt();
if (ENGINE(needToStopEngine(nowNt))) {
setStopped(PASS_ENGINE_PARAMETER_SIGNATURE);
return false;
}
@ -102,7 +101,6 @@ bool RpmCalculator::checkIfSpinning(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
*/
bool noTriggerEventsForTooLong = nowNt - engine->triggerCentral.previousShaftEventTimeNt >= US2NT(US_PER_SECOND_LL);
if (noRpmEventsForTooLong || noTriggerEventsForTooLong) {
setStopSpinning(PASS_ENGINE_PARAMETER_SIGNATURE);
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.
if (isStopped(PASS_ENGINE_PARAMETER_SIGNATURE) && !isSpinning) {
state = SPINNING_UP;
engine->triggerCentral.triggerState.spinningEventIndex = 0;
isSpinning = true;
}
// update variables needed by early instant RPM calc.
@ -238,8 +237,6 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
if (index == 0) {
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);
if (hadRpmRecently) {
@ -275,8 +272,11 @@ void rpmShaftPositionCallback(trigger_event_e ckpSignalType,
}
#endif
// Replace 'normal' RPM with instant RPM for the initial spin-up period
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 iRpm = engine->triggerCentral.triggerState.calculateInstantRpm(&prevIndex, nowNt PASS_ENGINE_PARAMETER_SUFFIX);
// validate instant RPM - we shouldn't skip the cranking state

View File

@ -126,17 +126,21 @@ int TriggerState::getTotalRevolutionCounter() const {
TriggerStateWithRunningStatistics::TriggerStateWithRunningStatistics() :
//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++) {
timeOfLastEvent[i] = nowNt;
}
void TriggerStateWithRunningStatistics::movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
// 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) {
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
*/
@ -149,7 +153,13 @@ float TriggerStateWithRunningStatistics::calculateInstantRpm(int *prevIndex, efi
// now let's get precise angle for that event
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;
// todo: angle diff should be pre-calculated
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;
instantRpmValue[current_index] = instantRpm;
timeOfLastEvent[current_index] = nowNt;
// This fixes early RPM instability based on incomplete data
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) {
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) {

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
@ -130,6 +133,10 @@ public:
* timestamp of each trigger wheel tooth
*/
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
*/
@ -138,6 +145,7 @@ public:
* Stores last non-zero instant RPM value to fix early instability
*/
float prevInstantRpmValue = 0;
void movePreSynchTimestamps(DECLARE_ENGINE_PARAMETER_SIGNATURE);
float calculateInstantRpm(int *prevIndex, 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
engineConfiguration->bc.isFasterEngineSpinUpEnabled = true;
eth.moveTimeForwardMs(1000 /*ms*/);
eth.firePrimaryTriggerRise();
// set ignition mode
engineConfiguration->ignitionMode = IM_INDIVIDUAL_COILS;
// 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
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
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
ASSERT_EQ(STOPPED, engine->rpmCalculator.getState());
ASSERT_EQ(SPINNING_UP, engine->rpmCalculator.getState());
// check RPM
ASSERT_EQ( 0, GET_RPM()) << "RPM=0";
// 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);
}
static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int expectedRpm) {
static void doTestFasterEngineSpinningUp60_2(int startUpDelayMs, int rpm1, int expectedRpm) {
WITH_ENGINE_TEST_HELPER(TEST_ENGINE);
// turn on FasterEngineSpinUp mode
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);
eth.moveTimeForwardMs(startUpDelayMs);
// fire 30 tooth rise/fall signals
eth.fireTriggerEvents2(30 /* count */, 1 /*ms*/);
eth.fireTriggerEvents2(1, 4);
EXPECT_EQ(expectedRpm, GET_RPM()) << "test RPM with " + std::to_string(startUpDelayMs) + " startUpDelayMs";
// now fire missed tooth rise/fall
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) {
// I do not get it. Startup delay is affecting instance RPM?
// todo: is this feature implementation issue or test framework issue?
doTestFasterEngineSpinningUp60_2(0, 220);
doTestFasterEngineSpinningUp60_2(100, 89);
doTestFasterEngineSpinningUp60_2(1000, 0);
doTestFasterEngineSpinningUp60_2(0, 288, 263);
doTestFasterEngineSpinningUp60_2(100, 288, 263);
doTestFasterEngineSpinningUp60_2(1000, 288, 263);
}