Make UARTs adhere to the std.Io.Reader interface

[pcolladosoto]

When trying to leverage an UART through an std.Io.Reader interface I was met with compilation errors such as

$ zig build
failed command: /Users/pcollado/.zvm/0.17.0-dev.1158+1d1193aa7/zig build-exe -fno-strip -fsingle-threaded -OReleaseSmall -target thumb-freestanding-eabi -mcpu cortex_m0plus --dep libmodbuz --dep microzig -Mroot=/Users/pcollado/repos/zig/zpi/src/main.zig -OReleaseSmall -target thumb-freestanding-eabi -mcpu cortex_m0plus -Mlibmodbuz=/Users/pcollado/repos/zig/zpi/zig-pkg/modbuz-0.1.0-0Q-9R9XEAgDzydH9_sRFHTaunW6CqI9rsSxKNNETe53K/src/root.zig --dep config --dep drivers --dep cpu --dep chip --dep hal --dep board -Mmicrozig=/Users/pcollado/repos/zig/zpi/../microzig-fork/core/src/microzig.zig -Mconfig=.zig-cache/o/9cc470c215c9d53763a20b54005a414b/options.zig -ODebug --dep link -Mdrivers=/Users/pcollado/repos/zig/zpi/../microzig-fork/drivers/src/root.zig --dep rtt --dep microzig -Mcpu=/Users/pcollado/repos/zig/zpi/../microzig-fork/core/src/cpus/cortex_m.zig --dep microzig -Mchip=.zig-cache/o/a4dc3ed0c6ced3b6b47ac543b2deba81/chips/RP2040.zig --dep bounded-array --dep microzig -Mhal=/Users/pcollado/repos/zig/zpi/../microzig-fork/port/raspberrypi/rp2xxx/src/hal.zig --dep bootloader --dep microzig -Mboard=/Users/pcollado/repos/zig/zpi/../microzig-fork/port/raspberrypi/rp2xxx/src/boards/raspberry_pi_pico.zig -ODebug -Mlink=/Users/pcollado/repos/zig/zpi/../microzig-fork/modules/network/link/link.zig -ODebug -Mrtt=/Users/pcollado/repos/zig/zpi/../microzig-fork/modules/rtt/src/rtt.zig -ODebug -Mbounded-array=/Users/pcollado/repos/zig/zpi/../microzig-fork/modules/bounded-array/src/bounded_array.zig -Mbootloader=.zig-cache/o/01d11e229d79fbdd352e20001304475f/w25q080.bin -ffunction-sections -fdata-sections --gc-sections --cache-dir .zig-cache --global-cache-dir /Users/pcollado/.cache/zig --name zpi -static -fcompiler-rt --script .zig-cache/o/498e1b9ecbd57f16bb8c6a4148dc6d7c/linker.ld --zig-lib-dir /Users/pcollado/.zvm/0.17.0-dev.1158+1d1193aa7/lib/ --listen=-

install
└─ install generated to zpi-dbg.elf
   └─ compile exe zpi-dbg Debug thumb-freestanding-eabi 3 errors
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:178:27: error: missing struct field: seek
            .interface = .{
                         ~^
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:178:27: note: missing struct field: end
/Users/pcollado/.zvm/0.17.0-dev.1158+1d1193aa7/lib/std/Io/Reader.zig:1:1: note: struct declared here
const Reader = @This();
^~~~~
referenced by:
    init: src/modbus.zig:161:30
    main: src/main.zig:87:16
    6 reference(s) hidden; use '-freference-trace=8' to see all references
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:210:28: error: @fieldParentPtr increases pointer alignment
        const r: *Reader = @fieldParentPtr("interface", io_reader);
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:210:28: note: parent pointer type '*hal.uart.UART.Reader' has alignment '2'
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:155:24: note: struct field 'interface' limits alignment to '2'
    pub const Reader = struct {
                       ^~~~~~
/Users/pcollado/repos/zig/microzig-fork/port/raspberrypi/rp2xxx/src/hal/uart.zig:210:28: note: use @alignCast to assert pointer alignment
src/modbus.zig:44:30: error: no field or member function named 'take_byte' in '?hal.uart.UART.Reader'
    const count = uart_reader.take_byte() catch |err| {
                  ~~~~~~~~~~~^~~~~~~~~~

$ zig version
0.17.0-dev.1158+1d1193aa7

This PR adds changes so that the UART's HAL adheres to the std.Io.Reader interface whilst also updating the stream function.

The reader implementation works on my side when driving the UART for performing Modbus requests.

If there's anything that should be changed don't hesitate to do so: these are my first steps with Zig!

Thanks a ton for your time.

[mattnite]

Since we're returning one error regardless, we don't need to capture the error here, you can write instead something like:

const b = uart.read_word_blocking(uart_reader.deadline) catch return error.ReadFailed;

[pcolladosoto]

Thanks a ton for the comment! It should be applied on 78b1d61.

Thanks again!

[pcolladosoto]

Quick question that popped up when using the interfaces. Is this deadline correct? It's initialised when instantiating the Reader struct, but at least for the RP2040 I'm creating these deadlines with time.deadline_in_ms() which simply adds the argument with the current time to define an absolute timestamp when the deadline elapses.

Now, given the deadline's instantiated at the very beginning and never recreated it basically becomes a no-op as it's always elapsed later down the road.

Would it make sense to instead of passing an instantiated mdf.time.Deadline pass an integer (i.e. deadline_ms) to instantiate the deadline whenever we make a call such as a in stream()?

Maybe I'm just using the API wrong...

Should this be an error I'd be more than happy to provide a separate PR handling this as it affects both the Reader and Writer.

Thanks a ton for your time.

[Grazfather]

It's 'correct', but yeah, it would be weird to use, since a deadline is an absolute time (typically since boot). I am not sure the best way to square our use of deadlines (vs. timeouts) with the reader interface. We could have a default timeout which becomes a deadline when the actual read/write functions are called.

[ikskuh]

We should always stick to absolute deadlines, as otherwise you can't compose the code correctly.

imagine you have a function which constructs a local reader, but is called 6 times. But you want to have these 6 calls timeout after a total of 1 second, and not each single one after 1 second.

We can just expose a .deadline as a public interface of the reader/writer and the user can simply update them as they wish.

[pcolladosoto]

Super! In order not to mix things up and given this 'issue' affects both the Reader and Writer types what do you think about spinning this off into another issue? That way we can merge the changes to the Reader so that it conforms to std.io.Reader and handle this elsewhere.

If you're okay with that I guess this can be merged as soon as you're happy with the changes and I'll prepare a follow-up PR in a couple of days; I'm currently on holiday! 😎