Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to set the encoder A and B pins on the MCU to use internal pull-down, pull-up, or leave floating #11706

Closed
wants to merge 6 commits into from
Closed

Add option to set the encoder A and B pins on the MCU to use internal pull-down, pull-up, or leave floating #11706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
749a8d3
added option to configure mcu pin behavior for encoders
Croktopus Jan 27, 2021
35edcd6
added documentation
Croktopus Jan 27, 2021
088ae7e
adjusted encoder pin code from feedback
Croktopus Feb 11, 2021
6255503
fixed missing #endif
Croktopus Feb 11, 2021
a589fce
fixed undefined variables
Croktopus Feb 11, 2021
9bc66f1
updated documentation
Croktopus Feb 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions docs/feature_encoders.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ If you are using different pinouts for the encoders on each half of a split keyb
#define ENCODER_RESOLUTIONS_RIGHT { 2, 4 }
```

## Configuring Input Pins

Rarely, you may want to set the MCU pin hooked up to the encoder to be left floating or use an internal pull down (not available on AVR), instead of the default which is to use an internal pull-up. This is mainly relevant for non-mechanical encoders, such as optical or hall effect. To use this feature `#define` ENCODER_PINS in your `config.h` as one of the following:

|Define |Description |
|---------------|---------------------------------------------------------------------------------------------------------|
|`PULL_UP` |Toggles on the internal pull-up (default behavior) |
|`PULL_DOWN` |Toggles on the internal pull-down (not on AVR) |
|`FLOAT` |Leaves the pin as floating |

## Callbacks

The callback functions can be inserted into your `<keyboard>.c`:
Expand Down
31 changes: 26 additions & 5 deletions quantum/encoder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,33 @@ void encoder_init(void) {
}
#endif

for (int i = 0; i < NUMBER_OF_ENCODERS; i++) {
setPinInputHigh(encoders_pad_a[i]);
setPinInputHigh(encoders_pad_b[i]);
#ifdef ENCODER_PINS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming things is difficult, however i'm cautious of too much crossover to existing setting, eg BACKLIGHT_PINS. While docs exists, it leads to questions like "why doesn't #define ENCODER_PINS {A1, B2} work?"

I cant find an existing setting to align to, but the closest thing that come to mind would be ENCODER_PIN_MODE, maybe ENCODER_PAD_MODE.

# define PULL_DOWN 1
# define PULL_UP 2
# define FLOAT 3
# if (ENCODER_PINS == PULL_DOWN)
# ifdef __AVR__
# error Pull Down is not supported on AVR
# endif
# define setEncPinInput(x) setPinInputLow(x)
# elif (ENCODER_PINS == PULL_UP)
# define setEncPinInput(x) setPinInputHigh(x)
# elif (ENCODER_PINS == FLOAT)
# define setEncPinInput(x) setPinInput(x)
# else
# error ENCODER_PINS must be PULL_UP, PULL_DOWN, or FLOAT
# define setEncPinInput(x) setPinInputHigh(x)
# endif
#else
# define setEncPinInput(x) setPinInputHigh(x)
#endif

encoder_state[i] = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1);
}
for (int i = 0; i < NUMBER_OF_ENCODERS; i++) {
setEncPinInput(encoders_pad_a[i]);
setEncPinInput(encoders_pad_b[i]);

encoder_state[i] = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1);
}
Comment on lines +105 to +110
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format

Suggested change
for (int i = 0; i < NUMBER_OF_ENCODERS; i++) {
setEncPinInput(encoders_pad_a[i]);
setEncPinInput(encoders_pad_b[i]);
encoder_state[i] = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1);
}
for (int i = 0; i < NUMBER_OF_ENCODERS; i++) {
setEncPinInput(encoders_pad_a[i]);
setEncPinInput(encoders_pad_b[i]);
encoder_state[i] = (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1);
}


#ifdef SPLIT_KEYBOARD
thisHand = isLeftHand ? 0 : NUMBER_OF_ENCODERS;
Expand Down