Update stream pkg

[bradleyharden]

See the first commit message for the major changes.

I haven't added any tests yet. But to be fair, the stream package never had any tests to begin with.

[bradleyharden]

As I mentioned on Gitter, combining the stream_master and stream_slave packages broke the documentation. Personally, I think it makes more sense for the entire VCI to be defined in one package, but I can split it up again, if requested

[bradleyharden]

I think the Python 2.7 AppVeyor GHDL version is out of date. I had to get some GHDL bugs fixed for this package to work, so GHDL needs to be up to date.

[kraigher]

I do not mind merging the features into one packages as long as the documentation is updated accordingly. It would be easier to review your changes if the merge into a single package was first done without any changes and then the changes could be reviewed separately on top of the single packege. Now it just look like a new file was added and the old ones removed.

[bradleyharden]

I rewrote the history to better show the changes. Commit 2afaa7a is the most significant one. That commit introduces all major changes.

[kraigher]

I am ok with the changes, I like that there is encapsulation of push and pop and that a record is used to protect from protocol mismatch in VC when new fields are added in the future.

It just needs to pass Travis CI and the documentation needs to be updated.

[bradleyharden]

Yes, I plan to add tests. I just wanted to get some feedback before continuing. I'll mark it as a draft to make that clear.

[bradleyharden]

Well, never mind. It looks like you can't go from ready back to draft. At least not yet. I'll work on finishing this soon.

[LarsAsplund]

I think you should provide as_sync functions to support that design pattern as well.

[bradleyharden]

What is the purpose of sync_handle_t? Right now it's just an alias for actor_t. Is it supposed to be a hook for future modifications? Right now, an actor function would be sufficient for sync_pkg as well, but I guess it's not quite as future proof. Is that the motivation?

[LarsAsplund]

Yes, that is the motivation. We try to make our public interface somewhat future-proof to avoid breaking backwards compatibility.

[LarsAsplund]

I think you should provide helper function to get the actor form the handles such that all com functionality becomes available while not accessing private fields of the handles

[bradleyharden]

Yes, I agree. After our conversation in Gitter, I realized that it's a much more general solution than overloading, even if it's not quite as clean when writing code.

[LarsAsplund]

I've started to think about best practices for VC/VCIs and I think they will include:

• Allow the user to specify actor. This allows for greater control when integrating the VC
• Allow the user to specify a logger and create a local checker based on that. Again it's about greater control for the integrator

Have a look at AXI stream for examples

[bradleyharden]

Good idea about a logger. I'll add access to both in this interface.

[LarsAsplund]

Use the local logger as the second parameter, see comment below

[bradleyharden]

I haven't forgotten about this. There are two issues.

One, I realized that I frequently (mis?)use stream_pkg as a way to push std_ulogic_vectors around without having to write or copy/paste a bunch of procedures. That actually covers a lot of use cases for me. However, the stream VCI ought to represent a generalized stream interface, not just be a collection of convenience procedures. I think my changes were more aimed at my own use cases. Maybe I can think of a new VC package that would cover my needs in that regard.

Two, I want to see the outcome of #520 before I update further.

[umarcor]

One, I realized that I frequently (mis?)use stream_pkg as a way to push std_ulogic_vectors around without having to write or copy/paste a bunch of procedures.

Just because of this reason, I've been thinking about FIFO VCs. I think this is a quite common use case and I'd love to have some kind of quick reference describing the alternatives provided by VUnit. E.g.:

• integer_vector along with to_unsigned/to_signed and to_integer. If a FIFO/LIFO buffer is desired, the user needs to handle the indices explicitly,
• queue along with to_unsigned/to_signed and to_integer. It behaves as a FIFO per se. I don't know if it can be used as a LIFO.
• (hypothetical) FIFO VC. Conversions to/from std_logic_vector and integer are handled, so the user does not need to do it. It could include some 'metadata', such as a variable containing the used capacity, or the percentage of clock cycles that it has been full/empty. This can be useful to do statistical analysis of the required FIFO sizes in a given design, provided that the application/context do not allow to infer then analytically.
• AXI Stream VC. It is quite similar to FIFO VC, plus multiple (optional) additional signals, a monitor and protocol checker.

Note that:

• Both FIFO VC and AXI Stream VC are expected to be built on top of Stream VCI.
• Stream VCI is built on top of queue.
• queue is built on top of integer_vector.

@bradleyharden, @LarsAsplund, am I forgetting/misunderstanding anything here? Does the FIFO VC make sense to you as a sweet spot between users (mis?)using stream_pkg and 'being forced' to use AXI Stream?

That actually covers a lot of use cases for me. However, the stream VCI ought to represent a generalized stream interface, not just be a collection of convenience procedures. I think my changes were more aimed at my own use cases. Maybe I can think of a new VC package that would cover my needs in that regard.

If it sounds good to you, I'm all in with creating a FIFO VC.

Two, I want to see the outcome of #520 before I update further.

Agree.