Change split_common to use RGBLIGHT_SPLIT #5509

mtei · 2019-03-29T14:47:49Z

This PR needs to merge #5020 first.

If you want to test the behavior of this PR, please temporarily cherry pick 647c0a9.

Description

Change split_common to synchronize rgblight using RGBLIGHT_SPLIT.

When using I2C master-slave communication
Enabling rgblight implicitly also enables RGBLIGHT_SPLIT.
(see quantum quantum/split_common/post_config.h)
When using serial master-slave communication
Enabling rgblight does not enable RGBLIGHT_SPLIT.
If necessary, define RGBLIGHT_SPLIT in config.h of the keyboard.
(see quantum quantum/split_common/post_config.h)

Types of Changes

Checklist

My code follows the code style of this project.
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).

Improvements to ease the maintenance of the I2C slave buffer layout. And this commit does not change the compilation results.

add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h

Add quantum/rgblight_post_config.h and quantum/split_common/post_config.h using POST_CONFIG_H variable of build_keyboard.mk. quantum/rgblight_post_config.h additionally defines RGBLIGHT_SPLIT if RGBLED_SPIT is defined. quantum/split_common/post_config.h defines RGBLIGHT_SPLIT additionally when master-slave communication is I2C.

…ure of rgblight.c

…eature of rgblight.c

This reverts commit 80118a6. [x] make RGBLIGHT_TEST=1 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/i2c:default (same RGBLIGHT_TEST=3) [x] make RGBLIGHT_TEST=3 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2_2oled:default

This reverts commit d35069f.

This reverts commit aebddfc.

…ci test. add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h

to see if it passes the travis-ci test." This reverts commit 647c0a9.

docs/config_options.md

…ransaction IDs. No change in build result.

Lenbok · 2019-04-03T09:04:41Z

@mtei It looks like some of the split keyboards are failing to build in travis. If this is otherwise ready for testing I can give this a whirl this weekend on my redox.

mtei · 2019-04-03T09:10:13Z

@Lenbok You can try checkout 647c0a9. I think it is work well.

drashna · 2019-04-03T17:05:20Z

Yeah, I think that this PR depends on #5020 to be merged first. So without that, this one may fail.
I'll see if I can get both PRs pushed through

drashna · 2019-04-07T14:42:53Z

Just to clarify, the issue still occurs, with the corrected code.

drashna · 2019-04-11T20:41:20Z

Okay, it looks like this may be a hardware issue with one of my boards. I can't reproduce it on another split keyboard, both iris keyboards, actually. Even swapping usb cables and TRRS cables. So ... yay.

But that means that this should be good to go!

This reverts commit be48ca1.

This reverts commit 978d26a.

drashna · 2019-04-18T21:04:33Z

build_keyboard.mk

@@ -280,6 +280,23 @@ ifneq ("$(wildcard $(KEYBOARD_PATH_1)/config.h)","")
    CONFIG_H += $(KEYBOARD_PATH_1)/config.h
 endif

+POST_CONFIG_H :=


Is this required to work properly?

Eg, can the post_config stuff be moved so this isn't needed?

This is necessary.
It is necessary to prevent unnecessary dependencies between header files between modules in lower layers.

For example, quantum/split_common/transport.c needs to define SERIAL_USE_MULTI_TRANSACTION of serial when using RGBLIGHT_SPLIT.
It is not good to solve this with serial.h and rgblight.h.

drashna · 2019-04-22T04:39:00Z

I was wrong.

The issue is when switching to mode 1 (solid color). Any time you change to it, the slave half does NOT switch.

* add I2C_slave_buffer_t to quantum/split_common/transport.c Improvements to ease the maintenance of the I2C slave buffer layout. And this commit does not change the compilation results. * add temporary pdhelix(Patched Helix) code * temporary cherry-pick from qmk#5020 add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h * add post_config.h support to build_keyboard.mk * add quantum/rgblight_post_config.h, quantum/split_common/post_config.h Add quantum/rgblight_post_config.h and quantum/split_common/post_config.h using POST_CONFIG_H variable of build_keyboard.mk. quantum/rgblight_post_config.h additionally defines RGBLIGHT_SPLIT if RGBLED_SPIT is defined. quantum/split_common/post_config.h defines RGBLIGHT_SPLIT additionally when master-slave communication is I2C. * Change split_common's transport.c I2C to use the synchronization feature of rgblight.c * Change split_common's transport.c serial to use the synchronization feature of rgblight.c * test RGBLIGHT_SPLIT on keyboards/handwired/pdhelix * Test End Revert "test RGBLIGHT_SPLIT on keyboards/handwired/pdhelix" This reverts commit 80118a6. [x] make RGBLIGHT_TEST=1 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/i2c:default (same RGBLIGHT_TEST=3) [x] make RGBLIGHT_TEST=3 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2_2oled:default * Test End, Revert "temporary cherry-pick from qmk#5020" This reverts commit d35069f. * Test End, Revert "add temporary pdhelix(Patched Helix) code" This reverts commit aebddfc. * temporarily cherry-pick from qmk#5020 to see if it passes the travis-ci test. add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h * Passed the travis-ci test. Revert "temporarily cherry-pick from qmk#5020 to see if it passes the travis-ci test." This reverts commit 647c0a9. * update docs/config_options.md * update split_common/transport.c, improves maintainability of serial transaction IDs. No change in build result. * temporary cherry-pick from qmk#5020 * fix build fail keebio/iris/rev3:default * fix build fail lets_split_eh/eh:default * Revert "temporary cherry-pick from qmk#5020" This reverts commit be48ca1. * temporary cherry-pick from qmk#5020 (0.6.336) * Revert "temporary cherry-pick from qmk#5020 (0.6.336)" This reverts commit 978d26a. * temporary cherry-pick from qmk#5020 (0.6.336)

mtei · 2019-04-22T18:58:39Z

I am sorry. I'm debugging now.

drashna · 2019-04-22T19:28:58Z

Well, I saw it earlier, but couldn't reliably reproduce. Unfortunately, now I can, for whatever reason.

And I wanted to make you aware of the issue, so it could get fixed.

Other than this issue, it work perfectly.

Lenbok · 2019-04-22T20:46:16Z

Switching to mode 1 is reliable for me on my redox, (at least when switching via RGB_MOD, both regular and shifted). Are you changing mode programmatically, or via a key that goes straight to the mode, or does it do it for you via RGB_MOD as well?

drashna · 2019-04-22T22:57:17Z

@Lenbok I'm using rgblight_mode_noeeprom specifically, and doing so via the layer_state_set_user function.

The base layer is solid, not animated... and triggers this. If I change the base layer to be animated instead of solid, there is no issue.

You can see my code here: https://github.com/qmk/qmk_firmware/blob/master/users/drashna/rgb_stuff.c#L278-L337

mtei · 2019-04-23T19:27:47Z

@drashna I found some bugs in rgblight.c (#5694), but unfortunately I was unable to reproduce the symptoms that were in your report.

I tested it with the following code and got back to mode 1 successfully.

https://github.com/mtei/qmk_firmware/blob/a89b747a8996700c9a87fda37ffe1326d1917af1/keyboards/handwired/pdhelix/keymaps/default/keymap.c#L138-L162

drashna · 2019-04-23T20:09:07Z

Even with the patch, I can reproduce the issue still.

And I can reliably reproduce it on 2 out of three Iris boards (the 3rd has backlight enabled, and doesn't cause the issue), and on both my orthodox rev1 and rev3.

It seems to be worse when the layers change rapidly.

* add I2C_slave_buffer_t to quantum/split_common/transport.c Improvements to ease the maintenance of the I2C slave buffer layout. And this commit does not change the compilation results. * add temporary pdhelix(Patched Helix) code * temporary cherry-pick from qmk#5020 add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h * add post_config.h support to build_keyboard.mk * add quantum/rgblight_post_config.h, quantum/split_common/post_config.h Add quantum/rgblight_post_config.h and quantum/split_common/post_config.h using POST_CONFIG_H variable of build_keyboard.mk. quantum/rgblight_post_config.h additionally defines RGBLIGHT_SPLIT if RGBLED_SPIT is defined. quantum/split_common/post_config.h defines RGBLIGHT_SPLIT additionally when master-slave communication is I2C. * Change split_common's transport.c I2C to use the synchronization feature of rgblight.c * Change split_common's transport.c serial to use the synchronization feature of rgblight.c * test RGBLIGHT_SPLIT on keyboards/handwired/pdhelix * Test End Revert "test RGBLIGHT_SPLIT on keyboards/handwired/pdhelix" This reverts commit 80118a6. [x] make RGBLIGHT_TEST=1 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/i2c:default (same RGBLIGHT_TEST=3) [x] make RGBLIGHT_TEST=3 handwired/pdhelix/i2c:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2:default [x] make RGBLIGHT_TEST=1 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=2 handwired/pdhelix/pd2_2oled:default [x] make RGBLIGHT_TEST=3 handwired/pdhelix/pd2_2oled:default * Test End, Revert "temporary cherry-pick from qmk#5020" This reverts commit d35069f. * Test End, Revert "add temporary pdhelix(Patched Helix) code" This reverts commit aebddfc. * temporarily cherry-pick from qmk#5020 to see if it passes the travis-ci test. add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h * Passed the travis-ci test. Revert "temporarily cherry-pick from qmk#5020 to see if it passes the travis-ci test." This reverts commit 647c0a9. * update docs/config_options.md * update split_common/transport.c, improves maintainability of serial transaction IDs. No change in build result. * temporary cherry-pick from qmk#5020 * fix build fail keebio/iris/rev3:default * fix build fail lets_split_eh/eh:default * Revert "temporary cherry-pick from qmk#5020" This reverts commit be48ca1. * temporary cherry-pick from qmk#5020 (0.6.336) * Revert "temporary cherry-pick from qmk#5020 (0.6.336)" This reverts commit 978d26a. * temporary cherry-pick from qmk#5020 (0.6.336)

mtei added 11 commits March 29, 2019 16:00

add temporary pdhelix(Patched Helix) code

aebddfc

add I2C_slave_buffer_t to quantum/split_common/transport.c

c01656a

Improvements to ease the maintenance of the I2C slave buffer layout. And this commit does not change the compilation results.

temporary cherry-pick from qmk#5020

d35069f

add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h

add post_config.h support to build_keyboard.mk

f6e8aeb

Change split_common's transport.c I2C to use the synchronization feat…

57e124c

…ure of rgblight.c

Change split_common's transport.c serial to use the synchronization f…

07c33fc

…eature of rgblight.c

test RGBLIGHT_SPLIT on keyboards/handwired/pdhelix

80118a6

Test End, Revert "temporary cherry-pick from qmk#5020"

4d7310f

This reverts commit d35069f.

Test End, Revert "add temporary pdhelix(Patched Helix) code"

c8d1c83

This reverts commit aebddfc.

mtei mentioned this pull request Mar 29, 2019

Added encoder support to split common code #5477

Merged

13 tasks

mtei added 2 commits March 30, 2019 01:19

temporarily cherry-pick from qmk#5020 to see if it passes the travis-…

647c0a9

…ci test. add new version(qmk#5020) quantum/rgblight.[ch], quantum/rgblight_modes.h

Passed the travis-ci test. Revert "temporarily cherry-pick from qmk#5020

6b77e60

to see if it passes the travis-ci test." This reverts commit 647c0a9.

drashna approved these changes Mar 29, 2019

View reviewed changes

drashna requested review from mechmerlin, fredizzimo, algernon, ezuk, jackhumbert, skullydazed and yanfali March 29, 2019 17:53

drashna added enhancement core labels Mar 29, 2019

yanfali reviewed Mar 29, 2019

View reviewed changes

docs/config_options.md Outdated Show resolved Hide resolved

mtei added 2 commits March 30, 2019 03:43

update docs/config_options.md

35d6855

update split_common/transport.c, improves maintainability of serial t…

c68ee60

…ransaction IDs. No change in build result.

mtei added 5 commits April 16, 2019 19:10

Revert "temporary cherry-pick from qmk#5020"

419f045

This reverts commit be48ca1.

temporary cherry-pick from qmk#5020 (0.6.336)

978d26a

Revert "temporary cherry-pick from qmk#5020 (0.6.336)"

4c56c82

This reverts commit 978d26a.

Merge branch 'master' into split_common-use-RGBLIGHT_SPLIT

e5f59ad

temporary cherry-pick from qmk#5020 (0.6.336)

fa28d10

drashna reviewed Apr 18, 2019

View reviewed changes

drashna added the awaiting review label Apr 18, 2019

skullydazed approved these changes Apr 19, 2019

View reviewed changes

drashna merged commit 7e67bd7 into qmk:master Apr 19, 2019

mtei mentioned this pull request Apr 24, 2019

FIX rgblight.c split sync bug #5694

Merged

13 tasks

mtei deleted the split_common-use-RGBLIGHT_SPLIT branch May 11, 2019 06:43

drashna mentioned this pull request Jul 7, 2019

RGB Light on Split Common doesn't properly sync #6277

Closed

drashna mentioned this pull request May 25, 2020

[Bug] RGB Light on split keyboard doesn't sync slave half correctly #8789

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change split_common to use RGBLIGHT_SPLIT #5509

Change split_common to use RGBLIGHT_SPLIT #5509

mtei commented Mar 29, 2019 •

edited

Loading

Lenbok commented Apr 3, 2019

mtei commented Apr 3, 2019 •

edited

Loading

drashna commented Apr 3, 2019

drashna commented Apr 7, 2019

drashna commented Apr 11, 2019 •

edited

Loading

drashna Apr 18, 2019

mtei Apr 18, 2019

drashna commented Apr 22, 2019

mtei commented Apr 22, 2019

drashna commented Apr 22, 2019

Lenbok commented Apr 22, 2019

drashna commented Apr 22, 2019

mtei commented Apr 23, 2019 •

edited

Loading

drashna commented Apr 23, 2019

Change split_common to use RGBLIGHT_SPLIT #5509

Change split_common to use RGBLIGHT_SPLIT #5509

Conversation

mtei commented Mar 29, 2019 • edited Loading

This PR needs to merge #5020 first.

Description

Types of Changes

Checklist

Lenbok commented Apr 3, 2019

mtei commented Apr 3, 2019 • edited Loading

drashna commented Apr 3, 2019

drashna commented Apr 7, 2019

drashna commented Apr 11, 2019 • edited Loading

drashna Apr 18, 2019

Choose a reason for hiding this comment

mtei Apr 18, 2019

Choose a reason for hiding this comment

drashna commented Apr 22, 2019

mtei commented Apr 22, 2019

drashna commented Apr 22, 2019

Lenbok commented Apr 22, 2019

drashna commented Apr 22, 2019

mtei commented Apr 23, 2019 • edited Loading

drashna commented Apr 23, 2019

mtei commented Mar 29, 2019 •

edited

Loading

mtei commented Apr 3, 2019 •

edited

Loading

drashna commented Apr 11, 2019 •

edited

Loading

mtei commented Apr 23, 2019 •

edited

Loading