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

[kr4fty]

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]

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 <#83>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMDZQEDOURGIJT3ALH73ZLZPZWMJAVCNFSM6AAAAABL7BL7PGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGQ2DOMRUGUYTIMQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>