ARMmbed / mbed-hal-st-stm32f4

mbed HAL for ST STM32F4-series microcontrollers
Other
7 stars 21 forks source link

ADC bit-shifts wrong? #42

Closed markus-becker-tridonic-com closed 8 years ago

markus-becker-tridonic-com commented 8 years ago

According to http://www.st.com/web/en/resource/technical/document/reference_manual/DM00031936.pdf Section 13.6.1 no shifts would be required to the sampled ADC values at https://github.com/ARMmbed/mbed-hal-st-stm32f4/blob/master/source/analogin_api.c#L164.

Same for F091: https://github.com/ARMmbed/mbed-hal-st-stm32f0/blob/master/source/analogin_api.c#L175

0xc0170 commented 8 years ago

The origin for this changeset is : https://github.com/mbedmicro/mbed/pull/465

cc @bcostm

markus-becker-tridonic-com commented 8 years ago

With out any bit shifts, I get 4095 for 3.3V and 0 for 0V, which is what I expect. With bit shifts, I get 65535 for 3.3V and 0 for 0V. This seems to be scaled, but the bit shifting does not look like scaling to me.

ciarmcom commented 8 years ago

ARM Internal Ref: IOTSFW-2350

MarceloSalazar commented 8 years ago

@bcostm could you please comment on this?

bcostm commented 8 years ago

Yes originally the function analogin_read_u16 output was 4095 for 3.3V. Then it has been requested to have instead 65535 for 3.3v which is the correct thing to do. I took this 12bits to 16bits convertion formula from other platforms on mbed Classic. You have the same for example in NXP, Renesas targets. I don't say this formula is correct just that it is the same in many targets... There is another solution to simply shift 4 bits to the left. Is it what you're looking for ?

markus-becker-tridonic-com commented 8 years ago

Just shifting 4 bits to the left is also not really, what I would expect. I would expect the result to be scaled.

For context: I was trying to sample a floating pin several times and using the lowest bits to get random values.

MarceloSalazar commented 8 years ago

@bcostm I understand there is a pending action from your side. Please comment if this is not the case.

markus-becker-tridonic-com commented 8 years ago

Sorry, this I have checked now. The bit-shifts are correct. Closing the issue. The remaining issue that one should be able to access the raw 12 bit value is tracked at https://github.com/ARMmbed/mbed-drivers/issues/186.

bcostm commented 8 years ago

Just a question: why you can't use the actual read function? The returned value is not rescaled to 16bits. Ok you need to re-multiply by 0xFFF to obtain the raw value. But is it a problem ?

markus-becker-tridonic-com commented 8 years ago

Do you mean to directly use adc_read(obj)? It is not part of the API https://github.com/ARMmbed/mbed-hal/blob/master/mbed-hal/analogin_api.h

bcostm commented 8 years ago

In fact you can simply type this for example:

AnalogIn temperature(p20); if (temperature > 0.5) { // or temperature.read() > 0.5 printf("Too hot! (%f)", temperature.read()); }

This will call the analog_read function. The returned value range is from 0.0 to 1.0.

markus-becker-tridonic-com commented 8 years ago

This is mapped to the float function, or? Which I then would have to revert to uint16_t.... Not so nice.

bcostm commented 8 years ago

Yes, it returns a float and then we'll have to multiply by 0xFFF and convert to a uint16_t... This is why I asked if this method was ok for you...

markus-becker-tridonic-com commented 8 years ago

I think we should continue this discussion in https://github.com/ARMmbed/mbed-drivers/issues/186