Don't keep a separate MultiChannelStateSequence for the trigger emulator, version 2. (#3517)

All it wants is to use the main trigger state, so don't bother copying it.  Instead, change
PwmConfig to take a const pointer to a Multi.*Sequence, then make all the users adapt.  Worse
fallout is that SimplePwm now has its own Multi.*Sequence, but PwmConfig is downgraded to a
pointer, so that's only a net +4 bytes.  And we can make the overhead of Multi.*Sequence much lower
using embedded arrays, which only the caller can do since it knows the maximum size... (for another
day...)

Also remove SimplePwm's 2nd copy of SingleChannelStateSequence.  It served no purpose.

Saves 1992 bytes of BSS and 24 bytes of RAM4 (latter probably due to SimplePwm change)
This commit is contained in:
Scott Smith 2021-11-10 04:01:20 -08:00 committed by GitHub
parent 1db9a02f1d
commit dac67235c4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 38 additions and 84 deletions

View File

@ -53,7 +53,7 @@ float MultiChannelStateSequence::getSwitchTime(const int index) const {
return switchTimes[index];
}
void MultiChannelStateSequence::checkSwitchTimes(const int size, const float scale) {
void MultiChannelStateSequence::checkSwitchTimes(const int size, const float scale) const {
if (switchTimes[size - 1] != 1) {
firmwareError(CUSTOM_ERR_WAVE_1, "last switch time has to be 1/%f not %.2f/%f", scale,
switchTimes[size - 1], scale * switchTimes[size - 1]);

View File

@ -72,7 +72,7 @@ public:
void reset(void);
float getSwitchTime(const int phaseIndex) const;
void setSwitchTime(const int phaseIndex, const float value);
void checkSwitchTimes(const int size, const float scale);
void checkSwitchTimes(const int size, const float scale) const;
pin_state_t getChannelState(const int channelIndex, const int phaseIndex) const;
int findAngleMatch(const float angle, const int size) const;

View File

@ -18,10 +18,11 @@
// 1% duty cycle
#define ZERO_PWM_THRESHOLD 0.01
SimplePwm::SimplePwm() {
waveInstance.init(pinStates);
sr[0] = waveInstance;
init(_switchTimes, sr);
SimplePwm::SimplePwm()
: sr(pinStates)
, seq(_switchTimes, &sr)
{
seq.waveCount = 1;
}
SimplePwm::SimplePwm(const char *name) : SimplePwm() {
@ -43,14 +44,6 @@ PwmConfig::PwmConfig() {
arg = this;
}
PwmConfig::PwmConfig(float *st, SingleChannelStateSequence *waves) : PwmConfig() {
multiChannelStateSequence.init(st, waves);
}
void PwmConfig::init(float *st, SingleChannelStateSequence *waves) {
multiChannelStateSequence.init(st, waves);
}
/**
* This method allows you to change duty cycle on the fly
* @param dutyCycle value between 0 and 1
@ -94,7 +87,7 @@ void SimplePwm::setSimplePwmDutyCycle(float dutyCycle) {
mode = PM_FULL;
} else {
mode = PM_NORMAL;
multiChannelStateSequence.setSwitchTime(0, dutyCycle);
_switchTimes[0] = dutyCycle;
}
}
@ -105,7 +98,7 @@ static efitick_t getNextSwitchTimeNt(PwmConfig *state) {
efiAssert(CUSTOM_ERR_ASSERT, state->safe.phaseIndex < PWM_PHASE_MAX_COUNT, "phaseIndex range", 0);
int iteration = state->safe.iteration;
// we handle PM_ZERO and PM_FULL separately
float switchTime = state->mode == PM_NORMAL ? state->multiChannelStateSequence.getSwitchTime(state->safe.phaseIndex) : 1;
float switchTime = state->mode == PM_NORMAL ? state->multiChannelStateSequence->getSwitchTime(state->safe.phaseIndex) : 1;
float periodNt = state->safe.periodNt;
#if DEBUG_PWM
efiPrintf("iteration=%d switchTime=%.2f period=%.2f", iteration, switchTime, period);
@ -272,21 +265,11 @@ static void timerCallback(PwmConfig *state) {
* Incoming parameters are potentially just values on current stack, so we have to copy
* into our own permanent storage, right?
*/
void copyPwmParameters(PwmConfig *state, int phaseCount, float const *switchTimes, int waveCount, pin_state_t *const *pinStates) {
void copyPwmParameters(PwmConfig *state, int phaseCount, MultiChannelStateSequence const * seq) {
state->phaseCount = phaseCount;
for (int phaseIndex = 0; phaseIndex < phaseCount; phaseIndex++) {
state->multiChannelStateSequence.setSwitchTime(phaseIndex, switchTimes[phaseIndex]);
for (int channelIndex = 0; channelIndex < waveCount; channelIndex++) {
// print("output switch time index (%d/%d) at %.2f to %d\r\n", phaseIndex, channelIndex,
// switchTimes[phaseIndex], pinStates[waveIndex][phaseIndex]);
pin_state_t value = pinStates[channelIndex][phaseIndex];
state->multiChannelStateSequence.channels[channelIndex].setState(phaseIndex, value);
}
}
state->multiChannelStateSequence = seq;
if (state->mode == PM_NORMAL) {
state->multiChannelStateSequence.checkSwitchTimes(phaseCount, 1);
state->multiChannelStateSequence->checkSwitchTimes(phaseCount, 1);
}
}
@ -296,9 +279,8 @@ void copyPwmParameters(PwmConfig *state, int phaseCount, float const *switchTime
*/
void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
const int phaseCount,
float const *switchTimes,
const int waveCount,
pin_state_t *const*pinStates, pwm_cycle_callback *pwmCycleCallback, pwm_gen_callback *stateChangeCallback) {
MultiChannelStateSequence const * seq,
pwm_cycle_callback *pwmCycleCallback, pwm_gen_callback *stateChangeCallback) {
UNUSED(msg);
this->executor = executor;
isStopRequested = false;
@ -312,14 +294,12 @@ void PwmConfig::weComplexInit(const char *msg, ExecutorInterface *executor,
firmwareError(CUSTOM_ERR_PWM_2, "too many phases in PWM");
return;
}
efiAssertVoid(CUSTOM_ERR_6583, waveCount > 0, "waveCount should be positive");
efiAssertVoid(CUSTOM_ERR_6583, seq->waveCount > 0, "waveCount should be positive");
this->pwmCycleCallback = pwmCycleCallback;
this->stateChangeCallback = stateChangeCallback;
multiChannelStateSequence.waveCount = waveCount;
copyPwmParameters(this, phaseCount, switchTimes, waveCount, pinStates);
copyPwmParameters(this, phaseCount, seq);
safe.phaseIndex = 0;
safe.periodNt = -1;
@ -338,16 +318,16 @@ void startSimplePwm(SimplePwm *state, const char *msg, ExecutorInterface *execut
return;
}
float switchTimes[] = { dutyCycle, 1 };
pin_state_t pinStates0[] = { TV_FALL, TV_RISE };
state->setSimplePwmDutyCycle(dutyCycle);
pin_state_t *pinStates[1] = { pinStates0 };
state->_switchTimes[0] = dutyCycle;
state->_switchTimes[1] = 1;
state->pinStates[0] = TV_FALL;
state->pinStates[1] = TV_RISE;
state->outputPins[0] = output;
state->setFrequency(frequency);
state->weComplexInit(msg, executor, 2, switchTimes, 1, pinStates, NULL, (pwm_gen_callback*)applyPinState);
state->setSimplePwmDutyCycle(dutyCycle); // TODO: DUP ABOVE?
state->weComplexInit(msg, executor, 2, &state->seq, NULL, (pwm_gen_callback*)applyPinState);
}
void startSimplePwmExt(SimplePwm *state, const char *msg,
@ -385,7 +365,7 @@ void startSimplePwmHard(SimplePwm *state, const char *msg,
void applyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ {
#if EFI_PROD_CODE
if (!engine->isPwmEnabled) {
for (int channelIndex = 0; channelIndex < state->multiChannelStateSequence.waveCount; channelIndex++) {
for (int channelIndex = 0; channelIndex < state->multiChannelStateSequence->waveCount; channelIndex++) {
OutputPin *output = state->outputPins[channelIndex];
output->setValue(0);
}
@ -394,10 +374,10 @@ void applyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_callback */ {
#endif // EFI_PROD_CODE
efiAssertVoid(CUSTOM_ERR_6663, stateIndex < PWM_PHASE_MAX_COUNT, "invalid stateIndex");
efiAssertVoid(CUSTOM_ERR_6664, state->multiChannelStateSequence.waveCount <= PWM_PHASE_MAX_WAVE_PER_PWM, "invalid waveCount");
for (int channelIndex = 0; channelIndex < state->multiChannelStateSequence.waveCount; channelIndex++) {
efiAssertVoid(CUSTOM_ERR_6664, state->multiChannelStateSequence->waveCount <= PWM_PHASE_MAX_WAVE_PER_PWM, "invalid waveCount");
for (int channelIndex = 0; channelIndex < state->multiChannelStateSequence->waveCount; channelIndex++) {
OutputPin *output = state->outputPins[channelIndex];
int value = state->multiChannelStateSequence.getChannelState(channelIndex, stateIndex);
int value = state->multiChannelStateSequence->getChannelState(channelIndex, stateIndex);
output->setValue(value);
}
}

View File

@ -53,13 +53,11 @@ typedef enum {
class PwmConfig {
public:
PwmConfig();
PwmConfig(float *switchTimes, SingleChannelStateSequence *waves);
void init(float *switchTimes, SingleChannelStateSequence *waves);
void *arg = nullptr;
void weComplexInit(const char *msg,
ExecutorInterface *executor,
const int phaseCount, float const *switchTimes, const int waveCount, pin_state_t *const*pinStates,
const int phaseCount, MultiChannelStateSequence const * seq,
pwm_cycle_callback *pwmCycleCallback,
pwm_gen_callback *callback);
@ -81,7 +79,7 @@ public:
// todo: 'outputPins' should be extracted away from here since technically one can want PWM scheduler without actual pin output
OutputPin *outputPins[PWM_PHASE_MAX_WAVE_PER_PWM];
MultiChannelStateSequence multiChannelStateSequence;
MultiChannelStateSequence const * multiChannelStateSequence = nullptr;
efitick_t togglePwmState();
void stop();
@ -127,12 +125,10 @@ public:
explicit SimplePwm(const char *name);
void setSimplePwmDutyCycle(float dutyCycle) override;
pin_state_t pinStates[2];
SingleChannelStateSequence sr[1];
SingleChannelStateSequence sr;
float _switchTimes[2];
MultiChannelStateSequence seq;
hardware_pwm* hardPwm = nullptr;
private:
SingleChannelStateSequence waveInstance;
};
/**
@ -167,6 +163,5 @@ void startSimplePwmHard(SimplePwm *state, const char *msg,
brain_pin_e brainPin, OutputPin *output, float frequency,
float dutyCycle);
void copyPwmParameters(PwmConfig *state, int phaseCount, float const *switchTimes,
int waveCount, pin_state_t *const *pinStates);
void copyPwmParameters(PwmConfig *state, int phaseCount, MultiChannelStateSequence const * seq);

View File

@ -87,6 +87,7 @@ void TriggerWaveform::initialize(operation_mode_e operationMode) {
memset(switchTimesBuffer, 0, sizeof(switchTimesBuffer));
memset(expectedEventCount, 0, sizeof(expectedEventCount));
wave.reset();
wave.waveCount = TRIGGER_CHANNEL_COUNT;
previousAngle = 0;
memset(isRiseEvent, 0, sizeof(isRiseEvent));
#if EFI_UNIT_TEST

View File

@ -168,7 +168,7 @@ void prepareEventAngles(TriggerWaveform *shape,
memset(details->eventAngles, 0, sizeof(details->eventAngles));
// this may be <length for some triggers like symmetrical crank Miata NB
int triggerShapeLength = shape->privateTriggerDefinitionSize;
int triggerShapeLength = shape->getSize();
assertAngleRange(shape->triggerShapeSynchPointIndex, "triggerShapeSynchPointIndex", CUSTOM_TRIGGER_SYNC_ANGLE2);
efiAssertVoid(CUSTOM_TRIGGER_CYCLE, engine->engineCycleEventCount != 0, "zero engineCycleEventCount");

View File

@ -53,20 +53,7 @@ void TriggerEmulatorHelper::handleEmulatorCallback(const int size, const MultiCh
}
}
/*
* todo: should we simply re-use instances used by trigger_decoder?
* todo: since we are emulating same shape we are decoding
*/
static pin_state_t pinStates1[PWM_PHASE_MAX_COUNT];
static pin_state_t pinStates2[PWM_PHASE_MAX_COUNT];
static pin_state_t pinStates3[PWM_PHASE_MAX_COUNT];
static SingleChannelStateSequence waves[PWM_PHASE_MAX_WAVE_PER_PWM] = { SingleChannelStateSequence(pinStates1), SingleChannelStateSequence(pinStates2),
SingleChannelStateSequence(pinStates3) };
static SingleChannelStateSequence sr[PWM_PHASE_MAX_WAVE_PER_PWM] = { waves[0], waves[1], waves[2] };
static float pwmSwitchTimesBuffer[PWM_PHASE_MAX_COUNT];
PwmConfig triggerSignal(pwmSwitchTimesBuffer, sr);
PwmConfig triggerSignal;
static int atTriggerVersion = 0;
@ -119,11 +106,7 @@ static void updateTriggerWaveformIfNeeded(PwmConfig *state DECLARE_ENGINE_PARAME
TriggerWaveform *s = &engine->triggerCentral.triggerShape;
pin_state_t *pinStates[PWM_PHASE_MAX_WAVE_PER_PWM] = {
s->wave.channels[0].pinStates,
s->wave.channels[1].pinStates,
s->wave.channels[2].pinStates };
copyPwmParameters(state, s->getSize(), s->wave.switchTimes, PWM_PHASE_MAX_WAVE_PER_PWM, pinStates);
copyPwmParameters(state, s->getSize(), &s->wave);
state->safe.periodNt = -1; // this would cause loop re-initialization
}
}
@ -141,7 +124,7 @@ static void emulatorApplyPinState(int stateIndex, PwmConfig *state) /* pwm_gen_c
* this callback would invoke the input signal handlers directly
*/
helper.handleEmulatorCallback(state->phaseCount,
state->multiChannelStateSequence,
*state->multiChannelStateSequence,
stateIndex);
}
@ -161,16 +144,11 @@ static void initTriggerPwm(DECLARE_ENGINE_PARAMETER_SIGNATURE) {
TriggerWaveform *s = &engine->triggerCentral.triggerShape;
setTriggerEmulatorRPM(engineConfiguration->triggerSimulatorFrequency PASS_ENGINE_PARAMETER_SUFFIX);
pin_state_t *pinStates[PWM_PHASE_MAX_WAVE_PER_PWM] = {
s->wave.channels[0].pinStates,
s->wave.channels[1].pinStates,
s->wave.channels[2].pinStates };
int phaseCount = s->getSize();
assertIsInBounds(phaseCount - 1, pwmSwitchTimesBuffer, "pwmSwitchTimesBuffer");
triggerSignal.weComplexInit("position sensor",
&engine->executor,
phaseCount, s->wave.switchTimes, PWM_PHASE_MAX_WAVE_PER_PWM,
pinStates, updateTriggerWaveformIfNeeded, (pwm_gen_callback*)emulatorApplyPinState);
phaseCount, &s->wave,
updateTriggerWaveformIfNeeded, (pwm_gen_callback*)emulatorApplyPinState);
hasInitTriggerEmulator = true;
}