#129 huge step forward

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
+	if (dutyCycle == 0) {
-	multiWave.setSwitchTime(0, dutyCycle);
+		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;
-	int cbStateIndex = safe.phaseIndex == 0 ? phaseCount - 1 : safe.phaseIndex - 1;
+	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 };
 

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
 	 */

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", 0 /* pin value */);
-	assertNextEvent("exec@3", 1 /* pin value */);
+	assertEqualsM("time4", 3000, timeNowUs);
-	assertEqualsM("time4", 2000, 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", 0 /* pin value */);
-	assertNextEvent("exec@5", 1 /* pin value */);
+	expectedTimeOfNextEvent += 1000;
+	assertEqualsM("time4", 5000, timeNowUs);
 	assertEqualsM2("7@1000/0", expectedTimeOfNextEvent, schedulingQueue.getForUnitText(0)->momentX, 0);