add voltage sensor on stm32f3 and correct the readme #1794
Conversation
dragonmux
added
Enhancement
There was a problem hiding this comment.
There are some items that need fixing before we can merge this contribution, but it is looking like a good start.
Please run clang-format on your contribution, despite the lint step being broken right now this is not optional. Please correct your commit message to have the correct suffix on the commit, which for this would be f3/platform: . The rest of the commit message is fine however.
| /* We want to read the temperature sensor, so we have to enable it. */ | ||
| //adc_enable_temperature_sensor(); | ||
| adc_set_sample_time_on_all_channels(ADC1, ADC_SMPR_SMP_601DOT5CYC); | ||
| uint8_t channel_array[] = { 1 }; /* ADC1_IN1 (PA0) */ |
There was a problem hiding this comment.
Rather than defining an array, please define just a channel value and take its address below as in:
uint8_t channel = 1U;
adc_set_regular_sequence(ADC1, 1U, &channel);| int i; | ||
| for (i = 0; i < 800000; i++) | ||
| __asm__("nop"); |
There was a problem hiding this comment.
i cannot go negative, so to avoid signed overflow issues and other such unnecessary badness, please use unsigned types here. size_t is ideal.
Note that the preferred style for this sort of busy loop is the one used in the native platform for this:
for (volatile size_t i = 0U; i < 800000U; ++i)
continue;The U suffix on the constants are not optional and makes them unsigned, preventing a signed-unsigned conversion that can drastically change the meaning of the code.
| return (val * 99U) / 8191U;; | ||
| //return val; | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
This is unreachable, please remove it, along with the commented out return val;
| /*static char ret[] = "0000000000"; | ||
| uint32_t val = platform_target_voltage_sense(); | ||
| for(uint8_t i = 10; i > 0; i--){ | ||
| ret[i-1] = '0' + val % 10U; | ||
| val = val / 10; | ||
| }*/ |
There was a problem hiding this comment.
Please remove this if you aren't including it as active code.
| while (!(adc_eoc(ADC1))) | ||
| continue; | ||
| uint32_t val=adc_read_regular(ADC1); | ||
| return (val * 99U) / 8191U;; |
| adc_disable_external_trigger_regular(ADC1); | ||
| adc_set_right_aligned(ADC1); | ||
| /* We want to read the temperature sensor, so we have to enable it. */ | ||
| //adc_enable_temperature_sensor(); |
There was a problem hiding this comment.
As this is not being used, and likewise with the gpio-mode_setup() above, please remove them.
Detailed description
I implemented the ability of having a voltage sensor when flashing BM firmware on a stm32f3.
The STM32f3 isn't able of calculating voltage highter than 3.6V, when the voltage is highter it anwser 4.95V every time.
I used the libcm3 adc interface to open an adc gpio and read it when voltage is asked
Your checklist for this pull request