nrf-rs / nrf-hal

A Rust HAL for the nRF family of devices
Apache License 2.0
507 stars 140 forks source link

[docs] UART data received outside a blocking `Uarte::{read,read_timeout}` call is discarded #289

Open japaric opened 3 years ago

japaric commented 3 years ago

I don't consider this a bug per se but I find the behavior surprising so I think it (the issue title) should be documented in the API docs.

Steps to reproduce

Running this program

let mut rx_buffer = [0];
defmt::info!("GO");
// do `$ echo hi > /dev/ttyACM0` in the middle of the delay
asm::delay(100_000_000);
defmt::info!("WAIT");

// do `$ echo yo > /dev/ttyACM0` in the middle of the infinite loop
loop {
    const TIMEOUT: u32 = 1_000_000;

    let res = uarte.read_timeout(&mut rx_buffer, &mut timer, TIMEOUT);

    if res.is_ok() {
        defmt::info!("{:?}", rx_buffer);
    } else {
        defmt::error!("timeout");
    }
}

And performing these serial operations:

$ # in parallel as specified in the source code
$ echo hi > /dev/ttyACM0
$ echo yo > /dev/ttyACM0

Produces this output:

$ cargo r
0.000000 INFO  WAIT
0.000000 INFO  GO
0.000000 ERROR timeout
0.000000 INFO  [121]
0.000000 INFO  [111]
0.000000 INFO  [13]
0.000000 INFO  [10]

[121, 111, 13, 10] = b"yo\r\n" The data sent by the first echo command (b"hi\r\n") is discarded -- I guess at the hardware level -- so it's never returned by the read_timeout operation The same behavior is observed with the read API.

This is different from other HALs / microcontrollers where incoming serial data is stored in a hardware buffer and embedded_hal::serial::Read implementations can still return that old data (module overrun errors)

P.S. this behavior may be present in other read APIs (I2C)? I haven't tested that

Initialization code

``` rust let p = unwrap!(pac::Peripherals::take()); let port0 = p0::Parts::new(p.P0); let mut timer = Timer::one_shot(p.TIMER0); let rxd = port0.p0_08.into_floating_input().degrade(); let txd = port0.p0_06.into_push_pull_output(Level::High).degrade(); let pins = Pins { rxd, txd, cts: None, rts: None, }; let mut uarte = Uarte::new(p.UARTE0, pins, Parity::EXCLUDED, Baudrate::BAUD9600); ```
jonas-schievink commented 3 years ago

Hmm, does the hardware not detect overrun? I'd expect the read_timeout to fail with an error, ideally.

japaric commented 3 years ago

I'd expect the read_timeout to fail with an error, ideally.

Now that you mention it. That would be better, yeah.

Hmm, does the hardware not detect overrun?

Not familiar with this peripheral or the implementation but perhaps it doesn't detect overrun when there's no configured DMA transfer?

taruti commented 3 years ago

I think the "correct" solution with nrf52 is to use hardware flow control. Which of course needs extra wires and depends on the other end...

Yatekii commented 3 years ago

I mean, normally it is configurable whether it should override the old contents of the buffer or throw away the new ones. I would not argue in favor of either. I am not sure how embedded-hal intends this to function, but I think both behaviors are perfectly valid! If you have let's say some sensory data that is transmitted via UART and you feed it into a control loop, you are never interested in the old value. Only ever in the new one. Your UART example @japaric of course has the opposite requirement :)

Sadly, I fear, for the nRF52 series this is not configurable and data is ALWAYS lost when an overrun occurs accoding to section 6.33.10.15 ERRORSRC in the ref man: https://infocenter.nordicsemi.com/pdf/nRF52840_PS_v1.1.pdf . We could flag a proper error and know that the previous data would be lost but I think there is no way to retain it.