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

Mac OS battery status #799

Open
Ocean48 opened this issue Jan 23, 2024 · 7 comments · May be fixed by #801
Open

Mac OS battery status #799

Ocean48 opened this issue Jan 23, 2024 · 7 comments · May be fixed by #801

Comments

@Ocean48
Copy link

Ocean48 commented Jan 23, 2024

When I try out the battery.status on Mac OS it seems like the CurrentCapacity is the actually current battery percentage already, the calculation for percentage is not needed?

Also, not sure if anyone else on Mac OS have the same issue

    max_capacity = float(line.rpartition('=')[-1].strip())
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: could not convert string to float: '80}'

There is an extra } at the end, my fix if to add a string slice to drop the }

max_capacity = float(line.rpartition('=')[-1].strip()[:-1])

To summarize this is the changes I made to have an output for battery status {'isCharging': True, 'percentage': 98.0}

for line in output.decode('utf-8').splitlines():
    if 'IsCharging' in line:
        is_charging = line.rpartition('=')[-1].strip()
    if 'MaxCapacity' in line:
        # max_capacity = float(line.rpartition('=')[-1].strip())
        max_capacity = float(line.rpartition('=')[-1].strip()[:-1])
    # if 'CurrentCapacity' in line:
    if 'CurrentCapacity' == line.rpartition('=')[0].strip()[1:-1]:
        current_capacity = float(line.rpartition('=')[-1].strip())

if is_charging:
    status['isCharging'] = is_charging == "Yes"

if current_capacity and max_capacity:
# status['percentage'] = 100.0 * current_capacity / max_capacity
status['percentage'] = current_capacity
@Julian-O
Copy link
Contributor

Also, not sure if anyone else on Mac OS have the same issue

I don't have a Mac and can't answer this directly.

Does the Plyer unit-tests exercise this code? If so, whether they pass when run on your machine.

What does the output of the command ioreg -rc AppleSmartBattery look like on your machine?

How does it compare to this example, which has CurrentCapacity and IsCharging on separate lines?

I was going to suggest that you submit a PR with your changes, but it seems to me there is a bigger issue with ioreg giving inconsistent output (based on macOS version? some sort of configuration?)

Do we need to specify more commandline options to override different defaults? Do we need the code to be able to handle the different formats?

@Ocean48
Copy link
Author

Ocean48 commented Jan 24, 2024

~ % ioreg -rc AppleSmartBattery
+-o AppleSmartBattery <class AppleSmartBattery, id 0x100000551, registered, ma$
{
"PostChargeWaitSeconds" = 120
"built-in" = Yes
"AppleRawAdapterDetails" = ()
"CurrentCapacity" = 74
"PackReserve" = 127
"DeviceName" = "bq40z651"
"PostDischargeWaitSeconds" = 120
"CarrierMode" = {"CarrierModeLowVoltage"=3600,"CarrierModeHighVoltage"=41$
"TimeRemaining" = 312
"ChargerConfiguration" = 0
"IOGeneralInterest" = "IOCommand is not serializable"
"IOReportLegend" = ({"IOReportChannels"=((7167869599145487988,6460407809,$
"AtCriticalLevel" = No
"BatteryCellDisconnectCount" = 0
"UpdateTime" = 1706062530
"Amperage" = 18446744073709550965
"AppleRawCurrentCapacity" = 2829
"AbsoluteCapacity" = 0
"AvgTimeToFull" = 65535
"ExternalConnected" = No
"ExternalChargeCapable" = No
"AppleRawBatteryVoltage" = 11934
"BootVoltage" = 0
"BatteryData" = {"Ra03"=91,"Ra10"=190,"CellWom"=(0,0),"RaTableRaw"=(<0000$
"BatteryInstalled" = Yes
"IOReportLegendPublic" = Yes
"Serial" = "F8Y24010B1K10X2AV"
"AppleRawExternalConnected" = No
"KioskMode" = {"KioskModeLastHighSocHours"=0,"KioskModeFullChargeVoltage"$
"NominalChargeCapacity" = 4158
"FullyCharged" = No
"FullPathUpdated" = 1706062470
"ManufacturerData" = <000000000b000100f91400000432323133033030420341544c0$
"FedDetails" = ({"FedStateOfCharge"=0,"FedPortPowerRole"=0,"FedProductID"$
"BatteryInvalidWakeSeconds" = 30
"ChargerData" = {"ChargerStatus"=<021c00009800000000000000000000000000000$
"BootPathUpdated" = 1705005538
"DesignCycleCount9C" = 1000
"AdapterDetails" = {"FamilyCode"=0}
"PowerTelemetryData" = {"WallEnergyEstimate"=0,"AccumulatedSystemPowerIn"$
"MaxCapacity" = 100
"InstantAmperage" = 18446744073709550965
"PortControllerInfo" = ({"PortControllerLoserReason"=1,"PortControllerEvt$
"GasGaugeFirmwareVersion" = 2
"AdapterInfo" = 0
"Location" = 0
"Temperature" = 2954
"AvgTimeToEmpty" = 312
"BestAdapterIndex" = 0
"DesignCapacity" = 4563
"IsCharging" = No
"PermanentFailureStatus" = 0
"Voltage" = 11934
"UserVisiblePathUpdated" = 1706062530
"CycleCount" = 152
"AppleRawMaxCapacity" = 4031
"VirtualTemperature" = 2219
}

It looks like the output for ioreg -rc AppleSmartBattery has changed. I'm on the newest MacOS, and CurrentCapacity is the actual battery percentage. Not sure starting from which MacOS version the output was changed.
But by the look of the output, maybe there is no need to look for MaxCapacity.

@Julian-O
Copy link
Contributor

What a mess.

Let's go through this.

Under the OLD system (e.g. in 2017, according to the link above):

MaxCapacity and CurrentCapacity were arbitrary units.
Plyer read them in, and normalised them into a percentage, using a formula.

Under the new system:
AppleRawMaxCapacity and AppleRawCurrentCapacity holds the values as arbitrary units.
MaxCapacity and CurrentCapacity are normalised, so MaxCapacity is always 100 and CurrentCapacity is effectively a percentage.
Plyer reads them, normalises them into a percentage again (which is effectively doesn't change it).

@Ocean48 is proposing to remove the second renormalising formula, and just use CurrentCapacity. I would argue against. The formula is doing no harm, and allows it to work on the old and new systems.

But wait... there is a bug. Plyer doesn't read in the field called CurrentCapacity. It reads in the last field that contains the text CurrentCapacity. Under the new system, this is whichever appears last: CurrentCapacity or AppleRawCurrentCapacity. Ditto with MaxCapacity or AppleRawMaxCapacity. Because the values are normalised, this will continue to work, but only if the order is consistent (e.g. both AppleRaws are last). This bug should be fixed so it isn't as fragile to changes in ioreg.

But there is still another issue: @Ocean48 complained that the MaxCapacity was "80}". I can't see how that happened here. This needs debugging.

So my position:

  • Leave in the normalisation function, so it works on old and new versions.
  • Fix the field look ups to be an exact match, not a "contains".
  • Ask @Ocean48 to debug why the brackets are slipping in.

@Ocean48
Copy link
Author

Ocean48 commented Jan 24, 2024

Long story short, the problem is because Plyer is checking string contains MaxCapacity or CurrentCapacity, so brackets just slips in. All needed is to do exact match lookup with string slicing leave everything else the same, and still will work on older version

@Julian-O
Copy link
Contributor

@Ocean48: Look at the string you posted a couple of hours ago. Where are these brackets? They are nowhere to be seen. It would be good to see what the value of line and output are when the error occurs.

Ocean48 added a commit to Ocean48/plyer that referenced this issue Jan 24, 2024
Fix kivy#799
Use exact string math instead of partial match to look for battery status. This will prevent fields like AppleRawMaxCapacity and AppleRawCurrentCapacity from being read as MaxCapacity and CurrentCapacity.
@Ocean48 Ocean48 linked a pull request Jan 24, 2024 that will close this issue
@Ocean48
Copy link
Author

Ocean48 commented Jan 24, 2024

@Ocean48: Look at the string you posted a couple of hours ago. Where are these brackets? They are nowhere to be seen. It would be good to see what the value of line and output are when the error occurs.

I forgot to include the string, here is the string
The string line is
"BatteryData" = {"Ra03"=98,"Ra10"=201,"CellWom"=(0,0),"RaTableRaw"=(<0055014a005200450053007e00480069006b007f009f00ad00bf010b0271045e>,<00550154006100470056007f0047006d00800093009c009c00c3012d0289049a>,<0055018e0064005200620088005000740088009400b400c900f10178034e0606>),"Qstart"=0,"AdapterPower"=1107416970,"TrueRemainingCapacity"=0,"DailyMinSoc"=39,"Ra04"=136,"CurrentSenseMonitorStatus"=0,"Ra11"=241,"CellVoltage"=(4030,4024,4032),"PackCurrentAccumulator"=1521278,"PassedCharge"=1067,"Flags"=16777216,"PresentDOD"=(48,48,48),"Ra05"=80,"Ra12"=376,"MiscStatus"=0,"FccComp1"=3988,"ChemID"=21353,"iMaxAndSocSmoothTable"=<0000000000000000000000000000000000000000000000000000000000000000>,"FccComp2"=3988,"PackCurrentAccumulatorCount"=1069098,"DOD0"=(4112,4112,4112),"Dod0AtQualifiedQmax"=0,"Ra06"=116,"ResScale"=0,"Ra13"=846,"FilteredCurrent"=0,"WeightedRa"=(108,114,126),"RSS"=0,"CellCurrentAccumulatorCount"=0,"Serial"="F8Y24010B1K10X2AV","DataFlashWriteCount"=7262,"DailyMaxSoc"=99,"DateOfFirstUse"=0,"Ra07"=136,"Ra14"=1542,"MaxCapacity"=100,"ChemicalWeightedRa"=0,"Ra00"=398,"BatteryHealthMetric"=0,"DesignCapacity"=4563,"Ra08"=148,"BatteryState"=<000000000000000083e4400200020000>,"CellCurrentAccumulator"=(0,0),"AlgoChemID"=21353,"MfgData"=<000000000b000100f91400000432323133033030420341544c00230000000000>,"ManufactureDate"=61788358979635,"ISS"=2128,"Ra01"=100,"Soc1Voltage"=0,"QmaxDisqualificationReason"=0,"ChargeAccum"=0,"SimRate"=0,"Qmax"=(4541,4496,4496),"PMUConfigured"=1648,"ITMiscStatus"=0,"StateOfCharge"=50,"Ra09"=180,"GaugeFlagRaw"=128,"CycleCount"=153,"Voltage"=12076,"SystemPower"=1091353460,"LifetimeData"={"Raw"=<02502ac5000037a30000000000000000025062688000c7e640b20000000000000025000511570aef33f0219013dcf48417b0f152f484f15200e50002a2000007>,"UpdateTime"=1706074562,"ResistanceUpdatedDisabledCount"=0,"CycleCountLastQmax"=143,"TimeAtHighSoc"=<00000000a1030000900000000000000000000000000000000000000000000000610200004c0000000000000000000000000000000000000000000000da0200003900000000000000000000000000000000000000070000005d1e0000c901000002000000000000000000000000000000>,"TemperatureSamples"=172544,"TotalOperatingTime"=10784,"MaximumDischargeCurrent"=18446744073709548676,"MinimumPackVoltage"=8592,"MaximumPackVoltage"=13296,"MaximumChargeCurrent"=5084,"AverageTemperature"=18446744073709551589,"MinimumTemperature"=5,"RDISCnt"=0,"MaximumTemperature"=37},"Ra02"=82}

    max_capacity = float(line.rpartition('=')[-1].strip())
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: could not convert string to float: '82}'

In this string it contains MaxCapacity, but it ends with 82}.
This is why I suggest exact match is better than a partial match

@Julian-O
Copy link
Contributor

Wha??? Ohhhhhh! I see!! It was truncated in the previous example. before, so I didn't know it contained the string.

I agree with your conclusion completely! Thanks for bringing me along.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants