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

STM32: F4: Increase ADC sampling time for VBAT #4691

Merged
merged 1 commit into from Jul 13, 2017

Conversation

Projects
None yet
6 participants
@LMESTM
Contributor

LMESTM commented Jul 3, 2017

Description

To get a valid VBAT measurement on F4 targets, it is required to increase
the sampling time to its maximum value.

This PR solves the issue reported here #4326

Status

READY

Tests

Tested on NUCLEO_F446RE target with VBAT measurements and non-regression OK

+-----------------------+---------------+-----------------------+--------+--------------------+-------------+
| target | platform_name | test suite | result | elapsed_time (sec) | copy_method |
+-----------------------+---------------+-----------------------+--------+--------------------+-------------+
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-analogin | OK | 12.65 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-analogout | OK | 11.8 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-businout | OK | 28.27 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-digitalio | OK | 13.17 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-i2c | OK | 14.54 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-interruptin | OK | 13.77 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-pwm | OK | 176.65 | shell |
| NUCLEO_F446RE-GCC_ARM | NUCLEO_F446RE | tests-api-spi | OK | 16.15 | shell |
+-----------------------+---------------+-----------------------+--------+--------------------+-------------+

More thoughts

Increasing the sample time may have impacts on performances (e.g. max number of samples per sec),
but this PR aims at getting valid sample first. There is way for improvement in MBED API to allow
configuration of this sampling time parameter. An issue will be entered accordingly.
Submitted #4692

@ohagendorf

This comment has been minimized.

Contributor

ohagendorf commented Jul 3, 2017

This has a huge impact on the normal usage of the ADC.
Why not put the long sample time only in the apropriate case when VBAT is sampled. During the normal usage the former config could be used:

    // Configure ADC channel
    sConfig.Rank         = 1;
    sConfig.SamplingTime = ADC_SAMPLETIME_15CYCLES;
    sConfig.Offset       = 0;

    switch (obj->channel) {
...
        case 18:
            sConfig.Channel = ADC_CHANNEL_VBAT;
            sConfig.SamplingTime = ADC_SAMPLETIME_480CYCLES;
            break;
@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jul 3, 2017

@ohagendorf I was hoping to get some feedback :-) thanks for that - I will proceed as suggested.
Do you think that adding MBED API for tuning the sampling time can help users ?

STM32: F4: Increase ADC sampling time
To get a valid VBAT measurement on F4 targets, it is required to increased
the sampling time to its maximum value.

@LMESTM LMESTM force-pushed the LMESTM:analogin_sample_time branch to f8d6f95 Jul 3, 2017

@LMESTM LMESTM changed the title from STM32: F4: Increase ADC sampling time to STM32: F4: Increase ADC sampling time for VBAT Jul 3, 2017

@ohagendorf

This comment has been minimized.

Contributor

ohagendorf commented Jul 3, 2017

Maybe two features or three would be nice to have:

  • setting sampling time
  • setting resolution

But curently for a fast conversion there is another huge problem. We analysed the adc read function:

    float read() {
        lock();
        float ret = analogin_read(&_adc);
        unlock();
        return ret;
    }

and lock()+unlock() need much more time than analogin_read(). I had to look into the project documentation for the exact numbers but it was something around 1.5µs for the conversion and 5...6µs for calling lock()+unlock() (Nucleo_F446RE).
One idea would be to add another read function with a callback under timer control or use continuous mode. In this case a fast equidistant conversion corresponding to the maximum conversion rate of the hardware would be possible. Or in combination with the above function to set the sampling time with a slower timing.

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jul 3, 2017

thanks @ohagendorf
I will expose your requests in #4692 and I suggest you to follow-up there as well.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Jul 7, 2017

@LMESTM So is there more work to come against this PR as a result of the above conversations ?

@LMESTM

This comment has been minimized.

Contributor

LMESTM commented Jul 7, 2017

@adbridge No more work - the latest PR takes into account comments from @ohagendorf

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 10, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Jul 11, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 761

All builds and test passed!

@0xc0170 0xc0170 merged commit 42548f3 into ARMmbed:master Jul 13, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment