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

feat: uart break detection interrupt #2858

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

zpg6
Copy link

@zpg6 zpg6 commented Dec 21, 2024

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs API guidelines

Extra:

Pull Request Details 📖

Description

Adding interrupt-based detection of UART break (closes #2753). Break condition is met when the receiver detects a NULL character (i.e. logic 0 for one NULL character transmission) after stop bits (register: UART_BRK_DET_INT).

Testing

Ongoing

@zpg6
Copy link
Author

zpg6 commented Dec 29, 2024

Opened discussion on interrupt handlers in general to better inform the design of this feature:

#2868

@zpg6
Copy link
Author

zpg6 commented Dec 29, 2024

Usage with UART Interrupt Handler

Added an example to test my implementation, it works well and handles some consistent load.

static SERIAL: Mutex<RefCell<Option<Uart<Blocking>>>> = Mutex::new(RefCell::new(None));

#[entry]
fn main() -> ! {
    let peripherals = esp_hal::init(esp_hal::Config::default());
    let uart_config = UartConfig::default()
        .baudrate(19200)
        .data_bits(DataBits::DataBits8)
        .parity_none()
        .stop_bits(StopBits::Stop1)
        .rx_fifo_full_threshold(1); // interrupt every time a byte is received
    let mut uart = Uart::new(
        peripherals.UART1,
        uart_config,
        peripherals.GPIO16, // RX
        peripherals.GPIO17, // TX
    )
    .expect("Failed to initialize UART");

    uart.set_interrupt_handler(handler);

    critical_section::with(|cs| {
        uart.clear_interrupts(UartInterrupt::RxBreakDetected | UartInterrupt::RxFifoFull);
        uart.listen(UartInterrupt::RxBreakDetected | UartInterrupt::RxFifoFull);
        SERIAL.borrow_ref_mut(cs).replace(uart);
    });

    loop {}
}

#[handler]
#[ram]
fn handler() {
    critical_section::with(|cs| {
        let mut serial = SERIAL.borrow_ref_mut(cs);
        let serial = serial.as_mut().unwrap();

        if serial.interrupts().contains(UartInterrupt::RxBreakDetected) {
            esp_println::print!("\nBREAK");
        }
        if serial.interrupts().contains(UartInterrupt::RxFifoFull) {
            esp_println::print!(" {:02X}", serial.read_byte().expect("Read byte failed"));
        }

        serial.clear_interrupts(UartInterrupt::RxBreakDetected | UartInterrupt::RxFifoFull);
    });
}

@zpg6
Copy link
Author

zpg6 commented Dec 29, 2024

Usage without Handler

Also added some simple examples for using break detection without handlers:

Blocking

#[entry]
fn main() -> ! {
    let peripherals = esp_hal::init(esp_hal::Config::default());
    let uart_config = UartConfig::default()
        .baudrate(19200)
        .data_bits(DataBits::DataBits8)
        .parity_none()
        .stop_bits(StopBits::Stop1)
        .rx_fifo_full_threshold(1); // interrupt every time a byte is received
    let mut uart = Uart::new(
        peripherals.UART1,
        uart_config,
        peripherals.GPIO16, // RX
        peripherals.GPIO17, // TX
    )
    .expect("Failed to initialize UART");

    loop {
        uart.wait_for_break();
        esp_println::print!("\nBREAK");

        while let Ok(byte) = uart.read_byte() {
            esp_println::print!(" {:02X}", byte);
        }
    }
}

Async

#[esp_hal_embassy::main]
async fn main(_spawner: Spawner) {
    let peripherals = esp_hal::init(esp_hal::Config::default());
    let uart_config = UartConfig::default()
        .baudrate(19200)
        .data_bits(DataBits::DataBits8)
        .parity_none()
        .stop_bits(StopBits::Stop1)
        .rx_fifo_full_threshold(1); // interrupt every time a byte is received
    let mut uart = Uart::new(
        peripherals.UART1,
        uart_config,
        peripherals.GPIO16, // RX
        peripherals.GPIO17, // TX
    )
    .expect("Failed to initialize UART")
    .into_async();

    loop {
        uart.wait_for_break_async().await;
        esp_println::print!("\nBREAK");

        let mut buf = [0u8; 11];
        let len = uart.read_async(&mut buf).await.unwrap();
        for byte in buf.iter().take(len) {
            esp_println::print!(" {:02X}", byte);
        }
    }
}

... these both work for me.

@zpg6
Copy link
Author

zpg6 commented Dec 30, 2024

My output for all three examples looks like:

BREAK 55 00 00 00 00 00 00 00 00 00 00
BREAK 55 01 00 00 00 00 00 00 00 00 00
BREAK 55 02 00 00 00 00 00 00 00 00 00
BREAK 55 03 00 00 00 00 00 00 00 00 00
BREAK 55 04 00 00 00 00 00 00 00 00 00
BREAK 55 05 00 00 00 00 00 00 00 00 00
BREAK 55 06 00 00 00 00 00 00 00 00 00
BREAK 55 07 00 00 00 00 00 00 00
BREAK 55 08 00 00 00 00 00 00 00 00 00
BREAK 55 22
BREAK 55 23

Works like a charm - for a test case that's not repeatable (you need to send yourself data and breaks with another tool or device). Maybe we can add the send_break and send_break_async first and add those to HIL capabilities.

@zpg6
Copy link
Author

zpg6 commented Dec 30, 2024

Added a HIL test for wait_for_break() and wait_for_break_async().await.

(They will fail on the 3sec test timeout until we add a break or simulated break)

@zpg6 zpg6 mentioned this pull request Dec 30, 2024
8 tasks
@zpg6 zpg6 marked this pull request as ready for review December 31, 2024 13:12
@MabezDev
Copy link
Member

MabezDev commented Jan 3, 2025

I suppose we should get the send break PR merged before this one, so that the HIL tests succeed.

@bugadani
Copy link
Contributor

bugadani commented Jan 6, 2025

IMHO wait_for_break(_async) need to be marked as unstable. We should figure out a policy how we want to introduce new functionality, and I'm not entirely sure this is the best way for this to look.

@zpg6
Copy link
Author

zpg6 commented Jan 6, 2025

IMHO wait_for_break(_async) need to be marked as unstable. We should figure out a policy how we want to introduce new functionality, and I'm not entirely sure this is the best way for this to look.

I suspect most will use the interrupt handler anyways, not the wait functions

@bugadani
Copy link
Contributor

bugadani commented Jan 6, 2025

I think that for the context of this PR it doesn't really matter what most users will use. I'm just saying this shouldn't be part of the initially stable API. Other than that, I don't have objections to include this unstably until we either accept that this is the best solution, or we find something better. An unstable solution is better than no solution after all :)

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.

UART: detecting Break conditions is not supported
3 participants