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] Sending unicode via XP macro uses a delayed Shift state #9533

Closed
1 of 3 tasks
Ceremony64 opened this issue Jun 24, 2020 · 4 comments
Closed
1 of 3 tasks

[Bug] Sending unicode via XP macro uses a delayed Shift state #9533

Ceremony64 opened this issue Jun 24, 2020 · 4 comments

Comments

@Ceremony64
Copy link
Contributor

Ceremony64 commented Jun 24, 2020

Describe the Bug

The XP keycode macro allows for using shift/capslock state to choose between two unicodes, just like a regular alphabet key outputs a lower or upper case character, depending on the current shift/capslock state.
Unfortunately, this is no longer working as intended: The shift state when triggering a XP macro gets delayed once every input. Let me explain with this very simple keyboard layout/config (keymap.c):

#include QMK_KEYBOARD_H

enum unicode_names {
    a,
    A
};

const uint32_t PROGMEM unicode_map[] = {
    [a] = 0x0061, // a
    [A] = 0x0041  // A
};

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
    [0] = LAYOUT_3(
        KC_LSFT, XP(a,A), KC_B
    )
};

When triggering the XP macro, the state of the last shift state of the previous XP trigger is used for the current one.
Here is the a sequential key press table of what is happening:

KC_LSFT Key Output Expected
No XP(a,A) a a
Yes XP(a,A) a A
Yes XP(a,A) A A
No XP(a,A) A a
No XP(a,A) a a

Also, the shift for the XP macro is saved even when shift has been let go or even when another key has been pressed inbetween:

KC_LSFT Key Output Expected
No XP(a,A) a a
Yes XP(a,A) a A
No KEY_B b b
No XP(a,A) A a
No XP(a,A) a a

System Information

  • Keyboard: XD96
    • Revision (if applicable):
  • Operating system: Windows 10 2004 & Linux Manjaro (kernel 5.7)
  • AVR GCC version: 5.4.0
  • ARM GCC version: 7.3.1
  • QMK Firmware version: 0.9.19
  • Any keyboard related software installed?
    • AutoHotKey
    • Karabiner
    • Other: WinCompose 0.9.4 (on Windows)

Additional Context

Issue occurs on both linux and windows, so its OS agnostic. Also, XP worked for me last time I compiled it, which was a little over a year ago tho, so something must have changed

@jobe451
Copy link

jobe451 commented Jul 27, 2020

Bug also confirmed on Win 10 and Ubuntu 20.04 with handwired/dactyl_manuform

@sigprof
Copy link
Contributor

sigprof commented Jul 29, 2020

Looks like this was broken in #8770. Before that change process_unicodemap() invoked unicodemap_index() after unicode_input_start(), and therefore unicodemap_index() needed to get the modifier state from unicode_saved_mods, because unicode_input_start() temporarily resets the modifier state to make Unicode input work properly. After #8770 process_unicodemap() invokes unicodemap_index() before invoking any Unicode input functions, therefore unicodemap_index() now needs to get the modifier state in the usual way (get_mods(), and probably also get_weak_mods() and get_oneshot_mods() to make those things affect the XP() keys properly, which may also fix #9405).

@rupa
Copy link
Contributor

rupa commented Jul 29, 2020

I was able to find and swap out the unicode_saved_mods reference @sigprof kindly pointed out for a call to get_mods() and it did fix this issue for me :)

@spidey3
Copy link
Contributor

spidey3 commented Dec 22, 2020

With the merging of #11220, this should now be fixed.

@drashna drashna closed this as completed Dec 27, 2020
nblyumberg pushed a commit to nblyumberg/qmk_firmware that referenced this issue Dec 31, 2020
xgnxs pushed a commit to xgnxs/qmk_firmware that referenced this issue Jan 9, 2021
drashna pushed a commit to zsa/qmk_firmware that referenced this issue Jan 13, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this issue May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants