search

igorantolic / ai-esp32-rotary-encoder

Easy implement rotary encoder to your application using microcontroler like ESP32
GNU General Public License v2.0
284 stars 70 forks source link

The function isEncoderButtonDown() returns an incorrect value when areEncoderPinsPulldown_forEsp32=false #83

Open kr4fty opened 2 months ago

kr4fty commented 2 months ago

if _arEncoderPinsPulldown_forEsp32 = true_, the button will always read a LOW. Then isEncoderButtonDown() returns a FALSE, which is correct.

button UP -----> isEncoderButtonDown() returns FALSE button DOWN ----> isEncoderButtonDown() returns TRUE

Now if areEncoderPinsPulldown_forEsp32 = false (center pin connected to GND), if we read the button state without having it pressed, it would give me a HIGH state. Then isEncoderButtonDown() returns a TRUE, but it should be FALSE

button UP -----> isEncoderButtonDown() returns TRUE, it should be FALSE button DOWN ----> isEncoderButtonDown() returns FALSE, it should be TRUE

The function should be something like:

bool AiEsp32RotaryEncoder::isEncoderButtonDown()
{
    if(!areEncoderPinsPulldownforEsp32)
        return digitalRead(encoderButtonPin) ? false : true;
    else
        return digitalRead(encoderButtonPin) ? true : false;
}
igorantolic commented 2 months ago

That is only partially true,

areEncoderPinsPulldownforEsp32 is primary made for encoder functionality, not button

you can just write your internal function

bool isButtonDown(){

return !rotary.isEncoderButtonDown()

}

Else we get info a discussion how to make parameters too complicated. Seems like we should introduce separate variable for button pulldown to cover all cases. But make more variables lead to confusion of significant number od users. For sure solution you proposed is not efficient since generating issue on another place and breaks the compatibility of the library. Compatibility is something very important. So new function can be created but I would not like to reverse the existing one.

On Sun, Aug 4, 2024 at 8:21 PM Tapia Favio @.***> wrote:

if arEncoderPinsPulldown_forEsp32 = true, the button will always read a LOW. Then isEncoderButtonDown() returns a FALSE, which is correct.

button UP -----> isEncoderButtonDown() returns FALSE button DOWN ----> isEncoderButtonDown() returns TRUE

Now if areEncoderPinsPulldown_forEsp32 = false (center pin connected to GND), if we read the button state without having it pressed, it would give me a HIGH state. Then isEncoderButtonDown() returns a TRUE, but it should be FALSE

button UP -----> isEncoderButtonDown() returns TRUE, it should be FALSE button DOWN ----> isEncoderButtonDown() returns FALSE, it should be TRUE

The function should be something like:

bool AiEsp32RotaryEncoder::isEncoderButtonDown() { if(!areEncoderPinsPulldownforEsp32) return digitalRead(encoderButtonPin) ? false : true; else return digitalRead(encoderButtonPin) ? true : false; }

— Reply to this email directly, view it on GitHub https://github.com/igorantolic/ai-esp32-rotary-encoder/issues/83, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMDZQEDOURGIJT3ALH73ZLZPZWMJAVCNFSM6AAAAABL7BL7PGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ2DOMRUGUYTIMQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>