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

Implement a configurable modifier indicator color #19

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

khoek
Copy link

@khoek khoek commented Apr 4, 2024

Built on top of #18. We add a Kconfig option for the modifier CAPS/SCOLL/NUM LOCK indicator LEDs, allowing their color to be configured, so that e.g. their accidental activation is more distinguishable. Tested on a real Adv360 Pro.

@ReFil
Copy link
Owner

ReFil commented Apr 5, 2024

As with the other PR. it would be great if you could open a PR to document this config option in the config repo

@khoek
Copy link
Author

khoek commented Apr 7, 2024

Documentation done in KinesisCorporation/Adv360-Pro-ZMK#431. I hope you think it looks OK!

Sorry about the noise, I rebased couple times a little too eagerly while I changed what was underneath. All done now.

@ReFil
Copy link
Owner

ReFil commented Apr 7, 2024

One last docs related thing. We changelog work to the base firmware too, so could you rebase your docs PR off the zephyr 3.5 PR (KinesisCorporation/Adv360-Pro-ZMK#426) and append the base repo changelog and config repo changelog accordingly please? I'll try and find the time to test all your work today and will merge in as soon as it all tests well

@khoek
Copy link
Author

khoek commented Apr 7, 2024

No worries. Hopefully I've changelogged things correctly now.

BTW, this is me just selfishly asking now: I found a race condition in the USB HID code zmkfirmware#2257 which unfortunately affects the Adv360's SoC, and so stopped my >F12 function keys from working when not over bluetooth. What is the process by which those sort of changes get pulled downstream here?

@ReFil
Copy link
Owner

ReFil commented Apr 8, 2024

As a general rule i try to avoid jumping the queue on pulling upstream PRs into the 360 pro's branch as there';s usually a conflict to resolve when that happens (such as the different naming of the extended nkro kconfig). I can see your PR is not that big, so i might cherry pick it if it doesn't get reviewed in the next few weeks

@khoek khoek force-pushed the adv360-mod-color branch from e40ced7 to e0eee7a Compare April 9, 2024 08:00
@ReFil
Copy link
Owner

ReFil commented Apr 9, 2024

Sorry, more conflicts that need to be untangled here

@khoek khoek force-pushed the adv360-mod-color branch from e0eee7a to ec4b77f Compare April 10, 2024 01:14
@khoek
Copy link
Author

khoek commented Apr 10, 2024

No worries, rebased again.

@ReFil ReFil merged commit 2fcd15d into ReFil:adv360-z3.5 Apr 10, 2024
31 of 32 checks passed
@ReFil
Copy link
Owner

ReFil commented Apr 10, 2024

Thank you very much for all your work on these features!

@ReFil
Copy link
Owner

ReFil commented Apr 10, 2024

Regarding your upstream USB PR, given it looks like it might be going all the way up to to the hal driver level I could be open to merging a short term fix now

@khoek khoek deleted the adv360-mod-color branch April 11, 2024 01:55
ReFil pushed a commit that referenced this pull request Nov 21, 2024
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.

2 participants