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

for discussion: use existing clock delta when applying kria reset #71

Merged
merged 1 commit into from Dec 2, 2019

Conversation

@csboling
Copy link
Contributor

@csboling csboling commented Aug 17, 2019

Kria measures the clock frequency to calculate gate durations and ratchet timing relative to the clock. When an incoming external clock stops completely, then starts again, the measurement code needs two rising edges to measure the clock rate, causing a different note duration for the first note played.
This patch, when resetting the Kria playhead, replaces the previous measured edge time t_{-1} with t_{0} - \delta, where t_{0} is the current time and \delta is the existing edge measurement t_{-1} - t_{-2} , causing the next clock measurement to evaluate to t_{0} - (t_{0} - \delta) = \delta.

This resolves the issue described here, using Pamela's New Workout to clock / reset Kria in sync with other modules. Since this potentially affects trigger ratcheting as well, I suspect this may also be a factor in issue #37, also reported when clocking / resetting Kria with Pam's. @fourhexagons if you would like to test this patch, there is a build posted here.

This change does however represent a change to behavior that users may be relying on. Specifically, when modulating the external clock frequency or changing Ansible's main internal clock rate, duration and ratchet timings will now lag by one clock cycle after a reset pulse, rather than tracking the clock edge-for-edge. Note that the same code will run when changing the active pattern sets pos_reset = true, so this could affect behavior when e.g. metasequencing and changing the Ansible main clock performatively. Another flag could be used to avoid this, but then stopping the clock, changing the pattern, and starting the clock would be affected by the same issue that this patch is intended to address.

@tehn
Copy link
Member

@tehn tehn commented Sep 5, 2019

whoa, apologies for the delay!

i'd be happy to go with the majority desire of the thread participants on this one.

@tehn
Copy link
Member

@tehn tehn commented Dec 2, 2019

another couple months gone by. shall we merge this?

@csboling
Copy link
Contributor Author

@csboling csboling commented Dec 2, 2019

I don't think anyone has complained, one user said it did fix the problem for them. Probably fine to merge it and revisit if it comes up again.

@tehn tehn merged commit f9b53f7 into monome:master Dec 2, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants