From 5443618037bdac687664d381594f57a9d1be7808 Mon Sep 17 00:00:00 2001 From: tyeth Date: Thu, 1 May 2025 00:09:02 +0100 Subject: [PATCH 1/5] Always publish initial pin value if on_change (so we have a previous value on boot) --- .../digitalIO/Wippersnapper_DigitalGPIO.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp index f3ab478d9..1545b5361 100644 --- a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp +++ b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp @@ -190,7 +190,11 @@ void Wippersnapper_DigitalGPIO::deinitDigitalPin( /********************************************************************/ int Wippersnapper_DigitalGPIO::digitalReadSvc(int pinName) { // Service using arduino `digitalRead` + WS_DEBUG_PRINT("\tDioPinRead: "); + WS_DEBUG_PRINT(pinName); int pinVal = digitalRead(pinName); + WS_DEBUG_PRINT("="); + WS_DEBUG_PRINT(pinVal); return pinVal; } @@ -284,8 +288,8 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { } else if (_digital_input_pins[i].period == 0L) { // read pin int pinVal = digitalReadSvc(_digital_input_pins[i].pinName); - // only send on-change - if (pinVal != _digital_input_pins[i].prvPinVal) { + // only send on-change, but we don't know initial state of feed (prvPinVal at boot) + if (_digital_input_pins[i].prvPeriod <= 0 || pinVal != _digital_input_pins[i].prvPinVal) { WS_DEBUG_PRINT("Executing state-based event on D"); WS_DEBUG_PRINTLN(_digital_input_pins[i].pinName); @@ -324,6 +328,10 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { // reset the digital pin _digital_input_pins[i].prvPeriod = curTime; + } else { + WS_DEBUG_PRINT("Dio: No change on "); + WS_DEBUG_PRINT(_digital_input_pins[i].pinName); + WS_DEBUG_PRINTLN(", skipping..."); } } } From 2f0c25b8037153f1c53ed5c91c2c8c53c1ab523c Mon Sep 17 00:00:00 2001 From: tyeth Date: Thu, 1 May 2025 13:44:14 +0100 Subject: [PATCH 2/5] Show logging less often --- .../digitalIO/Wippersnapper_DigitalGPIO.cpp | 43 ++++++++++++++----- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp index 1545b5361..36c86e21a 100644 --- a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp +++ b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp @@ -190,11 +190,26 @@ void Wippersnapper_DigitalGPIO::deinitDigitalPin( /********************************************************************/ int Wippersnapper_DigitalGPIO::digitalReadSvc(int pinName) { // Service using arduino `digitalRead` - WS_DEBUG_PRINT("\tDioPinRead: "); - WS_DEBUG_PRINT(pinName); int pinVal = digitalRead(pinName); - WS_DEBUG_PRINT("="); - WS_DEBUG_PRINT(pinVal); + // how do i check the digital IO pull up state for one of the + // _digital_input_pins, because I'm experiencing issues that resemble the + // situation where the initially set pull up value is no longer in effect + // For ESP32 + // #if defined(ESP32) + // #include "driver/gpio.h" + // #include "esp_err.h" + // #include "esp_log.h" + // #include "esp_system.h" + // #include "esp32-hal-log.h" + // #include "esp32-hal-gpio.h" + // gpio_pull_mode_t pull_mode; + // // can't find in idf, but merged issue + // https://github.com/espressif/esp-idf/issues/12176 + // esp_err_t result = gpio_get_pull_mode((gpio_num_t)pinName, &pull_mode); + // if (result == ESP_OK) { + // return (pull_mode == GPIO_PULLUP_ONLY); + // } + // #endif return pinVal; } @@ -288,8 +303,16 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { } else if (_digital_input_pins[i].period == 0L) { // read pin int pinVal = digitalReadSvc(_digital_input_pins[i].pinName); - // only send on-change, but we don't know initial state of feed (prvPinVal at boot) - if (_digital_input_pins[i].prvPeriod <= 0 || pinVal != _digital_input_pins[i].prvPinVal) { + if (curTime - _digital_input_pins[i].prvPeriod > 500) { + WS_DEBUG_PRINT("D"); + WS_DEBUG_PRINT(_digital_input_pins[i].pinName); + WS_DEBUG_PRINT(" is "); + WS_DEBUG_PRINT(pinVal); + } + // only send on-change, but we don't know initial state of feed + // (prvPinVal at boot) + if (_digital_input_pins[i].prvPeriod <= 0 || + pinVal != _digital_input_pins[i].prvPinVal) { WS_DEBUG_PRINT("Executing state-based event on D"); WS_DEBUG_PRINTLN(_digital_input_pins[i].pinName); @@ -328,10 +351,10 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { // reset the digital pin _digital_input_pins[i].prvPeriod = curTime; - } else { - WS_DEBUG_PRINT("Dio: No change on "); - WS_DEBUG_PRINT(_digital_input_pins[i].pinName); - WS_DEBUG_PRINTLN(", skipping..."); + // } else { + // WS_DEBUG_PRINT("Dio: No change on "); + // WS_DEBUG_PRINT(_digital_input_pins[i].pinName); + // WS_DEBUG_PRINTLN(", skipping..."); } } } From dc4f84a71bbcfe29d810c21f3c0452932baa5709 Mon Sep 17 00:00:00 2001 From: tyeth Date: Thu, 1 May 2025 15:06:09 +0100 Subject: [PATCH 3/5] Add prvPinVal based on pull direction + add pull down for esp32 --- .../digitalIO/Wippersnapper_DigitalGPIO.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp index 36c86e21a..2541752c9 100644 --- a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp +++ b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp @@ -99,6 +99,12 @@ void Wippersnapper_DigitalGPIO::initDigitalPin( if (pull == wippersnapper_pin_v1_ConfigurePinRequest_Pull_PULL_UP) { WS_DEBUG_PRINTLN("with internal pull-up enabled"); pinMode(pinName, INPUT_PULLUP); +#ifdef INPUT_PULLDOWN + } else if (pull == + wippersnapper_pin_v1_ConfigurePinRequest_Pull_PULL_DOWN) { + WS_DEBUG_PRINTLN("with internal pull-down enabled"); + pinMode(pinName, INPUT_PULLDOWN); +#endif } else { pinMode(pinName, INPUT); WS_DEBUG_PRINT("\n"); @@ -125,6 +131,11 @@ void Wippersnapper_DigitalGPIO::initDigitalPin( _digital_input_pins[i].pinName = pinName; _digital_input_pins[i].period = periodMs; _digital_input_pins[i].prvPeriod = curTime - periodMs; + if (pull == wippersnapper_pin_v1_ConfigurePinRequest_Pull_PULL_UP) { + _digital_input_pins[i].prvPinVal = HIGH; + } else { + _digital_input_pins[i].prvPinVal = LOW; + } break; } } @@ -311,8 +322,7 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { } // only send on-change, but we don't know initial state of feed // (prvPinVal at boot) - if (_digital_input_pins[i].prvPeriod <= 0 || - pinVal != _digital_input_pins[i].prvPinVal) { + if (pinVal != _digital_input_pins[i].prvPinVal) { WS_DEBUG_PRINT("Executing state-based event on D"); WS_DEBUG_PRINTLN(_digital_input_pins[i].pinName); From c76a4060a766049c7c9d6f7f1e711a90ee2b5e6e Mon Sep 17 00:00:00 2001 From: tyeth Date: Tue, 6 May 2025 21:41:36 +0100 Subject: [PATCH 4/5] Log GPIO Config for RP2040 --- .../digitalIO/Wippersnapper_DigitalGPIO.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp index 2541752c9..c77dde202 100644 --- a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp +++ b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp @@ -45,6 +45,26 @@ Wippersnapper_DigitalGPIO::~Wippersnapper_DigitalGPIO() { delete _digital_input_pins; } + +#ifdef ARDUINO_ARCH_RP2040 + void rp2040_dump_gpio_config(void) { + WS_PRINTER.printf("GPIO | FUNC | DIR | OUT | IN | PULL-UP | PULL-DOWN | SLEW\n"); + WS_PRINTER.printf("-----+------+-----+-----+-----+---------+-----------+-----\n"); + + for (int gpio = 0; gpio < NUM_BANK0_GPIOS; gpio++) { + WS_PRINTER.printf(" %2d | %2d | %s | %d | %d | %d | %d | %d\n", + gpio, + gpio_get_function(gpio), + gpio_get_dir(gpio) ? "OUT" : "IN ", + gpio_get_out_level(gpio), + gpio_get(gpio), + gpio_is_pulled_up(gpio), + gpio_is_pulled_down(gpio), + gpio_get_slew_rate(gpio)); + } + } +#endif + /*******************************************************************************************************************************/ /*! @brief Configures a digital pin to behave as an input or an output. @@ -140,6 +160,10 @@ void Wippersnapper_DigitalGPIO::initDigitalPin( } } +#ifdef ARDUINO_ARCH_RP2040 + rp2040_dump_gpio_config(); +#endif + } else { WS_DEBUG_PRINTLN("ERROR: Invalid digital pin direction!"); } @@ -183,6 +207,10 @@ void Wippersnapper_DigitalGPIO::deinitDigitalPin( itoa(pinName, cstr, 10); pinMode(pinName, INPUT); // hi-z +#ifdef ARDUINO_ARCH_RP2040 + rp2040_dump_gpio_config(); +#endif + // if prv. in-use by DIO, release pin back to application #ifdef STATUS_LED_PIN if (pinName == STATUS_LED_PIN) From 52fa2d6e61adb2fe6d56624c5ac22c28efa292b3 Mon Sep 17 00:00:00 2001 From: tyeth Date: Tue, 6 May 2025 21:42:51 +0100 Subject: [PATCH 5/5] Don't log pinState each 0.5s --- src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp index c77dde202..b1212eb9b 100644 --- a/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp +++ b/src/components/digitalIO/Wippersnapper_DigitalGPIO.cpp @@ -342,12 +342,6 @@ void Wippersnapper_DigitalGPIO::processDigitalInputs() { } else if (_digital_input_pins[i].period == 0L) { // read pin int pinVal = digitalReadSvc(_digital_input_pins[i].pinName); - if (curTime - _digital_input_pins[i].prvPeriod > 500) { - WS_DEBUG_PRINT("D"); - WS_DEBUG_PRINT(_digital_input_pins[i].pinName); - WS_DEBUG_PRINT(" is "); - WS_DEBUG_PRINT(pinVal); - } // only send on-change, but we don't know initial state of feed // (prvPinVal at boot) if (pinVal != _digital_input_pins[i].prvPinVal) {