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

Added Delay time to sending each Keys for VIA Macros function feature #16810

Merged
merged 6 commits into from
Jul 11, 2022

Conversation

FREEWING-JP
Copy link

Description

When playing back the VIA macro feature for UI manipulation, the UI may miss keystrokes because the keys are sent too fast.
This problem can be solved by adding a wait time for each key transmission.

Example.

config.h
#define VIA_MACRO_DELAY 10

to delay 10ms each key transmission in VIA macros.

Types of Changes

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

Issues Fixed or Closed by This PR

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 Apr 6, 2022
@zvecr zvecr changed the base branch from master to develop April 6, 2022 14:45
@zvecr zvecr added the via Adds via keymap and/or updates keyboard for via support label Apr 6, 2022
@drashna
Copy link
Member

drashna commented Apr 9, 2022

send_string_with_delay is probably going to be a MUCH better way to handle this.

And would prefer to use TAP_CODE_DELAY

@FREEWING-JP FREEWING-JP reopened this Apr 10, 2022
@github-actions github-actions bot removed the via Adds via keymap and/or updates keyboard for via support label Apr 10, 2022
@FREEWING-JP
Copy link
Author

send_string_with_delay is probably going to be a MUCH better way to handle this.

It good . Thank you .

And would prefer to use TAP_CODE_DELAY

But TAP_CODE_DELAY is also take effect other tap_code function .
So I define VIA_MACRO_DELAY . It take effect VIA only .

@drashna
Copy link
Member

drashna commented Apr 10, 2022

send_string_with_delay is probably going to be a MUCH better way to handle this.

It good . Thank you .

Glad to hear it. And yeah, it works better.

And would prefer to use TAP_CODE_DELAY

But TAP_CODE_DELAY is also take effect other tap_code function . So I define VIA_MACRO_DELAY . It take effect VIA only .

I do understand that, but it's another define that needs to be added, basically.

However, something like this may work:

#ifndef VIA_MACRO_DELAY
#    define VIA_MACRO_DELAY TAP_CODE_DELAY
#endif

This would default to the core TAP_CODE_DELAY value (which is 0, by default). But if somebody defines tap code delay, it would be picked up by this, as well. But it would also allow for this value to be overriden, as well.

@FREEWING-JP
Copy link
Author

FREEWING-JP commented Apr 12, 2022

@drashna
Thank you . Your suggestion and comment .
I fixed them .
994c44d

quantum/dynamic_keymap.c Outdated Show resolved Hide resolved
quantum/dynamic_keymap.c Outdated Show resolved Hide resolved
FREEWING-JP and others added 2 commits May 15, 2022 18:57
@drashna drashna requested a review from a team May 20, 2022 04:44
quantum/dynamic_keymap.c Outdated Show resolved Hide resolved
quantum/dynamic_keymap.c Outdated Show resolved Hide resolved
@FREEWING-JP FREEWING-JP requested a review from zvecr May 23, 2022 10:21
@drashna drashna merged commit 61df87a into qmk:develop Jul 11, 2022
@FREEWING-JP
Copy link
Author

Thank you .

@FREEWING-JP FREEWING-JP deleted the feature/add_via_macro_delay branch July 12, 2022 10:49
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants