From 0d4c7ef1d5191e7082cbe107a25cac85949a6130 Mon Sep 17 00:00:00 2001 From: tyeth Date: Tue, 20 Aug 2024 22:37:32 +0100 Subject: [PATCH 1/6] Switch Analog Voltage on esp32 to use analogReadMilliVolts and tweak hysteresis --- .../analogIO/Wippersnapper_AnalogIO.cpp | 69 ++++++++++++++----- .../analogIO/Wippersnapper_AnalogIO.h | 3 +- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index 660208de2..dfb9a48ac 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -87,8 +87,13 @@ void Wippersnapper_AnalogIO::setADCResolution(int resolution) { analogReadResolution(16); _nativeResolution = 12; #elif defined(ARDUINO_ARCH_ESP32) - scaleAnalogRead = true; - _nativeResolution = 13; + scaleAnalogRead = false; // handled in bsp (analogReadResolution) + analogReadResolution(resolution); // 16 bit values (shifted from 12 or 13bit) +#if defined(ESP32S3) + _nativeResolution = 13; // S3 ADC is 13-bit, others are 12-bit +#else + _nativeResolution = 12; +#endif #elif defined(ARDUINO_ARCH_RP2040) scaleAnalogRead = true; _nativeResolution = 10; @@ -96,7 +101,6 @@ void Wippersnapper_AnalogIO::setADCResolution(int resolution) { scaleAnalogRead = true; _nativeResolution = 10; #endif - _adcResolution = resolution; } @@ -232,8 +236,12 @@ uint16_t Wippersnapper_AnalogIO::getPinValue(int pin) { */ /**********************************************************/ float Wippersnapper_AnalogIO::getPinValueVolts(int pin) { +#ifdef ARDUINO_ARCH_ESP32 + return analogReadMilliVolts(pin) / 1000.0; +#else uint16_t rawValue = getPinValue(pin); return rawValue * getAref() / 65536; +#endif } /******************************************************************/ @@ -310,13 +318,17 @@ bool Wippersnapper_AnalogIO::encodePinEvent( The current software timer value. @param pin The desired analog pin to check + @param periodOffset + Offset to add to the pin's period (used for on_change). @returns True if pin's period expired, False otherwise. */ /**********************************************************/ -bool Wippersnapper_AnalogIO::timerExpired(long currentTime, - analogInputPin pin) { - if (currentTime - pin.prvPeriod > pin.period && pin.period != 0L) +bool Wippersnapper_AnalogIO::timerExpired(long currentTime, analogInputPin pin, + long periodOffset) { + if (pin.period + periodOffset != 0L && + currentTime - pin.prvPeriod > (pin.period + periodOffset)) { return true; + } return false; } @@ -335,9 +347,8 @@ void Wippersnapper_AnalogIO::update() { if (_analog_input_pins[i].enabled == true) { // Does the pin execute on-period? - if ((long)millis() - _analog_input_pins[i].prvPeriod > - _analog_input_pins[i].period && - _analog_input_pins[i].period != 0L) { + if (_analog_input_pins[i].period != 0L && + timerExpired(millis(), _analog_input_pins[i])) { WS_DEBUG_PRINT("Executing periodic event on A"); WS_DEBUG_PRINTLN(_analog_input_pins[i].pinName); @@ -359,28 +370,51 @@ void Wippersnapper_AnalogIO::update() { encodePinEvent(_analog_input_pins[i].pinName, _analog_input_pins[i].readMode, pinValRaw, pinValVolts); - // IMPT - reset the digital pin + // mark last execution time _analog_input_pins[i].prvPeriod = millis(); } // Does the pin execute on_change? else if (_analog_input_pins[i].period == 0L) { + // not first run and timer not expired, skip + if (_analog_input_pins[i].prvPeriod != 0L && + !timerExpired(millis(), _analog_input_pins[i], 500)) { + continue; + } + // note: on-change requires ADC DEFAULT_HYSTERISIS to check against prv // pin value uint16_t pinValRaw = getPinValue(_analog_input_pins[i].pinName); + // All boards ADC values scaled to 16bit, in future we may need to + // adjust dynamically + uint16_t maxDecimalValue = 65535; + + // Calculate threshold values - using DEFAULT_HYSTERISIS for first third + // (1/3) of the range, then 2x DEFAULT_HYSTERISIS for the middle 1/3, + // and 4x DEFAULT_HYSTERISIS for the last 1/3. This should allow a more + // wifi blip tolerant threshold for the both ends of the range. + float CURRENT_HYSTERISIS; + if (pinValRaw < maxDecimalValue / 3) { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS; + } else if (pinValRaw < (maxDecimalValue / 3) * 2) { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 2; + } else { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 4; + } + + // get the threshold values for previous pin value uint16_t _pinValThreshHi = - _analog_input_pins[i].prvPinVal + - (_analog_input_pins[i].prvPinVal * DEFAULT_HYSTERISIS); + _analog_input_pins[i].prvPinVal + CURRENT_HYSTERISIS; uint16_t _pinValThreshLow = - _analog_input_pins[i].prvPinVal - - (_analog_input_pins[i].prvPinVal * DEFAULT_HYSTERISIS); + _analog_input_pins[i].prvPinVal - CURRENT_HYSTERISIS; - if (pinValRaw > _pinValThreshHi || pinValRaw < _pinValThreshLow) { + if (_analog_input_pins[i].prvPeriod == 0 || + pinValRaw > _pinValThreshHi || pinValRaw < _pinValThreshLow) { // Perform voltage conversion if we need to if (_analog_input_pins[i].readMode == wippersnapper_pin_v1_ConfigurePinRequest_AnalogReadMode_ANALOG_READ_MODE_PIN_VOLTAGE) { - pinValVolts = pinValRaw * getAref() / 65536; + pinValVolts = getPinValueVolts(_analog_input_pins[i].pinName); } // Publish pin event to IO @@ -388,6 +422,9 @@ void Wippersnapper_AnalogIO::update() { _analog_input_pins[i].readMode, pinValRaw, pinValVolts); + // mark last execution time + _analog_input_pins[i].prvPeriod = millis(); + } else { // WS_DEBUG_PRINTLN("ADC has not changed enough, continue..."); continue; diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.h b/src/components/analogIO/Wippersnapper_AnalogIO.h index 91b62a489..d6a028e47 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.h +++ b/src/components/analogIO/Wippersnapper_AnalogIO.h @@ -63,7 +63,8 @@ class Wippersnapper_AnalogIO { void setADCResolution(int resolution); int getADCresolution(); int getNativeResolution(); - bool timerExpired(long currentTime, analogInputPin pin); + bool timerExpired(long currentTime, analogInputPin pin, + long periodOffset = 0); void update(); bool encodePinEvent( From 265ce3d5b98c5d940318b63e757fd959be0b05f5 Mon Sep 17 00:00:00 2001 From: tyeth Date: Wed, 16 Oct 2024 15:11:00 +0100 Subject: [PATCH 2/6] Refactor to calculateHysteresis --- .../analogIO/Wippersnapper_AnalogIO.cpp | 61 ++++++++++++------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index dfb9a48ac..c5c980867 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -311,6 +311,41 @@ bool Wippersnapper_AnalogIO::encodePinEvent( return true; } +/**********************************************************/ +/*! + @brief Calculates the hysteresis for the pin value. + @param pin + The desired analog pin to calculate hysteresis for. + @param _pinValThreshHi + The pin's high threshold value. + @param _pinValThreshLow + The pin's low threshold value. +*/ +/**********************************************************/ +void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, + uint16_t _pinValThreshHi, uint16_t _pinValThreshLow) { + // All boards ADC values scaled to 16bit, in future we may need to + // adjust dynamically + uint16_t maxDecimalValue = 65535; + + // Calculate threshold values - using DEFAULT_HYSTERISIS for first third + // (1/3) of the range, then 2x DEFAULT_HYSTERISIS for the middle 1/3, + // and 4x DEFAULT_HYSTERISIS for the last 1/3. This should allow a more + // wifi blip tolerant threshold for the both ends of the range. + float CURRENT_HYSTERISIS; + if (pinValRaw < maxDecimalValue / 3) { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS; + } else if (pinValRaw < (maxDecimalValue / 3) * 2) { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 2; + } else { + CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 4; + } + + // get the threshold values for previous pin value + _pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; + _pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; +} + /**********************************************************/ /*! @brief Checks if pin's period is expired. @@ -386,28 +421,10 @@ void Wippersnapper_AnalogIO::update() { // pin value uint16_t pinValRaw = getPinValue(_analog_input_pins[i].pinName); - // All boards ADC values scaled to 16bit, in future we may need to - // adjust dynamically - uint16_t maxDecimalValue = 65535; - - // Calculate threshold values - using DEFAULT_HYSTERISIS for first third - // (1/3) of the range, then 2x DEFAULT_HYSTERISIS for the middle 1/3, - // and 4x DEFAULT_HYSTERISIS for the last 1/3. This should allow a more - // wifi blip tolerant threshold for the both ends of the range. - float CURRENT_HYSTERISIS; - if (pinValRaw < maxDecimalValue / 3) { - CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS; - } else if (pinValRaw < (maxDecimalValue / 3) * 2) { - CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 2; - } else { - CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 4; - } - - // get the threshold values for previous pin value - uint16_t _pinValThreshHi = - _analog_input_pins[i].prvPinVal + CURRENT_HYSTERISIS; - uint16_t _pinValThreshLow = - _analog_input_pins[i].prvPinVal - CURRENT_HYSTERISIS; + // check if pin value has changed enough + uint16_t _pinValThreshHi, _pinValThreshLow; + calculateHysteresis(_analog_input_pins[i], pinValRaw, _pinValThreshHi, + _pinValThreshLow); if (_analog_input_pins[i].prvPeriod == 0 || pinValRaw > _pinValThreshHi || pinValRaw < _pinValThreshLow) { From 2947fb63bba550de0688fcc4bf442744f25ccd0f Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 18 Oct 2024 18:01:27 +0100 Subject: [PATCH 3/6] Add param to docstring --- src/components/analogIO/Wippersnapper_AnalogIO.cpp | 12 +++++++----- src/components/analogIO/Wippersnapper_AnalogIO.h | 2 ++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index c5c980867..a7e072c9c 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -316,14 +316,16 @@ bool Wippersnapper_AnalogIO::encodePinEvent( @brief Calculates the hysteresis for the pin value. @param pin The desired analog pin to calculate hysteresis for. - @param _pinValThreshHi + @param pinValRaw + The pin's raw value. + @param pinValThreshHi The pin's high threshold value. - @param _pinValThreshLow + @param pinValThreshLow The pin's low threshold value. */ /**********************************************************/ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, - uint16_t _pinValThreshHi, uint16_t _pinValThreshLow) { + uint16_t pinValThreshHi, uint16_t pinValThreshLow) { // All boards ADC values scaled to 16bit, in future we may need to // adjust dynamically uint16_t maxDecimalValue = 65535; @@ -342,8 +344,8 @@ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, } // get the threshold values for previous pin value - _pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; - _pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; + pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; + pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; } /**********************************************************/ diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.h b/src/components/analogIO/Wippersnapper_AnalogIO.h index d6a028e47..659e3e577 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.h +++ b/src/components/analogIO/Wippersnapper_AnalogIO.h @@ -57,6 +57,8 @@ class Wippersnapper_AnalogIO { int pin); void disableAnalogInPin(int pin); + void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, + uint16_t &_pinValThreshHi, uint16_t &_pinValThreshLow); uint16_t getPinValue(int pin); float getPinValueVolts(int pin); From 872a3f5d775cfe53fc410497ad5a3d760c4b48ab Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 18 Oct 2024 18:25:16 +0100 Subject: [PATCH 4/6] Cleanup + log thresholds --- src/components/analogIO/Wippersnapper_AnalogIO.cpp | 14 ++++++++++---- src/components/analogIO/Wippersnapper_AnalogIO.h | 2 +- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index a7e072c9c..86de51aaa 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -424,12 +424,18 @@ void Wippersnapper_AnalogIO::update() { uint16_t pinValRaw = getPinValue(_analog_input_pins[i].pinName); // check if pin value has changed enough - uint16_t _pinValThreshHi, _pinValThreshLow; - calculateHysteresis(_analog_input_pins[i], pinValRaw, _pinValThreshHi, - _pinValThreshLow); + uint16_t pinValThreshHi, pinValThreshLow; + calculateHysteresis(_analog_input_pins[i], pinValRaw, pinValThreshHi, + pinValThreshLow); + WS_DEBUG_PRINT("Returned pinValThreshHi: "); + WS_DEBUG_PRINTLN(pinValThreshHi); + WS_DEBUG_PRINT("Returned pinValThreshLow: "); + WS_DEBUG_PRINTLN(pinValThreshLow); + WS_DEBUG_PRINT("Current pinValRaw: "); + WS_DEBUG_PRINTLN(pinValRaw); if (_analog_input_pins[i].prvPeriod == 0 || - pinValRaw > _pinValThreshHi || pinValRaw < _pinValThreshLow) { + pinValRaw > pinValThreshHi || pinValRaw < pinValThreshLow) { // Perform voltage conversion if we need to if (_analog_input_pins[i].readMode == wippersnapper_pin_v1_ConfigurePinRequest_AnalogReadMode_ANALOG_READ_MODE_PIN_VOLTAGE) { diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.h b/src/components/analogIO/Wippersnapper_AnalogIO.h index 659e3e577..8ad0e31db 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.h +++ b/src/components/analogIO/Wippersnapper_AnalogIO.h @@ -58,7 +58,7 @@ class Wippersnapper_AnalogIO { void disableAnalogInPin(int pin); void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, - uint16_t &_pinValThreshHi, uint16_t &_pinValThreshLow); + uint16_t pinValThreshHi, uint16_t pinValThreshLow); uint16_t getPinValue(int pin); float getPinValueVolts(int pin); From 7b10b132ec0df784fd06902bb773b85ecc8783b8 Mon Sep 17 00:00:00 2001 From: tyeth Date: Fri, 18 Oct 2024 18:49:29 +0100 Subject: [PATCH 5/6] Tweak passed arguments --- src/components/analogIO/Wippersnapper_AnalogIO.cpp | 10 +++++----- src/components/analogIO/Wippersnapper_AnalogIO.h | 2 -- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index 86de51aaa..0272e8245 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -325,7 +325,7 @@ bool Wippersnapper_AnalogIO::encodePinEvent( */ /**********************************************************/ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, - uint16_t pinValThreshHi, uint16_t pinValThreshLow) { + uint16_t *pinValThreshHi, uint16_t *pinValThreshLow) { // All boards ADC values scaled to 16bit, in future we may need to // adjust dynamically uint16_t maxDecimalValue = 65535; @@ -344,8 +344,8 @@ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, } // get the threshold values for previous pin value - pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; - pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; + *pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; + *pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; } /**********************************************************/ @@ -425,8 +425,8 @@ void Wippersnapper_AnalogIO::update() { // check if pin value has changed enough uint16_t pinValThreshHi, pinValThreshLow; - calculateHysteresis(_analog_input_pins[i], pinValRaw, pinValThreshHi, - pinValThreshLow); + calculateHysteresis(_analog_input_pins[i], pinValRaw, &pinValThreshHi, + &pinValThreshLow); WS_DEBUG_PRINT("Returned pinValThreshHi: "); WS_DEBUG_PRINTLN(pinValThreshHi); WS_DEBUG_PRINT("Returned pinValThreshLow: "); diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.h b/src/components/analogIO/Wippersnapper_AnalogIO.h index 8ad0e31db..d6a028e47 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.h +++ b/src/components/analogIO/Wippersnapper_AnalogIO.h @@ -57,8 +57,6 @@ class Wippersnapper_AnalogIO { int pin); void disableAnalogInPin(int pin); - void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, - uint16_t pinValThreshHi, uint16_t pinValThreshLow); uint16_t getPinValue(int pin); float getPinValueVolts(int pin); From 9c6187f41ab6ad53ae5ad73f7b04a86cd153da4a Mon Sep 17 00:00:00 2001 From: tyeth Date: Thu, 14 Nov 2024 15:24:22 +0000 Subject: [PATCH 6/6] Correct hysteresis and stop overflow --- .../analogIO/Wippersnapper_AnalogIO.cpp | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/components/analogIO/Wippersnapper_AnalogIO.cpp b/src/components/analogIO/Wippersnapper_AnalogIO.cpp index 0272e8245..40df2e663 100644 --- a/src/components/analogIO/Wippersnapper_AnalogIO.cpp +++ b/src/components/analogIO/Wippersnapper_AnalogIO.cpp @@ -325,7 +325,7 @@ bool Wippersnapper_AnalogIO::encodePinEvent( */ /**********************************************************/ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, - uint16_t *pinValThreshHi, uint16_t *pinValThreshLow) { + uint16_t &pinValThreshHi, uint16_t &pinValThreshLow) { // All boards ADC values scaled to 16bit, in future we may need to // adjust dynamically uint16_t maxDecimalValue = 65535; @@ -342,10 +342,19 @@ void calculateHysteresis(analogInputPin pin, uint16_t pinValRaw, } else { CURRENT_HYSTERISIS = maxDecimalValue * DEFAULT_HYSTERISIS * 4; } - - // get the threshold values for previous pin value - *pinValThreshHi = pin.prvPinVal + CURRENT_HYSTERISIS; - *pinValThreshLow = pin.prvPinVal - CURRENT_HYSTERISIS; + // get the threshold values for previous pin value, but don't overflow + float overflowableThHi = pin.prvPinVal + CURRENT_HYSTERISIS; + float overflowableThLow = pin.prvPinVal - CURRENT_HYSTERISIS; + if (overflowableThHi > maxDecimalValue) { + pinValThreshHi = maxDecimalValue; + } else { + pinValThreshHi = overflowableThHi; + } + if (overflowableThLow < 0) { + pinValThreshLow = 0; + } else { + pinValThreshLow = overflowableThLow; + } } /**********************************************************/ @@ -425,14 +434,8 @@ void Wippersnapper_AnalogIO::update() { // check if pin value has changed enough uint16_t pinValThreshHi, pinValThreshLow; - calculateHysteresis(_analog_input_pins[i], pinValRaw, &pinValThreshHi, - &pinValThreshLow); - WS_DEBUG_PRINT("Returned pinValThreshHi: "); - WS_DEBUG_PRINTLN(pinValThreshHi); - WS_DEBUG_PRINT("Returned pinValThreshLow: "); - WS_DEBUG_PRINTLN(pinValThreshLow); - WS_DEBUG_PRINT("Current pinValRaw: "); - WS_DEBUG_PRINTLN(pinValRaw); + calculateHysteresis(_analog_input_pins[i], pinValRaw, pinValThreshHi, + pinValThreshLow); if (_analog_input_pins[i].prvPeriod == 0 || pinValRaw > pinValThreshHi || pinValRaw < pinValThreshLow) { @@ -450,12 +453,10 @@ void Wippersnapper_AnalogIO::update() { // mark last execution time _analog_input_pins[i].prvPeriod = millis(); - } else { - // WS_DEBUG_PRINTLN("ADC has not changed enough, continue..."); + } else { // ADC has not changed enough continue; } - // set the pin value in the digital pin object for comparison on next - // run + // set the pin value in the digital pin object for comparison next run _analog_input_pins[i].prvPinVal = pinValRaw; } }