From 2a276beb0d9a1e3f6ef5375c0611e6b0e6ad8258 Mon Sep 17 00:00:00 2001 From: Oscar Gustafsson Date: Sun, 18 Aug 2024 09:48:44 +0200 Subject: [PATCH] Fix review comments --- docs/news.d/1002.feature.rst | 12 ++++++------ vunit/sim_if/{_ossmixin.py => _viewermixin.py} | 5 ++--- vunit/sim_if/ghdl.py | 16 +++++++--------- vunit/sim_if/nvc.py | 14 ++++++-------- 4 files changed, 21 insertions(+), 26 deletions(-) rename vunit/sim_if/{_ossmixin.py => _viewermixin.py} (96%) diff --git a/docs/news.d/1002.feature.rst b/docs/news.d/1002.feature.rst index 17579d2a8..9032ad539 100644 --- a/docs/news.d/1002.feature.rst +++ b/docs/news.d/1002.feature.rst @@ -1,11 +1,11 @@ -[GHDL/NVC] Arbitrary waveform viewers are now supported by passing the `--viewer` -command line arugment. As a consequence, ``ghdl.gtkwave_script.gui`` and +[GHDL/NVC] Arbitrary waveform viewers are now supported by passing the ``--viewer`` +command line argument. As a consequence, ``ghdl.gtkwave_script.gui`` and ``nvc.gtkwave_script.gui`` are deprecated in favour of ``ghdl.viewer_script.gui`` and ``nvc.viewer_script.gui``, respectively. The ``--gtkwave-args`` and -``--gtkwave-fmt`` command line argument is deprecated in favour of ``--viewer-args`` +``--gtkwave-fmt`` command line arguments are deprecated in favour of ``--viewer-args`` and ``--viewer-fmt``, respectively. ``ghdl.viewer.gui`` and ``nvc.viewer.gui`` can -be used to set the preferred viewer from the run-file. Finally, if the requested -viewer does not exists, ``gtkwave`` or ``surfer`` is used, in that order. This -also means that VUnit uses ``surfer`` if ``gtkwave`` is not installed. +be used to set the preferred viewer from the run-file. If no viewer is explicitly +requested, ``gtkwave`` or ``surfer`` is used, in that order. This also means that +VUnit now uses ``surfer`` if ``gtkwave`` is not installed. [NVC] It is possible to get VCD waveform files by passing ``--viewer-fmt=vcd``. \ No newline at end of file diff --git a/vunit/sim_if/_ossmixin.py b/vunit/sim_if/_viewermixin.py similarity index 96% rename from vunit/sim_if/_ossmixin.py rename to vunit/sim_if/_viewermixin.py index 90fe8f181..fa0d50a21 100644 --- a/vunit/sim_if/_ossmixin.py +++ b/vunit/sim_if/_viewermixin.py @@ -8,7 +8,7 @@ """ -class OSSMixin: +class ViewerMixin: """ Mixin class for handling common viewer functionality for the GHDL and NVC simulators. """ @@ -35,8 +35,7 @@ def _get_viewer(self, config): """ Determine the waveform viewer to use. - Falls back to gtkwave or surfer, in that order, if none is provided or the provided - cannot be found. + Falls back to gtkwave or surfer, in that order, if none is provided. """ viewer = self._viewer or config.sim_options.get(self.name + ".viewer.gui", None) diff --git a/vunit/sim_if/ghdl.py b/vunit/sim_if/ghdl.py index 92788cc15..6a6b3ca7d 100644 --- a/vunit/sim_if/ghdl.py +++ b/vunit/sim_if/ghdl.py @@ -21,12 +21,12 @@ from ..ostools import Process from . import SimulatorInterface, ListOfStringOption, StringOption, BooleanOption from ..vhdl_standard import VHDL -from ._ossmixin import OSSMixin +from ._viewermixin import ViewerMixin LOGGER = logging.getLogger(__name__) -class GHDLInterface(SimulatorInterface, OSSMixin): # pylint: disable=too-many-instance-attributes +class GHDLInterface(SimulatorInterface, ViewerMixin): # pylint: disable=too-many-instance-attributes """ Interface for GHDL simulator """ @@ -100,7 +100,7 @@ def __init__( # pylint: disable=too-many-arguments backend="llvm", ): SimulatorInterface.__init__(self, output_path, gui) - OSSMixin.__init__( + ViewerMixin.__init__( self, gui=gui, viewer=viewer, @@ -132,7 +132,7 @@ def _get_version_output(cls, prefix): @classmethod def _get_help_output(cls, prefix): """ - Get the output of 'ghdl --version' + Get the output of 'ghdl --help' """ return subprocess.check_output([str(Path(prefix) / cls.executable), "--help"]).decode() @@ -392,11 +392,9 @@ def simulate(self, output_path, test_suite_name, config, elaborate_only): # pyl status = False if config.sim_options.get(self.name + ".gtkwave_script.gui", None): - warn_str = ( - "%s.gtkwave_script.gui is deprecated and will be removed " % self.name # pylint: disable=C0209 - + "in a future version, use %s.viewer_script.gui instead" % self.name # pylint: disable=C0209 - ) - LOGGER.warning(warn_str) + LOGGER.warning("%s.gtkwave_script.gui is deprecated and will be removed " + "in a future version, use %s.viewer_script.gui instead", + self.name, self.name) if self._gui and not elaborate_only: cmd = [self._get_viewer(config)] + shlex.split(self._viewer_args) + [data_file_name] diff --git a/vunit/sim_if/nvc.py b/vunit/sim_if/nvc.py index 1d84451fc..082c720e1 100644 --- a/vunit/sim_if/nvc.py +++ b/vunit/sim_if/nvc.py @@ -20,13 +20,13 @@ from ..ostools import Process from . import SimulatorInterface, ListOfStringOption, StringOption from . import run_command -from ._ossmixin import OSSMixin +from ._viewermixin import ViewerMixin from ..vhdl_standard import VHDL LOGGER = logging.getLogger(__name__) -class NVCInterface(SimulatorInterface, OSSMixin): # pylint: disable=too-many-instance-attributes +class NVCInterface(SimulatorInterface, ViewerMixin): # pylint: disable=too-many-instance-attributes """ Interface for NVC simulator """ @@ -80,7 +80,7 @@ def __init__( # pylint: disable=too-many-arguments if viewer_fmt == "ghw": LOGGER.warning("NVC does not support ghw, defaulting to fst") viewer_fmt = None # Defaults to FST later - OSSMixin.__init__(self, gui=gui, viewer=viewer, viewer_fmt=viewer_fmt, viewer_args=viewer_args) + ViewerMixin.__init__(self, gui=gui, viewer=viewer, viewer_fmt=viewer_fmt, viewer_args=viewer_args) self._prefix = prefix self._project = None @@ -305,11 +305,9 @@ def simulate( status = False if config.sim_options.get(self.name + ".gtkwave_script.gui", None): - warn_str = ( - "%s.gtkwave_script.gui is deprecated and will be removed " % self.name # pylint: disable=C0209 - + "in a future version, use %s.viewer_script.gui instead" % self.name # pylint: disable=C0209 - ) - LOGGER.warning(warn_str) + LOGGER.warning("%s.gtkwave_script.gui is deprecated and will be removed " + "in a future version, use %s.viewer_script.gui instead", + self.name, self.name) if self._gui and not elaborate_only: cmd = [self._get_viewer(config)] + shlex.split(self._viewer_args) + [str(wave_file)]