embassy-rs / embassy

Modern embedded framework, using Rust and async.
https://embassy.dev
Apache License 2.0
5.15k stars 713 forks source link

embassy-rp: UART flush isn't flushy enough #2464

Open jamesmunns opened 7 months ago

jamesmunns commented 7 months ago

The blocking uart's blocking+async flush only waits until the buffered data is enqueued, NOT until the data is all out on the wire:

https://github.com/embassy-rs/embassy/blob/871d82de7165781ca6164de72664c9a6a90b3e66/embassy-rp/src/uart/buffered.rs#L419-L427

Instead, you also need to busy loop on the uart.busy() flag that reports whether the transmitter is active. I do that here:

https://github.com/jamesmunns/erdnuss-pub/blob/3d98a55aa7a90141a525e4443c927400f8516dd2/source/rp2040/src/lib.rs#L74-L77

There is no interrupt for this state. AFAICT, you have to busy poll from the time of interrupt (when DMA has completed and all bytes enqueued), maybe past the interrupt when the FIFO falls low again, and then busy loop until the transmitter is idle.

This is not compliant with embedded-io's flush guarantees. For blocking, this is NBD, we can just busy loop. It's annoying for async tho.

jamesmunns commented 7 months ago

This is the same kind of deficiency being fixed by #2416

enbyted commented 7 months ago

The busy loop is exactly what I’m doing for controlling the RS485 transceiver. However, it would be nice to at least have an async flush for the fifo, then you only have to busy-loop on the last byte.

I guess one could start a (hardware) timer in the uart interrupt for TX fifo empty, that will expire as the last bit is expected to leave the shift register.

enbyted commented 7 months ago

I re-checked the datasheet and I remembered why in another project I disabled the FIFO for uart - there is no interrupt for an empty FIFO. The best you can get is an interrupt when 4 bytes are remaining to be sent (with watermark level set to 7/8.

That is still better than nothing I suppose - we can have async for everything, but last 5 bytes (4 in the fifo + 1 in the shift register).

We could get better by disabling the FIFO, but then we get a much higher risk of losing bytes on reception, since there are no separate controls for RX and TX FIFO. In the end, this is using the same interrupts and registers, so it could be a config option with no difference in code for different modes.

//offtopic: I remember scratching my head about how could they release a microcontroller with no hardware TE control and no TX complete style interrupt. I’ve only seen this on some PIC16 micros, never in a 32-bit chip. The best you can do is utilize PIO to control TE for you, but that is hard to make bit-accurate (i.e. make the TE rise right after stop bit).

jamesmunns commented 7 months ago

@enbyted yeah, there's also a detail I got bit with on my RS485 project: there is a "Line Idle" interrupt, but it only fires if the FIFO is not empty, which means if you use DMA then the FIFO is almost always empty.

That's why I ended up using a line break in my wire protocol instead of just a line idle time.

It's a standard Arm Prime Cell UART (so we can't really fully blame them, other than their choice of IP blocks), but it is a shame it is so meh.

enbyted commented 7 months ago

That’s also why I’m going with line break, though realistically for my needs 9-bit frames (i.e. what Atmel I think calls the uart with addressing mode or something like that) would be more efficient due to usually sending very short frames. Once I get some other parts of my system running I might work on some support for that, actually.

Anyhow, for this issue fixing the flush to also busy loop until transmission is fully complete sounds like the right approach. Would you be interested in getting a PIO-based async flush and/or TE controller into embassy? Since PIO can send interrupts it could wake up an async task for flush implementation, plus it’s nearly the same operation as controlling the TE line, I think that would be doable in a relatively-generic way. I’d need to look closer at the APIs, but I might be interested in working on that in near-ish future (a few weeks time).

EDIT: Now that I’m thinking about it, I wonder if it would be easier to simply implement a uart tx part in PIO that has the TE controller and TX complete interrupt. It can interface with DMA and the normal PL011 can simply work as the RX half with the FIFO and everything… It would also play very cleanly with the current APIs since you can already create just the RX part.

jamesmunns commented 6 months ago

Just for the record:

the only solutions I can think of are: