Discussion: plus_args as a dict rather than a list

[manunited10]

I have recently started working with cocotb-test, trying to get away from the makefile method. It's amazing!

plus_args:

My understanding of plus_args usage (PLUSARGS in cocotb) is a way to send python parameters to python test - at least that's how I do it through makefile (e.g. to send x=1, y=10, etc to python). These values are accessible in python under cocotb.plusargs variable.

If what I think is true, shouldn't plus_args in cocotb-test be a dict (rather than a list) like below?

plus_args = {'x': 1, 'y': 10,}

And that means
in simulator.py under Class Questa(Simulator) we may need another method like

    def get_plus_args_commands(self, parameters): 
        return [f"+{name}={str(value)}" for name, value in parameters.items()]

which is basically (almost) identical to get_parameter_commands.
and then later we use this method like

if not self.compile_only:
    if self.toplevel_lang == "vhdl":
        cmd.append(
              ...
            + [as_tcl_value(v) for v in self.get_plus_args_commands(self.plus_args)]

Thank you in advance.

p.s. I'm using cocotb 1.8.1, cocotb-test 0.2.5 and python 3.10.14

[themperek]

You can do plus_args = {'x=1', 'y=10'} but I agree it is not intuitive as it is.
I am open to PR for this but I would like to keep compatibility with the list maybe with deprecation warning.

[manunited10]

Thank you for the quick response.
I'm not sure that would work at least on questa.
For starters, self.plus_args is only being used on the else where it is compile_only=True toplevel_lang != "vhdl" (I am using vhdl).
On top of that, I think the generated vsim command should be something like

vsim ... +x=1 +y=10

which means the script should add + to each of the args.

Please take a look at my local diff (which works for vhdl):

diff --git a/cocotb_test/simulator.py b/cocotb_test/simulator.py
index acf6f7d..6cb71f8 100644
--- a/cocotb_test/simulator.py
+++ b/cocotb_test/simulator.py
@@ -148,7 +148,7 @@ class Simulator:
         self.simulation_args = sim_args + extra_args

         if plus_args is None:
-            plus_args = []
+            plus_args = {}

         self.plus_args = plus_args
         self.force_compile = force_compile
@@ -541,6 +541,9 @@ class Questa(Simulator):
     def get_parameter_commands(self, parameters):
         return [f"-g{name}={str(value)}" for name, value in parameters.items()]

+    def get_plus_args_commands(self, parameters):
+        return [f"+{name}={str(value)}" for name, value in parameters.items()]
+
     def do_script(self):
         do_script = ""
         if self.waves:
@@ -599,6 +602,7 @@ class Questa(Simulator):
                     + self.simulation_args
                     + [as_tcl_value(v) for v in self.get_parameter_commands(self.parameters)]
                     + self.toplevel
+                    + [as_tcl_value(v) for v in self.get_plus_args_commands(self.plus_args)]
                     + ["-do", do_script]
                 )
                 if self.verilog_sources:

[themperek]

I will try to change this but not sure when.

[manunited10]

@themperek any chance you could follow up on this? TIA.

[themperek]

I am happy to accept a PR on this. It should be backwards compatible and include some basic test.