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

[Core] Fix flipped logic bug with One Shot OS_ON / OS_OFF keys #16617

Merged
merged 1 commit into from
Mar 11, 2022
Merged

[Core] Fix flipped logic bug with One Shot OS_ON / OS_OFF keys #16617

merged 1 commit into from
Mar 11, 2022

Conversation

getreuer
Copy link
Contributor

The One Shot documentation says key OS_ON turns one-shot keys on and OS_OFF turns them off. The actual behavior is flipped: OS_ON turns them off and OS_OFF turns them on. This PR fixes this.

Description

This bug was discovered by reddit user u/Dutchnesss1. I can reproduce it on my keyboards. We were troubleshooting a problem where one-shot keys weren't work on Dutchnesss1's keyboard, and confusingly, OS_ON disabled one-shot keys when we thought we were enabling them.

The underlying problem is the logic for functions oneshot_set(), oneshot_enable(), oneshot_disabled(), and is_oneshot_enabled() are similarly flipped. The bug is rather obvious once looking at their implementation (code link):

void oneshot_set(bool active) {
    if (keymap_config.oneshot_disable != active) {
        keymap_config.oneshot_disable = active;
        eeconfig_update_keymap(keymap_config.raw);
        clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED);
        dprintf("Oneshot: active: %d\n", active);
    }
}

void oneshot_enable(void) {
    oneshot_set(true);
}

Calling oneshot_enable() calls oneshot_set(true), which sets keymap_config.oneshot_disable = true.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • 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).

@github-actions github-actions bot added the core label Mar 11, 2022
@drashna drashna requested a review from a team March 11, 2022 08:00
@zvecr zvecr added the bug label Mar 11, 2022
@zvecr zvecr merged commit 0eb42e0 into qmk:develop Mar 11, 2022
@drashna drashna mentioned this pull request Mar 13, 2022
9 tasks
0xcharly pushed a commit to Bastardkb/bastardkb-qmk that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants