Skip to content

Commit

Permalink
Removed acceptance testing issues with Riviera-PRO, Active-HDL and Mo…
Browse files Browse the repository at this point in the history 
…delsim.
  • Loading branch information
@LarsAsplund
LarsAsplund committed Nov 24, 2019
1 parent b13044a commit 45870be
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 6 deletions.
8 changes: 6 additions & 2 deletions tests/acceptance/test_artificial.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ class TestVunitArtificial(unittest.TestCase):
"""

def setUp(self):
# Spaces in path intentional to verify that it is supported
self.output_path = join(dirname(__file__), "artificial _out")
if simulator_is("activehdl"):
self.output_path = join(dirname(__file__), "artificial_out")
else:
# Spaces in path intentional to verify that it is supported
self.output_path = join(dirname(__file__), "artificial _out")

self.report_file = join(self.output_path, "xunit.xml")
self.artificial_run_vhdl = join(
dirname(__file__), "artificial", "vhdl", "run.py"
Expand Down
3 changes: 3 additions & 0 deletions tests/acceptance/test_external_run_scripts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ def test_vhdl_composite_generics_example_project(self):
],
)

@unittest.skipUnless(
simulator_is("ghdl"), "Support complex JSON strings as generic"

This comment has been minimized.

Sign in to view

Copy link
@eine

eine Nov 25, 2019

Collaborator

This test includes two examples: passing the JSON as a generic string or passing the path of a JSON file as a generic. While the former might fail, I think that the latter should work. Might be worth splitting the tests?

This comment has been minimized.

Sign in to view

Copy link
@felixn

felixn Nov 25, 2019

Contributor

I looked at making this testcase work with Modelsim, but failed miserably at trying to pass a double quote within a generic string. I tried all kinds of escaping, but nothing worked. I feel like this is not supported by Modelsim, but I haven't opened a support request.

Anyways, splitting the tests sounds good. Let me know if I should look into it, I don't want to cause double effort in case someone else is already looking into it.

This comment has been minimized.

Sign in to view

Copy link
@LarsAsplund

LarsAsplund Nov 25, 2019

Author Collaborator

Yes, this was a bit of a quickfix to make tests go green such that we can make a release and then improve later. If we just test/exemplify that a JSON file can be loaded we're not verifying the VUnit integration, only that the package works on its own and that's not really our responsibility. I think we need to get some details from the vendors on the restrictions they have on the string generics (@felixn maybe you can do that if you have access to their support. I can ask Aldec). It's always a trial and error. If they can't support complex JSON strings we need to add support for another level of "safe" encoding.

We can also make a simpler example. With Riviera-PRO I've successfully used JSON integer lists.

This comment has been minimized.

Sign in to view

Copy link
@eine

eine Nov 26, 2019

Collaborator

If we just test/exemplify that a JSON file can be loaded we're not verifying the VUnit integration, only that the package works on its own and that's not really our responsibility.

I beg to disagree. The JSON file can be generated in the run.py file with parameters that are also used in other VUnit features. From VUnit's point of view, the only difference between both approaches is using json.dump or vu.set_generic. Hence, it is useful to have a test that shows how VUnit can be used with JSON-for-VHDL, even on simulators with poor generics support. Otherwise, we are implicitly saying that the feature is not supported at all, which is not the case.

We can also make a simpler example. With Riviera-PRO I've successfully used JSON integer lists.

I updated #588 to follow this discussion.

Anyways, splitting the tests sounds good. Let me know if I should look into it, I don't want to cause double effort in case someone else is already looking into it.

I don't think anyone is working on it.

This comment has been minimized.

Sign in to view

Copy link
@LarsAsplund

LarsAsplund Nov 26, 2019

Author Collaborator

I beg to disagree...

You have a point. I've always seen the integration of JSON4VHDL as a way to enable complex generics.

If we can't find a way to pass plain JSON as generics we need to provide some other safe encoding that can easily be converted back to normal JSON with a to_json function implemented in VHDL. Maybe the easiest way forward, Trying to get things solved at the vendor side will only slow us down and end up in special solutions for each vendor

This comment has been minimized.

Sign in to view

Copy link
@eine

eine Nov 26, 2019

Collaborator

Good point. That safe encoding might be base64. It is quite common in web environments and Python provides a package to handle it. Precisely, it is used in https://github.com/VUnit/vunit/pull/568/files#diff-c9290a21e8aaf78614c59a6a9a178576. base64 is also mentioned in many references regarding IP encryption. Unfortunately, I am not aware of any open source VHDL library that allows base64 decoding.
)
def test_vhdl_json4vhdl_example_project(self):
self.check(join(ROOT, "examples", "vhdl", "json4vhdl", "run.py"))

Expand Down
2 changes: 1 addition & 1 deletion vunit/vhdl/data_types/src/string_ptr_pkg-body-93.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ package body string_ptr_pkg is

procedure reallocate (
ptr : ptr_t;
value : string
value : vec_t
) is
variable s : storage_t := st.idxs(ptr.ref);
variable n_value : string(1 to value'length) := value;
Expand Down
3 changes: 1 addition & 2 deletions vunit/vhdl/data_types/src/types.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ package types_pkg is
type integer_vector_access_vector_t is array (natural range <>) of integer_vector_access_t;
type integer_vector_access_vector_access_t is access integer_vector_access_vector_t;

type extintvec_access_t is access integer_vector_t(0 to integer'high-1);
type extintvec_access_t is access integer_vector_t(0 to integer'high / 2 - 1);
type extintvec_access_vector_t is array (natural range <>) of extintvec_access_t;
type extintvec_access_vector_access_t is access extintvec_access_vector_t;
end package;

2 changes: 1 addition & 1 deletion vunit/vhdl/logging/test/tb_deprecated.vhd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ begin
mock(default_logger);
verbose("hello", 17, "foo.vhd");
check_log(default_logger, "Mapping deprecated procedure verbose to trace", warning);
check_log(default_logger, "hello", verbose, 0 ns, 17, "foo.vhd");
check_log(default_logger, "hello", trace, 0 ns, 17, "foo.vhd");
unmock(default_logger);

end if;
Expand Down

0 comments on commit 45870be

Please sign in to comment.