Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configureLink call not always getting correct defaultTimeBase #366

Open
feldergast opened this issue Dec 13, 2018 · 2 comments
Open

configureLink call not always getting correct defaultTimeBase #366

feldergast opened this issue Dec 13, 2018 · 2 comments
Assignees
Labels
Milestone

Comments

@feldergast
Copy link
Contributor

Passing NULL for the time base argument to configureLink indicates the (sub)component's current default time base should be used as the time base for the link. However, configureLink doesn't actually put in the current defaultTimeBase for the component and most often you end up with the first registered defaultTimeBase.

So, the following code does not work properly:

registerTimeBase("1ns",true)
Link* first_link = configureLink("first_port");
registerTimeBase("2ns",true)
Link* second_link = configureLink("second_port");

Both first_link and second_link end up with a defaultTimeBase of 1ns, but second_link should end up with a defaultTimeBase of 2ns.

@feldergast feldergast added the Bug label Dec 13, 2018
@feldergast feldergast self-assigned this Dec 13, 2018
@jjwilke
Copy link
Contributor

jjwilke commented Apr 5, 2019

@feldergast: Do we want to support this sort of implicit behavior? I don't love the idea of allowing registerTimeBase to be called multiple times for a component. I would actually vote for this type of code to be disallowed. There's a component-wide default time base OR you set the time base directly on the link.

The regAll parameter defaults to true. Wouldn't the expected behavior actually be that both links end up with a time base of 2ns? It looks like the function loops all links if regAll = true and sets the time base on all of them.

@allevin
Copy link
Contributor

allevin commented Mar 11, 2020

@feldergast to look at this, it may be fixed.

@allevin allevin added this to the Future milestone Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants