Skip to content
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

hdl.rec: don't save casted shapes in Layout constructor #394

Merged
merged 1 commit into from Jun 5, 2020

Conversation

shawnanastasio
Copy link
Contributor

@shawnanastasio shawnanastasio commented May 31, 2020

Don't save the result of Shape.cast in the Layout constructor.
This allows the original user-provided shape to be preserved,
while still checking its validity.

This fixes Enum decoder inference for Signals (#393).

@codecov
Copy link

@codecov codecov bot commented May 31, 2020

Codecov Report

Merging #394 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #394   +/-   ##
=======================================
  Coverage   82.10%   82.10%           
=======================================
  Files          36       36           
  Lines        6075     6075           
  Branches     1234     1234           
=======================================
  Hits         4988     4988           
+ Misses        914      913    -1     
- Partials      173      174    +1     
Impacted Files Coverage Δ
nmigen/hdl/rec.py 96.91% <100.00%> (ø)
nmigen/build/run.py 31.25% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26a15b3...72a80fc. Read the comment docs.

@shawnanastasio
Copy link
Contributor Author

@shawnanastasio shawnanastasio commented May 31, 2020

Looks like I missed some tests, let me fix those up

nmigen/test/test_hdl_rec.py Outdated Show resolved Hide resolved
nmigen/test/test_lib_io.py Outdated Show resolved Hide resolved
@shawnanastasio
Copy link
Contributor Author

@shawnanastasio shawnanastasio commented Jun 2, 2020

Updated to address review comments

@whitequark
Copy link
Member

@whitequark whitequark commented Jun 2, 2020

Could you add a test that checks for the bug you're fixing here, i.e. the Enum decoder functionality?

@shawnanastasio
Copy link
Contributor Author

@shawnanastasio shawnanastasio commented Jun 3, 2020

Added a test for the enum decoder in RecordTestCase

Don't save the result of Shape.cast in the Layout constructor.
This allows the original user-provided shape to be preserved,
while still checking its validity.

This fixes Enum decoder inference for Signals (amaranth-lang#393).
@whitequark whitequark merged commit 2f7c3bf into amaranth-lang:master Jun 5, 2020
3 checks passed
@whitequark
Copy link
Member

@whitequark whitequark commented Jun 5, 2020

Thank you for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants