zmkfirmware / zmk

ZMK Firmware Repository
https://zmk.dev/
MIT License
2.48k stars 2.56k forks source link

Soft off on the right side of Corne split keyboard issues #2300

Closed GermanG closed 1 month ago

GermanG commented 1 month ago

This a sequel of #2274, which is partially solved in #2285 The behavior is not consistent, most of the times works perfectly, but sometimes the right side is still on. Reporting this now, I'll update my firmware to debug and update this issue when it happens. On the other hand I don't know where to look to use the wakeup-source in my config, because there are some use cases that have no reset switch.

GermanG commented 1 month ago

On the right side there isn't any event, on the left I saw this

[00:00:07.966,278] <dbg> zmk: position_state_changed_listener: 29 bubble (no undeci
ded hold_tap active)                                                               

[00:00:07.966,308] <dbg> zmk: zmk_keymap_apply_position_state: layer: 2 position: 2
9, binding name: soft_off  
[00:00:07.966,369] <dbg> zmk: split_bt_invoke_behavior_payload:                    
[00:00:07.966,430] <dbg> zmk: split_central_split_run_callback:                                                                                                      
[00:00:13.844,177] <dbg> zmk: kscan_matrix_read: Sending event at 2,5 state

But when it works I saw:

[00:00:02.845,306] <dbg> zmk: zmk_keymap_apply_position_state: layer: 2 position: 29, binding name: soft_off
[00:00:02.845,367] <dbg> zmk: split_bt_invoke_behavior_payload: 
[00:00:02.845,458] <dbg> zmk: split_central_split_run_callback: 
[00:00:05.342,163] <dbg> zmk: kscan_matrix_read: Sending event at 2,5 state off
[00:00:05.342,254] <dbg> zmk: zmk_ksc

My educated guess is that the left side goes down before completing the request to the right side.

GermanG commented 1 month ago

I added this to app/src/behaviors/behavior_soft_off.c

           if (!IS_SPLIT_PERIPHERAL) {
                k_usleep(100000);
            }

the whole fragment looks like

        if (hold_time > config->hold_time_ms) {
            if (!IS_SPLIT_PERIPHERAL) {
                k_usleep(100000);
            }
            zmk_pm_soft_off();
        } else {
            LOG_INF("Not triggering soft off: held for %d and hold time is %d", hold_time,
                    config->hold_time_ms);
        }

So far so good. I think the issue is introduced because the event is triggered when the key is released, so it's a matter of luck.

petejohanson commented 1 month ago

I added this to app/src/behaviors/behavior_soft_off.c

           if (!IS_SPLIT_PERIPHERAL) {
                k_usleep(100000);
            }

the whole fragment looks like

        if (hold_time > config->hold_time_ms) {
            if (!IS_SPLIT_PERIPHERAL) {
                k_usleep(100000);
            }
            zmk_pm_soft_off();
        } else {
            LOG_INF("Not triggering soft off: held for %d and hold time is %d", hold_time,
                    config->hold_time_ms);
        }

So far so good. I think the issue is introduced because the event is triggered when the key is released, so it's a matter of luck.

I think this should be tweaked a bit to only add the sleep if we are specifically a split central. Does it work as expected with that in place?

GermanG commented 1 month ago

So far my hack is working, I'm giving it more time to be sure. I'll open a PR if it goes ok for a couple of days.