This is not as straight-forward as the last. We have equipment that generates vcd traces from a 150MHz clock, and vcdcat can't parse it because of the odd timescale represented as 6666ps.
I feel OK suggesting removing that guard for a few reasons:
There's no code (left?) that actually uses the value of the timescale in such a way that odd numbers could upset some maths
The timescale vcd representation seems to have the same expressive power as Decimal() used in the library so it doesn't look like there's any reason the values will go wrong (eg as they might with float conversions)
its the only exit() call in the library portion - ie at the very least that exit-code should be replaced with an exception to allow library users to choose a different response
My only concern is that it removes all checking of the value of magnitude - the only downside I can think of is that if it might not properly discard the magnitude of 0, or accidentally accept a failure to parse as a 0 rather than rejecting it outright.
I added some representative (proprietaryness removed) vcd to the tests to match, but realised most of the metadata isn't parsed so added it simply in case future versions want to parse the $version, $date or whatever, they'll accept what we produce.
cirosantilli
commented
10 months ago
This is not as straight-forward as the last. We have equipment that generates vcd traces from a 150MHz clock, and
vcdcatcan't parse it because of the odd timescale represented as 6666ps.I feel OK suggesting removing that guard for a few reasons:
timescalein such a way that odd numbers could upset some mathstimescalevcd representation seems to have the same expressive power asDecimal()used in the library so it doesn't look like there's any reason the values will go wrong (eg as they might with float conversions)exit()call in the library portion - ie at the very least that exit-code should be replaced with an exception to allow library users to choose a different responseMy only concern is that it removes all checking of the value of
magnitude- the only downside I can think of is that if it might not properly discard themagnitudeof0, or accidentally accept a failure to parse as a0rather than rejecting it outright.I added some representative (proprietaryness removed) vcd to the tests to match, but realised most of the metadata isn't parsed so added it simply in case future versions want to parse the
$version,$dateor whatever, they'll accept what we produce.