rp-rs / pio-rs

Support crate for Raspberry Pi's PIO architecture.
MIT License
149 stars 22 forks source link

Clearer error/documentation on pio_asm macro when .side_set is set without opt #62

Open turmoni opened 4 months ago

turmoni commented 4 months ago

I've been attempting to port over some PIO code I wrote in Micropython to Rust, and the following error left me very confused:

error: proc macro panicked
   --> src/main.rs:69:21
    |
69  |       let read_cd32 = pio_proc::pio_asm!(
    |  _____________________^
70  | |         ".side_set 1",
71  | |         "begin:",
72  | |         "    set pins, 0    side 0 [2]",
...   |
101 | |         "    jmp begin",
102 | |     );
    | |_____^
    |
    = help: message: instruction requires 'side' set

Having come from Micropython, where I wasn't writing raw assembly, and with the error not pointing at a specific instruction, I hadn't realised that specifying .side_set without opt meant that a side set was mandatory on every instruction, and the PIO example is also slightly misleading:

        ".side_set 1", // each instruction may set 1 bit

The comment should probably say "each instruction must set 1 bit".

I think the proc macro error could be clearer either if it was possible to isolate the line with an issue (I don't know if this can be done), or alternatively perhaps a change to something like "every instruction requires 'side' set", but being new to Rust and not really knowing how things work yet, I don't want to propose anything more concrete.

The PIO section of the RP2040 datasheet does specify this behaviour, but already having something that was working elsewhere I wasn't looking too closely at it, and being brand new to Rust I was sort of assuming I'd messed up something somewhere else.

jannic commented 2 months ago

I updated the mentioned comment.

Updating the proc macros so the error message points to the correct line is likely possible, but a bigger task.