remove stateChangeCallback from simplepwm (#2644)

* remove callback parameter from cj125

* remove from ext

* remove stateChangeCallback

* alt doesn't need that

* demorgan

* remove call

* bad cut/paste

* check the actual pin in the test
This commit is contained in:
Matthew Kennedy 2021-05-07 06:38:41 -07:00 committed by GitHub
parent ea6cb14780
commit 37a3de07cf
9 changed files with 36 additions and 64 deletions

View File

@ -70,11 +70,14 @@ class AlternatorController : public PeriodicTimerController {
// todo: migrate this to FSIO
bool alternatorShouldBeEnabledAtCurrentRpm = GET_RPM() > engineConfiguration->cranking.rpm;
engine->isAlternatorControlEnabled = CONFIG(isAlternatorControlEnabled) && alternatorShouldBeEnabledAtCurrentRpm;
if (!engine->isAlternatorControlEnabled) {
if (!CONFIG(isAlternatorControlEnabled) || !alternatorShouldBeEnabledAtCurrentRpm) {
// we need to avoid accumulating iTerm while engine is not running
pidReset();
// Shut off output if not needed
alternatorControl.setSimplePwmDutyCycle(0);
return;
}
@ -135,18 +138,6 @@ void setAltPFactor(float p) {
showAltInfo();
}
static void applyAlternatorPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ {
efiAssertVoid(CUSTOM_ERR_6643, stateIndex < PWM_PHASE_MAX_COUNT, "invalid stateIndex");
efiAssertVoid(CUSTOM_IDLE_WAVE_CNT, state->multiChannelStateSequence.waveCount == 1, "invalid idle waveCount");
OutputPin *output = state->outputPins[0];
int value = state->multiChannelStateSequence.getChannelState(/*channelIndex*/0, stateIndex);
/**
* 'engine->isAlternatorControlEnabled' would be false is RPM is too low
*/
if (!value || engine->isAlternatorControlEnabled)
output->setValue(value);
}
void setDefaultAlternatorParameters(DECLARE_CONFIG_PARAMETER_SIGNATURE) {
engineConfiguration->alternatorOffAboveTps = 120;
@ -171,7 +162,7 @@ void initAlternatorCtrl(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
"Alternator control",
&engine->executor,
&enginePins.alternatorPin,
engineConfiguration->alternatorPwmFrequency, 0.1, (pwm_gen_callback*)applyAlternatorPinState);
engineConfiguration->alternatorPwmFrequency, 0);
}
instance.Start();
}

View File

@ -185,10 +185,6 @@ public:
* this is based on sensorChartMode and sensorSnifferRpmThreshold settings
*/
sensor_chart_e sensorChartMode = SC_OFF;
/**
* based on current RPM and isAlternatorControlEnabled setting
*/
bool isAlternatorControlEnabled = false;
bool slowCallBackWasInvoked = false;

View File

@ -335,10 +335,9 @@ void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
}
void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *executor,
OutputPin *output, float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback) {
OutputPin *output, float frequency, float dutyCycle) {
efiAssertVoid(CUSTOM_ERR_PWM_STATE_ASSERT, state != NULL, "state");
efiAssertVoid(CUSTOM_ERR_PWM_DUTY_ASSERT, dutyCycle >= 0 && dutyCycle <= 1, "dutyCycle");
efiAssertVoid(CUSTOM_ERR_PWM_CALLBACK_ASSERT, stateChangeCallback != NULL, "listener");
if (frequency < 1) {
warning(CUSTOM_OBD_LOW_FREQUENCY, "low frequency %.2f", frequency);
return;
@ -353,17 +352,17 @@ void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *execut
state->outputPins[0] = output;
state->setFrequency(frequency);
state->weComplexInit(msg, executor, 2, switchTimes, 1, pinStates, NULL, stateChangeCallback);
state->weComplexInit(msg, executor, 2, switchTimes, 1, pinStates, NULL, (pwm_gen_callback*)applyPinState);
}
void startSimplePwmExt(SimplePwm *state, const char *msg,
ExecutorInterface *executor,
brain_pin_e brainPin, OutputPin *output, float frequency,
float dutyCycle, pwm_gen_callback *stateChangeCallback) {
float dutyCycle) {
output->initPin(msg, brainPin);
startSimplePwm(state, msg, executor, output, frequency, dutyCycle, stateChangeCallback);
startSimplePwm(state, msg, executor, output, frequency, dutyCycle);
}
void startSimplePwmHard(SimplePwm *state, const char *msg,

View File

@ -145,7 +145,7 @@ void applyPinState(int stateIndex, PwmConfig* state) /* pwm_gen_callback */;
void startSimplePwm(SimplePwm *state, const char *msg,
ExecutorInterface *executor,
OutputPin *output,
float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback = (pwm_gen_callback*)applyPinState);
float frequency, float dutyCycle);
/**
* initialize GPIO pin and start a one-channel software PWM driver.
@ -156,7 +156,7 @@ void startSimplePwmExt(SimplePwm *state,
const char *msg,
ExecutorInterface *executor,
brain_pin_e brainPin, OutputPin *output,
float frequency, float dutyCycle, pwm_gen_callback *stateChangeCallback = (pwm_gen_callback*)applyPinState);
float frequency, float dutyCycle);
void startSimplePwmHard(SimplePwm *state, const char *msg,
ExecutorInterface *executor,

View File

@ -628,7 +628,7 @@ void initCJ125(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
if (!cjStartSpi(PASS_ENGINE_PARAMETER_SIGNATURE))
return;
efiPrintf("cj125: Starting heater control");
globalInstance.StartHeaterControl((pwm_gen_callback*)applyPinState PASS_ENGINE_PARAMETER_SUFFIX);
globalInstance.StartHeaterControl(PASS_ENGINE_PARAMETER_SIGNATURE);
cjStart(PASS_ENGINE_PARAMETER_SIGNATURE);
#ifdef CJ125_DEBUG

View File

@ -41,12 +41,12 @@ bool CJ125::isWorkingState(void) const {
return state != CJ125_ERROR && state != CJ125_INIT && state != CJ125_IDLE;
}
void CJ125::StartHeaterControl(pwm_gen_callback *stateChangeCallback DECLARE_ENGINE_PARAMETER_SUFFIX) {
void CJ125::StartHeaterControl(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
// todo: use custom pin state method, turn pin off while not running
startSimplePwmExt(&wboHeaterControl, "wboHeaterPin",
&engine->executor,
CONFIG(wboHeaterPin),
&wboHeaterPin, CJ125_HEATER_PWM_FREQ, 0.0f, stateChangeCallback);
&wboHeaterPin, CJ125_HEATER_PWM_FREQ, 0.0f);
SetIdleHeater(PASS_ENGINE_PARAMETER_SIGNATURE);
}

View File

@ -99,7 +99,7 @@ public:
bool isWorkingState(void) const;
void SetHeater(float value DECLARE_ENGINE_PARAMETER_SUFFIX);
void SetIdleHeater(DECLARE_ENGINE_PARAMETER_SIGNATURE);
void StartHeaterControl(pwm_gen_callback *stateChangeCallback DECLARE_ENGINE_PARAMETER_SUFFIX);
void StartHeaterControl(DECLARE_ENGINE_PARAMETER_SIGNATURE);
bool cjIdentify(DECLARE_ENGINE_PARAMETER_SIGNATURE);
void printDiag();
void calibrate(DECLARE_ENGINE_PARAMETER_SIGNATURE);

View File

@ -8,10 +8,6 @@
#include "cj125_logic.h"
#include "engine_test_helper.h"
static void applyHeaterPinState(PwmConfig *state, int stateIndex) {
}
class TestSpi : public Cj125SpiStream {
public:
MOCK_METHOD1(ReadRegister, uint8_t(uint8_t));
@ -27,7 +23,7 @@ TEST(testCJ125, testInitialState) {
WITH_ENGINE_TEST_HELPER(FORD_ASPIRE_1996);
cj.StartHeaterControl((pwm_gen_callback*)&applyHeaterPinState PASS_ENGINE_PARAMETER_SUFFIX);
cj.StartHeaterControl(PASS_ENGINE_PARAMETER_SIGNATURE);
ASSERT_EQ(cj.heaterDuty, CJ125_HEATER_IDLE_RATE);
TestSpi mock;

View File

@ -15,15 +15,8 @@
extern int timeNowUs;
static int expectedTimeOfNextEvent;
static int pinValue = -1;
static void testApplyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ {
pinValue = state->multiChannelStateSequence.getChannelState(/*channelIndex*/0, stateIndex);
printf("PWM_test: setPinValue=%d @ timeNow=%d\r\n", pinValue, timeNowUs);
}
static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor *executor) {
static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor *executor, OutputPin& pin) {
printf("PWM_test: Asserting event [%s]\r\n", msg);
// only one action expected in queue
ASSERT_EQ( 1, executor->size()) << "PWM_test: schedulingQueue size";
@ -33,7 +26,7 @@ static void assertNextEvent(const char *msg, int expectedPinState, TestExecutor
// execute pending actions and assert that only one action was executed
ASSERT_NEAR(1, executor->executeAll(timeNowUs), 0) << msg << " executed";
ASSERT_NEAR(expectedPinState, pinValue, 0) << msg << " pin state";
ASSERT_NEAR(expectedPinState, pin.currentLogicValue, 0) << msg << " pin state";
// assert that we have one new action in queue
ASSERT_EQ( 1, executor->size()) << "PWM_test: queue.size";
@ -51,19 +44,18 @@ static void test100dutyCycle() {
&executor,
&pin,
1000 /* frequency */,
1.0 /* duty cycle */,
(pwm_gen_callback*)&testApplyPinState);
1.0 /* duty cycle */);
expectedTimeOfNextEvent += 1000;
assertEqualsM2("1@1000/100", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@100", HIGH_VALUE, &executor);
assertNextEvent("exec@100", HIGH_VALUE, &executor, pin);
expectedTimeOfNextEvent += 1000;
assertNextEvent("exec2@100", HIGH_VALUE, &executor);
assertNextEvent("exec2@100", HIGH_VALUE, &executor, pin);
expectedTimeOfNextEvent += 1000;
assertNextEvent("exec3@100", HIGH_VALUE, &executor);
assertNextEvent("exec3@100", HIGH_VALUE, &executor, pin);
}
static void testSwitchToNanPeriod() {
@ -78,27 +70,26 @@ static void testSwitchToNanPeriod() {
&executor,
&pin,
1000 /* frequency */,
0.60 /* duty cycle */,
(pwm_gen_callback*)&testApplyPinState);
0.60 /* duty cycle */);
expectedTimeOfNextEvent += 600;
assertEqualsM2("1@1000/70", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@70", LOW_VALUE, &executor);
assertNextEvent("exec@70", LOW_VALUE, &executor, pin);
ASSERT_EQ( 600, timeNowUs) << "time1";
expectedTimeOfNextEvent += 400;
assertNextEvent("exec2@70", HIGH_VALUE, &executor);
assertNextEvent("exec2@70", HIGH_VALUE, &executor, pin);
pwm.setFrequency(NAN);
expectedTimeOfNextEvent += 600;
assertEqualsM2("1@1000/NAN", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec2@NAN", LOW_VALUE, &executor);
assertNextEvent("exec2@NAN", LOW_VALUE, &executor, pin);
expectedTimeOfNextEvent += MS2US(NAN_FREQUENCY_SLEEP_PERIOD_MS);
assertEqualsM2("2@1000/NAN", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec3@NAN", LOW_VALUE, &executor);
assertNextEvent("exec3@NAN", LOW_VALUE, &executor, pin);
}
TEST(misc, testPwmGenerator) {
@ -115,14 +106,13 @@ TEST(misc, testPwmGenerator) {
&executor,
&pin,
1000 /* frequency */,
0.80 /* duty cycle */,
(pwm_gen_callback*)&testApplyPinState);
0.80 /* duty cycle */);
expectedTimeOfNextEvent += 800;
assertEqualsM2("1@1000/80", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@0", LOW_VALUE, &executor);
assertNextEvent("exec@0", LOW_VALUE, &executor, pin);
ASSERT_EQ( 800, timeNowUs) << "time1";
expectedTimeOfNextEvent += 200;
@ -132,33 +122,33 @@ TEST(misc, testPwmGenerator) {
pwm.setSimplePwmDutyCycle(0);
assertEqualsM2("2@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@1", LOW_VALUE, &executor);
assertNextEvent("exec@1", LOW_VALUE, &executor, pin);
ASSERT_EQ( 1000, timeNowUs) << "time2";
expectedTimeOfNextEvent += 1000;
assertEqualsM2("3@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@2", LOW_VALUE /* pin value */, &executor);
assertNextEvent("exec@2", LOW_VALUE /* pin value */, &executor, pin);
ASSERT_EQ( 2000, timeNowUs) << "time3";
expectedTimeOfNextEvent += 1000;
assertEqualsM2("4@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@3", LOW_VALUE /* pin value */, &executor);
assertNextEvent("exec@3", LOW_VALUE /* pin value */, &executor, pin);
ASSERT_EQ( 3000, timeNowUs) << "time4";
expectedTimeOfNextEvent += 1000;
assertEqualsM2("5@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@4", LOW_VALUE /* pin value */, &executor);
assertNextEvent("exec@4", LOW_VALUE /* pin value */, &executor, pin);
expectedTimeOfNextEvent += 1000;
assertEqualsM2("6@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@5", LOW_VALUE /* pin value */, &executor);
assertNextEvent("exec@5", LOW_VALUE /* pin value */, &executor, pin);
expectedTimeOfNextEvent += 1000;
ASSERT_EQ( 5000, timeNowUs) << "time4";
assertEqualsM2("7@1000/0", expectedTimeOfNextEvent, executor.getForUnitTest(0)->momentX, 0);
assertNextEvent("exec@6", LOW_VALUE /* pin value */, &executor);
assertNextEvent("exec@6", LOW_VALUE /* pin value */, &executor, pin);
}