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

Addition to the Apple Globe key code to QMK #24661

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from

Conversation

ChrisGVE
Copy link
Contributor

The change brings the Apple Globe Key as KC_GLOBE to QMK.

Description

The change is based mainly on @lordpixel23 QMK patch and is being discussed in depth in QMK Apple Fn Key created by @fauxpark.

Importantly, this change does not rely on VID/PID to be set on specific values.

In addition to the patch mentioned above, it introduces a new compilation flag, GLOBE_KEY_ENABLE such that:
(a) the required KEYBOARD_SHARE_EP is enabled when the Globe keycode is used
(b) keyboards or user layouts can make use of this definition for further customizations

The QMK documentation has been amended accordingly.

Certain key combinations are not yet working, though; in particular, 🌐︎+arrow or 🌐︎+FN do not work.

Thanks go to the input of many contributors, as this topic has been long discussed: @fauxpark, @HVR88, @drashna, and @lordpixel23.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added core documentation dd Data Driven Changes labels Nov 30, 2024
@tzarc tzarc changed the base branch from master to develop November 30, 2024 12:51
Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

I'll leave the rest of the review for the team -- they know more about the status of this stuff than what I do.

data/constants/keycodes/keycodes_0.0.4_basic.hjson Outdated Show resolved Hide resolved
docs/keycodes.md Show resolved Hide resolved
docs/keycodes_basic.md Show resolved Hide resolved
keyboards/zlant_xl/config.h.old Outdated Show resolved Hide resolved
keyboards/zlant_xl/rules.mk.old Outdated Show resolved Hide resolved
tmk_core/protocol.mk Outdated Show resolved Hide resolved
tmk_core/protocol.mk Outdated Show resolved Hide resolved
SHARE_EP_ENABLE was incorrectly set with GLOBE_KEY_ENABLE, what was
needed is to set the KEYBOARD_SHARED_EP instead. This corrects it.
data/constants/keycodes/keycodes_0.0.7_basic.hjson Outdated Show resolved Hide resolved
docs/keycodes.md Outdated Show resolved Hide resolved
docs/keycodes_basic.md Outdated Show resolved Hide resolved
docs/keycodes.md Outdated Show resolved Hide resolved
tmk_core/protocol.mk Outdated Show resolved Hide resolved
@fauxpark fauxpark self-assigned this Dec 2, 2024
@github-actions github-actions bot added the keymap label Dec 2, 2024
@fauxpark
Copy link
Member

fauxpark commented Dec 2, 2024

Please undo the last merge commit.

@fauxpark
Copy link
Member

fauxpark commented Dec 2, 2024

Now everything else is gone...

@ChrisGVE
Copy link
Contributor Author

ChrisGVE commented Dec 2, 2024

Ok we should be back on track, sorry about the mishap.

@lordpixel23
Copy link

Is there anything truly still open or can this land?

@ChrisGVE
Copy link
Contributor Author

Nothing on my end

@drashna drashna added the breaking_change_2025q1 Targeting breaking changes Q1 2025 label Jan 12, 2025
@drashna
Copy link
Member

drashna commented Jan 12, 2025

Is there anything truly still open or can this land?

I can't speak for others, but I was waiting on @fauxpark, as he has more experience with the USB endpoint stuff, and the apple FN/globe key stuff.

However, develop isn't merged until end of feb

@fauxpark
Copy link
Member

To be completely honest, I'm still hesitant to hit approve on this myself simply due to the fact the only known way to make this key fully functional is to spoof the VID/PID.

My initial goal way back when was to figure out how the Fn key worked as it pertained to the F-keys, and as I learned more about that it became clear that their behaviour changes based on the particular Apple keyboard model. So it seems to me that the Globe key in this current implementation will never really be able to do everything it can on a real device - by design.

By all means, if people are happy with this in its current state, go ahead. It's already got two green ticks. Great work and not your fault @ChrisGVE, but to me personally, it doesn't quite close the case, which is a shame.

@lordpixel23
Copy link

I use it every day as it is because it is already useful. It is frustrating it cannot be 100% but if we find a way to do that it can always be in some future PR.

@tzarc
Copy link
Member

tzarc commented Jan 13, 2025

Can we clearly document the full list of shortcomings before merge, then? A single footnote is a bit hard to link to when it the next person who comes along complains that it doesn’t work as per their Apple-branded device.

@ChrisGVE
Copy link
Contributor Author

To be completely honest, I'm still hesitant to hit approve on this myself simply due to the fact the only known way to make this key fully functional is to spoof the VID/PID.

My initial goal way back when was to figure out how the Fn key worked as it pertained to the F-keys, and as I learned more about that it became clear that their behaviour changes based on the particular Apple keyboard model. So it seems to me that the Globe key in this current implementation will never really be able to do everything it can on a real device - by design.

By all means, if people are happy with this in its current state, go ahead. It's already got two green ticks. Great work and not your fault @ChrisGVE, but to me personally, it doesn't quite close the case, which is a shame.

This is really a shame because I am using this key on all my boards, and I have a few see my userspace repo without any problem. I don't have to change VIP/PID to make it work, whether I tap the key to obtain the emoji picker dialog or when I use it in combination with other keys (either as a single key or when using a hold-esc key + other key on HHKB).

I think I gave a list of cases where it does not work earlier in this PR, and the documentation could be changed to make it more obvious, but I still stand by its usefulness.

(My only challenge will be to port it back to the Keychron board Q60 Max, which is based on an earlier version of QMK...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking_change_2025q1 Targeting breaking changes Q1 2025 core dd Data Driven Changes documentation keyboard keymap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants