VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
722 stars 261 forks source link

Feature Request: Support set_sim_option name 'gtkwave.init_file.gui' in GTKWave similar to ModelSim, Active-HDL, and Riviera PRO #458

Open GlenNicholls opened 5 years ago

GlenNicholls commented 5 years ago

ModelSim, Active-HDL, and Riviera PRO all have the '<simulator_name>.init_file.gui' name option for set_sim_option(). However, the only way to accomplish this with GTKWave is to do python run.py -g --gtkwave-args=-S<script_name>.tcl and this is somewhat annoying to do everytime I want to run the same script/scripts everytime I open the GUI. I am looking to configure the waveform viewer with some of my scripts and this gets very annoying after doing it a few times. Can support be added to define the scripts to be run from run.py similar to the other supported simulators?

kraigher commented 5 years ago

Yes it makes sense to add it. If you want to make a PR just have a look in ghdl_interface.py and you can se how other sim options are handled there. It should be very straight forward to add this.

GlenNicholls commented 5 years ago

Okay cool, I will dig into the code here in the next couple weeks when I'm back from travel unless some time opens up this week.

GlenNicholls commented 5 years ago

@kraigher I have all of the code written and I'm working on testing it now. With this, it looks like it works when there are no spaces in the file path.

When the VUnit run.py has no spaces in the path name, i.e. ui.set_sim_option('ghdl.init_file.gui', 'C:/tmp/gtkAddWave.tcl'), everything looks fine. However, if I try to use the script in the run.py directory that has a space in the path name, it all blows up (ui.set_sim_option('ghdl.init_file.gui', 'gtkAddWave.tcl')):

$ python run.py "axi_dma_lib.tb_axi_dma_regs.Test destination address" -g -v
Re-compile not needed

Running test: axi_dma_lib.tb_axi_dma_regs.Test destination address
Running 1 tests

Starting axi_dma_lib.tb_axi_dma_regs.Test destination address
Output file: vunit_out\test_output\axi_dma_lib.tb_axi_dma_regs.Test_destinadb89a2c4a29e6dfdee0f89140e6749550048c785\output.txt
simulation stopped @105ns with status 0
init file: gtkAddWave.tcl
script path: C:\Users\Glen Nicholls\Desktop\vunit\examples\vhdl\axi_dma\vunit_out\test_output\axi_dma_lib.tb_axi_dma_regs.Test_destinadb89a2c4a29e6dfdee0f89140e6749550048c785\ghdl
gtkwave -SC:\Users\Glen Nicholls\Desktop\vunit\examples\vhdl\axi_dma\gtkAddWave.tcl C:\Users\Glen Nicholls\Desktop\vunit\examples\vhdl\axi_dma\vunit_out\test_output\axi_dma_lib.tb_axi_dma_regs.Test_destinadb89a2c4a29e6dfdee0f89140e6749550048c785\ghdl\wave.ghw

GTKWave Analyzer v3.3.98 (w)1999-2019 BSI

[0] start time.
[104500000] end time.
GTKWAVE | Executing Tcl script 'C:\Users\Glen Nicholls\Desktop\vunit\examples\vhdl\axi_dma\gtkAddWave.tcl'
GTKWAVE | wrong # args: should be "source ?-encoding name? fileName"

The bizarre issue to me is that GTK runs the provided wave file with no issues when theres a space in the path name, but only bombs when using the -S or --script flag. My best guess is this is something to do with TCL sourcing this file which seems to align with what this link shows for the source test.

The below is the code in ghdl_interface.py that produces this without the print statements I'm using to debug:

sim_options = [
        ListOfStringOption("ghdl.sim_flags"),
        ListOfStringOption("ghdl.elab_flags"),
        StringOption("ghdl.init_file.gui"),
    ]
...

def simulate(self,  # pylint: disable=too-many-locals
                 output_path,
                 test_suite_name,
                 config, elaborate_only):
        """
        Simulate with entity as top level using generics
        """

        script_path = join(output_path, self.name)

        if not exists(script_path):
            os.makedirs(script_path)

        cmd = self._get_sim_command(config, script_path)

        if elaborate_only:
            cmd += ["--no-run"]

        if self._gtkwave_fmt is not None:
            data_file_name = join(script_path, "wave.%s" % self._gtkwave_fmt)

            if exists(data_file_name):
                os.remove(data_file_name)

            if self._gtkwave_fmt == "ghw":
                cmd += ['--wave=%s' % data_file_name]
            elif self._gtkwave_fmt == "vcd":
                cmd += ['--vcd=%s' % data_file_name]

        else:
            data_file_name = None

        status = True
        try:
            proc = Process(cmd)
            proc.consume_output()
        except Process.NonZeroExitCode:
            status = False

        if self._gui and not elaborate_only:
            cmd = ["gtkwave"]

            init_file = config.sim_options.get(self.name + ".init_file.gui", None)
            if init_file is not None:
                cmd += ["-S%s" % abspath(init_file)]#.replace(' ', '\ ')] 

            cmd += shlex.split(self._gtkwave_args) + [data_file_name]

            stdout.write("%s\n" % " ".join(cmd))
            subprocess.call(cmd)

        return status

Do you know how to work around this issue?

GlenNicholls commented 5 years ago

It looks like if I make a new .tcl script with this:

source "C:/Users/Glen Nicholls/Desktop/vunit/examples/vhdl/axi_dma/gtkAddWave.tcl"

And run with this: python run.py "axi_dma_lib.tb_axi_dma_regs.Test destination address" -g --gtkwave-args=-SgtkSource.tcl -v

That gets around that issue. It seems like the -S flag in GTKWave doesn't expect the possibility of a pathname with a space in it. I'm trying to think of a workaround, but I'm not sure how to fix this. I sent a message to Joel, so hopefully, this is something they will add support for.

kraigher commented 5 years ago

Try ["-S", abspath(init_file)] instead of ["-S%s" % abspath(init_file)] and see if it makes a difference.

GlenNicholls commented 5 years ago

That didn't work, but I was able to get this to add it correctly cmd += ["--script", "\"{}\"".format(abspath(init_file))]

GlenNicholls commented 5 years ago

Just committed the changes after testing in 364f4ae, creating a PR now. One issue with GTKWave's TCL interface is it doesn't allow you to add multiple files as a list with --script and --tcl_init causes the --wish to be inferred. Because of this, if the ability to add multiple TCL files is desired, VUnit will need to generate a seperate TCL script that is passed to GTKWave that sources all of these. It seemed like a headache since my use case is simple and I found a way to work around that since I can just put a function to get the git root of my repo and have my wave scripts point to the files I need to add in my repo.

eine commented 5 years ago

@GlenNicholls, should this issue be closed now that #459 is merged?