OpenAnnePro / qmk_firmware

This has since been merged back to mainline QMK. Please use qmk/qmk_firmware
https://qmk.fm
GNU General Public License v2.0
196 stars 64 forks source link

Enable QMK RGB Matrix #49

Closed Jpe230 closed 1 year ago

Jpe230 commented 2 years ago

Solution to enable QMK RGB Matrix, this work by implementing a custom RGB matrix driver that updates shine.

Description

Made an extra keymap to avoid conflicts with other existing keymaps. Users have the choice of still using the previous implementation or use this new feature.

Types of Changes

Issues Fixed or Closed by This PR

Checklist

Codetector1374 commented 2 years ago

Okay yeah this is pretty cool

Jpe230 commented 2 years ago

I've added two new keymaps, both work the same as the default keymaps (full caps and layer indicators). but now using QMK RGB matrix to update the colors instead of talking directly to Shine.

Also in this new commit I moved "annepro2LedMaskSetRow" to matrix_scan_kb, with these changes we are effectively pushing to Shine the RGB buffer nearly all the time. Now effects that don't modify the whole buffer (raindrops, for example) display correctly.

blaa commented 2 years ago

I've ported my keymap to it and try to daily drive it - a version with openrgb support.
In general works, seems responsive.

Possible things to check:

Question is what we want to merge. Having LED QMK integration will be nice for QMK merge. But I think it would be better if it merged first, and then push this changes there.

OpenRGB is probably not mergeable into QMK, but fun and useful and could be kept on the master of ap2 repo.

I'd generally merge QMK-led first, and orgb support as a separate commit.

Jpe230 commented 2 years ago

My comments:

  • BT blinking won't work with this enabled as it overrides the blinking color.
  • In general the concept that the shine controls the color and main chip overrides some buttons (caps lock, blink, some fn keys) doesn't work with QMK/openrgb controlling the leds. The alpha channel could be therefore dropped completely (internal protocol will be faster then too), and blinking could get a priority inside shine chip.

For BT blinking maybe we can create a new command to override the mask (?). Dropping alpha means that we are committing in using QMK implementation, therefore existing keymaps would need to be updated. if @Codetector1374 likes the idea I'm all in.

OpenRGB is probably not mergeable into QMK, but fun and useful and could be kept on the master of ap2 repo. I'd generally merge QMK-led first, and orgb support as a separate commit.

Yes, totally agreed in this one, that's why this PR doesn't include any reference for OpenRGB. If you like I can make a new PR with OpenRGB commits or add them to this PR to be cherrypicked later.

Codetector1374 commented 1 year ago

Sorry I have not been keeping up wit the update here, this is some great work. I've tried this myself on my own board, I think we can merge this for now. ORGB support can also be ported if there is a need for it. I personally find the QMK RGB is all I need personally.