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

Battery indicator is misleading with two batteries #656

Closed
dbanetto opened this issue Nov 21, 2019 · 1 comment
Closed

Battery indicator is misleading with two batteries #656

dbanetto opened this issue Nov 21, 2019 · 1 comment
Labels
🐛 bug Something isn't working as expected. 🌱 good first issue Good first issue to get your feet wet.

Comments

@dbanetto
Copy link
Contributor

Bug Report

Current Behavior

The low battery will say that I have 5% left when one battery is low but the other battery has plenty left. See screenshots below.

Expected Behavior

The battery indicator will only show when the overall charge of multiple batteries is low, not when a single battery is.

Additional context/Screenshots

terminal-output
power-state

Environment

  • Starship version: starship 0.26.4
  • Shell type: zsh
  • Shell version: zsh 5.7.1 (x86_64-pc-linux-gnu)
  • Shell plugin manager: zgen
  • Terminal emulator: gnome-terminal
  • Operating system: Linux (Arch Linux)

Relevant Shell Configuration

zstyle ':prezto:*:*' color 'yes'

## zgen
source $HOME/.zgen/zgen.zsh
if ! zgen saved; then
  zgen prezto

  zgen prezto history-substring-search
  zgen prezto utility

  zgen save
fi

# conditionally run with starship
if command -v starship > /dev/null 2>&1 ; then
  eval "$(starship init zsh)"
else
  prompt pure
fi

Starship Configuration

I have not have a config file.

Possible Solution

  • Calculate the "total" battery (total remaining charge / total capacity of all batteries)
    • This could be done here or as a possible patch to the battery crate & used here
  • Use the battery with the highest remaining charge for the remaining
@dbanetto dbanetto added the 🐛 bug Something isn't working as expected. label Nov 21, 2019
@matchai
Copy link
Member

matchai commented Nov 21, 2019

Interesting! It looks like we can certainly add logic to handle multiple batteries:
https://github.com/starship/starship/blob/master/src/modules/battery.rs#L78

@matchai matchai added the 🌱 good first issue Good first issue to get your feet wet. label Nov 21, 2019
dbanetto added a commit to dbanetto/starship that referenced this issue Dec 1, 2019
Changes the calculation of the battery percentage to take into account
many batteries. This is by summing their current energy & capacity, then
getting a percentage from that. This is what the `battery` crate
describes what `Battery::state_of_charge` does but with less accuracy.
This should be fine because the percentage is just 2 s.f.

Since there is now many batteries the overall state of the battery must
be merged from many to one. The strategy here is: if any are discharging
then report as discharging; if none are discharging & at least one is
charging, then report as charging; if the battery states (e.g. full/empty)
are the same, report as that; otherwise, report as unknown.

Closes issue starship#656
dbanetto added a commit to dbanetto/starship that referenced this issue Dec 1, 2019
Changes the calculation of the battery percentage to take into account
many batteries. This is by summing their current energy & capacity, then
getting a percentage from that. This is what the `battery` crate
describes what `Battery::state_of_charge` does but with less accuracy.
This should be fine because the percentage is just 2 s.f.

Since there is now many batteries the overall state of the battery must
be merged from many to one. The strategy here is: if any are discharging
then report as discharging; if none are discharging & at least one is
charging, then report as charging; if the battery states (e.g. full/empty)
are the same, report as that; otherwise, report as unknown.

Closes issue starship#656
dbanetto added a commit to dbanetto/starship that referenced this issue Dec 1, 2019
Changes the calculation of the battery percentage to take into account
many batteries. This is by summing their current energy & capacity, then
getting a percentage from that. This is what the `battery` crate
describes what `Battery::state_of_charge` does but with less accuracy.
This should be fine because the percentage is just 2 s.f.

Since there is now many batteries the overall state of the battery must
be merged from many to one. The strategy here is: if any are discharging
then report as discharging; if none are discharging & at least one is
charging, then report as charging; if the battery states (e.g. full/empty)
are the same, report as that; otherwise, report as unknown.

Closes issue starship#656
@matchai matchai closed this as completed in 1558b22 Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working as expected. 🌱 good first issue Good first issue to get your feet wet.
Projects
None yet
Development

No branches or pull requests

2 participants