-
Notifications
You must be signed in to change notification settings - Fork 255
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
Refactor testing, add defmt, add async gpio test #1363
Conversation
One cool thing here is that it is actually possible to mix async and sync tests! Not super useful, but in the case of GPIO where they it can be used in async and blocking it makes sense. |
b7974b9
to
da1c4f7
Compare
Drafting until esp-rs/esp-pacs#221 as it will fail in the HIL stage. |
7bedf24
to
6fdf2e0
Compare
6fdf2e0
to
0902994
Compare
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.
Just a couple little nitpicks but otherwise LGTM
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.
Thanks for making those changes!
@SergioGasquez should we just try re-running the tests when this happens, or what is the best course of action? |
Seems to be working in my PR, so let's try again... |
I think this might actually be an issue, will investigate. |
Yeah so @bugadani already warned me about this, and then I subsequently forgot 😅. The last pin, pin 30 (31) is used for flash, for chips with embedded flash. When we change the pin operation, we break that and then the program goes nuts. I'm not sure how we test this without breaking functionality, any ideas? If there isn't a good way to do this, we can drop that test for now, there are still some good changes in here regardless. |
Eh, I just simplified the test. Add a changelog for the GPIO API too. |
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.
LGTM! Also, I will update all the VMs to use the latest probe-rs
as C6 has an older revision.
Thanks! In this case the corrupted control block was a real error that HIL caught 💪, but would be good to be running the latest code. |
Thank you for your contribution!
We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:
Submission Checklist 📝
CHANGELOG.md
in the proper section.Extra:
Pull Request Details 📖
Description
It's really annoying when a panic happens and you can't see the output, this also allows logging to be intermingled when testing, which will be very useful when debugging tests. The backtrace output via defmt though is really not that great, but at the very least a message is displayed.
Testing
Run the HIL suite locally.