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

fix(battery_plus): Implement not charging battery state #2275

Conversation

codercengiz
Copy link
Contributor

@codercengiz codercengiz commented Oct 18, 2023

Description

Normally, there are 5 different states to define the battery state on the Android side:
https://developer.android.com/reference/android/os/BatteryManager#BATTERY_STATUS_CHARGING
full, charging, discharging, not _charging, unknown. But in the current code base, discharging and not_charging are considered the same, and both are parsed as discharging.
With this change, if the Android SDK has a not_charging state, the Flutter side will also see this state.

On Samsung phones, if we enable battery protection, the phone charges up to 85%. When it reaches 85%, the status changes from charging to not_charging. When I checked, the current used is almost 0 mA.

Related Issues

Fix #2274

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

@codercengiz codercengiz changed the title Implement not charging battery state fix(battery_plus): Implement not charging battery state Oct 18, 2023
@vbuberen
Copy link
Collaborator

Thanks for your contribution, but I can't accept this PR as it is only covering Android platform.
If you would have really read the contribution guide instead of just setting such checkmark in PR template you would see that new features covering one platform can't be accepted: https://github.com/fluttercommunity/plus_plugins/blob/main/CONTRIBUTING.md#-cannot-be-accepted

@dkbast
Copy link
Contributor

dkbast commented Nov 28, 2023

I think what @vbuberen was trying to say is, that we appreciate the work that you have done, but that we have decided to only accept PRs that provide feature parity for the mobile platforms. As such you (or someone else) would need to add the same functionality for iOS for this PR to move out of a draft status and into the review phase. @codercengiz are you planning on creating an iOS implementation as well?

@vbuberen
Copy link
Collaborator

I think what @vbuberen was trying to say is, that we appreciate the work that you have done, but that we have decided to only accept PRs that provide feature parity for the mobile platforms.

There were no request to translate or interpret what I wrote there.

Also, I would ask you to not use we and not reply on behalf of Plus Plugins maintainers, because except being added to a chat on this topic there were no activity or contributions from you. Thus, please speak for yourself.

@vbuberen
Copy link
Collaborator

are you planning on creating an iOS implementation as well?

This can't be added because currently iOS has no separate state for not_charging like Android OS has: https://developer.apple.com/documentation/uikit/uidevice/batterystate

@dkbast
Copy link
Contributor

dkbast commented Nov 29, 2023

@vbuberen if it cannot be added to iOS, imho this would imply that this PR is already feature complete, because, even if a feature cannot be added for all platforms it should still be ok to add it to the ones which support it. If I remember correctly thats how it is handled in some of the other plugins.

@vbuberen
Copy link
Collaborator

I would like to do some tests to check a few assumptions first. Will get back to this PR later today when have more time.

@vbuberen vbuberen force-pushed the implement_not_charging_battery_state branch from 855f239 to 227da29 Compare November 30, 2023 16:33
@vbuberen
Copy link
Collaborator

After an additional research I found out that there is a possibility to implement same connected, but not charging state on MacOS as well, so updating platform interface enum with new item would be used not only by Android. Based on this fact I would like to accept this PR and merge it. Afterwards I will implement MacOS version and release an update when it is ready.

@vbuberen vbuberen merged commit 6595e03 into fluttercommunity:main Nov 30, 2023
19 checks passed
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.

[Bug]: Battery Status discharging is not equal to not charging
3 participants