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

2 Point calibration fix #232

Merged
merged 14 commits into from Mar 19, 2023
Merged

Conversation

redoxcode
Copy link
Contributor

This PR should fix an oversight in the way the calibration values are applied when using only 2 Points.
The issue was described here: #221 (comment)
In short: If we are not mistaken the adc offset measured when 0V is applied, is currently not taken into account.
Old:
result = 'adc raw value' / 'calibration at 10V'
New:
result = ('adc raw value' - 'calibration at 0V') / ('calibration at 10V' - 'calibration at 0V)

(some tests still fail, but it's only one contrib script (tests/contrib/test_europi_turing_machine.py) so I guess it's an issue of this test and not the code changes)

@awonak
Copy link
Collaborator

awonak commented Mar 15, 2023

@redoxcode do you have some time to sync this branch to HEAD, and fix the lint and test errors? We'd love to include this in our next release.

@redoxcode
Copy link
Contributor Author

redoxcode commented Mar 16, 2023

@awonak sync is done. I will try to work out why tests fail.
the lint part shouldn't be a problem.

@redoxcode
Copy link
Contributor Author

ok, there where some bugs in the implementation and some corner cases I missed (like when the input is below the 0 point calibration). But I think its good now!
But please check on actual hardware and do the calibration.
I also had to fix the 2 point calibration in the mock hardware.
And some how my black output was different than what lint wants 🤔

Copy link
Collaborator

@mjaskula mjaskula left a comment

Choose a reason for hiding this comment

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

@redoxcode Thanks for taking the time to dig into this, and thanks to @francoisgeorgy for describing the issue and solution in the first place. All of these changes look good to me. This should help some of our 0v reading issues.

Also, selfishly, I'm glad that the test failures didn't end up being issues in the turing machine tests themselves.

software/firmware/europi.py Outdated Show resolved Hide resolved
software/firmware/europi.py Outdated Show resolved Hide resolved
@awonak
Copy link
Collaborator

awonak commented Mar 19, 2023

LGTM, thank you for improving the calibration process @redoxcode & @francoisgeorgy

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.

[Bug Report] AnalogInput calibration improvement when using only two points
4 participants