Skip to content

rp2: Add StateMachine(may_exec=, offset=) #8223

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

Merged
merged 11 commits into from
Aug 9, 2023
Merged

Conversation

jepler
Copy link

@jepler jepler commented Jul 27, 2023

and also sanity-check the init instructions, thanks to the newly factored-out checker.

Closes: #8221

Not yet tested on hardware!

@jepler jepler changed the title rp2: Add StateMachine.may_exec rp2: Add StateMachine(may_exec=, offset=) Jul 27, 2023
jepler added 2 commits July 27, 2023 15:47
and also sanity-check the init instructions, thanks to the newly
factored-out checker.

Closes: adafruit#8221
@jepler
Copy link
Author

jepler commented Jul 27, 2023

and it also adds offset= which is needed for encoder implementations like https://github.com/adamgreen/QuadratureDecoder/blob/master/QuadratureDecoder.pio

@jepler
Copy link
Author

jepler commented Jul 28, 2023

a discord user reports this doesn't work right, so I'll need to revisit it and do more testing

@jepler
Copy link
Author

jepler commented Aug 5, 2023

I tested this today with a QT Py RP2040 & jog wheel encoder.

Tests may_exec=: https://gist.github.com/jepler/597b80caa61a4a71f094959c1df66001 (note that I think this PIO code may have a bug that occasionally allows the incorrect value ~x (bitwise not of the actual value) to be read; this is a bug in the PIO code and not in CP; I think this can be fixed by using 'y' as the temporary register)

Tests offset=0: https://gist.github.com/jepler/891a3c48e2f9f9fb9759bdb5ce081265 (revised by me so that values are continuously added to the FIFO, not only on change. otherwise, if the FIFO is allowed to fill, the latest value might not be read out because it was pushed while the FIFO was full) I don't know what happened but I did not test the code I thought I did. This needs further work.

@jepler
Copy link
Author

jepler commented Aug 5, 2023

@Ultrawipf please re-test if you have the time & interest!

@jepler jepler marked this pull request as ready for review August 5, 2023 19:39
@jepler
Copy link
Author

jepler commented Aug 5, 2023

updated yet again and using https://gist.github.com/jepler/0f162eb04c2b4e36e9ae9d6a228e4112 as test of offset=.

I also manually tested that offset conflicts are detected, though the error message says "All state machines in use" when the real problem is that the requested load address was not available.

This interactive session shows how the "same" program can be loaded multiple times at offset 0, then a 2nd program can be loaded at offset 0 of the second SM. A third program can't be loaded at offset 0 (it'd have to go in the third SM) but it can be loaded at another offset.

>>> p = adafruit_pioasm.assemble("jmp 0")
>>> sm1 = StateMachine(p, offset=0, frequency=1_000_000)
>>> sm2 = StateMachine(p, offset=0, frequency=1_000_000)
>>> sm3 = StateMachine(p, offset=0, frequency=1_000_000)
>>> p = adafruit_pioasm.assemble("nop")
>>> sm4 = StateMachine(p, offset=0, frequency=1_000_000)
>>> sm5 = StateMachine(p, offset=0, frequency=1_000_000)
>>> sm5 = StateMachine(p, offset=0, frequency=1_000_000)
>>> p = adafruit_pioasm.assemble("jmp x--, 0")
>>> sm6 = StateMachine(p, offset=0, frequency=1_000_000)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: All state machines in use
>>> sm6 = StateMachine(p, frequency=1_000_000)

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the refactor. Very cool to see these advanced uses of PIO.

jepler and others added 4 commits August 7, 2023 11:51
leave as a single structure because it's more efficient to call
functions with 4 or fewer arguments, and having two struct pointers
would make `consider_instruction` have 5 arguments instead.
@jepler jepler requested a review from tannewt August 8, 2023 15:45
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the updates!

@tannewt tannewt merged commit 472e6bc into adafruit:main Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rp2pio checking for "in" instruction when not always required
2 participants