schoeberl / chisel-book

Digital Design with Chisel
773 stars 144 forks source link

Version 2.1 #27

Closed hansemandse closed 3 years ago

hansemandse commented 3 years ago

In this PR, I have added many new things and updated some existing ones. The changes include:

schoeberl commented 3 years ago

Thank you for all those changes! Great work. However, I think this is a bit too large for a single commit. Can we please split this up into thematic changes? And discuss individual. I guess some of the changes are also a bit ahead. E.g., ChiselTest is still in beta, so I do not have made up my mind if I would like to go there now.

schoeberl commented 3 years ago

Let us start to talk about the tests. Did you write for each original iotester version a new one with ChiselTest? This is great! For using the tests internally I do not need to have both versions. That redundancy is actually bad. And we do not have them in different folders. I like to keep the Scala package = folder name. So a PR on simply changing the tests from the old to the new would be perfect. Except for the test(s) that are used in the book. For those, I would like to have both, so I can decide when to transition the description of testing in the book. I might for some time have both (wich will be a bit confusing).

schoeberl commented 3 years ago

I changed my mind. Let us move forward and I merge this PR.

Thanks, Martin

schoeberl commented 3 years ago

On 20 Apr, 2021, at 15:09, Hans Jakob Damsgaard @.***> wrote:

@hansemandse commented on this pull request.

In build.sbt https://github.com/schoeberl/chisel-book/pull/27#discussion_r616666719:

@@ -9,6 +9,6 @@ resolvers ++= Seq( )

// Chisel 3.4 -libraryDependencies += "edu.berkeley.cs" %% "chisel-iotesters" % "1.5.1" -libraryDependencies += "edu.berkeley.cs" %% "chiseltest" % "0.3.1"

+addCompilerPlugin("edu.berkeley.cs" % "chisel3-plugin" % "3.4.3" cross CrossVersion.full) It has to do with signal naming and should be included since Chisel3 3.4. At least that's what I get from here https://www.chisel-lang.org/chisel3/docs/explanations/naming.html.

Thanks for pointing me there. I got also an explanation today at the Chisel meeting. It is probably a good thing, but it adds another cryptic line to build.sbt. I don’t know if I should talk about this in the book.

Cheers, Martin