vunit
vunit copied to clipboard
VUnit
Support arbitrary waveform viewer
Closes #970
Haven't really had time to test this fully, so putting in draft. But since it seems like 5.0 is upcoming, this may be something to consider.
Basically, there is a --viewer argument that lets you select a wave form viewer. Also, all --gtkwave-* arguments are renamed --viewer-*, but with aliases.
(At least that is the idea once fully working.)
Motivated by https://surfer-project.org/ that is beginning to gain some traction. (Although possible to run in the browser there are also native versions.)
But since it seems like 5.0 is upcoming, this may be something to consider.
We cannot deprecate gtkwave arguments in the next release. We need to first release a version with the new feature, announcing that "old" aliases will be removed in a future version. Then, in a future release, we can remove the deprecated keywords/arguments. Therefore, this PR will not introduce breaking changes, and we can have it merged as soon as it is ready (no matter if it's 5.0 or 5.1).
Other than that, this looks very interesting. So, if it's ready by the time we want to tag 5.0, we can sure have it merged before.
The thing I am unsure how to handle are the option strings. Is there some previous example of aliasing those? But if the purpose is to deprecate in the long run, maybe they need to be handled explicitly anyway, so duplicated arguments and then check which has information.
@oscargus Previously, we've handled them explicitly. Here is an example: https://github.com/VUnit/vunit/blob/2c0b75e40aa59a42c51b5f6b2c40767f1b2b83d4/vunit/sim_if/ghdl.py#L255-L256
In your case it would be a warning rather than an error
Now I finally got around to finish this.
The news-item summarizes the changes quite good, but brief.
It seems like the emitted warnings are filtered, but I cannot really figure out how and where...
As a missing waveform viewer now raises after the simulation (which is not ideal, but I couldn't figure out another way to define the waveform viewer in the run-file), I had to remove one test. Let me know if this should be reverted or if there is another way to specify waveform viewer in the run-file.
It is of course possible to just limit the choice to surfer or gtkwave. That would simplify things a bit.
Now I have sorted things out (and passed the lint, wasn't easy with the long warning message...). I ended up factoring out the common viewer code to an OSSMixin-class that is used for both GHDL and NVC.
I'm a bit tempted to create a third class of options, maybe viewer_options as the viewer will be the same for/independent of ghdl and nvc it will also make sense to have common viewer, viewer_fmt, etc. This will also allow checking if it exists before actually trying to open the viewer (hence, keeping the previous behavior).
Taking the idea of being able to generate waves without opening the viewer, #1003 and #1042, one may also consider other naming.
All comments should be fixed. Sorry about that!
They are in a separate commit to make it easier to check, but I suggest to squash (I can squash if you do not have it enabled at GitHub).