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

Sync Timer feature #10997

Merged
merged 1 commit into from
Dec 1, 2020
Merged

Sync Timer feature #10997

merged 1 commit into from
Dec 1, 2020

Conversation

XScorpion2
Copy link
Contributor

@XScorpion2 XScorpion2 commented Nov 21, 2020

Description

Added a sync_timer set of apis that will keep their value in sync across split common keyboards. This helps keeps led animation effects in sync when run in split modes (RGBLIGHT_SPLIT or the upcoming RGB_MATRIX_SPLIT). This PR is targeting the development branch as this is a change to the Transport code which will require users to flash both master and slave halves.

Spent quite a bit of time poking at RGBLIGHT fixing the hitching of the previous attempts at a sync timer. Solved all the hitching, but it still does not stay in sync as well as I would like (when RGBLIGHT_SPLIT_NO_ANIMATION_SYNC is defined) due to how animation ticks are handled in RGBLIGHT. So while it's using the sync timer, it's not any better than what it was before. Additionally an option to disable the sync timer and fall back to normal timer is possible using the #define DISABLE_SYNC_TIMER

All hitching for RGBLIGHT with sync_timer has been fixed. Additionally RGBLIGHT now stays in sync with RGBLIGHT_SPLIT_NO_ANIMATION_SYNC defined Only remaining issue: boot / startup time hitching causes the animations to start out of sync. So there needs to be an initial sync event to get them lined up. RGBLIGHT_SPLIT_NO_ANIMATION_SYNC not defined still fixes this.

Note: In testing, this was used in conjunction with #10996

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).

@drashna drashna requested a review from a team November 25, 2020 18:52
Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also do a qmk cformat pass?

tmk_core/common/sync_timer.h Outdated Show resolved Hide resolved
tmk_core/common/sync_timer.h Show resolved Hide resolved
tmk_core/common/sync_timer.h Outdated Show resolved Hide resolved
tmk_core/common/sync_timer.h Outdated Show resolved Hide resolved
tmk_core/common/sync_timer.c Show resolved Hide resolved
@XScorpion2
Copy link
Contributor Author

Could you also do a qmk cformat pass?

Nope, because the qmk cli doesn't work for me:

$ qmk cformat
ERROR: It seems you are not using the MINGW64 terminal.
Please close this terminal and open a new MSYS2 MinGW 64-bit terminal.
Python: /usr/bin/python, MSYSTEM: MINGW64

This was after attempts to reinstall, and fight Kaspersky constantly flagging the installer as a virus. I ran the clang formatter in visual studio at least.

@fauxpark
Copy link
Member

It should be running in the MinGW64 Python, not MSYS (ie. /mingw64/bin/python). My guess is you have installed the CLI though the MSYS pip. The simplest and most reliable solution would be to nuke the C:\msys64 folder and start again, but you could also try to python3 -m pip uninstall qmk inside an MSYS terminal, then:

pacman -S mingw-w64-x86_64-python-pip
python3 -m pip install qmk

in the MINGW64 terminal.

@XScorpion2
Copy link
Contributor Author

QMK cli was installed from the MINGW64 terminal, I never use the other versions of the terminal. Will try the nuke and reinstall approach.

@XScorpion2 XScorpion2 requested a review from fauxpark November 27, 2020 17:14
@XScorpion2
Copy link
Contributor Author

@fauxpark fought quite a bit with the qmk cli setup process. Finally got it working and ran the cformat against the touched pr files and checked in the results.

@XScorpion2 XScorpion2 mentioned this pull request Nov 28, 2020
14 tasks
@noroadsleft noroadsleft deleted the branch qmk:develop November 28, 2020 20:02
@tzarc tzarc reopened this Nov 28, 2020
@XScorpion2
Copy link
Contributor Author

rebased & force push

@drashna drashna merged commit a8d0ec0 into qmk:develop Dec 1, 2020
@XScorpion2 XScorpion2 deleted the split_sync_timer branch December 1, 2020 18:18
nicsuzor added a commit to nicsuzor/qmk_firmware that referenced this pull request Dec 16, 2020
* XScorpion2/split_rgb_matrix_2020:
  Split RGB Matrix
  Split transport mirror support
  [Split] Sync Timer feature (qmk#10997)
  Branch point for 2021 Feb 27 Breaking Change
  Configurable serial usart timeout (qmk#11057)
@gsadams gsadams mentioned this pull request Feb 27, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
A timer that is kept in sync between the halves of a split keyboard
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.

5 participants