Pwm remap: another try #147

burrbull · 2019-11-27T04:53:31Z

This is something average between #124 and #146.

But it's need for testing.

burrbull · 2019-11-27T05:13:15Z

TIM Remaps should be in timer.rs I think.

geomatsi · 2019-11-27T12:00:47Z

I will help with testing in the next few days.

BTW, friendly reminder, you need to update document section in src/pwm.rs. Alternatively, you may just remove it and add another 1-2 sections into pwm_custom.rs example to show how to use different PWM mappings.

Regards,
Sergey

move Remap to timer.rs, add gpio::Mode

burrbull · 2019-11-28T06:28:45Z

Rebased.

cc @TheZoq2

burrbull · 2019-11-28T06:31:20Z

On this image PWM is not available for pins PA15 and PB3. Is this right?

CubeMX don't block for setting such combination

geomatsi · 2019-11-28T19:26:37Z

@burrbull ,
Why not available ? I see T2C2 for PB3 and T2C1E for PA15 which is according to TIM2 table from TRM.

geomatsi · 2019-11-28T19:42:34Z

Ok, now I see wave notation now. No idea why. I did a quick search through TRM, but couldn't find anything useful so far. The fastest option is just to give it a try.

geomatsi · 2019-11-30T13:56:24Z

Hi @burrbull ,

Just for the record... On BluePill all the TIM/PWM configurations available on that package work properly on all 4 channels. Here is the list of supported timers/mappings/pins combinations:

TIM2 (0b00): PA0, PA1, PA2, PA3
TIM2 (0b01): PA15, PB3, PA2, PA3
TIM2 (0b10): PA0, PA1, PB10, PB11
TIM2 (0b11): PA15, PB3, PB10, PB11
TIM3 (0b00): PA6, PA7, PB0, PB1
TIM3 (0b10): PB4, PB5, PB0, PB1
TIM4 (0b00): PB6, PB7, PB8, PB9

In several cases afio.mapr.disable_jtag shall be used to reconfigure pins PA15, PB3, PB4 before using them as PWM output. Otherwise all works as expected, w/o surprises.

Note that for now these are the reference results that were obtained w/o your PR.

Regards,
Sergey

This example is based on new PWM implementation in stm32f1xx HAL: see stm32-rs/stm32f1xx-hal#147 Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>

geomatsi · 2019-11-30T17:26:21Z

Ok, PWM testing is completed for all the 4-channels combinations available on BluePill. Besides, random selection of 1/2/3-channels combinations has been tested as well. No testing for pwm_input and QEI from me though...

In brief, using this implementation is very convenient. With this implementation it is hard to end up with a misconfigured PWM. Full example is available here. Note that test example explicitly specifies all the mappings in the form pwm::<Tim3PartialRemap, _, _, _>. But in fact it is needed only if there is ambiguity in pin selection. Normally it can be skipped on the first pass and compiler will complain if it can not figure out the specific pin combination.

Tested-by: Sergey Matyukevich geomatsi@gmail.com

Regards,
Sergey

geomatsi · 2019-11-30T17:52:40Z

src/pwm.rs

@@ -14,10 +14,10 @@
  ```rust


The whole document subsection about pre-defined channel combinations can be removed.

geomatsi · 2019-11-30T18:36:33Z

src/pwm_input.rs

+    feature = "stm32f105",
+))]
+impl Timer<TIM1> {
+    pub fn pwm_input<REMAP, PINS, T>(


I am not familiar with using pwm_input and qei. So the question based on comparison with pwm implementation. I assume it is not pwm_input<REMAP, P, PINS, T> just because only (Ch1, Ch2) are allowed for pwm_input mode as well as QEI mode. Is this correct ? Or more configurations are supported, but this is current limitation of stm32f1xx_hal ?

burrbull · 2019-11-30T19:20:59Z

src/pwm_input.rs

+where
+    REMAP: Remap<Periph = TIM>,
+    P1: Ch1<REMAP> + gpio::Mode<Input<Floating>>,
+    P2: Ch2<REMAP> + gpio::Mode<Input<Floating>> {}


@geomatsi pwm_input use other Pins trait/implementation with only first two channels. I did not change this.

TheZoq2

Looks good overall (though i'm not super familiar with PWM input or QEI). I left some nitpick comments, mostly related to docs

TheZoq2 · 2019-11-28T13:08:52Z

CHANGELOG.md

@@ -19,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

 ### Breaking changes

+- Implement more pin combinations for PWM configuration, added PWM for TIM1 


This doesn't seem like a breaking change, or am I missing something?

If someone use old custom API, now he need to rewrite his code. So yes, it is breaking change.

Ah yea, makes sense. Perhaps the changelog should clarify that though

TheZoq2 · 2019-11-28T13:09:42Z

examples/pwm_custom.rs

@@ -58,12 +34,12 @@ fn main() -> ! {
    let p0 = pb4.into_alternate_push_pull(&mut gpiob.crl);
    let p1 = gpiob.pb5.into_alternate_push_pull(&mut gpiob.crl);

+    //let pa1 = gpioa.pa1.into_alternate_push_pull(&mut gpioa.crl);


Remove unused comments

TheZoq2 · 2019-12-01T09:31:44Z

examples/pwm_custom.rs

    timer::Timer,
+    //timer::Tim2NoRemap,


Unused comment

TheZoq2 · 2019-12-01T09:33:54Z

src/pwm.rs

@@ -28,227 +28,157 @@

    // Put the timer in PWM mode using the specified pins
    // with a frequency of 100 hz.
+    // In some cases you need to specify remap you need, especcially for TIM2:


Would be nice to point users to where they can find what remaps they can specify.

Also double 'c' in especially

TheZoq2 · 2019-12-01T11:57:42Z

Perfect, thanks!

burrbull mentioned this pull request Nov 27, 2019

Pwm remap #146

Closed

implement remap for Timers when pwm is used

5dddd32

move Remap to timer.rs, add gpio::Mode

burrbull force-pushed the pwm3 branch from 0370a16 to 5dddd32 Compare November 28, 2019 06:27

geomatsi added a commit to geomatsi/rust-blue-pill-tests that referenced this pull request Nov 30, 2019

[WIP]: pwm: new example

6017c67

This example is based on new PWM implementation in stm32f1xx HAL: see stm32-rs/stm32f1xx-hal#147 Signed-off-by: Sergey Matyukevich <geomatsi@gmail.com>

geomatsi approved these changes Nov 30, 2019

View reviewed changes

burrbull commented Nov 30, 2019

View reviewed changes

TheZoq2 reviewed Dec 1, 2019

View reviewed changes

burrbull added 2 commits December 1, 2019 14:11

add remap tables

e498e44

fix changelog

bb19444

TheZoq2 merged commit d006750 into stm32-rs:master Dec 1, 2019

This was referenced Dec 2, 2019

Support for different PWM pins configurations out of the box #124

Closed

PWM expose 2 channels, take their pins and force their mode. #42

Closed

burrbull deleted the pwm3 branch December 3, 2019 15:50

burrbull mentioned this pull request Dec 7, 2019

PWM on TIM1 outputs #119

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pwm remap: another try #147

Pwm remap: another try #147

burrbull commented Nov 27, 2019

burrbull commented Nov 27, 2019 •

edited

geomatsi commented Nov 27, 2019

burrbull commented Nov 28, 2019

burrbull commented Nov 28, 2019 •

edited

geomatsi commented Nov 28, 2019

geomatsi commented Nov 28, 2019

geomatsi commented Nov 30, 2019

geomatsi commented Nov 30, 2019 •

edited

geomatsi Nov 30, 2019

geomatsi Nov 30, 2019

burrbull Nov 30, 2019

TheZoq2 left a comment

TheZoq2 Nov 28, 2019

burrbull Dec 1, 2019

TheZoq2 Dec 1, 2019

TheZoq2 Nov 28, 2019

TheZoq2 Dec 1, 2019

TheZoq2 Dec 1, 2019

TheZoq2 commented Dec 1, 2019

		@@ -19,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

		### Breaking changes

		- Implement more pin combinations for PWM configuration, added PWM for TIM1

Pwm remap: another try #147

Pwm remap: another try #147

Conversation

burrbull commented Nov 27, 2019

burrbull commented Nov 27, 2019 • edited

geomatsi commented Nov 27, 2019

burrbull commented Nov 28, 2019

burrbull commented Nov 28, 2019 • edited

geomatsi commented Nov 28, 2019

geomatsi commented Nov 28, 2019

geomatsi commented Nov 30, 2019

geomatsi commented Nov 30, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheZoq2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheZoq2 commented Dec 1, 2019

burrbull commented Nov 27, 2019 •

edited

burrbull commented Nov 28, 2019 •

edited

geomatsi commented Nov 30, 2019 •

edited