Implement send_break support

[ithinuel]

No description provided.

[jannic]

The failing test is the MSRV check also discussed in #698. It's fixed with #698, so no need to worry about it here.

[jannic]

While trying to write a good example for this function, I noticed a few things I don't like about this API:
First, this looks like valid code but is completely wrong:

     uart.write_full_blocking(data: b"before break\r\n");                         
     uart.send_break();                                            
     uart.write_full_blocking(data: b"after break\r\n");             

This compiles without a warning, but wouldn't ever clear the break bit.
By adding a #[must_use] annotation, this would at least trigger a warning:

warning: unused `rp2040_hal::uart::writer::Break` that must be used
   --> rp2040-hal/examples/uart.rs:101:5
    |
101 |     uart.send_break();
    |     ^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_must_use)]` on by default
help: use `let _ = ...` to ignore the resulting value
    |
101 |     let _ = uart.send_break();
    |     +++++++

But the suggestion is misleading (let _ = uart.send_break() would still be wrong), and doesn't provide any hint how to use it correctly.
If you are aware that you have to clear the break manually, and have to implement the delay yourself, the next attempt could be this:

    uart.write_full_blocking(b"before break\r\n");
    let brk = uart.send_break();
    let bit_duration: MicrosDurationU32 = baudrate.into_duration();
    let break_duration = bit_duration * (2 * 10); // at least two full frames of 10 bits
    delay.delay_us( break_duration.to_micros() );
    brk.clear();
    uart.write_full_blocking(b"after break\r\n");

But as far as I understand, it's still not correct. The break bit is specified as:
"If this bit is set to 1, a low-level is continually output on the UARTTXD output, after completing transmission of the current character. "
So the code has two issues:

1. The break might start somewhere in the middle of the string "before break", because it will start after the currently transmitted byte, not flushing the TX fifo first. This is probably not what the user wants to do.
2. The delay starts immediately, while the last frame before the break is still being transmitted. So at worst, the break might be one frame too short.
   To fix this, one needs to add a flush before starting the break.

All in all, this interface is easier to use incorrectly than correctly.

While less flexible, wouldn't something like fn send_break(delay: &mut DelayUs, duration_bits: u32) be much more usable? However, I'd hesitate to implement it like that before embedded_hal 1.0 is released, to avoid adding a dependency on embedded_hal 0.2.

For now, we could add two low-level functions set_break() / clear_break(), which just set and clear the break bit? This would be as difficult to use correctly as described above, but at least it wouldn't give the impression of being a high-level API.

Realized that the API is too difficult to use correctly.

[ithinuel]

I know it makes it harder to handle but would changing the type of the uart be acceptable? (via a wrapping newtype or a generic).

// with
fn break(self) -> Breaking<Self>;

let foo: Breaking<_> = serial.break();
// some wait logic
let serial = foo.unbreak();

[TKFRvisionOfficial]

I'm not a fan of the @ithinuel solution to the issue, because I store my UART in a struct, this would break things cuz now we got a consuming function. Im of the opinion that @jannic thoughts about the misusage in regards to bypassing the must_use via "_=" is unfounded because ppl who do this probably haven't really understood what a uart break actually is yet. A issue I do get is the TX buffer not being cleared, which can be confusing to beginners that don't understand a 100% how the rp2040 uart works yet. In my small world proper docs would suffice but I get that rust is all about enforcing the correctness via the compiler. I don't really have an issue with @jannic less flexible API, it doesn't effect my use case, but their might be some edge case out there where this doesn't suffice. Adding the low level functions (maybe as unsafe) might fix this as well.

[9names]

Rust is all about enforcing the correctness via the compiler.

Rust is all about enforcing memory safety, sometimes via the compiler, sometimes via the standard library.
We can not and should not solve all logic errors through the language and the typesystem.

Adding the low level functions (maybe as unsafe)

Putting low-level access behind "unsafe" is a terrible solution to this. This isn't a memory safety issue.
Having unsafe on a function like this means you unnecessarily opt out of some of the safety that safe Rust provides.

If you really want low-level interfaces to be super obvious (and therefore, not what you choose when looking for a high-level API) we have better practices for logically separating functions that live at different levels of the API.
Namespaces are a nice one (that would require more change to the existing code). But even just prefixing the function with the words "lowlevel_" makes it quite obvious that it's a low level function that you should take more care with:

serial.lowlevel_set_break();

[jannic]

While I'm not convinced that the lowlevel_-prefix is needed, it doesn't really hurt either. And instead of endlessly bikeshedding function naming, we should start providing a way to send a break using the HAL. → Approved.