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 "extra" keycodes in a tapdance causes them to be sent repeatedly #17036

Closed
3 tasks
ollien opened this issue May 8, 2022 · 2 comments
Closed
3 tasks

Comments

@ollien
Copy link

ollien commented May 8, 2022

Describe the Bug

I have a tap dance that sends certain media codes based on the number of presses on a specific key

static void tap_dance_media(qk_tap_dance_state_t *state, void *user_data) {
	switch (state->count) {
		case 1:
			tap_code(KC_MPLY);
			break;
		case 2:
			tap_code(KC_MNXT);
			break;
		case 3:
			tap_code(KC_MPRV);
			break;
	}
}
Full keymap
/*
Copyright 2020 Álvaro "Gondolindrim" Volpato <alvaro.volpato@usp.br>

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program.  If not, see <http://www.gnu.org/licenses/>.
*/

#include QMK_KEYBOARD_H

static void tap_dance_media(qk_tap_dance_state_t *state, void *user_data);

LEADER_EXTERNS();

enum {
	TD_END,
	TD_MEDIA
};

qk_tap_dance_action_t tap_dance_actions[] = {
	[TD_END] = ACTION_TAP_DANCE_DOUBLE(KC_END, KC_HOME),
	[TD_MEDIA] = ACTION_TAP_DANCE_FN(tap_dance_media),
};

const uint16_t PROGMEM keymaps[][MATRIX_ROWS][MATRIX_COLS] = {
   [0] = LAYOUT_all(
    KC_GRAVE,         KC_1,    KC_2,    KC_3,    KC_4,    KC_5,    KC_6,    KC_7,    KC_8,     KC_9,    KC_0,    KC_MINS,    KC_EQL,     KC_BSPC,    XXXXXXX, KC_LEAD,
    KC_TAB,           KC_Q,    KC_W,    KC_E,    KC_R,    KC_T,    KC_Y,    KC_U,    KC_I,     KC_O,    KC_P,    KC_LBRC,    KC_RBRC,    KC_BSLS,             KC_PGUP,
    KC_CAPS,          KC_A,    KC_S,    KC_D,    KC_F,    KC_G,    KC_H,    KC_J,    KC_K,     KC_L,    KC_SCLN, KC_QUOT,    XXXXXXX,    KC_ENT,              KC_PGDN,
	KC_LSFT, XXXXXXX, KC_Z,    KC_X,    KC_C,    KC_V,    KC_B,    KC_N,    KC_M,    KC_COMM,  KC_DOT,  KC_SLSH, MO(1),                  KC_UP,               TD(TD_END),
    KC_LCTL,          KC_LGUI, KC_LALT,                            KC_SPC ,                             MO(2),   KC_RCTL,    KC_LEFT,    KC_DOWN,             KC_RGHT
	),
   [1] = LAYOUT_all(
    KC_ESC,           KC_F1,      KC_F2,      KC_F3,      KC_F4,      KC_F5,      KC_F6,      KC_F7,      KC_F8,       KC_F9,      KC_F10,     KC_TRNS,    KC_TRNS,    KC_DEL,     XXXXXXX, KC_TRNS,
    KC_TRNS,          KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,             KC_HOME,
    KC_TRNS,          KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,    XXXXXXX,    KC_TRNS,             KC_END,
	KC_TRNS, XXXXXXX, KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,                KC_TRNS,             KC_TRNS,
    KC_TRNS,          KC_TRNS,    KC_TRNS,                            KC_TRNS,                            KC_TRNS,     RESET,      KC_TRNS,    KC_TRNS,                KC_TRNS
	),
   [2] = LAYOUT_all(
    KC_TRNS,          KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    XXXXXXX, KC_TRNS,
    KC_TRNS,          KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,             KC_VOLU,
    KC_TRNS,          KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,    XXXXXXX,    KC_TRNS,             KC_VOLD,
	KC_TRNS, XXXXXXX, KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,    KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,                KC_TRNS,             TD(TD_MEDIA),
    KC_TRNS,          KC_TRNS,    KC_TRNS,                            KC_TRNS,                            KC_TRNS,     KC_TRNS,    KC_TRNS,    KC_TRNS,                KC_TRNS
	),
};

static void tap_dance_media(qk_tap_dance_state_t *state, void *user_data) {
	switch (state->count) {
		case 1:
			tap_code(KC_MPLY);
			break;
		case 2:
			tap_code(KC_MNXT);
			break;
		case 3:
			tap_code(KC_MPRV);
			break;
	}
}

void matrix_scan_user(void) {
	LEADER_DICTIONARY() {
		leading = false;
		leader_end();

		SEQ_TWO_KEYS(KC_P, KC_R) {
			tap_code(KC_PSCR);
		}

		SEQ_FIVE_KEYS(KC_R, KC_E, KC_S, KC_E, KC_T) {
			reset_keyboard();
		}
	}
}

It seems that whenever I activate any of these tapdance branches, the keys are sent repeatedly (which was verified by looking at the output of xev), rapidly pausing/unpausing music, for instance. Interestingly, this does not happen with simple keycodes like letters; those simply press/release once as expected.

I asked in the Discord and @daskygit pointed me to a similar fix they made in ChibiOS. #14643. They theorized there might be a similar bug in vusb, pointing to

if (usbInterruptIsReadyShared()) {
usbSetInterruptShared((void *)&report, sizeof(report_extra_t));
}
, noting that there might be a possibility of missing interrupts. The quick fix is to change that to a busy-wait to poll USB, but I'm not sure if that's a "proper" solution (and might just lead to more missed keypresses)

System Information

Keyboard: KB Elmo Sesame
Revision (if applicable):
Operating system: Fedora 35
qmk doctor output:

[~/Documents/code/qmk_firmware] nick@cubert$ qmk doctor
Ψ QMK Doctor is checking your environment.
Ψ CLI version: 1.0.0
Ψ QMK home: /home/nick/Documents/code/qmk_firmware
Ψ Detected Linux.
⚠ Missing or outdated udev rules for 'atmel-dfu' boards. Run 'sudo cp /home/nick/Documents/code/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
⚠ Missing or outdated udev rules for 'hid-bootloader' boards. Run 'sudo cp /home/nick/Documents/code/qmk_firmware/util/udev/50-qmk.rules /etc/udev/rules.d/'.
Ψ Git branch: master
Ψ Repo version: 0.16.9
⚠ Git has unstashed/uncommitted changes.
Ψ All dependencies are installed.
Ψ Found arm-none-eabi-gcc version 11.1.0
Ψ Found avr-gcc version 11.1.0
⚠ We do not recommend avr-gcc newer than 8. Downgrading to 8.x is recommended.
Ψ Found avrdude version 6.3
Ψ Found dfu-util version 0.11
Ψ Found dfu-programmer version 0.7.2
Ψ Submodules are up to date.
Ψ QMK is ready to go, but minor problems were found

Any keyboard related software installed?

  • AutoHotKey (Windows)
  • Karabiner (macOS)
  • Other:

Additional Context

@fauxpark
Copy link
Member

fauxpark commented May 8, 2022

Add #define TAP_CODE_DELAY 10 to your config.h.

@ollien
Copy link
Author

ollien commented May 8, 2022

@fauxpark Yep - that totally worked. Thanks!

@ollien ollien closed this as completed May 8, 2022
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

2 participants