-
Notifications
You must be signed in to change notification settings - Fork 212
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
Add HIL test for ECC #1418
Add HIL test for ECC #1418
Conversation
For future reference (whenever we add C2 support), the tests are also passing for C2! Including the
As usual some noisy-irrelevant probe-rs errors in C2, feel free to ignore them |
358b573
to
8de0b29
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.
Changes mostly LGTM! Left a few things to fix
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! As discussed, the only thing that I see is that we could make the test faster if we already have the expected output hard-coded instead of calculating them with sw and comparing them. Not sure how much this will save us and if it's relevant, though
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, thanks!
Since #1414 got already merged, well need to rebase this PR |
Yeah my bad didn't notice the conflict before adding it to the queue 😅 |
00ff9c6
to
67a9d2d
Compare
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
Adds a HIL test for ECC - it's basically a copy-pasted example.
Testing
Tested locally on C6 and @SergioGasquez helped me with testing on H2 (my H2 is dead). Also, the HIL CI run in my fork/branch of this PR: https://github.com/esp-rs/esp-hal/actions/runs/8630865547