From 9a0b66791bd8fe6e3223b8f727f2fb5446fe122f Mon Sep 17 00:00:00 2001 From: andreika-git Date: Mon, 25 Nov 2019 07:02:53 +0200 Subject: [PATCH] unregisterOutput() + isPinOrModeChanged() + Stepper::stepPin+enablePin + Kinetis fix (#1029) * Fix isConfigurationChanged() for EFI_ACTIVE_CONFIGURATION_IN_FLASH * unregisterOutput() -> unregisterOutputIfPinOrModeChanged() * clutchUpPinMode for unregisterPin() * Fix Kinetis: EFI_ACTIVE_CONFIGURATION_IN_FLASH & EFI_MAIN_RELAY_CONTROL * Stepper: enablePin & stepPin with modes support --- firmware/config/boards/kinetis/efifeatures.h | 4 +- .../controllers/algo/engine_configuration.cpp | 4 + firmware/controllers/core/common_headers.h | 11 +++ firmware/controllers/injector_central.cpp | 10 --- firmware/controllers/system/efi_gpio.cpp | 81 ++++++++++--------- firmware/controllers/system/efi_gpio.h | 2 +- firmware/hw_layer/hardware.cpp | 9 +-- firmware/hw_layer/stepper.cpp | 43 +++------- firmware/hw_layer/stepper.h | 12 +-- 9 files changed, 74 insertions(+), 102 deletions(-) diff --git a/firmware/config/boards/kinetis/efifeatures.h b/firmware/config/boards/kinetis/efifeatures.h index 891dbe1d96..5543f0ce04 100644 --- a/firmware/config/boards/kinetis/efifeatures.h +++ b/firmware/config/boards/kinetis/efifeatures.h @@ -181,7 +181,7 @@ /** * Control the main power relay based on measured ignition voltage (Vbatt) */ -#define EFI_MAIN_RELAY_CONTROL FALSE +#define EFI_MAIN_RELAY_CONTROL TRUE #ifndef EFI_PWM #define EFI_PWM FALSE @@ -407,7 +407,7 @@ #define EFI_PRINT_ERRORS_AS_WARNINGS TRUE #define EFI_PRINT_MESSAGES_TO_TERMINAL TRUE -#define EFI_ACTIVE_CONFIGURATION_IN_FLASH FLASH_ADDR +#define EFI_ACTIVE_CONFIGURATION_IN_FLASH (FLASH_ADDR + offsetof(persistent_config_container_s, persistentConfiguration.engineConfiguration)) //#define PWM_PHASE_MAX_COUNT 122 diff --git a/firmware/controllers/algo/engine_configuration.cpp b/firmware/controllers/algo/engine_configuration.cpp index 574058ae01..4faeaaa577 100644 --- a/firmware/controllers/algo/engine_configuration.cpp +++ b/firmware/controllers/algo/engine_configuration.cpp @@ -143,6 +143,8 @@ static fuel_table_t alphaNfuel = { */ #ifdef EFI_ACTIVE_CONFIGURATION_IN_FLASH engine_configuration_s & activeConfiguration = *(engine_configuration_s *)EFI_ACTIVE_CONFIGURATION_IN_FLASH; +// we cannot use this activeConfiguration until we call rememberCurrentConfiguration() +bool isActiveConfigurationVoid = true; #else static engine_configuration_s activeConfigurationLocalStorage; engine_configuration_s & activeConfiguration = activeConfigurationLocalStorage; @@ -153,6 +155,8 @@ extern engine_configuration_s *engineConfiguration; void rememberCurrentConfiguration(DECLARE_ENGINE_PARAMETER_SIGNATURE) { #ifndef EFI_ACTIVE_CONFIGURATION_IN_FLASH memcpy(&activeConfiguration, engineConfiguration, sizeof(engine_configuration_s)); +#else + isActiveConfigurationVoid = false; #endif /* EFI_ACTIVE_CONFIGURATION_IN_FLASH */ } diff --git a/firmware/controllers/core/common_headers.h b/firmware/controllers/core/common_headers.h index aa1b846cb0..1010d8bf6a 100644 --- a/firmware/controllers/core/common_headers.h +++ b/firmware/controllers/core/common_headers.h @@ -83,6 +83,17 @@ persistent_config_s *config = engine->config; \ board_configuration_s *boardConfiguration = &engineConfiguration->bc; +#ifndef EFI_ACTIVE_CONFIGURATION_IN_FLASH +// We store a special changeable copy of configuration is RAM, so we can just compare them #define isConfigurationChanged(x) (engineConfiguration->x != activeConfiguration.x) +#else +// We cannot call prepareVoidConfiguration() for activeConfiguration if it's stored in flash, +// so we need to tell the firmware that it's "void" (i.e. zeroed, invalid) by setting a special flag variable, +// and then we consider 'x' as changed if it's just non-zero. +extern bool isActiveConfigurationVoid; +#define isConfigurationChanged(x) ((engineConfiguration->x != activeConfiguration.x) || (isActiveConfigurationVoid && engineConfiguration->x != 0)) +#endif /* EFI_ACTIVE_CONFIGURATION_IN_FLASH */ + +#define isPinOrModeChanged(pin, mode) (isConfigurationChanged(pin) || isConfigurationChanged(mode)) #endif /* CONTROLLERS_CORE_COMMON_HEADERS_H_ */ diff --git a/firmware/controllers/injector_central.cpp b/firmware/controllers/injector_central.cpp index 74bc56ecc9..5effd8280f 100644 --- a/firmware/controllers/injector_central.cpp +++ b/firmware/controllers/injector_central.cpp @@ -250,16 +250,6 @@ private: static BenchController instance; -void OutputPin::unregisterOutput(brain_pin_e oldPin, brain_pin_e newPin) { - if (oldPin != GPIO_UNASSIGNED && oldPin != newPin) { - scheduleMsg(logger, "unregistering %s", hwPortname(oldPin)); -#if EFI_GPIO_HARDWARE - brain_pin_markUnused(oldPin); - port = nullptr; -#endif /* EFI_GPIO_HARDWARE */ - } -} - static void handleCommandX14(uint16_t index) { switch (index) { case 1: diff --git a/firmware/controllers/system/efi_gpio.cpp b/firmware/controllers/system/efi_gpio.cpp index ff31a5afbe..20ff5f4f5b 100644 --- a/firmware/controllers/system/efi_gpio.cpp +++ b/firmware/controllers/system/efi_gpio.cpp @@ -85,6 +85,18 @@ EnginePins::EnginePins() { } \ } +#define unregisterOutputIfPinChanged(output, pin) { \ + if (isConfigurationChanged(pin)) { \ + (output).unregisterOutput(activeConfiguration.pin); \ + } \ +} + +#define unregisterOutputIfPinOrModeChanged(output, pin, mode) { \ + if (isPinOrModeChanged(pin, mode)) { \ + (output).unregisterOutput(activeConfiguration.pin); \ + } \ +} + #else /* EFI_PROD_CODE */ #define setPinValue(outputPin, electricalValue, logicValue) \ @@ -114,37 +126,24 @@ void EnginePins::unregisterPins() { unregisterEtbPins(); #endif /* EFI_ELECTRONIC_THROTTLE_BODY */ #if EFI_PROD_CODE - fuelPumpRelay.unregisterOutput(activeConfiguration.bc.fuelPumpPin, engineConfiguration->bc.fuelPumpPin); - fanRelay.unregisterOutput(activeConfiguration.bc.fanPin, engineConfiguration->bc.fanPin); - acRelay.unregisterOutput(activeConfiguration.bc.acRelayPin, engineConfiguration->bc.acRelayPin); - hipCs.unregisterOutput(activeConfiguration.bc.hip9011CsPin, engineConfiguration->bc.hip9011CsPin); - triggerDecoderErrorPin.unregisterOutput(activeConfiguration.bc.triggerErrorPin, - engineConfiguration->bc.triggerErrorPin); - - sdCsPin.unregisterOutput(activeConfiguration.bc.sdCardCsPin, engineConfiguration->bc.sdCardCsPin); - accelerometerCs.unregisterOutput(activeConfiguration.LIS302DLCsPin, engineConfiguration->LIS302DLCsPin); -// etbOutput1.unregisterOutput(activeConfiguration.bc.etb1.directionPin1, -// engineConfiguration->bc.etb1.directionPin1); -// etbOutput2.unregisterOutput(activeConfiguration.bc.etb1.directionPin2, -// engineConfiguration->bc.etb1.directionPin2); - checkEnginePin.unregisterOutput(activeConfiguration.bc.malfunctionIndicatorPin, - engineConfiguration->bc.malfunctionIndicatorPin); - dizzyOutput.unregisterOutput(activeConfiguration.dizzySparkOutputPin, - engineConfiguration->dizzySparkOutputPin); - tachOut.unregisterOutput(activeConfiguration.bc.tachOutputPin, - engineConfiguration->bc.tachOutputPin); - idleSolenoidPin.unregisterOutput(activeConfiguration.bc.idle.solenoidPin, - engineConfiguration->bc.idle.solenoidPin); + unregisterOutputIfPinOrModeChanged(fuelPumpRelay, bc.fuelPumpPin, bc.fuelPumpPinMode); + unregisterOutputIfPinOrModeChanged(fanRelay, bc.fanPin, bc.fanPinMode); + unregisterOutputIfPinOrModeChanged(acRelay, bc.acRelayPin, bc.acRelayPinMode); + unregisterOutputIfPinOrModeChanged(hipCs, bc.hip9011CsPin, bc.hip9011CsPinMode); + unregisterOutputIfPinOrModeChanged(triggerDecoderErrorPin, bc.triggerErrorPin, bc.triggerErrorPinMode); + unregisterOutputIfPinOrModeChanged(checkEnginePin, bc.malfunctionIndicatorPin, bc.malfunctionIndicatorPinMode); + unregisterOutputIfPinOrModeChanged(dizzyOutput, dizzySparkOutputPin, dizzySparkOutputPinMode); + unregisterOutputIfPinOrModeChanged(tachOut, bc.tachOutputPin, bc.tachOutputPinMode); + unregisterOutputIfPinOrModeChanged(idleSolenoidPin, bc.idle.solenoidPin, bc.idle.solenoidPinMode); + unregisterOutputIfPinChanged(sdCsPin, bc.sdCardCsPin); + unregisterOutputIfPinChanged(accelerometerCs, LIS302DLCsPin); for (int i = 0;i < FSIO_COMMAND_COUNT;i++) { - fsioOutputs[i].unregisterOutput(activeConfiguration.bc.fsioOutputPins[i], - engineConfiguration->bc.fsioOutputPins[i]); + unregisterOutputIfPinChanged(fsioOutputs[i], bc.fsioOutputPins[i]); } - alternatorPin.unregisterOutput(activeConfiguration.bc.alternatorControlPin, - engineConfiguration->bc.alternatorControlPin); - mainRelay.unregisterOutput(activeConfiguration.bc.mainRelayPin, - engineConfiguration->bc.mainRelayPin); + unregisterOutputIfPinOrModeChanged(alternatorPin, bc.alternatorControlPin, bc.alternatorControlPinMode); + unregisterOutputIfPinOrModeChanged(mainRelay, bc.mainRelayPin, bc.mainRelayPinMode); #endif /* EFI_PROD_CODE */ } @@ -161,9 +160,7 @@ void EnginePins::reset() { void EnginePins::stopIgnitionPins(void) { #if EFI_PROD_CODE for (int i = 0; i < IGNITION_PIN_COUNT; i++) { - NamedOutputPin *output = &enginePins.coils[i]; - output->unregisterOutput(activeConfiguration.bc.ignitionPins[i], - engineConfiguration->bc.ignitionPins[i]); + unregisterOutputIfPinOrModeChanged(enginePins.coils[i], bc.ignitionPins[i], bc.ignitionPinMode); } #endif /* EFI_PROD_CODE */ } @@ -171,9 +168,7 @@ void EnginePins::stopIgnitionPins(void) { void EnginePins::stopInjectionPins(void) { #if EFI_PROD_CODE for (int i = 0; i < INJECTION_PIN_COUNT; i++) { - NamedOutputPin *output = &enginePins.injectors[i]; - output->unregisterOutput(activeConfiguration.bc.injectionPins[i], - engineConfiguration->bc.injectionPins[i]); + unregisterOutputIfPinOrModeChanged(enginePins.injectors[i], bc.injectionPins[i], bc.injectionPinMode); } #endif /* EFI_PROD_CODE */ } @@ -191,13 +186,11 @@ void EnginePins::startIgnitionPins(void) { #if EFI_PROD_CODE for (int i = 0; i < engineConfiguration->specs.cylindersCount; i++) { NamedOutputPin *output = &enginePins.coils[i]; - // todo: we need to check if mode has changed - if (isConfigurationChanged(bc.ignitionPins[i])) { + if (isPinOrModeChanged(bc.ignitionPins[i], bc.ignitionPinMode)) { output->initPin(output->name, CONFIGB(ignitionPins)[i], &CONFIGB(ignitionPinMode)); } } - // todo: we need to check if mode has changed - if (isConfigurationChanged(dizzySparkOutputPin)) { + if (isPinOrModeChanged(dizzySparkOutputPin, dizzySparkOutputPinMode)) { enginePins.dizzyOutput.initPin("dizzy tach", engineConfiguration->dizzySparkOutputPin, &engineConfiguration->dizzySparkOutputPinMode); @@ -210,9 +203,7 @@ void EnginePins::startInjectionPins(void) { // todo: should we move this code closer to the injection logic? for (int i = 0; i < engineConfiguration->specs.cylindersCount; i++) { NamedOutputPin *output = &enginePins.injectors[i]; - // todo: we need to check if mode has changed - if (engineConfiguration->bc.injectionPins[i] != activeConfiguration.bc.injectionPins[i]) { - + if (isPinOrModeChanged(bc.injectionPins[i], bc.injectionPinMode)) { output->initPin(output->name, CONFIGB(injectionPins)[i], &CONFIGB(injectionPinMode)); } @@ -481,6 +472,16 @@ void OutputPin::initPin(const char *msg, brain_pin_e brainPin, const pin_output_ #endif /* EFI_GPIO_HARDWARE */ } +void OutputPin::unregisterOutput(brain_pin_e oldPin) { + if (oldPin != GPIO_UNASSIGNED) { + scheduleMsg(&sharedLogger, "unregistering %s", hwPortname(oldPin)); +#if EFI_GPIO_HARDWARE && EFI_PROD_CODE + brain_pin_markUnused(oldPin); + port = nullptr; +#endif /* EFI_GPIO_HARDWARE */ + } +} + #if EFI_GPIO_HARDWARE // questionable trick: we avoid using 'getHwPort' and 'getHwPin' in case of errors in order to increase the changes of turning the LED diff --git a/firmware/controllers/system/efi_gpio.h b/firmware/controllers/system/efi_gpio.h index fd919abd0c..f9d5b4fe33 100644 --- a/firmware/controllers/system/efi_gpio.h +++ b/firmware/controllers/system/efi_gpio.h @@ -41,7 +41,7 @@ public: /** * dissociates pin from this output and un-registers it in pin repository */ - void unregisterOutput(brain_pin_e oldPin, brain_pin_e newPin); + void unregisterOutput(brain_pin_e oldPin); bool isInitialized(); diff --git a/firmware/hw_layer/hardware.cpp b/firmware/hw_layer/hardware.cpp index f9e3828930..d4b5cdde01 100644 --- a/firmware/hw_layer/hardware.cpp +++ b/firmware/hw_layer/hardware.cpp @@ -257,12 +257,6 @@ void turnOnHardware(Logging *sharedLogger) { #endif /* EFI_SHAFT_POSITION_INPUT */ } -static void unregisterPin(brain_pin_e currentPin, brain_pin_e prevPin) { - if (currentPin != prevPin) { - brain_pin_markUnused(prevPin); - } -} - void stopSpi(spi_device_e device) { #if HAL_USE_SPI if (!isSpiInitialized[device]) @@ -339,7 +333,8 @@ void applyNewHardwareSettings(void) { stopHD44780_pins(); #endif /* #if EFI_HD44780_LCD */ - unregisterPin(engineConfiguration->bc.clutchUpPin, activeConfiguration.bc.clutchUpPin); + if (isPinOrModeChanged(bc.clutchUpPin, bc.clutchUpPinMode)) + brain_pin_markUnused(activeConfiguration.bc.clutchUpPin); enginePins.unregisterPins(); diff --git a/firmware/hw_layer/stepper.cpp b/firmware/hw_layer/stepper.cpp index cceb0daa9c..663804a243 100644 --- a/firmware/hw_layer/stepper.cpp +++ b/firmware/hw_layer/stepper.cpp @@ -112,13 +112,6 @@ StepperMotor::StepperMotor() { currentPosition = 0; targetPosition = 0; -#if EFI_PROD_CODE - enablePort = NULL; - enablePin = 0; - stepPort = NULL; - stepPin = 0; -#endif - reactionTime = 0; totalSteps = 0; } @@ -142,22 +135,20 @@ void StepperMotor::setDirection(bool isIncrementing) { } void StepperMotor::pulse() { -#if EFI_PROD_CODE - palWritePad(enablePort, enablePin, 0); // enable stepper - palWritePad(stepPort, stepPin, 1); + enablePin.setValue(false); // enable stepper + + stepPin.setValue(true); chThdSleepMilliseconds(reactionTime); - palWritePad(stepPort, stepPin, 0); + stepPin.setValue(false); chThdSleepMilliseconds(reactionTime); - palWritePad(enablePort, enablePin, 1); // disable stepper -#endif /* EFI_PROD_CODE */ + enablePin.setValue(true); // disable stepper } void StepperMotor::initialize(brain_pin_e stepPin, brain_pin_e directionPin, pin_output_mode_e directionPinMode, float reactionTime, int totalSteps, brain_pin_e enablePin, pin_output_mode_e enablePinMode, Logging *sharedLogger) { - UNUSED(enablePinMode); this->reactionTime = maxF(1, reactionTime); this->totalSteps = maxI(3, totalSteps); @@ -167,30 +158,18 @@ void StepperMotor::initialize(brain_pin_e stepPin, brain_pin_e directionPin, pin return; } -#if EFI_PROD_CODE - stepPort = getHwPort("step", stepPin); - this->stepPin = getHwPin("step", stepPin); -#endif /* EFI_PROD_CODE */ - this->directionPinMode = directionPinMode; this->directionPin.initPin("stepper dir", directionPin, &this->directionPinMode); -#if EFI_PROD_CODE - enablePort = getHwPort("enable", enablePin); - this->enablePin = getHwPin("enable", enablePin); -#endif /* EFI_PROD_CODE */ + this->stepPinMode = OM_DEFAULT; // todo: do we need configurable stepPinMode? + this->stepPin.initPin("stepper step", stepPin, &this->stepPinMode); - efiSetPadMode("stepper step", stepPin, PAL_MODE_OUTPUT_PUSHPULL); - // todo: start using enablePinMode parameter here #718 - efiSetPadMode("stepper enable", enablePin, PAL_MODE_OUTPUT_PUSHPULL); - -#if EFI_PROD_CODE - palWritePad(this->enablePort, enablePin, 1); // disable stepper + this->enablePinMode = enablePinMode; + this->enablePin.initPin("stepper enable", enablePin, &this->enablePinMode); // All pins must be 0 for correct hardware startup (e.g. stepper auto-disabling circuit etc.). - palWritePad(this->stepPort, this->stepPin, 0); -#endif - + this->enablePin.setValue(true); // disable stepper + this->stepPin.setValue(false); this->directionPin.setValue(false); this->currentDirection = false; diff --git a/firmware/hw_layer/stepper.h b/firmware/hw_layer/stepper.h index 168e2cebf1..c951add875 100644 --- a/firmware/hw_layer/stepper.h +++ b/firmware/hw_layer/stepper.h @@ -21,7 +21,7 @@ public: int getTargetPosition() const; void setDirection(bool isIncrementing); - OutputPin directionPin; + OutputPin directionPin, stepPin, enablePin; int currentPosition; bool currentDirection; float reactionTime; @@ -29,15 +29,7 @@ public: private: int targetPosition; -#if EFI_PROD_CODE - ioportid_t stepPort; - ioportmask_t stepPin; - - ioportid_t enablePort; - ioportmask_t enablePin; -#endif - - pin_output_mode_e directionPinMode; + pin_output_mode_e directionPinMode, stepPinMode, enablePinMode; THD_WORKING_AREA(stThreadStack, UTILITY_THREAD_STACK_SIZE); };