-
Notifications
You must be signed in to change notification settings - Fork 18
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
Port Commands to Pure Python #26
Conversation
Please create a separate PR with the tests. |
""" | ||
if CommandScheduler._instance is not None: | ||
return | ||
CommandScheduler._instance = self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's weird that this is assigned here instead of __new__
, but I see why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I can only get rid of it if I force the constructor to crash.
Added a run of the tests with wrapped commands to CI. To make more tests pass, I put in some forwards compatibility changes at https://github.com/TheTripleV/robotpy-commands-v2/blob/mclaren/tests/util.py#L31. Pure commands still works with the old syntax is most cases. breaking changesCommandBase and Command were merged into a unified Command. Methods like
to In fact, all of the CommandGroups now use SelectCommand's constructor has been changed from |
Ran my tests, things look good for me. From an existing example we had to teach students in 2023, it's only four-ish sets of changes. My workflow: Changes that are forced with this PR: More verbose info in the CD thread: |
1b46350
to
c2c2280
Compare
A version of this has been merged to main and pushed to pypi as version 2024.0.0b3. There are a lot of missing pieces, tracking those at #28 |
No description provided.