Skip to content

Settable analogio voltage reference #8781

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

Closed
wants to merge 1 commit into from

Conversation

bill88t
Copy link

@bill88t bill88t commented Jan 3, 2024

Internally stored as int, converted on / from float to maintain compatibility.
This means getting values is just as fast as it was, since there is no float conversion.

It has to now be done for every port, so it will take a moment.

Fixes #8754.

@tannewt
Copy link
Member

tannewt commented Jan 17, 2024

Do the ESP chips actually expose a vref pin? Or is the value passed here fixed for each chip variant? If it is fixed, then this should be done internally.

@bill88t
Copy link
Author

bill88t commented Jan 17, 2024

Do the ESP chips actually expose a vref pin?

No.. I have never seen an exposed vref pin on any esp module or chip.
I did a quick sanity check on the esp32 wroom module datasheet and there is none.

Or is the value passed here fixed for each chip variant?

As far as I can tell, every variant has a different vref. So yes, the value set in this would be different for each variant to have a correct 0-65535 value.

If it is fixed, then this should be done internally.

I would really love to, but hardcoding the fixed vref per chip would make .value return vastly different values compared to what it's returning now, potentially breaking existing applications in a pretty bad way.
This api addition however doesn't break anything.
It's not perfect, but it's better than having to slap big warnings to the release page and/or rework the adc api.

Something that could alternatively be done is having an "internal percentage calculator".
Please let me know what you think would work best.

@anecdata
Copy link
Member

I think the question is: at what layer of abstraction should the API operate?

At the lowest level internally, I believe all Espressif chips use ~1.1v as VREF (going back to ESP8266 days), ± some per-chip variance that's (mostly?) calibrated out in software. ESP8266 users were warned not to put more than 1.1v on the ADC pin.

Espressif chips are 3.3v devices, and we've chosen 11dB attenuation to give a natural voltage range for ADC (whether or not the particular variant is fully capable of delivering that range, or whether there's good linearity within the capable range). From a [Circuit]Python code perspective, ADC is always a 16-bit number across ports, and it's natural to think of 65535 as 3.3v in this context, realizing that not all variants can actually read that value.

My understanding of this PR is that it would allow the user to set a VREF equal to the highest practical value in the range per-variant (and potentially in the future, also per-attenuation)? The result could be greater precision, though I don't think there is greater accuracy. I'm also not sure there is current authoritative info on what the high-end-of-the-range(s) values are, so the user may be scratching their head trying to figure out how to set VREF. If it's hard-coded, I wouldn't know what to set the values to, other than by going back to outdated documentation.

@tannewt
Copy link
Member

tannewt commented Jan 18, 2024

Something that could alternatively be done is having an "internal percentage calculator".
Please let me know what you think would work best.

It isn't clear to me what issue this PR solves to justify changing the behavior. There isn't a clear bug here that I can see.

@bill88t
Copy link
Author

bill88t commented Jan 18, 2024

The bug is that CP has no way to internally handle different vrefs properly.
But I agree this PR isn't the solution.

I'm not feeling like pursuing this any further in the core.
I will make a module that handles the math for every mcu automatically.

@bill88t bill88t closed this Jan 18, 2024
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.

Espressif ADC is not accurate, has a deadzone and differs per cpu
3 participants