qmk / qmk_firmware

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

Windows Remote Desktop ignores auto shift [Bug] #13708

Open gerhard-h opened 3 years ago

gerhard-h commented 3 years ago

Describe the Bug

On my Computer auto shift works as intended, but Windows Remote Desktop ignores auto shift. I tried AUTO_SHIFT_TIMEOUT between 80 - 150 with no effect. Setting Remote Desktop to run Windows-Shortcuts on the client only, would resolve the auto shift problem, but alt+tab, win-key would run locally, wich is not intended

System Information

Additional Context

drashna commented 3 years ago

It's a known issue, and is a timing issue.

Add this to your config.h:

#define TAP_CODE_DELAY 10

And see if that fixes your issue.

drashna commented 3 years ago

Any luck?

gerhard-h commented 3 years ago

I tried TAP_CODE_DELAY from 10 to 90 at wich point the keyboard is nearly unusable but autoshift still didn't work. Maybe TAP_CODE_DELAY is only applied to the letter key but not to shift? Workaround: I finally activated "run Windows-Shortcuts on the client only" and used qmk to remap PgUp, PgDn and Home; in a way I can ALT them easily.

IsaacElenbaas commented 3 years ago

Auto Shift uses TAP_CODE_DELAY here, but not above that when it registers shift because no report is sent. We could manually send one and wait if TAP_CODE_DELAY is set and see if that helps.

gerhard-h commented 3 years ago

As suggested I moved the wait_ms( part after the Shift istead of after the tap But I couldn't test because internet connection is to good today everthing works even without the delay. I also removed all delays from my logitech mouse macros and they also kept working.

I'll report back on another day.

static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) {
   ...
        } else {
            // Simulate pressing the shift key.
            add_weak_mods(MOD_BIT(KC_LSFT));
#if TAP_CODE_DELAY > 0
        wait_ms(TAP_CODE_DELAY);
#endif
            register_code(autoshift_lastkey);
            autoshift_flags.lastshifted = true;
#    if defined(AUTO_SHIFT_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT)
            if (matrix_trigger) {
                // Prevents release.
                return;
            }
#    endif
        }

//    if TAP_CODE_DELAY > 0
//        wait_ms(TAP_CODE_DELAY);
//    endif
IsaacElenbaas commented 3 years ago

That would add delay, but not help with the problem as no keyboard report is sent. See the comment just below where you're changing things - changing weak mods does not send one. Put send_keyboard_report(); above the uppermost delay (the one you added).

gerhard-h commented 3 years ago

I added send_keyboard_report and voila autoshift works 100% of the time.

// Simulate pressing the shift key.
 add_weak_mods(MOD_BIT(KC_LSFT));
#if TAP_CODE_DELAY > 0
            send_keyboard_report();
            wait_ms(TAP_CODE_DELAY);
#endif

But now I realized the other caveat for remote desktop: using shifted key_codes! ~30% of the time when I want to type a shifted keycode like S(KC_4) I get a 4 instead of a $. This is mentioned here: https://beta.docs.qmk.fm/using-qmk/advanced-keycodes/keycodes_us_ansi_shifted#caveats

I could fix this in quantum.c by adding a delay after the f() callback (f is calling send_keyboard_report )

static void do_code16(uint16_t code, void (*f)(uint8_t)) {
   ...
    f(mods_to_send);
    #if TAP_CODE_DELAY > 0
            wait_ms(TAP_CODE_DELAY);
    #endif
}
gerhard-h commented 3 years ago

my patch from above has shortcommings like delaying also on unregister_code16 and delaying twice when using tap_code16.

looking at this patch and the patch to autoshift TAP_CODE_DELAY has now become a little bit of a misleading name; MOD_CODE_DELAY might be a better one. And I think it might be worth to be added to the master branch.

I am also not sure if the original delay in tap_code16 had additional purposes, but because it is also available for tap_code it should maybe not be removed from tap_code16.

static void do_code16(uint16_t code, void (*f)(uint8_t)) {
    switch (code) {
        case QK_MODS ... QK_MODS_MAX:
            break;
        default:
            return;
    }

    uint8_t mods_to_send = 0;

    if (code & QK_RMODS_MIN) {  // Right mod flag is set
        if (code & QK_LCTL) mods_to_send |= MOD_BIT(KC_RCTL);
        if (code & QK_LSFT) mods_to_send |= MOD_BIT(KC_RSFT);
        if (code & QK_LALT) mods_to_send |= MOD_BIT(KC_RALT);
        if (code & QK_LGUI) mods_to_send |= MOD_BIT(KC_RGUI);
    } else {
        if (code & QK_LCTL) mods_to_send |= MOD_BIT(KC_LCTL);
        if (code & QK_LSFT) mods_to_send |= MOD_BIT(KC_LSFT);
        if (code & QK_LALT) mods_to_send |= MOD_BIT(KC_LALT);
        if (code & QK_LGUI) mods_to_send |= MOD_BIT(KC_LGUI);
    }

    f(mods_to_send);
}

void register_code16(uint16_t code) {
    if (IS_MOD(code) || code == KC_NO) {
        do_code16(code, register_mods);

    } else {
        do_code16(code, register_weak_mods);
    }
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
#    if TAP_CODE_DELAY > 0
            wait_ms(TAP_CODE_DELAY);
#    endif
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    register_code(code);
}

void unregister_code16(uint16_t code) {
    unregister_code(code);
    if (IS_MOD(code) || code == KC_NO) {
        do_code16(code, unregister_mods);
    } else {
        do_code16(code, unregister_weak_mods);
    }
}

void tap_code16(uint16_t code) {
    register_code16(code);
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
//Removing the delay from production branch
//#if TAP_CODE_DELAY > 0
//    wait_ms(TAP_CODE_DELAY);
//#endif
// !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    unregister_code16(code);
}