adc-dma timer #99

burrbull · 2019-08-17T07:33:52Z

TheZoq2 · 2019-08-18T14:59:10Z

Looks like the build failed for stm32f100

Example: reading multiple ADC channels using DMA. Note: this example uses PR that has not yet been merged: see stm32-rs/stm32f1xx-hal#99 Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>

geomatsi · 2019-09-01T07:55:41Z

Hi @burrbull ,

Thanks for working on extremely useful DMA bits !

I have been testing suggested functionality using the following simple setup:

stm32f103 Blue Pill
4 ADC channels (PA0-PA3) connected either to 3V3 or to GND

It looks like suggested use of continuous mode is not what should be used for multiple channels specified in SQRx registers. In this case I always get 4 readings of ADC_CH0 in my tests. IMHO documentation (RM0008 Technical Reference Manual) is a bit fuzzy on what exactly should be used. But after some googling and experiments, it looks like enabled scan mode with disabled continuous mode is what should be used. In this case ADC scans through specified regular sequence and then stops, so we can read data in DMA IRQ handler and setup next scan if needed.

Here is a patch on top of your PR that made things work properly for me:

diff --git a/src/adc.rs b/src/adc.rs
index b15abe9..fa56b43 100644
--- a/src/adc.rs
+++ b/src/adc.rs
@@ -545,12 +545,10 @@ impl<PINS> Receive for AdcDma<PINS> {
 impl<PINS> TransferPayload for AdcDma<PINS> {
     fn start(&mut self) {
         self.channel.start();
-        self.payload.adc.rb.cr2.modify(|_, w| w.cont().set_bit());
         self.payload.adc.rb.cr2.modify(|_, w| w.adon().set_bit());
     }
     fn stop(&mut self) {
         self.channel.stop();
-        self.payload.adc.rb.cr2.modify(|_, w| w.cont().clear_bit());
     }
 }
 
@@ -559,6 +557,8 @@ impl Adc<ADC1> {
     where
         Self:SetChannels<PINS>
     {
+        self.rb.cr1.modify(|_, w| w.scan().set_bit());
+        self.rb.cr2.modify(|_, w| w.cont().clear_bit());
         self.rb.cr1.modify(|_, w| w.discen().clear_bit());
         self.rb.cr2.modify(|_, w| w.align().bit(self.align.into()));
         self.set_samples();
@@ -583,6 +583,7 @@ impl<PINS> AdcDma<PINS> {
         let AdcDma {payload, channel} = self;
         payload.adc.rb.cr2.modify(|_, w| w.dma().clear_bit());
         payload.adc.rb.cr1.modify(|_, w| w.discen().set_bit());
+        payload.adc.rb.cr1.modify(|_, w| w.scan().clear_bit());
 
         (payload.adc, payload.pins, channel)
     }

Thoughts ? Comments ?

Regards,
Sergey

burrbull · 2019-09-01T08:16:14Z

Thank you for testing. I could not do it last month.

And you are right:

scan mode with disabled continuous mode is what should be used

burrbull · 2019-09-01T08:20:27Z

I tried to do changes compatible with existent code base and forgot about mode.

What about make generic Adc<ADC1, MODE> and AdcDma<PINS, MODE> ?

geomatsi · 2019-09-01T08:34:50Z

src/adc.rs

+                        14 => self.rb.smpr1.modify(|_, w| w.smp14().bits(sample_time)),
+                        15 => self.rb.smpr1.modify(|_, w| w.smp15().bits(sample_time)),
+                        16 => self.rb.smpr1.modify(|_, w| w.smp16().bits(sample_time)),
+                        17 => self.rb.smpr1.modify(|_, w| w.smp17().bits(sample_time)),
                        _ => unreachable!(),
                    }

                    return;


clippy suggests to remove this return

geomatsi · 2019-09-01T08:47:04Z

src/adc.rs

+    fn set_sequence(&mut self);
+}
+
+impl<PIN> SetChannels<PIN> for Adc<ADC1>


In basic OneShot implementation single channel sequence is configured in convert function. In the case of multiple channels, we have to implement SetChannel anyway to specify particular list of channels. So could you please clarify why do we need this implementation of SetChannels for single channel ? Does it help to maintain backward compatibility for with_dma ?

geomatsi · 2019-09-01T13:26:55Z

src/adc.rs

 impl Adc<ADC1> {
-    pub fn with_dma<PINS>(mut self, pins: PINS, dma_ch: C1) -> AdcDma<PINS>
+    pub fn with_dma<PIN>(mut self, pins: PIN, dma_ch: C1) -> AdcDma<PIN, Continuous>


Just a nitpick: pin instead of pins ?

geomatsi · 2019-09-01T13:28:29Z

Looks good and works fine. Thanks ! Do you have any specific plans about stm32f100 build failure ?

burrbull · 2019-09-01T13:58:14Z

Looks good and works fine. Thanks ! Do you have any specific plans about stm32f100 build failure ?

stm32-rs/stm32-rs#270

Waiting for stm32-rs release.

geomatsi · 2019-09-08T20:03:02Z

Hi @burrbull and all,

I did more experiments with ADC/DMA trying to re-configure ADC channels to read for each new transfer. The sequence of operations is as follows:

use with_scan_dma to setup AdcDma for channels 0, 1
use read and wait to start reading and get the result
use split to release adc
use with_scan_dma to setup AdcDma for channels 2, 3, 4
use read and wait to start reading and get the result
use split to release adc
repeat

Full example is available here

It turned out, that this approach provided incorrect results. In particular, readings were shifted as if DMA was started from the last ADC channel of the previous configuration. To be more specific, suppose that I have the following levels on ADC inputs:

ch0 = 0, ch1 = 3v3
ch2 = 3v3, ch3 = 0, ch4 = 0

If I don't switch between these two channel configurations, then I get proper readings:

[0, ~4095] for channels 0, 1
[~4095, 0, 0] for channels 2, 3, 4

However if I switch between those configurations after each reading, then I get the following readings:

[~4095, 0] for channels 0, 1
[0, ~4095, 0] for channels 2, 3, 4

It looks like DMA somehow does not reset its starting position after split. The following simple change in with_dma_scan fixes things:

diff --git a/src/adc.rs b/src/adc.rs
index 9b686fc..d99d82a 100644
--- a/src/adc.rs
+++ b/src/adc.rs
@@ -581,6 +581,7 @@ impl Adc<ADC1> {
     where
         Self:SetChannels<PINS>
     {
+        self.rb.cr2.modify(|_, w| w.adon().clear_bit());
         self.rb.cr1.modify(|_, w| w.scan().set_bit());
         self.rb.cr2.modify(|_, w| w.cont().clear_bit());
         self.rb.cr1.modify(|_, w| w.discen().clear_bit());
@@ -588,6 +589,7 @@ impl Adc<ADC1> {
         self.set_samples();
         self.set_sequence();
         self.rb.cr2.modify(|_, w| w.dma().set_bit());
+        self.rb.cr2.modify(|_, w| w.adon().set_bit());
 
         let payload = AdcPayload {
             adc: self,

Though I am not really sure if this is the right thing to do...

Thoughts ? Comments ?

Regards,
Sergey

TheZoq2 · 2019-09-24T09:11:36Z

Is the build error here related to stm32-rs/stm32-rs#270?

geomatsi · 2019-11-02T20:40:25Z

To: @TheZoq2 , @burrbull

As far as I can see, required ADC changes have been merged to stm32-rs, so we are waiting for release v0.9.0. Meanwhile I failed to build the following setup:

HEAD of stm32f1xx-hal with this PR
HEAD of stm32-rs

The build failed for all the 3 targets stm32f10[013]. The root cause was in some name changes for EXTSELW and MMSW. Finally, build succeeded with the following additional patch:

diff --git a/src/adc.rs b/src/adc.rs
index d99d82a..198474a 100644
--- a/src/adc.rs
+++ b/src/adc.rs
@@ -238,7 +238,7 @@ macro_rules! adc_hal {
                 }
 
                 #[inline(always)]
-                pub fn set_external_trigger(&mut self, trigger: crate::pac::$adc::cr2::EXTSELW) {
+                pub fn set_external_trigger(&mut self, trigger: crate::pac::$adc::cr2::EXTSEL_A) {
                     self.rb.cr2.modify(|_, w| w.extsel().variant(trigger))
                 }
 
diff --git a/src/timer.rs b/src/timer.rs
index e03c241..7421363 100644
--- a/src/timer.rs
+++ b/src/timer.rs
@@ -147,7 +147,7 @@ macro_rules! hal {
                     timer
                 }
 
-                pub fn start_master<T>(self, timeout: T, mode: crate::pac::$timbase::cr2::MMSW) -> CountDownTimer<$TIMX>
+                pub fn start_master<T>(self, timeout: T, mode: crate::pac::$timbase::cr2::MMS_A) -> CountDownTimer<$TIMX>
                 where
                     T: Into<Hertz>,
                 {

Regards,
Sergey

burrbull · 2019-11-03T05:04:50Z

Does it work as expected with disabling ADC?

geomatsi · 2019-11-03T15:24:38Z

Not sure what you mean. Could you please clarify your question ?

I did not test with up-to-date stm32-rs. I am testing with stm32f101 where your current version is working fine. But in my tests I still use additional patch #99 (comment) on top of your PR to make sure that DMA is properly reset on split.

burrbull · 2019-11-04T06:57:52Z

Rebased.

TheZoq2 · 2019-11-11T09:14:23Z

@burrbull Would you mind rebasing this again? :)

burrbull · 2019-11-11T10:17:15Z

Rebased

TheZoq2 · 2019-11-17T10:08:00Z

Looks like some great changes, thanks!

geomatsi reviewed Sep 1, 2019

View reviewed changes

burrbull force-pushed the master branch 4 times, most recently from 3080353 to 679a898 Compare September 1, 2019 11:28

geomatsi reviewed Sep 1, 2019

View reviewed changes

burrbull force-pushed the master branch from 679a898 to a2fa503 Compare November 4, 2019 06:57

adc-dma timer & Adc Mode

95354e4

burrbull force-pushed the master branch from a2fa503 to 95354e4 Compare November 11, 2019 10:00

TheZoq2 merged commit 7d323d4 into stm32-rs:master Nov 17, 2019

cs2dsb mentioned this pull request Nov 17, 2019

Added all timers for all variants as described by CubeMX #133

Merged

TheZoq2 mentioned this pull request Apr 14, 2020

RX DMA Access on the SPI Peripheral #200

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adc-dma timer #99

adc-dma timer #99

burrbull commented Aug 17, 2019

TheZoq2 commented Aug 18, 2019

geomatsi commented Sep 1, 2019

burrbull commented Sep 1, 2019

burrbull commented Sep 1, 2019

geomatsi Sep 1, 2019

geomatsi Sep 1, 2019

geomatsi Sep 1, 2019

geomatsi commented Sep 1, 2019

burrbull commented Sep 1, 2019

geomatsi commented Sep 8, 2019

TheZoq2 commented Sep 24, 2019

geomatsi commented Nov 2, 2019

burrbull commented Nov 3, 2019

geomatsi commented Nov 3, 2019

burrbull commented Nov 4, 2019

TheZoq2 commented Nov 11, 2019

burrbull commented Nov 11, 2019

TheZoq2 commented Nov 17, 2019

adc-dma timer #99

adc-dma timer #99

Conversation

burrbull commented Aug 17, 2019

TheZoq2 commented Aug 18, 2019

geomatsi commented Sep 1, 2019

burrbull commented Sep 1, 2019

burrbull commented Sep 1, 2019

geomatsi Sep 1, 2019

Choose a reason for hiding this comment

geomatsi Sep 1, 2019

Choose a reason for hiding this comment

geomatsi Sep 1, 2019

Choose a reason for hiding this comment

geomatsi commented Sep 1, 2019

burrbull commented Sep 1, 2019

geomatsi commented Sep 8, 2019

TheZoq2 commented Sep 24, 2019

geomatsi commented Nov 2, 2019

burrbull commented Nov 3, 2019

geomatsi commented Nov 3, 2019

burrbull commented Nov 4, 2019

TheZoq2 commented Nov 11, 2019

burrbull commented Nov 11, 2019

TheZoq2 commented Nov 17, 2019