Alternative for #176 #177
Alternative for #176 #177
Conversation
src/pwm.rs
Outdated
| Some(Channel::C2) => unsafe { (*$TIMX::ptr()).ccr2.read().ccr().bits() }, | ||
| Some(Channel::C3) => unsafe { (*$TIMX::ptr()).ccr3.read().ccr().bits() }, | ||
| Some(Channel::C4) => unsafe { (*$TIMX::ptr()).ccr4.read().ccr().bits() }, | ||
| None => 0, |
There was a problem hiding this comment.
Is 0 always equivalent to a disabled channel? Or can the channel be in some sort of floating state if disabled?
There was a problem hiding this comment.
What about to panic here?
There was a problem hiding this comment.
I would really like to avoid panics as far as possible. I don't see a great way to accomplish that though :/
There was a problem hiding this comment.
By experience, disable is different to duty of 0
There was a problem hiding this comment.
But it should be OK to give the duty whatever it is enabled or disabled, according to enabling the channel would give you this duty.
There was a problem hiding this comment.
Yep, that seems like the best solution
src/pwm.rs
Outdated
| Some(Channel::C2) => unsafe { (*$TIMX::ptr()).ccr2.write(|w| w.ccr().bits(duty)) }, | ||
| Some(Channel::C3) => unsafe { (*$TIMX::ptr()).ccr3.write(|w| w.ccr().bits(duty)) }, | ||
| Some(Channel::C4) => unsafe { (*$TIMX::ptr()).ccr4.write(|w| w.ccr().bits(duty)) }, | ||
| None => {}, |
There was a problem hiding this comment.
In this code:
pwm.disable(C1);
pwm.set_duty(C1, 0.5);
pwm.enable(C1);I would expect it to get the updated duty after being enabled, but this seems to not be the case here. As far as I can tell,
match channel {
Channel::C1 => unsafe{ /*ccr1.write...*/ }
Channel::C2 => unsafe{ /*ccr2.write...*/ }
Channel::C3 => unsafe{ /*ccr3.write...*/ }
Channel::C4 => unsafe{ /*ccr3.write...*/ }
}would result in that behaviour
src/pwm.rs
Outdated
|
|
||
| // Length in ms of an internal clock pulse | ||
| (clk.0 / u32(psc * arr)).hz() | ||
| // (((psc as u32) / clk.0) * (1_000_000 as u32) * (arr as u32)).ms() |
There was a problem hiding this comment.
This should probably be removed
src/pwm.rs
Outdated
| T: Into<Self::Time> { | ||
| let clk = self.clk; | ||
|
|
||
| // let freq = u16(1 / period.into()); |
|
I've renamed function to check_used and added panic. |
| fn check_used(c: Channel) -> Channel { | ||
| if (c == Channel::C1 && Self::C1) | ||
| || (c == Channel::C2 && Self::C2) | ||
| || (c == Channel::C3 && Self::C3) | ||
| || (c == Channel::C4 && Self::C4) | ||
| { | ||
| c | ||
| } else { | ||
| panic!("Unused channel") | ||
| } |
There was a problem hiding this comment.
Ideally I want to avoid panics if the user specifies the wrong channel as well :) But I don't see a great way to do that at the moment
| //// Operations affecting all defined channels on the Timer | ||
|
|
||
| // Adjust period to 0.5 seconds | ||
| pwm.set_period(500.us()); |
|
I just integrated this into my branch |
Ok. I close PR, but branch will be available. |
No description provided.