Skip to content
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

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

Closed
nraynaud opened this issue Mar 10, 2019 · 5 comments
Closed

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

nraynaud opened this issue Mar 10, 2019 · 5 comments

Comments

@nraynaud
Copy link

There are a few issues with the current code:

  • timers only expose 2 channels
  • timers takes the channels pins wether we want it or not.
  • timers force the pin mode to be OpenDrain or PushPull

example form timer_input:

impl Pins<TIM1> for (PA8<Alternate<OpenDrain>>, PA9<Alternate<OpenDrain>>) {
    const REMAP: u8 = 0b00;
}

I propose having the channels pins using the Option and a generic mode:

pub type Pwm1Mapping0<M1, M2, M3, M4> = (Option<PA8<Alternate<M1>>>,
                                         Option<PA9<Alternate<M2>>>,
                                         Option<PA10<Alternate<M3>>>,
                                         Option<PA11<Alternate<M4>>>);

here is a more complete example:
https://github.com/japaric/stm32f103xx-hal/blob/9f132cd3a8a95d0b726f925c239fad6e74b60d76/src/pwm2.rs#L219

@nraynaud nraynaud changed the title Timers expose 2 channels and take their pins. Timers expose 2 channels, take their pins and force their mode. Mar 10, 2019
@TheZoq2 TheZoq2 changed the title Timers expose 2 channels, take their pins and force their mode. PWM expose 2 channels, take their pins and force their mode. Mar 11, 2019
@TheZoq2
Copy link
Member

TheZoq2 commented Oct 7, 2019

This looks like it has been fixed by allowing external impls of the Pins trait. This was documented in #60

However, I feel like it might be more ergonomic to impl all the possible pin/channel combinations, probably via a macro. Is there a reason we haven't done that before?

@geomatsi
Copy link
Contributor

geomatsi commented Oct 7, 2019

Hi @TheZoq2 ,
At first I tried to come up with macro, but did not succeeded. So I switched to the newtype approach. IIUC some clever macro still would be an option here. My only concern is firmware size: is it possible to list all the combinations without bloating the final binary ?

Regards,
Sergey

@TeXitoi
Copy link
Contributor

TeXitoi commented Oct 8, 2019

Only thé used code Will be on the binary so it shouldn't be a problem.

@TheZoq2
Copy link
Member

TheZoq2 commented Oct 10, 2019

I'll try to experiment with this when I get time

@TheZoq2
Copy link
Member

TheZoq2 commented Dec 2, 2019

This has been fixed in #147

@TheZoq2 TheZoq2 closed this as completed Dec 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants