lesterlo / Notched-Shaft-Encoder

Control a Notched Shaft Encoder with and Arduino
MIT License
7 stars 1 forks source link

Button doesn't work as desired #1

Open gitterman opened 3 years ago

gitterman commented 3 years ago

I tried the examples with my encoder attached to A1 and A2 and the button attached to A0. The examples without button do work but when I use the button example I see button status messages although I do not touch the encoder. I mostly get "Btn status: Held" When I press the button several times I sometimes manage to get "Btn status: Released". If I then rotate the encoder I get the encoder output as desired but intermixed with "Btn status: Pressed" messages although I did not press the button. I set enable_doublePress=false but did not get any improvement. I also uncommented the intervals for the button in setup as I noticed that they are different from the definitions in NSEncoder.h but no change either. If I use the ClickEncoder library from 0xPIT the button works as it should. Thus, I think the encoder hardware is OK.

Here is an excerpt from the serial monitor: Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Held Btn status: Released Encoder Diff: 1 Encoder Diff: 1 Encoder Diff: 1 Encoder Diff: -1 Encoder Diff: -1 Btn status: Pressed Encoder Diff: -1 Btn status: Pressed Encoder Diff: -1 Encoder Diff: -1 Btn status: Pressed Encoder Diff: 1 Encoder Diff: 1

gitterman commented 3 years ago

I found that if I explicitly set the button pin mode as INPUT_PULLUP the button works as it should. Thus, I think the mode setting in NSEncoder.cpp doesn't work correctly. Looking at the code the logic seems to be wrong. But even if I declare the encoder as NSEncoder_btn enc( 15, 16, 14, 4, false, HIGH ) the button does not work. So there may be something else going on here too.

lesterlo commented 3 years ago

Hi,

About the INPUT_PULLUP problem, did you set the pinMode to INPUT_PULLUP explicitly in this line and make the button works as expected?

My button hardware is using INPUT rather than INPUT_PULUP.

Could you help me to test this code, changing the pin_activeType to _pin_activeType may help?

pinMode(_pin_BTN, ((_pin_activeType == LOW )? INPUT : INPUT_PULLUP));

or this?

pinMode(_pin_BTN, ((_pin_activeType == true)? INPUT : INPUT_PULLUP));

Maybe I should not use HIGH, LOW with bool data type. I will address this issue in the future.

gitterman commented 3 years ago

To make it work, I simply but this line in the setup of the NSEncoder_btn.ino pinMode( A0, INPUT_PULLUP ) ; Changing pin_activeType to _pin_activeType didn't show any change. Compared with line 52 of the cpp from https://github.com/0xPIT/encoder the logic for active low needs to be reversed. I.e. if pin_activeType == LOW then the INPUT_PULLUP must be used unless you have an external pullup in your circuit. Thus, to make it work the line in your cpp should read: pinMode(_pin_BTN, ((pin_activeType == LOW )? INPUT_PULLUP : INPUT));

lesterlo commented 3 years ago

@gitterman , Thank you for fixing the bug in this project.😆

I see your point. So, the problem is a logical problem rather than a code/hardware problem?

Just flip the order in this line, when the _pin_activeType is LOW thwn the arduino will use INPUT_PULLUP to handle the input connection.

In short, the following modification can solve the problem?

pinMode(_pin_BTN, ((_pin_activeType == LOW )? INPUT : INPUT_PULLUP));

to

pinMode(_pin_BTN, ((pin_activeType == LOW )? INPUT_PULLUP : INPUT));

If yes, I will make a commit to fix this problem.

gitterman commented 3 years ago

Yes, reversing the logic solves the problem. As I said, an active low input needs a pullup either internal or external.

lesterlo commented 3 years ago

@gitterman

Thank you for your help, I will update the main branch after I tested the new code. (around 1-2 weeks)

I8NHJ commented 1 year ago

Hi @lesterlo. I'm having the same issue reported by @gitterman. Did you test the code and updated it?

Thanks, Max

I8NHJ commented 1 year ago

JFYI, I checked my NSEncoder_btn.cpp downloaded few weeks ago and, as of today, that fix has not been applied.

lesterlo commented 1 year ago

Currently, I don't have the hardware in my hands. Let me check it when getting it.

thank you very much

I8NHJ commented 1 year ago

Hi Lesterlo.You don’t need the hardware to compile.Just select the DUE platform in the Arduino IDE.Let me know if i can help.MassimoOn Mar 5, 2023, at 6:55 AM, Lester Lo @.***> wrote: Currently, I don't have the hardware in my hands. Let me check it when getting it. thank you very much

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>