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

Vr id67 rgb #3

Merged
merged 9 commits into from
Jan 14, 2022
Merged

Vr id67 rgb #3

merged 9 commits into from
Jan 14, 2022

Conversation

vinorodrigues
Copy link
Collaborator

@vinorodrigues vinorodrigues commented Jan 14, 2022

Proposing a few changes on this one:

config.h

  1. Remove "RGB Light" config, lines 49-70. This is redundant and not used
  2. Introduce a ID67_DISABLE_UNDERGLOW define that disables LED's 68-77 (the 10 LEDs on the underside of the board). This will allow self-compile users to disable these LEDs if they have boards that have a solid backplate, like a standard ID67, ID67V2, and ID67 Bestype. The "default" compiles will INCLUDE the LED's as QMK admin will want the base code to include all the features of the PCB.
  3. Minor fixes to the spaces between #ifdef's to make the code more readable.

id67.c

  1. @peepeetee - I know you put a lot of work into manually mapping the LEDs for the g_led_config global, but the doc does state to keep to the {224, 64} window. This mapping in based on my spreadsheet method covered in my Gist. I've tested most of the animations, all the spirals are centred, & all the crosshair effects are in a "straight" line.
  2. Added ‌#ifndef ID67_DISABLE_UNDERGLOW to the matrix to optionally disable the underglow LEDs. Note this is a "if NOT def" ... so the default complies will always include the underglow.
  3. Added a __attribute__ ((weak)) void rgb_matrix_indicators_user() function to give the base compile a Caps-Lock LED function (as used in other 67's in the QMK repo).

id67.h

  1. Minor clean of code (using __ instead of KC_NO to make it more readable).

default_nug/* & via_nug/*

  1. Added "NUG" (No Under-Glow) variants of the default and via keycaps, with appropriate note.

readme.md

  1. Note in the maintainer section that this is @thewerther 's work.
  2. Added list of ID67 variants. Only because someday someone will ask if this works for the V2 or something.

Final thoughts

  1. I've tested all five keymaps, all compile no problem.
  2. I have not tested if the underglow actually does get disabled in the *_nug maps. @peepeetee, you have the PCB only test bench, can you check for me?
  3. I would like @Thewether 's input on this before you reject or accept this pull to your own repo. - that will propagate into the [Keyboard]Remove RGB light implementation of ID67, revert the file structure changes by #15558 qmk/qmk_firmware#15854
  4. I have my own keymap that I build around a Mac layout and some nifty Caps-Lock lighting effects. I DID NOT INCLUDE that (didn't want to be presumptuous), but I would be honoured if you / @Thewether would allow me to add it to this. :)

--made with ♡, Vino Rodrigues

Copy link

@thewerther thewerther left a comment

Choose a reason for hiding this comment

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

Looks good to me, go ahead and also add your keymap, it'll add an additional example on how to customize the keymap/effects and whatnot.

@peepeetee peepeetee merged commit 55d8a8a into peepeetee:id67_rgb Jan 14, 2022
@vinorodrigues vinorodrigues deleted the vr_id67_rgb branch January 14, 2022 07:38
peepeetee pushed a commit that referenced this pull request Jan 16, 2022
Changes all mentions of 0.15.12 to 0.14.23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants