amaranth
amaranth copied to clipboard
Emit signal groups in GTKW files
Fixes #764.
Things to look at in the review:
- Group names for views and interfaces. The current code uses "view" and "interface". Maybe the name should reference a layout/signature in some way?
- Treating single-field view as in https://github.com/amaranth-lang/amaranth/issues/764#issuecomment-2014220740 is not implemented. It could be implemented by adding a special case.
- I've added tests that check if the code doesn't raise an exception, the resulting gtkw files are not checked. I'm not sure how to best check them; I could compare them with a reference, but this would freeze the signal naming. Is signal naming for views and interfaces specified?
- Had to add another change related to the recent private names commit. Now that there are three separate recursive functions in this PR, maybe the recursion pattern should be abstracted away?
Codecov Report
Attention: Patch coverage is 88.70968% with 7 lines in your changes are missing coverage. Please review.
Project coverage is 91.05%. Comparing base (
dde8334) to head (35380ec). Report is 27 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| amaranth/sim/pysim.py | 86.00% | 3 Missing and 4 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1245 +/- ##
==========================================
+ Coverage 90.48% 91.05% +0.57%
==========================================
Files 43 45 +2
Lines 10864 11256 +392
Branches 2660 2449 -211
==========================================
+ Hits 9830 10249 +419
+ Misses 855 828 -27
Partials 179 179
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@tilk Could you rebase the PR please? We've finally landed MemoryData, a proper replacement for MemoryIdentity that also improves tracing experience, but this caused a conflict here.
@tilk, the rework of the simulator necessary for the RFC 36 has introduced some merge conflicts here--could you fix them please?
Could you squash the PR please? The commit message could be something like
sim: group signal traces according to their function.
Could you squash the PR please?
Done.
Thanks!