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

Remove deprecated cocotb functionality #83

Merged
merged 10 commits into from
Sep 10, 2024

Conversation

p12tic
Copy link
Contributor

@p12tic p12tic commented Sep 5, 2024

This PR removes uses of a few things that have been deprecated or removed in the upcoming cocotb 2.0 release.

The changes either do not change the API of cocotb-bus or change it in a way that few, if any users will be affected. The commit messages have more detailed explanations of rationale where the lack of impact wasn't obvious.

I hope that it will be possible to have a single code base seamlessly supporting both pre-2.0 cocotb and post-2.0 cocotb. This should significantly reduce the pain of migration to cocotb-bus users as it's one less thing to worry about.

@p12tic p12tic force-pushed the remove-deprecated-cocotb branch from 5c890f3 to a300d65 Compare September 5, 2024 17:24
@cmarqu
Copy link
Contributor

cmarqu commented Sep 5, 2024

Thanks for working on this, it will also be a good example helping users migrate their own code to 2.0.
I suppose we should also extend CI to run against cocotb master or a recent sha then?

@p12tic p12tic force-pushed the remove-deprecated-cocotb branch from 1d6330a to f51af89 Compare September 6, 2024 02:59
@p12tic
Copy link
Contributor Author

p12tic commented Sep 6, 2024

I suppose we should also extend CI to run against cocotb master or a recent sha then?

Agreed. However the more nasty changes have not been included into this PR yet. This is all uncontroversial stuff for now. The tests still fail.

The most significant change is removal of BinaryValue without seamless migration to LogicArray. A PR with this migration will come later.

cocotb.log.SimLog has been removed from upcoming cocotb 2.0 release.
cocotb.result.TestFailure has been removed from upcoming cocotb 2.0
release.
int(handle) have been deprecated in cocotb 2.0.
cocotb.result.TestError has been removed from upcoming cocotb 2.0
release.
@p12tic p12tic force-pushed the remove-deprecated-cocotb branch 2 times, most recently from d03583f to 295f3d4 Compare September 6, 2024 09:53
@cmarqu
Copy link
Contributor

cmarqu commented Sep 6, 2024

@p12tic By the way, there is this apparently nice tool libCST which could be set up to modernize a codebase (see https://libcst.readthedocs.io/en/latest/codemods_tutorial.html).
Just in case you like to play :)

@p12tic
Copy link
Contributor Author

p12tic commented Sep 6, 2024

Just in case you like to play :)

If only I had time. The migration from BinaryValue to LogicVector seems to require non-obvious manual intervention in a lot of places.

By the way, I have a further enhancements of cocotb-bus that complete support of cocotb v2.0 with tests and everything. It depends on this PR being merged first.

@p12tic p12tic force-pushed the remove-deprecated-cocotb branch from 295f3d4 to 3a8248f Compare September 6, 2024 21:09
@cmarqu cmarqu requested a review from ktbarrett September 7, 2024 19:58
tests/test_cases/test_axi4/test_axi4.py Show resolved Hide resolved
src/cocotb_bus/compat.py Outdated Show resolved Hide resolved
cocotb.triggers.Combine still doesn't support coroutines being passed to
it. Accordingly, on pre v2.0 versions coroutine decorator still must be
used.
TestFactory has been deprecated in upcoming cocotb v2.0 without a
migration path in v1.x series.
Upcoming cocotb 2.0 will raise a warning and suggest that an explicit
comparison to string is used:

cocotb/cocotb#4143

Explicit conversion to str is no longer needed as of cocotb 1.9.0. Once
this is the oldest supported version of cocotb the casts could be
dropped.
cocotb.wavedrom has been removed in upcoming cocotb 2.0 release.
On upcoming cocotb 2.0 LogicObject has less implicit conversions and
accordingly the tests should explicitly acquire its value
The .data field of cocotb.triggers.Event has been removed in cocotb 2.0.
This is a part of the API exposed by cocotb-bus because Monitor receives
event from external source. It is probably best to keep the current API
until support for cocotb 1.x has been removed from cocotb-bus.

Accordingly, the simpliest solution for this problem is to detect cocotb
2.0 version and re-add the .data field when event.set is being called on
cocotb 2.0.
@p12tic p12tic force-pushed the remove-deprecated-cocotb branch from 3a8248f to 25ef08c Compare September 8, 2024 07:24
@p12tic
Copy link
Contributor Author

p12tic commented Sep 10, 2024

@ktbarrett Is there anything else left in this PR? Thanks!

@ktbarrett
Copy link
Member

I suppose not. Thanks!

@ktbarrett ktbarrett merged commit b99f1fa into cocotb:master Sep 10, 2024
5 checks passed
@p12tic p12tic deleted the remove-deprecated-cocotb branch September 14, 2024 19:23
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.

3 participants