-
Notifications
You must be signed in to change notification settings - Fork 58
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
build: add DSL for defining platform resources. #59
Conversation
Codecov Report
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
+ Coverage 85.73% 85.91% +0.17%
==========================================
Files 27 28 +1
Lines 4010 4060 +50
Branches 803 817 +14
==========================================
+ Hits 3438 3488 +50
Misses 479 479
Partials 93 93
Continue to review full report at Codecov.
|
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.
See review comments above.
Also, please rename the entire module such that it becomes nmigen.build.dsl
.
Also, please write tests for Pins
and DiffPairs
to verify that these APIs work correctly. I think I will ask for more changes to Subsignal
so you don't need to write tests for that for now.
return "(subsignal {} {} {})".format(self.name, " ".join(map(repr, self.io)), self.extras) | ||
|
||
|
||
class Resource(Subsignal): |
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.
What is the purpose of this class?
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.
Resources are just "top-level" Subsignals.
In case multiple IO resources share the same name, they have an additional number
attribute to identify them (e.g. platform.request("user_led", 1)
).
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.
Thank you, this is very good! I thought about Resource
and it is basically OK. Please add a test for that and I'll merge this.
No description provided.