Increase ADC resolution for FYSETC S6, STM32

[LinFor]

Description

STM32 HAL currently using 10-bit ADC resolution as classic arduino does. FYSETC S6 can use 12-bit ADC.
Seems this must be done for all STM32 boards, maybe except F1xx.

Benefits

Better PID stability

Configurations

None

Related Issues

#15719, #20463

[sjasonsmith]

What have you done to verify this works properly?

[rhapsodyv]

Is ADC_RESOLUTION always set on all other variants? Looking at the variants, all ADC_RESOLUTION are commented out. Is there any fallback to 10 anywhere or is it missing?

[rhapsodyv]

Is this fix related to this too? #20463

[LinFor]

What have you done to verify this works properly?

Now I use it on my printer.

Is ADC_RESOLUTION always set on all other variants? Looking at the variants, all ADC_RESOLUTION are commented out. Is there any fallback to 10 anywhere or is it missing?

It defaults to 10 in stm32 arduino: https://github.com/stm32duino/Arduino_Core_STM32/blob/a4058ae1c9e6234b7411ae8214bbd12c99961cca/cores/arduino/pins_arduino.h#L343

Is this fix related to this too? #20463

Very likely. Graphs starting from 11/01/2020 shows demonstrate very non-smooth temperature changes, which hints at insufficient ADC resolution. As a consequence, the PID can produce abrupt, late and excessive responses to temperature changes it sees.

[sjasonsmith]

Graphs starting from 11/01/2020 shows demonstrate very non-smooth temperature changes, which hints at insufficient ADC resolution.

Do you have any idea why this would have just started on 11/1?

[sjasonsmith]

What does this change achieve. I'm not doubting it, Is this simply because fewer samples are needed to generate an averaged reading when using the higher resolution?

Is the number choice here arbitrary?

[LinFor]

In fact, this is a crutch so as not to change the existing oversampling mechanism. All measurements are summed in a 16-bit field, so a high ADC bit depth simultaneously with a large oversampling ratio will cause an overflow.
A few lines below there is a static validation for this (MAX_RAW_THERMISTOR_VALUE).

Since there is no easy way to transfer OVERSAMPLENR from hal (well, at least there are doubts that this needs to be introduced), I preferred to calculate it automatically for high-bit ADCs to avoid overflow. There is, of course, an opportunity to set HAL_ADC_FILTERED at the hal level, which will completely disable oversampling, but this is semantically incorrect, since no filtration is performed in this hal.

[thinkyhead]

Works for me. The main thing is to have at least some number of oversamples to deal with the natural variation in ADC readings. Guaranteed to be over 10 is reasonable.

[LinFor]

Do you have any idea why this would have just started on 11/1?

As far as I know, some time ago the GTR board was transferred from a separate F4 HAL to a common stm32 HAL. Probably, in common stm32 HAL there was always a problem with ADC accuracy, they simply did not pay attention to it. Or maybe some internal logic in upstream ststm32 lib changed.

But as far as I understood from that thread, the problem is not only in the accuracy of the ADC, but also probably in the incorrect / inaccurate behavior of the auto-tuning pid. So, in the version from "03/01/2020" there is almost perfect coincidence of the set temperature, and already in "04/14/2020" a shortage of temperature is noticeable. Moreover, in these versions, the temperature graphs are quite smooth, i.e. bit ADC was sufficient.

Thus, there is probably more than one problem reflected there, and maybe not even two :)

[rhapsodyv]

Can we make 12 a marlin default for STM32?

[LinFor]

Can we make 12 a marlin default for STM32?

I would have done so, but I'm not sure about the capabilities of the ADC of the lower series of stm32 processors, so I did it in the standard way - at the level of variant.h. I think we can safely set 12-bit for all boards with F4 and higher.
But I won't be able to reread all the datasheets. Maybe someone more experienced in processors will tell us.

[rhapsodyv]

Seems all STM32 chips support at least 12 bit ADC resolution, and some support 16 bit. The time overhead from 10 to 12 bit seems minimal.
So I vote to keep our hal default in 12, as temperature control is big concern on Marlin.

This PR added defaults to 10 on STM32F1 (but seem this value is not use anywhere), but 12 on LPC: #15719.

Looking at the maple code (STM32F1), even if we have a macro with 10, seems maple is fixed at 12, as it explicitly use 4096 for calcs.... (framework-arduinoststm32-maple/STM32F1/libraries/STM32ADC/src/STM32ADC.cpp)

Reference: https://www.st.com/resource/en/application_note/cd00211314-how-to-get-the-best-adc-accuracy-in-stm32-microcontrollers-stmicroelectronics.pdf

[rhapsodyv]

Missing the default to 12 on STM32. We just need to add it before stm32duino header, or in all variants.

If we ship STM32 with 10 as default could give users the sensation that stm32 is less stable than the old STM32F1, as maple always used 12...