-
Notifications
You must be signed in to change notification settings - Fork 6
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
Make changes to support HitL testing. #69
Conversation
test/hitl/pyboard.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how we want to support this dependency. Single file submodule? It really is a great tool, and it works well with both MicroPython and CircuitPython.
25373cc
to
9c106bb
Compare
…ort can still remain locked, causing the next test to fail.
…he main example. This allows different boards to be easily targeted.
… tests only when key files changed, so they don't run unnecessarily.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff @m-mcgowan!
exit 1 | ||
fi | ||
|
||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline at end of file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added pre-commit checks to catch this.
- name: Make CircuitPython filesystem writeable to pyboard | ||
if: ${{ matrix.lock_cpy_filesystem }} | ||
run: | | ||
timeout $USB_MSD_ATTACH_TIME bash test/scripts/wait_for_file.sh "$CPY_FS_CIRCUITPY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this redundant since we already waited for the file to exist at line 104?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous step may not have run since it's conditional. But I want the check in the previous step to keep the check close to the behavior that affects it.
|
||
# only copy if it's changed or not present. After the device has reset, no further changes can be made | ||
# until the filesystem is erased. This allows the workflow to be rerun flash_device=false | ||
diff test/hitl/boot.py "$CPY_FS_CIRCUITPY/boot.py" || test/hitl/boot.py "$CPY_FS_CIRCUITPY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should test/hitl/boot.py "$CPY_FS_CIRCUITPY"
be cp test/hitl/boot.py "$CPY_FS_CIRCUITPY"
?
default: true | ||
|
||
jobs: | ||
huzzah32: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be called test
instead, to mirror hil-circuitpython.yml?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had the same thought when I renamed it. The matrix parameters show up in the job run so no need to name it specifically.
exit 1 | ||
fi | ||
|
||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline at EOF.
examples/notecard-basics/board.py
Outdated
UART=2 | ||
|
||
""" | ||
The SCL pin of the I2C bus connected to Notecard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment seems slightly misplaced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it up when making a pep8 pass through.
``` b'Performing initial setup\r\nMicroPython v1.20.0 on 2023-04-26; ESP32 module with ESP32\r\nType "help()" for more information.\r\n>>> ' INTERNALERROR> pyb.enter_raw_repl() INTERNALERROR> File "/home/mat/runners/notecard-nbgl-carr_f-huzzah32-serial/_work/note-python/note-python/test/hitl/deps/pyboard.py", line 367, in enter_raw_repl INTERNALERROR> raise PyboardError("could not enter raw repl") INTERNALERROR> pyboard.PyboardError: could not enter raw repl ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll leave it to you to approve the final review.
exit 1 | ||
fi | ||
|
||
exit 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added pre-commit checks to catch this.
- name: Make CircuitPython filesystem writeable to pyboard | ||
if: ${{ matrix.lock_cpy_filesystem }} | ||
run: | | ||
timeout $USB_MSD_ATTACH_TIME bash test/scripts/wait_for_file.sh "$CPY_FS_CIRCUITPY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous step may not have run since it's conditional. But I want the check in the previous step to keep the check close to the behavior that affects it.
default: true | ||
|
||
jobs: | ||
huzzah32: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had the same thought when I renamed it. The matrix parameters show up in the job run so no need to name it specifically.
examples/notecard-basics/board.py
Outdated
UART=2 | ||
|
||
""" | ||
The SCL pin of the I2C bus connected to Notecard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it up when making a pep8 pass through.
Since Hayden is also an author for this PR, he cannot approve a review. Merging with offline approval from him. |
Mat's Notes: