From 070475f1ffbbc8149ae9a5a8634ef9c92b790c2e Mon Sep 17 00:00:00 2001 From: rusefi Date: Fri, 21 Apr 2017 15:14:37 -0400 Subject: [PATCH] refactoring - reducing GPIO complexity --- firmware/config/stm32f4ems/efifeatures.h | 2 +- firmware/controllers/system/efiGpio.cpp | 167 +++++++++++++++--- firmware/controllers/system/efiGpio.h | 33 +++- .../trigger/main_trigger_callback.cpp | 4 +- firmware/hw_layer/io_pins.cpp | 98 +--------- firmware/hw_layer/io_pins.h | 27 --- simulator/simulator/efifeatures.h | 2 +- unit_tests/efifeatures.h | 2 +- 8 files changed, 175 insertions(+), 160 deletions(-) diff --git a/firmware/config/stm32f4ems/efifeatures.h b/firmware/config/stm32f4ems/efifeatures.h index 22b676b13b..91fee21780 100644 --- a/firmware/config/stm32f4ems/efifeatures.h +++ b/firmware/config/stm32f4ems/efifeatures.h @@ -10,7 +10,7 @@ #ifndef EFIFEATURES_H_ #define EFIFEATURES_H_ -#define EFI_GPIO TRUE +#define EFI_GPIO_HARDWARE TRUE #define EFI_FSIO TRUE diff --git a/firmware/controllers/system/efiGpio.cpp b/firmware/controllers/system/efiGpio.cpp index d55556eb6a..6e1fe6f40e 100644 --- a/firmware/controllers/system/efiGpio.cpp +++ b/firmware/controllers/system/efiGpio.cpp @@ -7,28 +7,18 @@ */ #include "main.h" -#if EFI_GPIO || defined(__DOXYGEN__) +#include "engine.h" #include "efiGpio.h" + +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) #include "io_pins.h" +// todo: maybe remove this from here? pin_repository.h has a lot of stuff it should not have +#include "pin_repository.h" +#endif /* EFI_GPIO_HARDWARE */ -pin_output_mode_e OUTPUT_MODE_DEFAULT = OM_DEFAULT; +EXTERN_ENGINE; -// todo: clean this mess, this should become 'static'/private -EnginePins enginePins; -extern LoggingWithStorage sharedLogger; - -NamedOutputPin::NamedOutputPin() : OutputPin() { - name = NULL; -} - -NamedOutputPin::NamedOutputPin(const char *name) : OutputPin() { - this->name = name; -} - -InjectorOutputPin::InjectorOutputPin() : NamedOutputPin() { - reset(); - injectorIndex = -1; -} +static pin_output_mode_e OUTPUT_MODE_DEFAULT = OM_DEFAULT; static const char *sparkNames[IGNITION_PIN_COUNT] = { "c1", "c2", "c3", "c4", "c5", "c6", "c7", "c8", "c9", "cA", "cB", "cD"}; @@ -70,6 +60,26 @@ void EnginePins::reset() { } } + + +// todo: clean this mess, this should become 'static'/private +EnginePins enginePins; +extern LoggingWithStorage sharedLogger; + +NamedOutputPin::NamedOutputPin() : OutputPin() { + name = NULL; +} + +NamedOutputPin::NamedOutputPin(const char *name) : OutputPin() { + this->name = name; +} + +InjectorOutputPin::InjectorOutputPin() : NamedOutputPin() { + reset(); + injectorIndex = -1; +} + + bool NamedOutputPin::stop() { #if EFI_PROD_CODE || defined(__DOXYGEN__) if (isInitialized() && getLogicValue()) { @@ -77,7 +87,7 @@ bool NamedOutputPin::stop() { scheduleMsg(&sharedLogger, "turning off %s", name); return true; } -#endif +#endif /* EFI_PROD_CODE */ return false; } @@ -100,19 +110,19 @@ void IgnitionOutputPin::reset() { OutputPin::OutputPin() { modePtr = &OUTPUT_MODE_DEFAULT; -#if EFI_PROD_CODE || defined(__DOXYGEN__) +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) port = NULL; pin = 0; -#endif +#endif /* EFI_GPIO_HARDWARE */ currentLogicValue = INITIAL_PIN_STATE; } bool OutputPin::isInitialized() { -#if EFI_PROD_CODE || defined(__DOXYGEN__) +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) return port != NULL; -#else +#else /* EFI_GPIO_HARDWARE */ return false; -#endif +#endif /* EFI_GPIO_HARDWARE */ } void OutputPin::setValue(int logicValue) { @@ -124,9 +134,9 @@ bool OutputPin::getLogicValue() { } void OutputPin::unregister() { -#if EFI_PROD_CODE || defined(__DOXYGEN__) +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) port = NULL; -#endif +#endif /* EFI_PROD_CODE */ } void OutputPin::setDefaultPinState(pin_output_mode_e *outputMode) { @@ -136,4 +146,107 @@ void OutputPin::setDefaultPinState(pin_output_mode_e *outputMode) { setValue(false); // initial state } -#endif /* EFI_GPIO */ +pin_output_mode_e DEFAULT_OUTPUT = OM_DEFAULT; +pin_output_mode_e OPENDRAIN_OUTPUT = OM_OPENDRAIN; + +void initOutputPins(void) { +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) + /** + * want to make sure it's all zeros so that we can compare in initOutputPinExt() method + */ +// todo: it's too late to clear now? this breaks default status LEDs +// todo: fix this? +// memset(&outputs, 0, sizeof(outputs)); +// outputPinRegister("ext led 1", LED_EXT_1, EXTRA_LED_1_PORT, EXTRA_LED_1_PIN); +// outputPinRegister("ext led 2", LED_EXT_2, EXTRA_LED_2_PORT, EXTRA_LED_2_PIN); +// outputPinRegister("ext led 3", LED_EXT_3, EXTRA_LED_2_PORT, EXTRA_LED_3_PIN); +// outputPinRegister("alive1", LED_DEBUG, GPIOD, 6); + +#if HAL_USE_SPI || defined(__DOXYGEN__) + outputPinRegisterExt2("spi CS5", &enginePins.sdCsPin, boardConfiguration->sdCardCsPin, &DEFAULT_OUTPUT); +#endif /* HAL_USE_SPI */ + + // todo: should we move this code closer to the fuel pump logic? + outputPinRegisterExt2("fuel pump relay", &enginePins.fuelPumpRelay, boardConfiguration->fuelPumpPin, &DEFAULT_OUTPUT); + + outputPinRegisterExt2("main relay", &enginePins.mainRelay, boardConfiguration->mainRelayPin, &boardConfiguration->mainRelayPinMode); + + outputPinRegisterExt2("fan relay", &enginePins.fanRelay, boardConfiguration->fanPin, &DEFAULT_OUTPUT); + outputPinRegisterExt2("o2 heater", &enginePins.o2heater, boardConfiguration->o2heaterPin, &DEFAULT_OUTPUT); + outputPinRegisterExt2("A/C relay", &enginePins.acRelay, boardConfiguration->acRelayPin, &boardConfiguration->acRelayPinMode); + + // digit 1 + /* + ledRegister(LED_HUGE_0, GPIOB, 2); + ledRegister(LED_HUGE_1, GPIOE, 7); + ledRegister(LED_HUGE_2, GPIOE, 8); + ledRegister(LED_HUGE_3, GPIOE, 9); + ledRegister(LED_HUGE_4, GPIOE, 10); + ledRegister(LED_HUGE_5, GPIOE, 11); + ledRegister(LED_HUGE_6, GPIOE, 12); + + // digit 2 + ledRegister(LED_HUGE_7, GPIOE, 13); + ledRegister(LED_HUGE_8, GPIOE, 14); + ledRegister(LED_HUGE_9, GPIOE, 15); + ledRegister(LED_HUGE_10, GPIOB, 10); + ledRegister(LED_HUGE_11, GPIOB, 11); + ledRegister(LED_HUGE_12, GPIOB, 12); + ledRegister(LED_HUGE_13, GPIOB, 13); + + // digit 3 + ledRegister(LED_HUGE_14, GPIOE, 0); + ledRegister(LED_HUGE_15, GPIOE, 2); + ledRegister(LED_HUGE_16, GPIOE, 4); + ledRegister(LED_HUGE_17, GPIOE, 6); + ledRegister(LED_HUGE_18, GPIOE, 5); + ledRegister(LED_HUGE_19, GPIOE, 3); + ledRegister(LED_HUGE_20, GPIOE, 1); + */ +#endif /* EFI_GPIO_HARDWARE */ +} + +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) + +/** + * This method is used for digital GPIO pins only, for peripheral pins see mySetPadMode + */ +static void outputPinRegisterExt(const char *msg, OutputPin *output, ioportid_t port, uint32_t pin, + pin_output_mode_e *outputMode) { +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) + if (port == GPIO_NULL) { + // that's for GRIO_NONE + output->port = port; + return; + } + + assertOMode(*outputMode); + iomode_t mode = (*outputMode == OM_DEFAULT || *outputMode == OM_INVERTED) ? + PAL_MODE_OUTPUT_PUSHPULL : PAL_MODE_OUTPUT_OPENDRAIN; + + initOutputPinExt(msg, output, port, pin, mode); + + output->setDefaultPinState(outputMode); +#endif /* EFI_GPIO_HARDWARE */ +} + +void outputPinRegister(const char *msg, OutputPin *output, ioportid_t port, uint32_t pin) { + outputPinRegisterExt(msg, output, port, pin, &DEFAULT_OUTPUT); +} + + +void initPrimaryPins(void) { + outputPinRegister("led: ERROR status", &enginePins.errorLedPin, LED_ERROR_PORT, LED_ERROR_PIN); +} + +void outputPinRegisterExt2(const char *msg, OutputPin *output, brain_pin_e brainPin, pin_output_mode_e *outputMode) { + if (brainPin == GPIO_UNASSIGNED) + return; + ioportid_t hwPort = getHwPort(brainPin); + int hwPin = getHwPin(brainPin); + + outputPinRegisterExt(msg, output, hwPort, hwPin, outputMode); +} + + +#endif /* EFI_GPIO_HARDWARE */ diff --git a/firmware/controllers/system/efiGpio.h b/firmware/controllers/system/efiGpio.h index 991186271e..215310292d 100644 --- a/firmware/controllers/system/efiGpio.h +++ b/firmware/controllers/system/efiGpio.h @@ -15,11 +15,36 @@ void initPrimaryPins(void); void initOutputPins(void); -#if EFI_GPIO || defined(__DOXYGEN__) +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) void turnAllPinsOff(void); -#else /* EFI_GPIO */ +#else /* EFI_GPIO_HARDWARE */ #define turnAllPinsOff() {} -#endif /* EFI_GPIO */ +#endif /* EFI_GPIO_HARDWARE */ + +/** + * @brief Single output pin reference and state + */ +class OutputPin { +public: + OutputPin(); + bool isInitialized(); + void setValue(int logicValue); + void setDefaultPinState(pin_output_mode_e *defaultState); + bool getLogicValue(); + void unregister(); +#if EFI_PROD_CODE || defined(__DOXYGEN__) + ioportid_t port; + uint8_t pin; +#endif /* EFI_PROD_CODE */ + int8_t currentLogicValue; + // 4 byte pointer is a bit of a memory waste here + pin_output_mode_e *modePtr; + /** + * we track current pin status so that we do not touch the actual hardware if we want to write new pin bit + * which is same as current pin value. This maybe helps in case of status leds, but maybe it's a total over-engineering + */ +}; + class NamedOutputPin : public OutputPin { public: @@ -160,7 +185,7 @@ public: void turnPinHigh(NamedOutputPin *output); void turnPinLow(NamedOutputPin *output); -#if EFI_PROD_CODE +#if EFI_PROD_CODE || defined(__DOXYGEN__) void initOutputPin(const char *msg, OutputPin *outputPin, ioportid_t port, uint32_t pinNumber); void initOutputPinExt(const char *msg, OutputPin *outputPin, ioportid_t port, uint32_t pinNumber, iomode_t mode); diff --git a/firmware/controllers/trigger/main_trigger_callback.cpp b/firmware/controllers/trigger/main_trigger_callback.cpp index 9b8379e3a8..02a1c4974f 100644 --- a/firmware/controllers/trigger/main_trigger_callback.cpp +++ b/firmware/controllers/trigger/main_trigger_callback.cpp @@ -349,7 +349,7 @@ static ALWAYS_INLINE void handleFuelInjectionEvent(int injEventIndex, InjectionE * */ static void scheduleOutput2(OutputSignalPair *pair, efitimeus_t nowUs, float delayUs, float durationUs, InjectorOutputPin *output) { -#if EFI_GPIO || defined(__DOXYGEN__) +#if EFI_GPIO_HARDWARE || defined(__DOXYGEN__) #if EFI_UNIT_TEST || defined(__DOXYGEN__) printf("scheduling output %s\r\n", output->name); @@ -366,7 +366,7 @@ static void scheduleOutput2(OutputSignalPair *pair, efitimeus_t nowUs, float del efitimeus_t turnOffTime = nowUs + (int) (delayUs + durationUs); seScheduleByTime("out down", sDown, turnOffTime, (schfunc_t) &seTurnPinLow, pair); -#endif /* EFI_GPIO */ +#endif /* EFI_GPIO_HARDWARE */ } static void fuelClosedLoopCorrection(DECLARE_ENGINE_PARAMETER_F) { diff --git a/firmware/hw_layer/io_pins.cpp b/firmware/hw_layer/io_pins.cpp index f66e0cbcba..ffd6dd529f 100644 --- a/firmware/hw_layer/io_pins.cpp +++ b/firmware/hw_layer/io_pins.cpp @@ -36,31 +36,7 @@ static ioportid_t PORTS[] = { GPIOA, GPIOB, GPIOC, GPIOD, GPIOE, GPIOF, GPIOG, G static ioportid_t PORTS[] = { GPIOA, GPIOB, GPIOC, GPIOD, GPIOF}; #endif -pin_output_mode_e DEFAULT_OUTPUT = OM_DEFAULT; -pin_output_mode_e OPENDRAIN_OUTPUT = OM_OPENDRAIN; -/** - * This method is used for digital GPIO pins only, for peripheral pins see mySetPadMode - */ -static void outputPinRegisterExt(const char *msg, OutputPin *output, ioportid_t port, uint32_t pin, - pin_output_mode_e *outputMode) { -#if EFI_GPIO - if (port == GPIO_NULL) { - // that's for GRIO_NONE - output->port = port; - return; - } - - assertOMode(*outputMode); - iomode_t mode = (*outputMode == OM_DEFAULT || *outputMode == OM_INVERTED) ? - PAL_MODE_OUTPUT_PUSHPULL : - PAL_MODE_OUTPUT_OPENDRAIN; - - initOutputPinExt(msg, output, port, pin, mode); - - output->setDefaultPinState(outputMode); -#endif -} ioportid_t getHwPort(brain_pin_e brainPin) { if (brainPin == GPIO_UNASSIGNED) @@ -82,78 +58,6 @@ ioportmask_t getHwPin(brain_pin_e brainPin) { return brainPin % PORT_SIZE; } -void outputPinRegisterExt2(const char *msg, OutputPin *output, brain_pin_e brainPin, pin_output_mode_e *outputMode) { - if (brainPin == GPIO_UNASSIGNED) - return; - ioportid_t hwPort = getHwPort(brainPin); - int hwPin = getHwPin(brainPin); - - outputPinRegisterExt(msg, output, hwPort, hwPin, outputMode); -} - -void outputPinRegister(const char *msg, OutputPin *output, ioportid_t port, uint32_t pin) { - outputPinRegisterExt(msg, output, port, pin, &DEFAULT_OUTPUT); -} - - -void initPrimaryPins(void) { - outputPinRegister("led: ERROR status", &enginePins.errorLedPin, LED_ERROR_PORT, LED_ERROR_PIN); -} - -void initOutputPins(void) { - /** - * want to make sure it's all zeros so that we can compare in initOutputPinExt() method - */ -// todo: it's too late to clear now? this breaks default status LEDs -// todo: fix this? -// memset(&outputs, 0, sizeof(outputs)); -// outputPinRegister("ext led 1", LED_EXT_1, EXTRA_LED_1_PORT, EXTRA_LED_1_PIN); -// outputPinRegister("ext led 2", LED_EXT_2, EXTRA_LED_2_PORT, EXTRA_LED_2_PIN); -// outputPinRegister("ext led 3", LED_EXT_3, EXTRA_LED_2_PORT, EXTRA_LED_3_PIN); -// outputPinRegister("alive1", LED_DEBUG, GPIOD, 6); - -#if HAL_USE_SPI || defined(__DOXYGEN__) - outputPinRegisterExt2("spi CS5", &enginePins.sdCsPin, boardConfiguration->sdCardCsPin, &DEFAULT_OUTPUT); -#endif - - // todo: should we move this code closer to the fuel pump logic? - outputPinRegisterExt2("fuel pump relay", &enginePins.fuelPumpRelay, boardConfiguration->fuelPumpPin, &DEFAULT_OUTPUT); - - outputPinRegisterExt2("main relay", &enginePins.mainRelay, boardConfiguration->mainRelayPin, &boardConfiguration->mainRelayPinMode); - - outputPinRegisterExt2("fan relay", &enginePins.fanRelay, boardConfiguration->fanPin, &DEFAULT_OUTPUT); - outputPinRegisterExt2("o2 heater", &enginePins.o2heater, boardConfiguration->o2heaterPin, &DEFAULT_OUTPUT); - outputPinRegisterExt2("A/C relay", &enginePins.acRelay, boardConfiguration->acRelayPin, &boardConfiguration->acRelayPinMode); - - // digit 1 - /* - ledRegister(LED_HUGE_0, GPIOB, 2); - ledRegister(LED_HUGE_1, GPIOE, 7); - ledRegister(LED_HUGE_2, GPIOE, 8); - ledRegister(LED_HUGE_3, GPIOE, 9); - ledRegister(LED_HUGE_4, GPIOE, 10); - ledRegister(LED_HUGE_5, GPIOE, 11); - ledRegister(LED_HUGE_6, GPIOE, 12); - - // digit 2 - ledRegister(LED_HUGE_7, GPIOE, 13); - ledRegister(LED_HUGE_8, GPIOE, 14); - ledRegister(LED_HUGE_9, GPIOE, 15); - ledRegister(LED_HUGE_10, GPIOB, 10); - ledRegister(LED_HUGE_11, GPIOB, 11); - ledRegister(LED_HUGE_12, GPIOB, 12); - ledRegister(LED_HUGE_13, GPIOB, 13); - - // digit 3 - ledRegister(LED_HUGE_14, GPIOE, 0); - ledRegister(LED_HUGE_15, GPIOE, 2); - ledRegister(LED_HUGE_16, GPIOE, 4); - ledRegister(LED_HUGE_17, GPIOE, 6); - ledRegister(LED_HUGE_18, GPIOE, 5); - ledRegister(LED_HUGE_19, GPIOE, 3); - ledRegister(LED_HUGE_20, GPIOE, 1); - */ -} /** * @brief Initialize the hardware output pin while also assigning it a logical name @@ -180,7 +84,7 @@ void initOutputPin(const char *msg, OutputPin *outputPin, ioportid_t port, uint3 } -#if EFI_GPIO +#if EFI_GPIO_HARDWARE /** * This method is part of fatal error handling. diff --git a/firmware/hw_layer/io_pins.h b/firmware/hw_layer/io_pins.h index a7a094e747..da7528ecf7 100644 --- a/firmware/hw_layer/io_pins.h +++ b/firmware/hw_layer/io_pins.h @@ -52,31 +52,4 @@ // LED_HUGE_19, // LED_HUGE_20, -#ifdef __cplusplus - -/** - * @brief Single output pin reference and state - */ -class OutputPin { -public: - OutputPin(); - bool isInitialized(); - void setValue(int logicValue); - void setDefaultPinState(pin_output_mode_e *defaultState); - bool getLogicValue(); - void unregister(); -#if EFI_PROD_CODE || defined(__DOXYGEN__) - ioportid_t port; - uint8_t pin; -#endif /* EFI_PROD_CODE */ - int8_t currentLogicValue; - // 4 byte pointer is a bit of a memory waste here - pin_output_mode_e *modePtr; - /** - * we track current pin status so that we do not touch the actual hardware if we want to write new pin bit - * which is same as current pin value. This maybe helps in case of status leds, but maybe it's a total over-engineering - */ -}; -#endif /* __cplusplus */ - #endif /* IO_PINS_H_ */ diff --git a/simulator/simulator/efifeatures.h b/simulator/simulator/efifeatures.h index 608265594a..e5aa8e4cb0 100644 --- a/simulator/simulator/efifeatures.h +++ b/simulator/simulator/efifeatures.h @@ -14,7 +14,7 @@ #define EFI_PRINTF_FUEL_DETAILS TRUE -#define EFI_GPIO TRUE +#define EFI_GPIO_HARDWARE FALSE #define EFI_FSIO TRUE diff --git a/unit_tests/efifeatures.h b/unit_tests/efifeatures.h index 4c5f414f53..7afcd20723 100644 --- a/unit_tests/efifeatures.h +++ b/unit_tests/efifeatures.h @@ -12,7 +12,7 @@ #define SPARK_EXTREME_LOGGING TRUE -#define EFI_GPIO TRUE +#define EFI_GPIO_HARDWARE FALSE #define EFI_UNIT_TEST TRUE