From a60bc912052dd88a8628a1e411b27911f6892356 Mon Sep 17 00:00:00 2001 From: Mimoja Date: Mon, 3 Jul 2023 21:17:24 +0200 Subject: [PATCH] ESP: Fix Powermanagement for non-configured POWER We currently have two ways of undefining the PowerPin in software for the ESP32 For once we can set POWER_NO_SOFT_POWER which will lead to some helpful screen messages _but_ will still require the definition of the FLASHER_AP_POWER pin. The second way (which was replaced with POWER_NO_SOFT_POWER) allowed to set the powerPin array to {-1} to indicate that the pin is undefined. This would then be handled in pretty far down the call stack right before toggeling the pin in powerControl: ``` void powerControl(bool powerState, uint8_t* pin, uint8_t pincount) { if (pin[0] == -1) return; ``` This however is leading to an error: pin is n pointer (here an array with additional length) to an uint8_t which can never be -1. Therefore the check wont ever be reached and an invalid gpio number is passed to the gpio hal. This caused the error: ``` E (XXXX) gpio: gpio_set_level(226): GPIO output gpio_num error ``` to show up in the logs. We are now proposing three seperate changes to mitigate this behaviour: 1) We are addng the POWER_NO_SOFT_POWER to the Wemos and the M5Stack boards default configs for new users to find. 2) We are replacing the pin[0] check in powerControl with a check on the pincount and a nullptr check as the zbs_interface->begin() allows for no powerPin to be passed in which case we are dereferencing a nullptr. 3) ZBS_interface->begin() is losing its powerPin related default configurations and sanity checks are put in place before every calling. We have opted to do 3) in this way and not adding the checked into the ZBS_interface->begin() function which is also passed an uint8_t pointer to keep the API of the class stable for reuse in other projects. Changing the interface however would be ideal here Signed-off-by: Mimoja --- ESP32_AP-Flasher/include/zbs_interface.h | 2 +- ESP32_AP-Flasher/platformio.ini | 5 +++++ ESP32_AP-Flasher/src/flasher.cpp | 17 ++++++++++++++--- ESP32_AP-Flasher/src/powermgt.cpp | 3 ++- ESP32_AP-Flasher/src/serialap.cpp | 7 +++++-- ESP32_AP-Flasher/src/usbflasher.cpp | 10 +++++++--- ESP32_AP-Flasher/src/zbs_interface.cpp | 5 ++++- 7 files changed, 38 insertions(+), 11 deletions(-) diff --git a/ESP32_AP-Flasher/include/zbs_interface.h b/ESP32_AP-Flasher/include/zbs_interface.h index 8b74965e..2bdaafa4 100644 --- a/ESP32_AP-Flasher/include/zbs_interface.h +++ b/ESP32_AP-Flasher/include/zbs_interface.h @@ -9,7 +9,7 @@ class ZBS_interface { public: - uint8_t begin(uint8_t SS, uint8_t CLK, uint8_t MOSI, uint8_t MISO, uint8_t RESET, uint8_t* POWER = nullptr, uint8_t powerPins = 1, uint32_t spi_speed = 8000000); + uint8_t begin(uint8_t SS, uint8_t CLK, uint8_t MOSI, uint8_t MISO, uint8_t RESET, uint8_t* POWER, uint8_t powerPins, uint32_t spi_speed = 8000000); void setSpeed(uint32_t speed); void set_power(uint8_t state); void enable_debug(); diff --git a/ESP32_AP-Flasher/platformio.ini b/ESP32_AP-Flasher/platformio.ini index 556103d0..7e8db8a9 100644 --- a/ESP32_AP-Flasher/platformio.ini +++ b/ESP32_AP-Flasher/platformio.ini @@ -214,6 +214,9 @@ build_flags = -D BUILD_ENV_NAME=$PIOENV -D BUILD_TIME=$UNIX_TIME -D CORE_DEBUG_LEVEL=0 + + -D POWER_NO_SOFT_POWER + -D FLASHER_AP_SS=5 -D FLASHER_AP_CLK=18 -D FLASHER_AP_MOSI=23 @@ -243,6 +246,8 @@ build_flags = -D BUILD_ENV_NAME=$PIOENV -D BUILD_TIME=$UNIX_TIME -D CORE_DEBUG_LEVEL=0 + + -D POWER_NO_SOFT_POWER -D HAS_SDCARD -D USE_SOFTSPI -D SD_CARD_SS=4 diff --git a/ESP32_AP-Flasher/src/flasher.cpp b/ESP32_AP-Flasher/src/flasher.cpp index adaa7ea6..b4a218e8 100644 --- a/ESP32_AP-Flasher/src/flasher.cpp +++ b/ESP32_AP-Flasher/src/flasher.cpp @@ -117,22 +117,33 @@ flasher::~flasher() { Storage.begin(); } +static uint8_t validatePowerPinCount(int8_t *powerPin, uint8_t pinCount) { + if (pinCount > 0) { + pinCount = powerPinsAP[0] != -1 ? pinCount : 0; + } + return pinCount; +} + #ifndef FLASHER_AP_SPEED #define FLASHER_AP_SPEED 4000000 #endif bool flasher::connectTag(uint8_t port) { bool result; + uint8_t power_pins = 0; switch (port) { case 0: - result = zbs->begin(FLASHER_AP_SS, FLASHER_AP_CLK, FLASHER_AP_MOSI, FLASHER_AP_MISO, FLASHER_AP_RESET, (uint8_t *)powerPinsAP, sizeof(powerPinsAP), FLASHER_AP_SPEED); + power_pins = validatePowerPinCount(powerPinsAP, sizeof(powerPinsAP)); + result = zbs->begin(FLASHER_AP_SS, FLASHER_AP_CLK, FLASHER_AP_MOSI, FLASHER_AP_MISO, FLASHER_AP_RESET, (uint8_t *)powerPinsAP, power_pins, FLASHER_AP_SPEED); break; #ifdef OPENEPAPERLINK_PCB case 1: - result = zbs->begin(FLASHER_EXT_SS, FLASHER_EXT_CLK, FLASHER_EXT_MOSI, FLASHER_EXT_MISO, FLASHER_EXT_RESET, (uint8_t *)powerPinsExt, sizeof(powerPinsExt), FLASHER_AP_SPEED); + power_pins = validatePowerPinCount(powerPinsExt, sizeof(powerPinsExt)); + result = zbs->begin(FLASHER_EXT_SS, FLASHER_EXT_CLK, FLASHER_EXT_MOSI, FLASHER_EXT_MISO, FLASHER_EXT_RESET, (uint8_t *)powerPinsExt, power_pins, FLASHER_AP_SPEED); break; case 2: - result = zbs->begin(FLASHER_ALT_SS, FLASHER_ALT_CLK, FLASHER_ALT_MOSI, FLASHER_ALT_MISO, FLASHER_ALT_RESET, (uint8_t *)powerPinsAlt, sizeof(powerPinsAlt), FLASHER_AP_SPEED); + power_pins = validatePowerPinCount(powerPinsAlt, sizeof(powerPinsAlt)); + result = zbs->begin(FLASHER_ALT_SS, FLASHER_ALT_CLK, FLASHER_ALT_MOSI, FLASHER_ALT_MISO, FLASHER_ALT_RESET, (uint8_t *)powerPinsAlt, power_pins, FLASHER_AP_SPEED); break; #endif default: diff --git a/ESP32_AP-Flasher/src/powermgt.cpp b/ESP32_AP-Flasher/src/powermgt.cpp index 8bbe8549..dba55014 100644 --- a/ESP32_AP-Flasher/src/powermgt.cpp +++ b/ESP32_AP-Flasher/src/powermgt.cpp @@ -75,7 +75,8 @@ void rampTagPower(uint8_t* pin, bool up) { } void powerControl(bool powerState, uint8_t* pin, uint8_t pincount) { - if (pin[0] == -1) return; + if (pincount == 0) return; + if (pin == nullptr) return; #ifdef POWER_RAMPING if (powerState == true) { diff --git a/ESP32_AP-Flasher/src/serialap.cpp b/ESP32_AP-Flasher/src/serialap.cpp index e70c821a..23e224c1 100644 --- a/ESP32_AP-Flasher/src/serialap.cpp +++ b/ESP32_AP-Flasher/src/serialap.cpp @@ -134,12 +134,15 @@ void APEnterEarlyReset() { // Reset the tag void APTagReset() { + uint8_t powerPins = sizeof(APpowerPins); + if (powerPins > 0 && APpowerPins[0] == -1) + powerPins = 0; pinMode(AP_RESET_PIN, OUTPUT); digitalWrite(AP_RESET_PIN, LOW); vTaskDelay(50 / portTICK_PERIOD_MS); - powerControl(false, (uint8_t*)APpowerPins, sizeof(APpowerPins)); + powerControl(false, (uint8_t*)APpowerPins, powerPins); vTaskDelay(300 / portTICK_PERIOD_MS); - powerControl(true, (uint8_t*)APpowerPins, sizeof(APpowerPins)); + powerControl(true, (uint8_t*)APpowerPins, powerPins); vTaskDelay(100 / portTICK_PERIOD_MS); digitalWrite(AP_RESET_PIN, HIGH); vTaskDelay(100 / portTICK_PERIOD_MS); diff --git a/ESP32_AP-Flasher/src/usbflasher.cpp b/ESP32_AP-Flasher/src/usbflasher.cpp index f3de81bd..0fc9f3cc 100644 --- a/ESP32_AP-Flasher/src/usbflasher.cpp +++ b/ESP32_AP-Flasher/src/usbflasher.cpp @@ -239,6 +239,7 @@ void processFlasherCommand(struct flasherCommand* cmd) { uint8_t* tempbuffer; uint8_t temp_buff[16]; uint32_t spi_speed = 0; + uint8_t powerPinCount = 1; static uint32_t curspeed = 0; switch (cmd->command) { @@ -267,14 +268,17 @@ void processFlasherCommand(struct flasherCommand* cmd) { curspeed = spi_speed; if (cmd->data[0] & 2) { - temp_buff[0] = zbs->begin(FLASHER_AP_SS, FLASHER_AP_CLK, FLASHER_AP_MOSI, FLASHER_AP_MISO, FLASHER_AP_RESET, (uint8_t*)powerPins, spi_speed); + powerPinCount = powerPins[0] != -1 ? sizeof(powerPins) : 0; + temp_buff[0] = zbs->begin(FLASHER_AP_SS, FLASHER_AP_CLK, FLASHER_AP_MOSI, FLASHER_AP_MISO, FLASHER_AP_RESET, (uint8_t*)powerPins, powerPinCount, spi_speed); } else if (cmd->data[0] & 4) { #ifdef OPENEPAPERLINK_PCB - temp_buff[0] = zbs->begin(FLASHER_ALT_SS, FLASHER_ALT_CLK, FLASHER_ALT_MOSI, FLASHER_ALT_MISO, FLASHER_ALT_RESET, (uint8_t*)powerPins3, spi_speed); + powerPinCount = powerPins3[0] != -1 ? sizeof(powerPins3) : 0; + temp_buff[0] = zbs->begin(FLASHER_ALT_SS, FLASHER_ALT_CLK, FLASHER_ALT_MOSI, FLASHER_ALT_MISO, FLASHER_ALT_RESET, (uint8_t*)powerPins3, powerPinCount, spi_speed); #endif } else { #ifdef OPENEPAPERLINK_PCB - temp_buff[0] = zbs->begin(FLASHER_EXT_SS, FLASHER_EXT_CLK, FLASHER_EXT_MOSI, FLASHER_EXT_MISO, FLASHER_EXT_RESET, (uint8_t*)powerPins2, spi_speed); + powerPinCount = powerPins2[0] != -1 ? sizeof(powerPins2) : 0; + temp_buff[0] = zbs->begin(FLASHER_EXT_SS, FLASHER_EXT_CLK, FLASHER_EXT_MOSI, FLASHER_EXT_MISO, FLASHER_EXT_RESET, (uint8_t*)powerPins2, powerPinCount, spi_speed); #endif } sendFlasherAnswer(cmd->command, temp_buff, 1); diff --git a/ESP32_AP-Flasher/src/zbs_interface.cpp b/ESP32_AP-Flasher/src/zbs_interface.cpp index 13d5352b..e835a520 100644 --- a/ESP32_AP-Flasher/src/zbs_interface.cpp +++ b/ESP32_AP-Flasher/src/zbs_interface.cpp @@ -19,7 +19,10 @@ uint8_t ZBS_interface::begin(uint8_t SS, uint8_t CLK, uint8_t MOSI, uint8_t MISO _MOSI_PIN = MOSI; _MISO_PIN = MISO; _RESET_PIN = RESET; - _POWER_PIN = POWER; + if (powerPins > 0) + _POWER_PIN = POWER; + else + _POWER_PIN = nullptr; pinMode(_SS_PIN, OUTPUT); pinMode(_RESET_PIN, OUTPUT); digitalWrite(_SS_PIN, HIGH);