From 41bcceae407d8924ecb6bbeac16d85d463367e78 Mon Sep 17 00:00:00 2001 From: rusefi Date: Sat, 8 Dec 2018 22:57:00 -0500 Subject: [PATCH] #129 huge step forward --- .../system/pwm_generator_logic.cpp | 47 ++++++++-- .../controllers/system/pwm_generator_logic.h | 13 +++ unit_tests/test_pwm_generator.cpp | 85 ++++++++++++++++--- 3 files changed, 125 insertions(+), 20 deletions(-) diff --git a/firmware/controllers/system/pwm_generator_logic.cpp b/firmware/controllers/system/pwm_generator_logic.cpp index e1ec64d234..4931938ee1 100644 --- a/firmware/controllers/system/pwm_generator_logic.cpp +++ b/firmware/controllers/system/pwm_generator_logic.cpp @@ -28,6 +28,7 @@ void PwmConfig::baseConstructor() { memset(&safe, 0, sizeof(safe)); dbgNestingLevel = 0; periodNt = NAN; + mode = PM_NORMAL; memset(&outputPins, 0, sizeof(outputPins)); phaseCount = 0; pwmCycleCallback = NULL; @@ -50,6 +51,7 @@ void PwmConfig::init(float *st, single_wave_s *waves) { /** * This method allows you to change duty cycle on the fly * @param dutyCycle value between 0 and 1 + * See also setFrequency */ void SimplePwm::setSimplePwmDutyCycle(float dutyCycle) { if (cisnan(dutyCycle)) { @@ -60,8 +62,14 @@ void SimplePwm::setSimplePwmDutyCycle(float dutyCycle) { warning(CUSTOM_ERR_6579, "spwd:dutyCycle %.2f", dutyCycle); return; } - // todo: need to fix PWM so that it supports zero duty cycle - multiWave.setSwitchTime(0, dutyCycle); + if (dutyCycle == 0) { + mode = PM_ZERO; + } else if (dutyCycle == 1) { + mode = PM_FULL; + } else { + mode = PM_NORMAL; + multiWave.setSwitchTime(0, dutyCycle); + } } /** @@ -70,7 +78,8 @@ void SimplePwm::setSimplePwmDutyCycle(float dutyCycle) { static efitimeus_t getNextSwitchTimeUs(PwmConfig *state) { efiAssert(CUSTOM_ERR_ASSERT, state->safe.phaseIndex < PWM_PHASE_MAX_COUNT, "phaseIndex range", 0); int iteration = state->safe.iteration; - float switchTime = state->multiWave.getSwitchTime(state->safe.phaseIndex); + // we handle PM_ZERO and PM_FULL separately + float switchTime = state->mode == PM_NORMAL ? state->multiWave.getSwitchTime(state->safe.phaseIndex) : 1; float periodNt = state->safe.periodNt; #if DEBUG_PWM scheduleMsg(&logger, "iteration=%d switchTime=%.2f period=%.2f", iteration, switchTime, period); @@ -132,9 +141,13 @@ efitimeus_t PwmConfig::togglePwmState() { /** * NaN period means PWM is paused * TODO: what about pin state? low, high or random? - * TODO: cover this by a unit test + * TODO: cover this with a unit test */ - return getTimeNowUs() + MS2US(100); + return getTimeNowUs() + MS2US(NAN_FREQUENCY_SLEEP_PERIOD_MS); + } + if (mode != PM_NORMAL) { + // in case of ZERO or FULL we are always at starting index + safe.phaseIndex = 0; } if (safe.phaseIndex == 0) { @@ -144,8 +157,16 @@ efitimeus_t PwmConfig::togglePwmState() { /** * Here is where the 'business logic' - the actual pin state change is happening */ - // callback state index is offset by one. todo: why? can we simplify this? - int cbStateIndex = safe.phaseIndex == 0 ? phaseCount - 1 : safe.phaseIndex - 1; + int cbStateIndex; + if (mode == PM_NORMAL) { + // callback state index is offset by one. todo: why? can we simplify this? + cbStateIndex = safe.phaseIndex == 0 ? phaseCount - 1 : safe.phaseIndex - 1; + } else if (mode == PM_ZERO) { + cbStateIndex = 0; + } else { + cbStateIndex = 1; + } + stateChangeCallback(this, cbStateIndex); efitimeus_t nextSwitchTimeUs = getNextSwitchTimeUs(this); @@ -168,10 +189,15 @@ efitimeus_t PwmConfig::togglePwmState() { // } safe.phaseIndex++; - if (safe.phaseIndex == phaseCount) { + if (safe.phaseIndex == phaseCount || mode != PM_NORMAL) { safe.phaseIndex = 0; // restart safe.iteration++; } +#if EFI_UNIT_TEST + printf("PWM: nextSwitchTimeUs=%d phaseIndex=%d iteration=%d\r\n", nextSwitchTimeUs, + safe.phaseIndex, + safe.iteration); +#endif /* EFI_UNIT_TEST */ return nextSwitchTimeUs; } @@ -205,7 +231,9 @@ void copyPwmParameters(PwmConfig *state, int phaseCount, float *switchTimes, int state->multiWave.waves[waveIndex].pinStates[phaseIndex] = pinStates[waveIndex][phaseIndex]; } } - state->multiWave.checkSwitchTimes(phaseCount); + if (state->mode == PM_NORMAL) { + state->multiWave.checkSwitchTimes(phaseCount); + } } /** @@ -251,6 +279,7 @@ void startSimplePwm(SimplePwm *state, const char *msg, OutputPin *output, float float switchTimes[] = { dutyCycle, 1 }; pin_state_t pinStates0[] = { 0, 1 }; + state->setSimplePwmDutyCycle(dutyCycle); pin_state_t *pinStates[1] = { pinStates0 }; diff --git a/firmware/controllers/system/pwm_generator_logic.h b/firmware/controllers/system/pwm_generator_logic.h index 404e6eb33a..901a60eea2 100644 --- a/firmware/controllers/system/pwm_generator_logic.h +++ b/firmware/controllers/system/pwm_generator_logic.h @@ -13,6 +13,8 @@ #include "scheduler.h" #include "efiGpio.h" +#define NAN_FREQUENCY_SLEEP_PERIOD_MS 100 + typedef struct { /** * a copy so that all phases are executed on the same period, even if another thread @@ -35,6 +37,12 @@ class PwmConfig; typedef void (pwm_cycle_callback)(PwmConfig *state); typedef void (pwm_gen_callback)(PwmConfig *state, int stateIndex); +typedef enum { + PM_ZERO, + PM_NORMAL, + PM_FULL +} pwm_mode_e; + /** * @brief Multi-channel software PWM output configuration */ @@ -50,6 +58,11 @@ public: pwm_cycle_callback *pwmCycleCallback, pwm_gen_callback *callback); + /** + * We need to handle zero duty cycle and 100% duty cycle in a special way + */ + pwm_mode_e mode; + /** * @param use NAN frequency to pause PWM */ diff --git a/unit_tests/test_pwm_generator.cpp b/unit_tests/test_pwm_generator.cpp index 1da592d7a8..c04837d98c 100644 --- a/unit_tests/test_pwm_generator.cpp +++ b/unit_tests/test_pwm_generator.cpp @@ -13,7 +13,7 @@ extern EventQueue schedulingQueue; extern int timeNowUs; -static int expectedTimeOfNextEvent = 0; +static int expectedTimeOfNextEvent; static int pinValue = -1; static void testApplyPinState(PwmConfig *state, int stateIndex) { @@ -38,15 +38,76 @@ static void assertNextEvent(const char *msg, int expectedPinState) { assertEqualsM("PWM_test: queue.size", 1, schedulingQueue.size()); } +static void test100dutyCycle() { + print("*************************************** test100dutyCycle\r\n"); + + expectedTimeOfNextEvent = timeNowUs = 0; + SimplePwm pwm; + OutputPin pin; + schedulingQueue.clear(); + + startSimplePwm(&pwm, "unit_test", + &pin, + 1000 /* frequency */, + 1.0 /* duty cycle */, + &testApplyPinState); + + expectedTimeOfNextEvent += 1000; + assertEqualsM2("1@1000/100", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); + + assertNextEvent("exec@100", 1); + + expectedTimeOfNextEvent += 1000; + assertNextEvent("exec2@100", 1); + + expectedTimeOfNextEvent += 1000; + assertNextEvent("exec3@100", 1); +} + +static void testSwitchToNanPeriod() { + print("*************************************** testSwitchToNanPeriod\r\n"); + + expectedTimeOfNextEvent = timeNowUs = 0; + SimplePwm pwm; + OutputPin pin; + schedulingQueue.clear(); + + startSimplePwm(&pwm, "unit_test", + &pin, + 1000 /* frequency */, + 0.60 /* duty cycle */, + &testApplyPinState); + + expectedTimeOfNextEvent += 600; + assertEqualsM2("1@1000/70", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); + + assertNextEvent("exec@70", 0); + assertEqualsM("time1", 600, timeNowUs); + + expectedTimeOfNextEvent += 400; + assertNextEvent("exec2@70", 1); + + pwm.setFrequency(NAN); + + expectedTimeOfNextEvent += 600; + assertEqualsM2("1@1000/NAN", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); + assertNextEvent("exec2@70", 1); + + expectedTimeOfNextEvent += MS2US(NAN_FREQUENCY_SLEEP_PERIOD_MS); + assertEqualsM2("2@1000/NAN", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); + assertNextEvent("exec3@NAN", 1); +} + void testPwmGenerator() { + test100dutyCycle(); + testSwitchToNanPeriod(); + print("*************************************** testPwmGenerator\r\n"); + expectedTimeOfNextEvent = timeNowUs = 0; SimplePwm pwm; - OutputPin pin; - schedulingQueue.clear(); - timeNowUs = 0; startSimplePwm(&pwm, "unit_test", &pin, @@ -68,27 +129,29 @@ void testPwmGenerator() { pwm.setSimplePwmDutyCycle(0); assertEqualsM2("2@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); - assertNextEvent("exec@1", 1); + assertNextEvent("exec@1", 0); assertEqualsM("time2", 1000, timeNowUs); + expectedTimeOfNextEvent += 1000; assertEqualsM2("3@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); assertNextEvent("exec@2", 0 /* pin value */); - assertEqualsM("time3", 1000, timeNowUs); + assertEqualsM("time3", 2000, timeNowUs); expectedTimeOfNextEvent += 1000; assertEqualsM2("4@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); - // todo: this is bad - pin is high with zero duty cycle - assertNextEvent("exec@3", 1 /* pin value */); - assertEqualsM("time4", 2000, timeNowUs); + assertNextEvent("exec@3", 0 /* pin value */); + assertEqualsM("time4", 3000, timeNowUs); + expectedTimeOfNextEvent += 1000; assertEqualsM2("5@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); assertNextEvent("exec@4", 0 /* pin value */); expectedTimeOfNextEvent += 1000; assertEqualsM2("6@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0); - // todo: this is bad - pin is high with zero duty cycle - assertNextEvent("exec@5", 1 /* pin value */); + assertNextEvent("exec@5", 0 /* pin value */); + expectedTimeOfNextEvent += 1000; + assertEqualsM("time4", 5000, timeNowUs); assertEqualsM2("7@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0);