WIP: add InputPin trait to output pins #88
Conversation
| @@ -699,6 +695,19 @@ macro_rules! gpio { | |||
|
|
|||
| #[cfg(feature = "unproven")] | |||
| impl<MODE> toggleable::Default for $PXi<Output<MODE>> {} | |||
|
|
|||
| impl<MODE> InputPin for $PXi<Output<MODE>> { | |||
There was a problem hiding this comment.
Almost done, this also has to be implemented at line 231 for PXx which is a "fully reased pin", line 425 $PXx, which is a partially erase pin. :) The implementation is almost the same. The compiler will scream the difference to you, or you could look at the other implementations 😉
There was a problem hiding this comment.
Minor nitpick: I think I would move this below
impl<MODE> OutputPin for $PXi<Output<MODE>> {at line 668, because this is another implementation for Output, so this position would make more sense.
| let mode = 0b01; | ||
| moder.moder().modify(|r, w| unsafe { | ||
| w.bits((r.bits() & !(0b11 << offset)) | (mode << offset)) | ||
| pub fn into_open_drain_output(self) -> $PXi<Output<OpenDrain>> { |
There was a problem hiding this comment.
Breaking changes in itself are fine. But changing only this, would mean other parts which take the moder and otyper register must be changed as well, to make it coherent.
We had a discussion about this a while ago in #37. I've no opinion on this, but I'd rather tackle that as whole in another PR :)
There was a problem hiding this comment.
I agree, we should do this in a separate PR.
But also this implementation here is not safe. The reason we pass &mut references for the relevant GPIO registers in is that this way we can ensure (at compile time) that we have exclusive access to them. We need this, because otherwise race conditions can happen, as explained in #37. To make this safe we'd need something else that ensures exclusive access, like a critical section.
| type Error = (); | ||
|
|
||
| fn is_high(&self) -> Result<bool, Self::Error> { | ||
| self.is_low().map(|v| !v) | ||
| } | ||
|
|
||
| fn is_low(&self) -> Result<bool, Self::Error> { | ||
| // NOTE(unsafe) atomic read with no side effects | ||
| Ok(unsafe { (*$GPIOX::ptr()).idr.read().bits() & (1 << $i) == 0 }) | ||
| } | ||
| } | ||
| )+ |
There was a problem hiding this comment.
| type Error = (); | |
| fn is_high(&self) -> Result<bool, Self::Error> { | |
| self.is_low().map(|v| !v) | |
| } | |
| fn is_low(&self) -> Result<bool, Self::Error> { | |
| // NOTE(unsafe) atomic read with no side effects | |
| Ok(unsafe { (*$GPIOX::ptr()).idr.read().bits() & (1 << $i) == 0 }) | |
| } | |
| } | |
| )+ | |
| type Error = (); | |
| fn is_high(&self) -> Result<bool, Self::Error> { | |
| self.is_low().map(|v| !v) | |
| } | |
| fn is_low(&self) -> Result<bool, Self::Error> { | |
| // NOTE(unsafe) atomic read with no side effects | |
| Ok(unsafe { (*$GPIOX::ptr()).idr.read().bits() & (1 << $i) == 0 }) | |
| } | |
| } | |
| )+ |
Indentation is wrong. I wonder why rustfmt did not complaint 🤔
Sh3Rm4n
changed the title
|
Closed in favor of #114. |
Closes #84
This builds and runs (I'm not getting a reading from sensor but that is probably some other issue like my wiring)
Out of curiosity, I've updated into_open_drain_output with the api from stm32f4 library which is simpler imo. Aware this is a breaking change so should probably not do this :)