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

pybricks.common.charger: change return value of connected() #274

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

dlech
Copy link
Member

@dlech dlech commented Nov 16, 2024

Change return value of connected() property from bool to int using the value of pbdrv_usb_get_bcd(). This will allow pro users to be able to tell if they have a "nonstandard" charger that could prevent proper charging.

Related discussion: https://github.com/orgs/pybricks/discussions/1872#discussioncomment-11278180

Alternative to consider: We could have the battery status light reflect this status somehow to make it even easier for users. Although having programmatically accessible in addition to that still seems useful for advanced use cases.

@coveralls
Copy link

coveralls commented Nov 16, 2024

Coverage Status

coverage: 56.385%. remained the same
when pulling f4703a1 on dlech-patch-1
into 9e5cd4c on master.

@laurensvalk
Copy link
Member

This looks good to me. Thanks!

Could you add a notice to the CHANGELOG? I will be going through that one later to make the documentation updates to match.

dlech added 2 commits December 5, 2024 08:27
Change return value of `connected()` property from `bool` to `int` using the value of `pbdrv_usb_get_bcd()`. This will allow pro users to be able to tell if they have a "nonstandard" charger that could prevent proper charging.
Since these are part of the API now, we don't want the values to change.
@laurensvalk laurensvalk merged commit f4703a1 into master Dec 5, 2024
38 checks passed
@laurensvalk
Copy link
Member

Merged, thanks!

@laurensvalk laurensvalk deleted the dlech-patch-1 branch December 5, 2024 11:00
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