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.

Sign up for GitHub

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

Jump to bottom

nvc: Waveform files are only generated with --gui option #1099

Open
tmeissner opened this issue Feb 25, 2025 · 3 comments · May be fixed by #1101
Open

nvc: Waveform files are only generated with --gui option #1099

tmeissner opened this issue Feb 25, 2025 · 3 comments · May be fixed by #1101

Comments

@tmeissner
Copy link
Contributor

It seems that when using nvc as simulator, the waveforms set with --viewer-fmt format are only generated when using the --gui option of the Python runner. With ghdl, this isn't the case, waveforms are generated regardless of the --gui option.

I think, the problem is in nvc.py simulate() method:

First, wave_file is only set to a value if _gui is set:

vunit/vunit/sim_if/nvc.py

Lines 260 to 265 in acf7e7f

if self._gui:
wave_file = script_path / (f"{config.entity_name}.{self._viewer_fmt or 'fst'}")
if wave_file.exists():
remove(wave_file)
else:
wave_file = None

Later if wave_file is set, the nvc --wave is added to the command line:

vunit/vunit/sim_if/nvc.py

Lines 295 to 296 in acf7e7f

if wave_file:
cmd += [f"--wave={wave_file}"]

In ghdl.py, creation of the waveform export command line option is only dependent on _viewer_fmt, but not on _gui:

vunit/vunit/sim_if/ghdl.py

Lines 368 to 375 in acf7e7f

if self._viewer_fmt is not None:
data_file_name = str(Path(script_path) / f"wave.{self._viewer_fmt!s}")
if Path(data_file_name).exists():
remove(data_file_name)
else:
data_file_name = None
cmd = self._get_command(config, script_path, elaborate_only, ghdl_e, test_suite_name, data_file_name)

vunit/vunit/sim_if/ghdl.py

Lines 327 to 333 in acf7e7f

if wave_file:
if self._viewer_fmt == "ghw":
sim += [f"--wave={wave_file!s}"]
elif self._viewer_fmt == "vcd":
sim += [f"--vcd={wave_file!s}"]
elif self._viewer_fmt == "fst":
sim += [f"--fst={wave_file!s}"]

@LarsAsplund
Copy link
Collaborator

@oscargus Do you have any comments on this?

@oscargus
Copy link
Contributor

oscargus commented Mar 4, 2025

There is #1042 which solves it. (There is also a bit of discussions/rambling in #1003 about the need for it.)

My thought has been to add a command to generate the waveforms in a more "non-surprising" way than specifying --viewer-fmt, but got wound up in what to call it...

--waveform(s) or --wave(s), but then one should probably have called --viewer-fmt --wave-fmt or something. And there I got stuck as I had just renamed it once... The idea is, to clarify, to specify that waveforms should be generated (but the gui should not be opened, so calling it something related to viewer seems confusing).

Coming back to it, I guess it can make sense to have a flag like that (--wave*) and then add aliases for --viewer-fmt that matches the selected name.

If you agree @LarsAsplund , and can make an executive decision on the naming, I should be able to provide a PR within a week or so.

@oscargus oscargus linked a pull request Mar 5, 2025 that will close this issue
Open
@oscargus
Copy link
Contributor

oscargus commented Mar 5, 2025

I added #1101 which unifies things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add --waves argument to generate waveforms for GHDL and NVC oscargus/vunit
3 participants
@tmeissner @LarsAsplund @oscargus