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

Join Serial for USART and UART again. Make inner traits with different implementation for USART and UART. #636

Merged
merged 3 commits into from
May 11, 2023

Conversation

qwerty19106
Copy link
Contributor

@qwerty19106 qwerty19106 commented May 11, 2023

cc #630
@burrbull

Main idea

#[cfg(feature = "uart4")]
pub(crate) use crate::pac::uart4::RegisterBlock as RegisterBlockUart;
pub(crate) use crate::pac::usart1::RegisterBlock as RegisterBlockUsart;

pub trait RegisterBlockImpl {
    fn new<UART: Instance<RegisterBlock = Self>, WORD>(
        uart: UART,
        pins: (impl Into<UART::Tx<PushPull>>, impl Into<UART::Rx<PushPull>>),
        config: impl Into<config::Config>,
        clocks: &Clocks,
    ) -> Result<Serial<UART, WORD>, config::InvalidConfig>;

    fn read_u16(&self) -> nb::Result<u16, Error>;
    fn write_u16(&self, word: u16) -> nb::Result<(), Error>;

    fn read_u8(&self) -> nb::Result<u8, Error> {
        // Delegate to u16 version, then truncate to 8 bits
        self.read_u16().map(|word16| word16 as u8)
    }

    fn write_u8(&self, word: u8) -> nb::Result<(), Error> {
        // Delegate to u16 version
        self.write_u16(u16::from(word))
    }

    fn flush(&self) -> nb::Result<(), Error>;

    fn bwrite_all_u8(&self, buffer: &[u8]) -> Result<(), Error> {
        for &b in buffer {
            nb::block!(self.write_u8(b))?;
        }
        Ok(())
    }

    fn bwrite_all_u16(&self, buffer: &[u16]) -> Result<(), Error> {
        for &b in buffer {
            nb::block!(self.write_u16(b))?;
        }
        Ok(())
    }

    fn bflush(&self) -> Result<(), Error> {
        nb::block!(self.flush())
    }

    // RxISR
    fn is_idle(&self) -> bool;
    fn is_rx_not_empty(&self) -> bool;
    fn clear_idle_interrupt(&self);

    // TxISR
    fn is_tx_empty(&self) -> bool;

    // RxListen
    fn listen_rxne(&self);
    fn unlisten_rxne(&self);
    fn listen_idle(&self);
    fn unlisten_idle(&self);

    // TxListen
    fn listen_txe(&self);
    fn unlisten_txe(&self);

    // Listen
    fn listen(&self, event: Event);
    fn unlisten(&self, event: Event);

    // PeriAddress
    fn peri_address(&self) -> u32;
}

macro_rules! uartCommon {
    ($RegisterBlock:ty) => {
        impl RegisterBlockImpl for $RegisterBlock {
			...
		}
    }
}

uartCommon! { RegisterBlockUsart }

#[cfg(feature = "uart4")]
uartCommon! { RegisterBlockUart }

Note that RegisterBlockImpl not exports from stm32f4xx-hal.

Changes in public API

  • add type RegisterBlock; to Instance
pub trait Instance: crate::Sealed + rcc::Enable + rcc::Reset + rcc::BusClock + CommonPins {
    type RegisterBlock;

    #[doc(hidden)]
    fn ptr() -> *const Self::RegisterBlock;
    #[doc(hidden)]
    fn set_stopbits(&self, bits: config::StopBits);
}
  • remove uart::{Serial, Rx, Tx}
  • add RxListen, TxListen, Listen traits
/// Trait for listening [`Rx`] interrupt events.
pub trait RxListen {
    /// Start listening for an rx not empty interrupt event
    ///
    /// Note, you will also have to enable the corresponding interrupt
    /// in the NVIC to start receiving events.
    fn listen(&mut self);

    /// Stop listening for the rx not empty interrupt event
    fn unlisten(&mut self);

    /// Start listening for a line idle interrupt event
    ///
    /// Note, you will also have to enable the corresponding interrupt
    /// in the NVIC to start receiving events.
    fn listen_idle(&mut self);

    /// Stop listening for the line idle interrupt event
    fn unlisten_idle(&mut self);
}

/// Trait for listening [`Tx`] interrupt event.
pub trait TxListen {
    /// Start listening for a tx empty interrupt event
    ///
    /// Note, you will also have to enable the corresponding interrupt
    /// in the NVIC to start receiving events.
    fn listen(&mut self);

    /// Stop listening for the tx empty interrupt event
    fn unlisten(&mut self);
}

/// Trait for listening [`Serial`] interrupt events.
pub trait Listen {
    /// Starts listening for an interrupt event
    ///
    /// Note, you will also have to enable the corresponding interrupt
    /// in the NVIC to start receiving events.
    fn listen(&mut self, event: Event);

    /// Stop listening for an interrupt event
    fn unlisten(&mut self, event: Event);
}
  • relax Serial.split and Serial.release trait bounds to UART: CommonPins
  • relax Rx.join and Tx.join trait bounds to UART: CommonPins

Questions

  • Why PeriAddress and DMASet implemented for Rx<UART, u8>, not Rx<UART, WORD>? And Tx too.

P.S.

I have tried use #![feature(specialization)] and #![feature(min_specialization)] and failed miserably.
The min_specialization not does not cover this case, the specialization cause ICE.

Besides I think that current specialization RFC not suitable for our case at all, because we have impl InstanceUsart and impl InstanceUart with the same "specialization", but not trait bounds inheritance.

@qwerty19106 qwerty19106 changed the title Make one struct Serial for USART and UART. Make inner traits with different implementation for USART and UART. Join Serial for USART and UART again. Make inner traits with different implementation for USART and UART. May 11, 2023
@qwerty19106 qwerty19106 changed the title Join Serial for USART and UART again. Make inner traits with different implementation for USART and UART. Join Serial for USART and UART again. Make inner traits with different implementation for USART and UART. May 11, 2023
@burrbull
Copy link
Member

stm32f4 has several issues with uart.
You can experiment on https://github.com/stm32-rs/stm32-rs-nightlies to simplify initial implementation.

@qwerty19106
Copy link
Contributor Author

You plan release new version of stm32-rs in the near future?

@burrbull
Copy link
Member

You plan release new version of stm32-rs in the near future?

I don't have access. Only @adamgreig can do this.

@burrbull
Copy link
Member

pub trait RegisterBlockImpl: crate::Sealed {
}
impl crate::Sealed for RegisterBlockUart { }

src/serial/uart_impls.rs Outdated Show resolved Hide resolved
@qwerty19106 qwerty19106 marked this pull request as ready for review May 11, 2023 12:17
@burrbull
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented May 11, 2023

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

Successfully merging this pull request may close these issues.

2 participants