qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.16k stars 39.1k forks source link

[Feature Request] RGB_MATRIX feature for split keyboards #9619

Closed tyalie closed 3 years ago

tyalie commented 4 years ago

As of now split keyboards don't support rgb_matrix out of the box. The nature of the problem lies in the asynchron internal timers of both halves, the disabled keypress evaluation on the slave side, ...

Making it work seamlessly is the goal of the current repo, but there're a few problems to solve first.

Feature Request Type

Description

As mentioned before rgb matrix doesn't work together with split keyboards. There're some forks (#5998 and an iteration on that: #9613) that seem promising, but aren't perfect either. Major problems that need to be fixed first are:

tyalie commented 4 years ago

Suggestions are welcome

tzarc commented 4 years ago

Probably need layer state sync too, in order to deal with lighting layers? (Is that RGBLIGHT-only?)

tyalie commented 4 years ago

I haven't really looked into the matter on the RGB Matrix side, but I'm not even sure RGB Matrix has something like that without some hacks. At least the rgb_matrix.h doesn't mention anything to my knowledge

Probably what #717 was for.

sigprof commented 4 years ago

Probably need layer state sync too, in order to deal with lighting layers? (Is that RGBLIGHT-only?)

Yes, lighting layers are currently implemented only in the rgblight code, not in rgbmatrix; and the state of rgblight layers is already synced, so they technically should not need anything more.

Actually there might be some conflicts if we attempt to sync the layer state to the slave side (which would presumably call layer_state_set_user() to update various kinds of indicators) and at the same time send the rgblight layer state there directly.

The RGB Matrix code, on the other hand, has rgb_matrix_indicators_user(), which works in a completely different way (it can pull information for various places to update the LED state), and it would need layer state sync if the user wants some LEDs to show the layer state.

tyalie commented 4 years ago

Hmh. I haven't thought about that. But rgb_matrix_indicator_user() could be very ugly to implement. I'll add it to the list.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

drashna commented 3 years ago

Removing the "in progress" tag, so that this can be closed.

develop (as of May 25th) has everything in place for this to work, now. And in a week, the code will hit master. (as well as the removal of the "legacy" corne revision)

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.