AxisStreamMonitor using **kwargs results in a TypeError

[rprechelt]

I was attempting to hookup an AxiStreamMonitor to a SystemVerilog interface.

Just specifying the name of the interface obviously doesn't work because the default value of bus_separator is _ so it formats the interface signals as iface_tready instead of the (correct) iface.tready. On L251 of axis.py, **kwargs passed to the AxisMonitor.__init__() method are passed on to the Bus constructor.

Passing bus_separator='.' into the AxisMonitor constructor appears to construct the bus correctly, however **kwargs is also passed onto the parent constructor on L258 which then fails because that constructor (for Reset) takes no arguments and therefore fails with TypeError: object.__init__() takes no parameters.

Using **kwargs in both of these contexts is a bug. Either **kwargs should be passed to the Bus constructor or it should be passed to the parent constructor but not both (especially since it causes the code to crash regardless of the keyword argument that is passed). Since the parent constructor takes no arguments, I would argue that the optimal behaviour would be to pass **kwargs on to the Bus constructor (especially since this allows hooking up to SV interfaces easily, in theory).

Let me know if I've missed something here! If not, I'm making to make the PR that implements whatever changes you think are best.

[alexforencich]

Yeah, that's true, that's definitely a problem. Unfortunately it's kinda unavoidable unless I either add all bus arguments explicitly to init or don't properly implement python multiple inheritance. Or manually filter things out of kwargs.

I'm planning on reworking how the bus object is handled anyway - not only is cocotb deprecating bus and moving it into a different repo, I also have a number of issues with how the current bus object works, especially in terms of making manual connections. I'm thinking about making my own bus replacement that's more flexible, and then passing an instance of that object in instead of entity+name. That way, you can configure any extra options separately. Something along these lines:

inst = AxiStreamSource(NewBus(dut, 'm_axis', options....), dut.clk, dut.rst, ....)

[elgorwi]

If I remember correctly, as a workaround passing “dut.interface” rather than “dut” as the first argument to AxiStreamSource() allows it to be used with SystemVerilog interfaces.

[alexforencich]

Hmm, that's a good point, I suppose in that case a "null" name needs to be supported with no separator added. Not sure if the current bus object supports that or not.

[elgorwi]

@rprechelt Just checked again and following works for a SystemVerilog interface defined in dut:

source = AxiStreamSource(dut.interface, None, dut.clk, dut.rst)

[rprechelt]

@elgorwi Thanks for the workaround for SystemVerilog interfaces - most appreciated!

The **kwargs stuff in the original issue is still a bug but if the entire Bus architecture and usage is currently being revamped (as per the first reply from Alex) then it doesn't seem pressing (now that this workaround exists).

Is this workaround something that could be (or is) documented somewhere in a FAQ?

[alexforencich]

I would not call it a workaround, I would call it the correct way of using an SV interface in cocotb. That certainly will be clean with the new bus setup, it will look like:

inst = AxiStreamMonitor(NewBus(dut.interface), dut.clk, dut.rst, ...)

In fact, what I can probably even do is cast that arg to a bus object, so if you're using SV interfaces you don't need to play with bus objects at all and instead can do:

inst = AxiStreamMonitor(dut.interface, dut.clk, dut.rst, ...)

which is super clean, with no extra None.

[alexforencich]

And the workaround, if you really want to have the prefix as a string, is to use getattr like so:

inst = AxiStreamMonitor(getattr(dut, 'interface'), dut.clk, dut.rst, ...)

[rprechelt]

Sounds perfect and looks super clean - excited to see it in a future release!

Thanks for your help!

[alexforencich]

In the latest release, I added new objects for handling these connections. Currently they are wrappers around Bus, but I'm planning on reworking that later to improve flexibility. With the new setup, you should be able to do:

inst = AxiStreamMonitor(AxiBus.from_entity(dut.interface), dut.clk, dut.rst, ...)

Eventually I will probably try to support passing in an object directly - basically, if the first parameter is not an instance of AxiBus, then assume it is an entity and pass it through AxiBus.from_entity(). But this isn't implemented yet.

[rprechelt]

Great - this is fantastic! Thanks for the update and look forward to your further changes.

I think this issue can be closed.