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 segment showing as charging color/icon when it should show as discharging #644

Closed
2 tasks done
devhawk opened this issue Apr 14, 2021 · 9 comments
Closed
2 tasks done
Labels
🤔 info needed Further information is requested

Comments

@devhawk
Copy link

devhawk commented Apr 14, 2021

Prerequisites

  • I have read and understand the CONTRIBUTING guide
  • I looked for duplicate issues before submitting this one

Description

Battery segment shows as charging when power is disconnected.

Problem seems to be isolated to Windows. I have configured my WSL distros on this box to use the oh-my-posh-wsl binary and the same theme as I do on Windows (bash instead of powershell tho). On Ubuntu, the battery segment displays correctly when discharging.

Environment

  • Oh my Posh version: 3.137.0
  • Theme: custom
  • Operating System: Windows 10 20H2
  • Shell: Powershell
  • Terminal: Windows Terminal

Steps to Reproduce

Battery segment of my theme:

                {
                    "type": "battery",
                    "style": "powerline",
                    "powerline_symbol": "\uE0B0",
                    "foreground": "#000000",
                    "background": "#ffeb3b",
                    "properties": {
                        "battery_icon": "",
                        "discharging_icon": "\uE234 ",
                        "charging_icon": "\uF1E6 ",
                        "charged_icon": "\uF1E6 ",
                        "color_background": true,
                        "charged_color": "#4caf50",
                        "charging_color": "#40c4ff",
                        "discharging_color": "#ff5722",
                        "postfix": "\uF295 ",
                        "display_charging": true
                    }
                },

Expected behavior: [What you expected to happen]

How it looks on WSL:
image

Actual behavior: [What actually happened]

How it looks on Windows:
image

@lnu
Copy link
Contributor

lnu commented Apr 14, 2021

Are you sure you're using the same theme?
I tried on my side and I always get the same layout if I use the same theme on windows and wsl.
If I try your config, I always get the second one:
image

And is it always different because I noticed that sometimes if I open two different terminal windows I don't have the same color/icon but if I clear both then it's ok. The reading can be different.

And there's something different in the readings between wsl and windows(not omp but distatus/battery which is used to get battery data):
image

@JanDeDobbeleer
Copy link
Owner

The code is actually the same, so there's no way oh-my-posh can inform a difference in behaviour unless the OS (or distaffs/battery) reports something else. The only thing impacting this can be when Windows reports maybe more batteries than WSL, which indeed maps the status the most logical way.

@JanDeDobbeleer JanDeDobbeleer added the 🤔 info needed Further information is requested label Apr 14, 2021
@devhawk
Copy link
Author

devhawk commented Apr 14, 2021

Are you sure you're using the same theme?

Yes, I'm sure

The code is actually the same, so there's no way oh-my-posh can inform a difference in behaviour unless the OS (or distaffs/battery) reports something else. The only thing impacting this can be when Windows reports maybe more batteries than WSL, which indeed maps the status the most logical way.

This is a surface book which has multiple batteries, so that must be the source of the behavior difference. I charged overnight and unplugged, now my battery toolbar widget lists both batteries as "100% and in use"

image

I'm not sure why OMP on Windows displays this state using the charging icon + color

image

@JanDeDobbeleer
Copy link
Owner

@devhawk would it make more sense for you to see both batteries btw? I'll validate the logic as maybe I made a silly mistake.

@devhawk
Copy link
Author

devhawk commented Apr 14, 2021

@devhawk would it make more sense for you to see both batteries btw? I'll validate the logic as maybe I made a silly mistake.

I would think displaying a single combined battery segment makes most sense. Thanks for taking a look at the logic. Let me know how I can help. Unfortunately, my go skills are VERY limited so I can't really help troubleshoot locally

JanDeDobbeleer added a commit that referenced this issue Apr 15, 2021
JanDeDobbeleer added a commit that referenced this issue Apr 16, 2021
JanDeDobbeleer added a commit that referenced this issue Apr 16, 2021
JanDeDobbeleer added a commit that referenced this issue Apr 16, 2021
@JanDeDobbeleer
Copy link
Owner

@devhawk I tweaked the mapping and adjusted it so it can be tested. Can you validate the latest release?

@blauwkeut
Copy link

Also had the same behaviour on my Windows machine, but seems resolved by the v3.137.3 release...

@devhawk
Copy link
Author

devhawk commented Apr 16, 2021

@devhawk I tweaked the mapping and adjusted it so it can be tested. Can you validate the latest release?

v3.137.3 also appears to have resolved this issue on my Surface Book. Thanks @JanDeDobbeleer!

Copy link

This issue has been automatically locked since there has not been any recent activity (i.e. last half year) after it was closed. It helps our maintainers focus on the active issues.
If you have found a problem that seems similar, please open a discussion first, complete the body with all the details necessary to reproduce, and mention this issue as reference.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🤔 info needed Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants