Closed NickShaffner closed 8 years ago
@cfelton I'm a bit confused as to why Clock.gen() can override hticks via a passed in parameter - ignoring the frequency and period. I see several of the tests seem to make use of this, but I don't understand why? It seems to me that the 'official' and only way to set hticks should be through the Clock() constructor. either via the frequency parameter, or perhaps an alternate optional hticks parameter that computes frequency. Would you be amiable to me doing such a refactoring?
I'm a bit confused as to why Clock.gen() can override hticks via a passed in parameter - ignoring the frequency and period.
In myhdl the simulation step is not tied to an absolute timescale (well there is in some cases, like trace signals). To determine what the tick/htick should be either need all the possibly clocks (to set them relative to each other), know that absolute time you want sim step to represent, or let the user set it.
In the code the absolute (1) and user (3) are currently supported but the absolute is currently tied to 1ns the user (modifying hticks) lets the user override this - cases where frequency greater than 500 MHz is desired.
When the user sets the htick, it doesn't change the frequency of the clock but rather the absolute time unit that a simulation step will represent.
Sorry this pull request keeps going :) I suppose I need to figure out how to create separate pull requests in github.
Because the build tools are not tested by travis-ci (FPGA tools not available) I will have to pull from your branch to my local computer and tests the FPGA board builds. Some of the Vivado build changes I don't understand, I want to make sure they work with multiple versions etc.
This might take me a couple days because I have MyHDL GSoC items I need to finish in this week.
@cfelton No problem or rush, and sorry that things are confusing. I've just been fixing things as I go along to get my basic flow working.
The changes were related to:
These changes would likely also benefit the other toolflows as well, but I only have vivado installed, so can only test it there.
I'll make the changes you requested and submit another changelist later today.
Apologies for cramming so much stuff into this one pull request. In future, I'll separate changes into branches so they are more topical.
Coverage remained the same at 70.364% when pulling 558a84fdf316a5fc39d4e411283cbe1eb9c88303 on NickShaffner:master into aae5e3f469f829f26f97387718d23a6d2c4315c1 on cfelton:master.