Skip to content

Fails to parse the SPI example #7

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

Closed
jepler opened this issue Feb 10, 2021 · 2 comments · Fixed by #8
Closed

Fails to parse the SPI example #7

jepler opened this issue Feb 10, 2021 · 2 comments · Fixed by #8

Comments

@jepler
Copy link
Contributor

jepler commented Feb 10, 2021

The SPI example has this code:

    out pins, 1 side 0 [1] ; Stall here on empty (sideset proceeds even if
    in pins, 1  side 1 [1] ; instruction stalls, so we stall with SCK low)

It fails to parse, because it searches for "pins," in IN_SOURCES and OUT_DESTINATIONS (note the trailing ,).

I put a hacky fix in and it did assemble, though there are remaining limitations in the core before it can be used:

diff --git a/adafruit_pioasm.py b/adafruit_pioasm.py
index 96891f9..c847298 100644
--- a/adafruit_pioasm.py
+++ b/adafruit_pioasm.py
@@ -106,7 +106,8 @@ def assemble(text_program):
         elif instruction[0] == "in":
             #                instr delay src count
             assembled.append(0b010_00000_000_00000)
-            assembled[-1] |= IN_SOURCES.index(instruction[1]) << 5
+            source = instruction[1].rstrip(",")
+            assembled[-1] |= IN_SOURCES.index(source) << 5
             count = int(instruction[-1])
             if not 1 <= count <= 32:
                 raise RuntimeError("Count out of range")
@@ -114,7 +115,8 @@ def assemble(text_program):
         elif instruction[0] == "out":
             #                instr delay dst count
             assembled.append(0b011_00000_000_00000)
-            assembled[-1] |= OUT_DESTINATIONS.index(instruction[1]) << 5
+            destination = instruction[1].rstrip(",")
+            assembled[-1] |= OUT_DESTINATIONS.index(destination) << 5
             count = int(instruction[-1])
             if not 1 <= count <= 32:
                 raise RuntimeError("Count out of range")

I'm totally sure this is not the right fix so I'm entering this as an issue instead of as a PR.

@dglaude
Copy link
Contributor

dglaude commented Feb 11, 2021

If someone in Adafruit Circuit Python community know how to setup a way to automate testing, I believe this library could benefit from it. It is pure software, no hardware involved. You can have various piece of assembly code and you know exactly what binary result you expect.

I did manually run this kind of test in this PR comment: #5 (comment)

This would permit to detect regression and make sure that code that "assemble" in the past still work in the future.

@tannewt
Copy link
Member

tannewt commented Feb 11, 2021

I was hoping to make decision to omit support for commas. I don't like entirely optional portions of a language. There should only be one canonical form. Perhaps we should simply add a friendly exception for it.

jepler added a commit to jepler/Adafruit_CircuitPython_PIOASM that referenced this issue Feb 12, 2021
All of the following now parse as `["mov", "pins", "1"]` and assemble to
0x6001:
```
out pins , 1 ;
out pins, 1  ;
out pins,1   ;
out pins ,1  ;
out pins 1   ;
```

This brings pioasm closer to what upstream's examples show

Closes: adafruit#7
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 a pull request may close this issue.

3 participants