0xPIT / encoder

Atmel AVR C++ RotaryEncoder Implementation
309 stars 162 forks source link

Sometimes Misses ClickEncoder::Released #14

Open wrhastings opened 7 years ago

wrhastings commented 7 years ago

I'm trying to use the library with an Adafruit Rotarty Encoder. The reading of the encoder seems to work fine. The problem I'm having is that it sometimes misses ClickEncoder::Released. My code is watching for ClickEncoder::Held (which always works). Once I see it, I want to wait for the ClickEncoder::Released before continuing. Unfortunately, about 1 in 4 times, the ClickEncoder::Released is not caught. I have to re-hold the button and the release for it to catch up.

soligen2010 commented 7 years ago

Please try my fork of this library. The original library by 0xPIT as a number of bugs that I fixed.

On Oct 31, 2016 5:38 PM, "Bill Hastings" notifications@github.com wrote:

I'm trying to use the library with an Adafruit Rotarty Encoder. The reading of the encoder seems to work fine. The problem I'm having is that it sometimes misses ClickEncoder::Released. My code is watching for ClickEncoder::Held (which always works). Once I see it, I want to wait for the ClickEncoder::Released before continuing. Unfortunately, about 1 in 4 times, the ClickEncoder::Released is not caught. I have to re-hold the button and the release for it to catch up.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/0xPIT/encoder/issues/14, or mute the thread https://github.com/notifications/unsubscribe-auth/APx9EM0tpEdrZ1zzV2JV1zap7PmQf86Oks5q5l_qgaJpZM4KliPT .

wrhastings commented 7 years ago

Thanks, but I tried your fork and the problem still exists. Doesn't seem to happen quite as often, but it's still there. FYI, I'm running this on an Arduino Mega 2650.

soligen2010 commented 7 years ago

Please strip your sketch down to just the logic that uses the library and shows the issue and post your code. I don't have that hardware so never tested on it, but I'll look at your code.

You might also want to try using a different pins just in case something else in your sketch or another library is interfering.

soligen2010 commented 7 years ago

Also I should mention that there is a minimum amount of time that the the button must be held before it goes into held/released state. If you let go too soon, then it comes out as a clicked event instead. In the original library is set to 1.2 seconds. My version reduces this to 1.0 seconds, but it can also be over-ridden in my library to whatever you want. This library is designed to only go into the hold/released process AFTER enough time has passed that the clicked event is bypassed.

You can try putting in some prints to see what event is actually coming back to see if you are getting clicked events instead. If you don't want clicked events, you can try changing hold/release interval (in my fork, not the original) to zero and see if this works - I have not specifically tested the interval at zero, so I would be interested in the results. Or you could trigger the same logic on both clicked or released events.

I am pretty much guessing here. There is not enough info provided to tell anything for sure. Please try these things and post back what happens.

wrhastings commented 7 years ago

I'll give all of those a try, but I just finished stripping down the code and it still shows the issue. It only happens 1 in 8 times, or so.

`

include

include

define ENCODERPINA 40

define ENCODERPINB 41

define ENCODERBTNPIN 42

define ENCODERSTEPSPERNOTCH 4

// Rotary Encoder definitions ClickEncoder *encoder;

// Timer/encoder Interupt service routine void timerIsr() { encoder->service(); }

void setup() { Serial.begin(9600);

encoder = new ClickEncoder(ENCODERPINB, ENCODERPINA, ENCODERBTNPIN, ENCODERSTEPSPERNOTCH);
encoder->setAccelerationEnabled(false);

Timer1.initialize(1000);
Timer1.attachInterrupt(timerIsr); 

}

void loop() { ClickEncoder::Button btn = encoder->getButton(); if(btn == ClickEncoder::Held) { Serial.println("Button held");Serial.println();

    // do stuff here

    // wait for button release
    while (encoder->getButton() != ClickEncoder::Released) ;
    Serial.println("Button released");Serial.println();
}

}`

wrhastings commented 7 years ago

So I made a small change to the code to check for clicked events. I've verified that it a held event and not a click. The interesting point is that when I fail to see the release, it also does not see a click. This tells me that it is hung up in the Held state. I didn't try changing any of the timing - didn't seem necessary. Here's the updated test code. `

include

include

define ENCODERPINA 40

define ENCODERPINB 41

define ENCODERBTNPIN 42

define ENCODERSTEPSPERNOTCH 4

// Rotary Encoder definitions ClickEncoder *encoder;

// Timer/encoder Interupt service routine void timerIsr() { encoder->service(); }

void setup() { Serial.begin(9600);

encoder = new ClickEncoder(ENCODERPINB, ENCODERPINA, ENCODERBTNPIN, ENCODERSTEPSPERNOTCH);
encoder->setAccelerationEnabled(false);

Timer1.initialize(1000);
Timer1.attachInterrupt(timerIsr); 

}

void loop() { ClickEncoder::Button btn = encoder->getButton();

if(btn == ClickEncoder::Clicked)
{
    Serial.println("Button clicked");Serial.println();       
}
else if(btn == ClickEncoder::Held)
{
    Serial.println("Button held");Serial.println();

    // do stuff here

    // wait for button release
    while (encoder->getButton() != ClickEncoder::Released) ;
    Serial.println("Button released");Serial.println();
}

}`

soligen2010 commented 7 years ago

Did you verify you are actually going into HELD when this happens? you are not processing the clicked event, so I think it is may be because you are not holding the button long enough to get into held state.

I just rand a test setting the hold time to 0 using setHoldTime. This will effectively get rid on both Clicked and DoubleClicked events and just do held/released. I think this is what you should try.

I think all the events are likely firing properly, but you are not catching all the possible events. You either need to catch them all and do something with them, or turn them off. (requires my fork to turn them off) .

soligen2010 commented 7 years ago

I just duplicated the problem. It does seem to enter held, just never leaves it. May be a race condition. Stay tuned.....

soligen2010 commented 7 years ago

OK, my fork has a fix (along with some other new functionality I was working on) that I think takes care of it. I tried about 50 presses and it didn't happen.

If you are not processing clicked or double clicked events, I do recommend trying a HoldTime of zero... Unless of course you like the delay before entering the held state.

wrhastings commented 7 years ago

Yep, that looks like it took care of it. Thanks for the quick response. For my application, I want to use, Clicked, Held, and Release. I'm not using DoubleClick, so I'm leaving HoldTime where it is. Thanks, again.

soligen2010 commented 7 years ago

You can explicitly turn off just doubleClick using setDoubleClickEnabled. Turning it off makes the click event more responsive becasue it doesn't have to wait and see if a second click happens.

Also, I just made a tweak to the encoder logic just in case a similar race condition can happen there. Could you please download the latest again and test it.

Thanks

wrhastings commented 7 years ago

Downloaded the latest and set setDoubleClickEnabled false. The problem I'm seeing is that a single click is now very unreliable. I've got to click it 3, 4 or even 5 times before it takes. The hold and release are working fine.

soligen2010 commented 7 years ago

This result I an not able to reproduce. Here is the sketch I am using - basically yours with pins changed for using a nano. It is working fine for me. Any more info?

include

include

define ENCODERPINA 7

define ENCODERPINB 8

define ENCODERBTNPIN 9

define ENCODERSTEPSPERNOTCH 4

// Rotary Encoder definitions ClickEncoder *encoder;

// Timer/encoder Interupt service routine void timerIsr() { encoder->service(); }

void setup() { Serial.begin(74880);

encoder = new ClickEncoder(ENCODERPINB, ENCODERPINA, ENCODERBTNPIN, ENCODERSTEPSPERNOTCH); encoder->setAccelerationEnabled(false); encoder->setDoubleClickEnabled(false);

Timer1.initialize(1000); Timer1.attachInterrupt(timerIsr); }

void loop() { ClickEncoder::Button btn = encoder->getButton(); if(btn == ClickEncoder::Clicked) {Serial.println("Button click");Serial.println(); } if(btn == ClickEncoder::Held) { Serial.println("Button held");Serial.println();

// do stuff here

// wait for button release
while (encoder->getButton() != ClickEncoder::Released) ;
Serial.println("Button released");Serial.println();

} }

wrhastings commented 7 years ago

The stripped down code works fine for me also. When I try to use it with my full sketch (which also supports a TFT display) is when I'm seeing problems with Click. Let me try eliminating portions of the sketch to try and determine what's causing the problem. That's a job for tomorrow, though. Thanks for your help.

soligen2010 commented 7 years ago

One think to check is that you only call getButton in one place. getButton resets the state of the button (except for held). If, for example you call getButton, check for held, then call it again and check for clicked, you will miss any click events that came in on the first call

wrhastings commented 7 years ago

My sketch basically runs as a state machine. A long click (held) causes a change of state, but I do nothing until I see a release. Once released, I use single clicks to select options within the state.

wrhastings commented 7 years ago

Nevermind. Sorry, it was pilot error. Too many calls to getButton(). I've now reduced it to a single call, each time through the loop() function. It all seems to be working well. Thanks, again, for your help.

soligen2010 commented 7 years ago

Good. I am glad it is working now.

For anyone in the future looking at this thread, please go to the soligen2010/encoder fork for the code worked on here.

DrDiodac commented 6 years ago

Hi. I have problem with setting. I need values in range 0-127. How to do in proper way? Thank you

no-trick-pony commented 4 years ago

Thanks a lot. This fork fixed the problem I had, that the code would somehow recognize clicks, double click and holdes when just rotating. It would recognize regular clicks of the button too. But I get ghost inputs. Even if I disconnect the button completely. Not happening with @soligen2010's fork.