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

Zoom65 Lite patch #473

Closed
wants to merge 2 commits into from
Closed

Zoom65 Lite patch #473

wants to merge 2 commits into from

Conversation

juanlufont
Copy link
Contributor

This PR aims to:

  • fix the mapping for the encoder CW and CCW, originally they were swapped, volume was going up when CCW and viceversa
  • add some missing keys to the default Vial keymap (Home, Page Down, End, Print Screen, Lock Scroll and Pause)

Original configuration had CW and CCW swapped. This is, despite "proper"
configuration, volume was going up when encoder was turned CCW and
viceversa.
Add Home, PageDown, Print Screen, Scroll Lock and Pause
@juanlufont
Copy link
Contributor Author

Did the build process failed for something not related to the code?

Comment on lines -34 to +35
#define ENCODERS_PAD_A { B1 }
#define ENCODERS_PAD_B { B0 }
#define ENCODERS_PAD_A { B0 }
#define ENCODERS_PAD_B { B1 }
Copy link
Contributor

@lesshonor lesshonor May 29, 2023

Choose a reason for hiding this comment

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

This doesn't match upstream QMK: https://github.com/qmk/qmk_firmware/blob/d02ff2edd9b265c37782abb312693d1876aec54d/keyboards/meletrix/zoom65_lite/info.json#L18

...and if it's wrong there, you should fix it there too. In a best-case scenario for you, this change would be accepted to Vial and then undone when updates are merged in from QMK.

If your specific encoder is backwards, you could put #define ENCODER_DIRECTION_FLIP in your vial keymap's config.h instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed to check the info.json, but I saw the config.h for vial-qmk contains several lines that are not in upstream.

About the upstream code, I have to flash it again, but IIRC, it worked fine regarding the encoder, but it also included custom functions for the knob that we removed in my previous PR: #467 (comment)

Copy link
Contributor

@lesshonor lesshonor May 29, 2023

Choose a reason for hiding this comment

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

I saw the config.h for vial-qmk contains several lines that are not in upstream.

Yes. A lot of modifications have been made upstream that have not been brought over to Vial.
If no one touches the keyboard level files in the Vial repository, then all the changes made upstream will apply cleanly with no conflicts.

If the files are changed in the Vial repository, then someone (read: xyz) will have to resolve any conflicting changes manually. xyz has mentioned the general process of this: the upstream version wins by default, and if the default or vial keymaps don't compile, then the keyboard gets a second look.

The encoder hacks put in place for VIA will be removed in August. So if this keyboard is incorrect upstream, there is every chance in the world that it eventually end up backwards for everyone -- unless you or someone else who actually owns this board creates/modifies a keymap to use encoder mapping, tests it, and if necessary opens a PR to fix the pin ordering.

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, a recent comment from the closed QMK PR for this board:
qmk/qmk_firmware#15876 (comment)

there are two encoder, two different encoders in different batches ,and they are in different directions.

...So it looks like it'll always be backwards for someone. 🤷🏾

Here's another idea: add a layout option, "Swap Encoder Direction", that flips the order of the 0,0 and 0,1 encoder labels.

@lesshonor lesshonor mentioned this pull request Aug 10, 2023
@xyzz xyzz closed this Oct 14, 2023
@juanlufont juanlufont deleted the zoom65-patch branch February 18, 2024 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants