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

Implements Sample Rate, resolves #21 #46

Merged
merged 1 commit into from
Feb 13, 2017
Merged

Implements Sample Rate, resolves #21 #46

merged 1 commit into from
Feb 13, 2017

Conversation

daniel-j-h
Copy link
Collaborator

@daniel-j-h daniel-j-h commented Feb 11, 2017

500 Hz:

500hz

750 Hz:

750hz

Looks like the signal strength goes down quite a bit from my first experiences. No real difference between 750 Hz and 1000 Hz. Resolution goes up (even though not that much (?)), looks good over all!

@dcyoung @kent-williams can you take a quick look. Especially since you know more about firmware plans. I opted for the user having to explicitly specify the Hz, and let the README document the support for 500 Hz, 750 Hz and 1000 Hz.

From your docs it looks like the device can be off by ~50-100 Hz. Which means high-precision applications probably require accurate timestamps (?) (see #19).

References:

@dcyoung
Copy link

dcyoung commented Feb 11, 2017

Hmm, sample rate code 03 (~1000Hz) should be noticeably different. Were you able to verify that the device was returning code 03 after setting it?

In general, I'm wondering if we should avoid having the user specify a value in HZ. If we eventually add support for specifying more specific sample rates, this could be of use... but I'm not sure that is even possible @kent-williams. This is a similar discussion to the baud rate comments we had in #44. But for sample rate, i think the specification should be avoided for now. Here is why....

Currently the 3 supported sample rate codes (01, 02, 03), correspond more to a target range than a specific value. It might be better if we allow the user to specify a sample rate code which we define clearly in the protocol and API as targeting a certain sample rate range. This way people are not inclined to misinterpret the parameter as a true variable. Otherwise, we should specify a sample rate enum or mapping of code to HZ value. Making structures or queries like this available through the API will allow people to build application logic around the codes and values. Then we can update the values of codes without breaking user's code.

@daniel-j-h
Copy link
Collaborator Author

Yes I made sure to check the get sample rate response. You can try it yourself, e.g. add a the set function call to one of the examples and run it. Then run the viewer.

Re. the API: we could add enums for those codes for sure, as in

enum {
  500_600_HZ = 1,
  ..
};

I'd like to hear your plans for future firmware updates / devices on this, though.
The enum would need a bit of wrapping for the bindings, too.

@daniel-j-h
Copy link
Collaborator Author

Here are some more comparisons from today (click to enlarge).

With motor speed 3Hz. Sample rate from left to right: 500Hz, 750Hz, 1000Hz:

Motor 3Hz, Sample Rates

With motor speed 7Hz. Sample rate from left to right: 500Hz, 750Hz, 1000Hz:

Motor 7Hz, Sample Rates

Do these observations look reasonable from your experiences @kent-williams @dcyoung? Not sure if we should recommend users leaving the sample rate at 500Hz except they know what they're doing.

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.

2 participants