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

[Bug] RGBLIGHT_SLEEP only turns off master half on split keyboard #22794

Open
2 tasks
mikastiv opened this issue Jan 1, 2024 · 6 comments
Open
2 tasks

[Bug] RGBLIGHT_SLEEP only turns off master half on split keyboard #22794

mikastiv opened this issue Jan 1, 2024 · 6 comments

Comments

@mikastiv
Copy link

mikastiv commented Jan 1, 2024

Describe the Bug

The define only works for the master keyboard on my setup when my pc is turned off. I am using a corne keyboard

Keyboard Used

corne/rev1

Link to product page (if applicable)

No response

Operating System

windows 10

qmk doctor Output

Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.1.1
Ψ QMK home: C:/Users/mlebl/qmk_firmware
Ψ Detected Windows 10 (10.0.19045).
Ψ QMK MSYS version: 1.7.2
Ψ Userspace enabled: False
Ψ Git branch: master
⚠ Git has unstashed/uncommitted changes.
Ψ - Latest master: 2023-12-31 18:27:03 -0500 (50a9658502) -- zig logo
Ψ - Latest upstream/master: 2023-12-31 19:25:58 +0100 (90811118b7) -- docs(skeletyl): fix readme instructions (#22791)
Ψ - Latest upstream/develop: None
Ψ - Common ancestor with upstream/master: 2023-12-23 17:49:37 -0600 (a1d29982dc) -- Add Momokai Aurora Image (#22728)
Ψ - Common ancestor with upstream/develop: None
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 10.1.0
Ψ Found avr-gcc version 8.5.0
Ψ Found avrdude version 7.0
Ψ Found dfu-programmer version 0.7.2
Ψ Found dfu-util version 0.11
Ψ Submodules are up to date.
Ψ Submodule status:
Ψ - lib/chibios: 2023-04-15 13:48:04 +0000 --  (11edb1610)
Ψ - lib/chibios-contrib: 2023-07-17 11:39:05 +0200 --  (da78eb37)
Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 --  (e2239ee6)
Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 --  (549b97320)
Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 --  (819dbc1)
Ψ - lib/printf: 2022-06-29 23:59:58 +0300 --  (c2e3b4e)
Ψ - lib/pico-sdk: 2023-02-12 20:19:37 +0100 --  (a3398d8)
Ψ - lib/lvgl: 2022-04-11 04:44:53 -0600 --  (e19410f8)
Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

  • AutoHotKey (Windows)
  • Karabiner (macOS)

Other keyboard-related software installed

No response

Additional Context

I have pro-micro microcontrollers https://keebmaker.com/collections/parts/products/controllers-pro-micro-c?_pos=1&_fid=bdf159e90&_ss=c

@drashna
Copy link
Member

drashna commented Jan 4, 2024

To make sure, when updating the config, have you flashed both sides?

@mikastiv
Copy link
Author

mikastiv commented Jan 4, 2024

Yes I did. I tried multiple times and it doesn't work for me.
I would actually be willing to work on this if no body does and someone could guide me a bit; it annoys me to see my keyboard with half the lights on 😅

I tried to investigate a bit and from what I understood, the master keyboard is stuck in a loop in protocol_pre_task during suspend and never sends to the slave that is should suspend, but maybe I'm wrong

@mikastiv
Copy link
Author

mikastiv commented Jan 16, 2024

I found why it was not working on my keyboard; here:

if (USB_Device_RemoteWakeupEnabled && suspend_wakeup_condition()) {
USB_Device_SendRemoteWakeup();
clear_keyboard();

USB_Device_RemoteWakeupEnabled is false for me so suspend_wakeup_condition() is never run and the lights suspend state is never synced to the keyboard slave. For myself. I added a matrix_scan call inside the suspend loop to run the same code from suspend_wakeup_condition() and it works. I don't know if this is a good solution or not. Thoughts?

@mikastiv
Copy link
Author

I would propose this as a fix

const bool wakeup = suspend_wakeup_condition();
if (USB_Device_RemoteWakeupEnabled && wakeup) {
    ...
}

@Filios92
Copy link
Contributor

I've been having the same issue (altough I think it didn't always happened on PC turn off) - I have been mitigating it by suspending RGB after 20s of idle / not using keyboard (which has some other issue, details below).
Seems yours proposed fix helped!

(The other issue I have, which I thought could be related, is after calling rgblight_suspend the slave sometimes doesn't set it's rgblight_config.enable to the one indicated by master - maybe there is some race or maybe just serial comm error, but I haven't seen any. I have fixed it with (in rgblight_update_sync which I always call in rgblight_handlers_slave):

if (syncinfo->config.enable != rgblight_config.enable)
        syncinfo->status.change_flags |= RGBLIGHT_STATUS_CHANGE_MODE;
if (syncinfo->status.change_flags == 0) // not needed really
        return;

to force slave's enable update if changed (as the whole config is synced periodically either way, not just on status change)
But this seems to me other issue / PR, as yours fix solved PC turn off issue, I may yet to check mine rgb_suspend fix alone after turning off PC)

@JuanoD
Copy link
Contributor

JuanoD commented Jul 29, 2024

#24055 fixes this. We just have to wait for it to be merged into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants