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.

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

Sofle/via - Switch to ENCODER_MAP, "handiness" comment update, swap PGUP/PGDOWN encoder mapping #20630

Merged
merged 3 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 2 additions & 3 deletions keyboards/sofle/keymaps/via/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@

#pragma once

/* The way how "handedness" is decided (which half is which),
/* By default left side is selected as master,
see https://docs.qmk.fm/#/feature_split_keyboard?id=setting-handedness
for more options.
*/
for more options. */

#if defined(KEYBOARD_sofle_rev1)
// Add RGB underglow and top facing lighting
Expand Down
39 changes: 0 additions & 39 deletions keyboards/sofle/keymaps/via/encoder.c

This file was deleted.

10 changes: 8 additions & 2 deletions keyboards/sofle/keymaps/via/keymap.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,15 @@

#include QMK_KEYBOARD_H
#include "oled.c"
#include "encoder.c"

//Default keymap. This can be changed in Via. Use oled.c and encoder.c to change beavior that Via cannot change.
#ifdef ENCODER_MAP_ENABLE
const uint16_t PROGMEM encoder_map[][NUM_ENCODERS][2] = {
[0] = { ENCODER_CCW_CW(KC_VOLD, KC_VOLU), ENCODER_CCW_CW(KC_PGDN, KC_PGUP) },
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe there's a mistake in the second encoder here, it should be in this order from the original encoder.c code: KC_PGUP, KC_PGDN if ENCODER_CCW_CW from my understanding is from couterclockwise to clockwise @ireun

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I'd say this makes more sense now, since both KC_VOLxx and KC_PGxx are in the same order. Usually in my experience rotating clockwise corresponds to increasing, that applies to clocks, volume knobs or even gas burners.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is there's a change in behavior between what u had before with encoder.c file and the new encoder_map. Before, the encoder at index 1 was doing KC_PGDN on clockwise rotation while now it's being reversed to KC_PGUP on clockwise rotation.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe mention this in the PR about the behavior change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, thank You for pointing that out.

[1] = { ENCODER_CCW_CW(_______, _______), ENCODER_CCW_CW(_______, _______) },
[2] = { ENCODER_CCW_CW(RGB_HUD, RGB_HUI), ENCODER_CCW_CW(RGB_SAD, RGB_SAI) },
[3] = { ENCODER_CCW_CW(RGB_VAD, RGB_VAI), ENCODER_CCW_CW(RGB_RMOD, RGB_MOD)}
};
#endif

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
/*
Expand Down
1 change: 1 addition & 0 deletions keyboards/sofle/keymaps/via/rules.mk
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ EXTRAKEY_ENABLE = yes
VIA_ENABLE = yes
LTO_ENABLE = yes
RGBLIGHT_ENABLE = yes
ENCODER_MAP_ENABLE = yes