feat: uart break detection interrupt #2858
Conversation
|
Opened discussion on interrupt handlers in general to better inform the design of this feature: |
Usage with UART Interrupt HandlerAdded 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);
});
} |
Usage without HandlerAlso 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. |
|
My output for all three examples looks like: 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 |
|
Added a HIL test for (They will fail on the 3sec test timeout until we add a break or simulated break) |
|
I suppose we should get the send break PR merged before this one, so that the HIL tests succeed. |
|
IMHO |
I suspect most will use the interrupt handler anyways, not the wait functions |
|
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 :) |
#3177 is ready for review with the send break |
|
Seems main branch's uart.rs is failing to build? I noticed when I went to run my async examples for the uart break I think it should say UPDATE:I fixed this in 8d01053 and it seems to work fine. |
|
example |
| /// | ||
| /// Clears the break detection interrupt before returning. | ||
| #[instability::unstable] | ||
| pub fn wait_for_break(&mut self) { |
There was a problem hiding this comment.
- Why do you need the
int_enabit to be set? - You shouldn't clear
int_enaat the end of the function without actually checking if it was cleared before - This function can end up hanging if the user has an interrupt handled that clears the brk_det bit. I know it would be their own fault but we should try our best to minimize the change of bugs.
There was a problem hiding this comment.
- Why do you need the int_ena bit to be set?
- I am operating under the assumption that the
int_rawonly functions ifint_enais set. Is this incorrect?
- I am operating under the assumption that the
- You shouldn't clear int_ena at the end of the function without actually checking if it was cleared before
- Sorry, do you mean
int_clr?
- Sorry, do you mean
- This function can end up hanging if the user has an interrupt handled that clears the brk_det bit. I know it would be their own fault but we should try our best to minimize the change of bugs.
- Makes sense
There was a problem hiding this comment.
Is this incorrect?
No, as far as I know only int_st and the actual interrupt handling is affected, where int_st = int_raw & int_ena. Usually.
Sorry, do you mean int_clr?
Hmm yeah disregard that point, I saw something else, not what you wrote. But in that case you should disable listening for the interrupt :)
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 📝
cargo xtask fmt-packagescommand to ensure that all changed code is formatted correctly.CHANGELOG.mdin the proper section.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