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.

Sign up for GitHub

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

Jump to bottom

[Bug] hub.system.set_stop_button(None) not working #837

Closed
laurensvalk opened this issue Dec 7, 2022 · 2 comments
Closed

[Bug] hub.system.set_stop_button(None) not working #837

laurensvalk opened this issue Dec 7, 2022 · 2 comments
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime)

Comments

@laurensvalk
Copy link
Member

laurensvalk commented Dec 7, 2022 •
edited
Loading

Describe the bug
In the REPL, do:

>>> hub.system.set_stop_button(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: argument should be a 'System' not a 'NoneType'
>>>

I suspect this happened due to pybricks/pybricks-micropython@f09d810 combined with the way System() is implemented. All its methods don't have a self_in argument.

We could change that, or see if there is a way to set them as class methods so this check gets bypassed.
@laurensvalk laurensvalk added triage Issues that have not been triaged yet bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) and removed triage Issues that have not been triaged yet labels Dec 7, 2022
@laurensvalk
Copy link
Member Author

Another way to do it, potentially without size increase, is to do it like the battery methods.

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Dec 8, 2022
@laurensvalk
pybricks.common.System: Change type definition to skip self check.
2d851f0 
All of the methods in the System class are implemented as staticmethods
so self was not being passed, which causes the self-type check to fail.

This works around it without changing the methods by implementing the
System instance as a module, just like we do for the battery.

Fixes: pybricks/support#837

See also pybricks/support#840 for the cleanup
to make the implementation consistent across modules.
@laurensvalk
Copy link
Member Author

I went for the battery-like solution for minimal impact.

I have reopened #840 to make it more consistent in a future release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@laurensvalk