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

Fix issue preventing BPH below 14400 from working #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xyzzy42
Copy link
Contributor

@xyzzy42 xyzzy42 commented Feb 13, 2021

Periods (two beats) longer than 500 ms were rejected as too long. This
effectively limits the low end of BPH to 14400. Any slower and the
correct period length is greater than 500 ms, e.g. 12000 BPH is a 600 ms
period.

Change the limit to be 20% slower than the nominal value for the BPH.
If the BPH is set, then use that, when BPH is guessed, use MIN_BPH.

When MIN_BPH of 1200, this increases the max period to 720 ms when
guessing. It could be longer if one were able to set a slower than
MIN_BPH period length.

Copy link

@jakelewis3d jakelewis3d left a comment

Choose a reason for hiding this comment

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

This works well.
I can confirm that this works at least down to 10800 bph, if MIN_BPH is set to 10800, and that value is also added to PRESET_BPH []

@marjanw
Copy link

marjanw commented Mar 15, 2021

I also confirm that this works fine at 10800 bph!

tg.h:
#define MIN_BPH 10800

#define PRESET_BPH { 10800, 12000, 14400, 17280, 18000, 19800, 21600, 25200, 28800, 36000, 43200, 72000, 0 };

@xyzzy42
Copy link
Contributor Author

xyzzy42 commented Mar 15, 2021

Note that it's not absolutely necessary to add new rates to the list. Any value can be manually entered in bph box, as long as it is in MIN-MAX BPH range. If it is not in list, it will not be guessed in guess mode.

The beat guessing logic has issues when there are rates at whole multiples of other rates in the list. E.g., adding 9000 to the list which is exactly half of 18000. It will be likely to guess the slower rate for a faster movement, e.g. guess 9000 for a 18000 bph movement.

In a 18000 bph movement, there are also ticks at proper time for 9000 and since it will see every other tick, there will be less beat error. Also, either the tick sound vs the tock sound will be louder. Using half the correct bph will just use the louder sounds, and so it appears to be better.

So adding all possible slow bph values to list has a drawback: It will interfere with auto-detection of more common higher bph values that are exactly double (or triple, etc.) the slower value.

Periods (two beats) longer than 500 ms were rejected as too long.  This
effectively limits the low end of BPH to 14400.  Any slower and the
correct period length is greater than 500 ms, e.g. 12000 BPH is a 600 ms
period.

Change the limit to be 20% slower than the nominal value for the BPH.
If the BPH is set, then use that, when BPH is guessed, use MIN_BPH.

A MIN_BPH of 12000 will increases the max period to 720 ms when
guessing.  It could be longer if one were able to set a slower than
MIN_BPH period length.
Allow lower BPH values.  Create another setting that affects the BPH
guessing code so it doesn't get confused by fractions of faster beats.

Lowest value is 2.25 Hz or 8100 BPH.  Lower rates do not work well or at
all.

Tg uses a minimum 2 second detection window and needs to see two periods
in this window.  So that it can see the 1st period matches the 2nd
period and the time between them is about what it should be from the
BPH.  In order to get two complete periods (two beats each) in a 2
second window, one needs 7200 BPH.  Below this and there might only be
one period in the window and nothing to detect.

But even at 7200, the beats at the beginning and end of the window might
be cut off, so it's necessary to go a little faster to be reliable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants