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

Switch from fork to start_soon #62

Closed
wants to merge 3 commits into from

Conversation

toddstrader
Copy link
Contributor

@toddstrader toddstrader commented Oct 18, 2023

Closes #61 I've s/fork/start_soon which resolves my particular problem, but I have no idea if this is generically correct. Also, I don't know how far back start_soon goes in cocotb, so clearly this would break some people until they also update cocotb.

@toddstrader
Copy link
Contributor Author

CI is mad because it's trying to use Python 3.6. I could update:

python-version: 3.6

but I'm going to take a beat to see if I'm pulling on a thread that requires more thought than I'm putting into it.

Also, I'm guessing this should be updated to whatever the correct supported versions of cocotb would be:

"cocotb>=1.5.0.dev,<2.0"

But I'm also not sure what to put there.

@marlonjames
Copy link
Contributor

But I'm also not sure what to put there.

cocotb is now 2.0.0.dev0, so:
cocotb>=2.0.0.dev

@marlonjames
Copy link
Contributor

For the attrs deprecation warnings on 3.6, we use this in the main cocotb repo:

https://github.com/cocotb/cocotb/blob/20266934f8fc3a15ea9b459981e3534704c28b0e/tests/Makefile#L28-L31

@toddstrader
Copy link
Contributor Author

But I'm also not sure what to put there.

cocotb is now 2.0.0.dev0, so: cocotb>=2.0.0.dev

Yeah, I was more uncertain about the starting point. Looks like start_soon() was added here:

commit ac465b968a241136dbd0cdc4509331ef49a61ea1
Author: Marlon James <marlon.james@garmin.com>
Date:   Sun Aug 15 13:54:57 2021 -0700

    Add start and start_soon scheduling functions (#2660)

So we can probably start at 1.6.0:
https://github.com/cocotb/cocotb/releases/tag/v1.6.0

@toddstrader
Copy link
Contributor Author

For the attrs deprecation warnings on 3.6, we use this in the main cocotb repo:

https://github.com/cocotb/cocotb/blob/20266934f8fc3a15ea9b459981e3534704c28b0e/tests/Makefile#L28-L31

Thanks, I can try this. But should these tests just run on a newer version of Python instead? Maybe I'll try that first.

1.6 adds start_soon()
Ignore attrs warning
@toddstrader toddstrader mentioned this pull request Oct 31, 2023
@jahagirdar
Copy link
Contributor

@toddstrader ,
Can you also add fix for

self._thread = cocotb.scheduler.add(self._send_thread())
in this PR.

@jahagirdar
Copy link
Contributor

Additional patches
scheduler.txt

@cmarqu
Copy link
Contributor

cmarqu commented Mar 1, 2024

#75 (once merged) makes CI work again, but it doesn't pass because of the issues fixed by this PR.

@toddstrader Would you mind updating this PR (resolve conflicts and add the changes that @jahagirdar mentioned), or are you okay with me doing it?

@toddstrader
Copy link
Contributor Author

#75 (once merged) makes CI work again, but it doesn't pass because of the issues fixed by this PR.

@toddstrader Would you mind updating this PR (resolve conflicts and add the changes that @jahagirdar mentioned), or are you okay with me doing it?

@cmarqu if you're willing please do. We abandoned ship on cocotb-bus since trying to keep cocotb and cocotb-bus coherent in this interstitial period between v1 and v2 is quite painful.

@Botnic
Copy link

Botnic commented Jul 17, 2024

Hi @cmarqu

What's the state of this PR?

@cmarqu
Copy link
Contributor

cmarqu commented Jul 17, 2024

What's the state of this PR?

Nothing new. I'll try to take a look at it soon.

@cmarqu cmarqu mentioned this pull request Jul 17, 2024
@cmarqu
Copy link
Contributor

cmarqu commented Jul 17, 2024

Replaced by #78

@cmarqu cmarqu closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forks have been removed in cocotb
5 participants