Fix battery tooltip text when minutesLeft is unknown #2160
Conversation
src/util/battery/batterywindows.cpp
Outdated
| m_dPercentage = 0.0; | ||
| m_chargingState = Battery::UNKNOWN; | ||
| int seconds_left; |
There was a problem hiding this comment.
This declaration can be combined with the assignment below.
src/util/battery/batterywindows.cpp
Outdated
| @@ -15,9 +15,10 @@ BatteryWindows::~BatteryWindows() { | |||
| } | |||
|
|
|||
| void BatteryWindows::read() { | |||
| m_iMinutesLeft = 0; | |||
| m_iMinutesLeft = -1; | |||
There was a problem hiding this comment.
It does not matter at all here and it is not you fault, but in general I prefer not to mess with member variables during a function call. During to later changes, one may not aware of this and use the same member in a subroutine, wit surprising effects.
Can you move that into the else branch below. This way the member value is always valid.
There was a problem hiding this comment.
Can we do this later, in a separate pull request?
Fixing this correctly for all OSs is a big change..
src/util/battery/batterymac.cpp
Outdated
| @@ -16,7 +16,7 @@ BatteryMac::~BatteryMac() { | |||
| } | |||
|
|
|||
| void BatteryMac::read() { | |||
| m_iMinutesLeft = 0; | |||
| m_iMinutesLeft = -1; | |||
There was a problem hiding this comment.
Please define -1 as constexp in a anonymous namespace.
There was a problem hiding this comment.
I've done this in the latest commit (I hope I didn't misunderstand ;)
Fix battery tooltip text when minutesLeft is unknown
Currently, the tooltip says "Time until charged: 0" or "Time left: 0" when the time is actually unknown.
The change makes all versions of the util return
-1ifminutesLeftis unknown.In src/widget/wbattery.cpp, we have:
(minutesLeft can be 0 if the time is less than an minute)
Note that upower returns
0for unknown:https://upower.freedesktop.org/docs/Device.html#Device:TimeToEmpty
Windows uses
-1:https://docs.microsoft.com/en-us/windows/desktop/api/winbase/ns-winbase-_system_power_status
On OSX, the property is optional and not set when unknown (i guess).