Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
turmoni opened this issue Jul 20, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@turmoni
Copy link

turmoni commented Jul 20, 2024

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 added a commit to jannic/rp-hal that referenced this issue Aug 27, 2024
The rp2040 and rp235x example of pio_side_set have diverged a bit.
Port the changes made to the rp235x back to rp2040, where applicable.

Also update a comment, as suggested in rp-rs/pio-rs#62
@jannic
Copy link
Member

jannic commented Aug 27, 2024

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.

@jannic jannic added the enhancement New feature or request label Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants