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

[Feature Request] Timers between two halves of a split keyboard are not synced #9665

Closed
2 of 4 tasks
tyalie opened this issue Jul 6, 2020 · 4 comments
Closed
2 of 4 tasks
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.

Comments

@tyalie
Copy link

tyalie commented Jul 6, 2020

The main problem is, that for RGB Matrix and other uses the internal clock between both halves should be synced, which they aren't right now. As most of us don't use an RTC on their keyboard, both timers run at different speeds and can get heavily out of sync in a very short amount of time. To my understanding RGB Light circumvents this problem, by splitting the animation into multiple parts and informing the minion when a new part of the animation was started. But this concept is not feasible when using RGB Matrix. As the only other alternative that comes to my mind (sending all rgb data over the serial connection) isn't feasible easy, due to low data speeds, I'd suggest that syncing both timers is essential to the idea.

In #9613 and #5998 a new timer class called sync_timer was introduced to separate the animation and internal timers. But I would suggest a general change to a synced timer.h to stream line the usage.

Description

This would change timer.h directly and (hopefully) doesn't change anything for current applications.

  • the master will have no alteration to their timer class
  • the minion's timer class also runs normally without support, but the current time can be corrected using a function call
  • the slave can request the current time from the master using Christian's Algorithm
    • this should occur maybe every >10ms, but not to often to prevent overload of the serial communication (i.e. every time serial loop is run), for the time between two syncs the internal clock shouldn't run to much out of sync to pose a problem.
    • on a successful timer update, minion's timer is updated accordingly using the exposed function

This description is open for discussion and is a sub issue of #9619.

Feature Request Type

  • Core functionality
  • Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • Alteration (enhancement/optimization) of existing feature(s)
  • New behavior
@XScorpion2
Copy link
Contributor

XScorpion2 commented Jul 20, 2020

But I would suggest a general change to a synced timer.h to stream line the usage.

So this is what I first did in #5998 and ran into quite a few problems. Pretty much nearly everything in QMK uses timer.h to handle quite a bit and just about any jump forward or backward in that timer, even 4ms was enough to lock the slave half and require rebooting the keyboard. From experience, I would recommend going with just the sync_timer approach and allowing systems to opt in to using it once they have been updated and tested with the sync timer and only if they really need to stay in sync. In addition, it's also worth noting that, while the timers don't stay in sync, it's doesn't matter for most systems as they are just trying to generically time start and stop events, and are not used like a real time tracker such as their use in RGB Light and RGB Matrix.

@tyalie
Copy link
Author

tyalie commented Jul 20, 2020

Thanks. That's an interesting point. I kinda expected, but thought that the timers wouldn't run off by that much normally. But it seems they do.

One could try to introduce a timer start / stop class but that seems quite disruptive to me.

@stale
Copy link

stale bot commented Oct 19, 2020

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

@stale stale bot added the stale Issues or pull requests that have become inactive without resolution. label Oct 19, 2020
@stale
Copy link

stale bot commented Nov 19, 2020

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know.

@stale stale bot closed this as completed Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

No branches or pull requests

2 participants