Skip to content

Take ADC resolution into account#17018

Merged
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-2.0.xfrom
ManuelMcLure:FixADformula
Feb 29, 2020
Merged

Take ADC resolution into account#17018
thinkyhead merged 2 commits intoMarlinFirmware:bugfix-2.0.xfrom
ManuelMcLure:FixADformula

Conversation

@ManuelMcLure
Copy link
Contributor

Description

Take the ADC resolution into account in the AD595 and AD8495 temperature formulas.

Benefits

Makes the AD595 and AD8495 formulas work correctly on boards with 12 bit ADCs.

Related Issues

Related to #16993.

@AnHardt
Copy link
Contributor

AnHardt commented Feb 29, 2020

Does the 6.6 still happily apply with this change? There have been some questions about it. As I recall the original submission for the AD8495 was a little different, but to make it line up with the AD595 formula above it I changed the code so it just has the literal 6.6 value in the same spot as the 5.0. It was probably something like 660 instead of 6.6 * 100. I'll take a look at the original PR at some point.

#define TEMP_AD595(RAW) ((RAW) * 5.0 * 100.0 / float(HAL_ADC_RANGE) / (OVERSAMPLENR) * (TEMP_SENSOR_AD595_GAIN) + TEMP_SENSOR_AD595_OFFSET)
#define TEMP_AD8495(RAW) ((RAW) * 6.6 * 100.0 / float(HAL_ADC_RANGE) / (OVERSAMPLENR) * (TEMP_SENSOR_AD8495_GAIN) + TEMP_SENSOR_AD8495_OFFSET)

The best to read and likely most understandable version could be:
#define TEMP_AD8495(RAW) ((RAW) / float(OVERSAMPLENR) ... // the average ADC_value of the samples
... * 3.3 / float(HAL_ADC_RANGE) ... // Volt per ADC_LSB
... / 0.005 ... The direct number for the sensor gain from the datasheet - not its reciprocal value 200. If that's really speeding up the calculation the reciprocal is acceptable. That's the only number distinguishing AD595 from AD8495.
... * (TEMP_SENSOR_AD8495_GAIN) + TEMP_SENSOR_AD8495_OFFSET) // the user calibration.

Ideally the direct value for the ADC_REFERENCE_VOLTAGE 3.3 or 5.0 Volt should be taken from a define in the cores. Likely we have to define something like HAL_ADC_REFERENCE_VOLTAGE in the HALs because the cores, if they define something like that, don't name it uniformly. And likely it makes sense to make that over-writable by the configs.

Every value, except RAW, in this calculation is constant. Maybe we should help the compiler to see that by pre-calculating. Then the actual runtime calculation can be abbreviated to #define TEMP_AD8495(RAW) ((RAW) * a + b)

@thinkyhead
Copy link
Member

Maybe we should help the compiler to see that by pre-calculating.

No need. The compiler can easily reduce the constant parts to a single multiply and add on its own.

I agree that if there's are reference values to be used —such as those at #14257— they would be better to define at the board level.

@ManuelMcLure ManuelMcLure deleted the FixADformula branch February 29, 2020 18:16
oechslein pushed a commit to oechslein/Marlin that referenced this pull request Mar 21, 2020
Fixes thermocouple reading on LP1768 (which uses 12-bit resolution).
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.

3 participants