search

RehabMan / OS-X-ACPI-Battery-Driver

Implements an Advanced Configuration and Power Interface (ACPI) based battery manager kernel extension (kext/driver) for non-Apple laptops running OS X.
Other
178 stars 58 forks source link

When checking if any batteries are discharging, check for their presence too #2

Closed jafd closed 10 years ago

jafd commented 10 years ago

When a battery is not present, due to the fact that we always set fACConnected for it to true to enable boot without batteries, the tracker cannot rely on the fACConnected flag alone.

Thus, check also that the battery is marked as present.

jafd commented 10 years ago

Hm, I think it needs some debugging for the cases when there are two batteries actually present. A few replugging attempts and the system thinks AC is not connected when it in fact is.

Anyway, my personal thought is that connect/disconnect messages from the AC adapter should prevail over the current logic which is „Assume connected when not discharging”. First, in a Thinkpad X220, the battery does not move to discharging state right away, so ACPIBatteryManager only reacts on next polling — which by default only happens after 30 seconds. Second, If the AC adapter is not powerful enough for some burst modes, the battery can well be draining despite being connected, to complement the power supply (which happens in Macbook Pros — they always throttle to minimum speed when they are connected to AC but don't have a battery).

RehabMan commented 10 years ago

So far, I've been trying to avoid using the status from the AC Adapter. Not all DSDTs have working AC adapters and I don't want to break computers that are already working. Given that 99.9% of laptops have only one battery, I don't want to risk it.

But I could look into only trusting the AC adapter when there are multiple batteries. Then it is up to DSDT patch authors, where multiple batteries (the 0.1%) are possible, to insure their AC adapter DSDT code is working properly. For the single battery case, I can continue to use only charging/discharging status as a way to infer the status of the AC adapter connection.

Will consider the above after I have a chance to review the code.

jafd commented 10 years ago

I'd rather suggest that the driver trusts the disconnect/connect notifications it itself is broadcasting. On Jan 27, 2014 6:38 AM, "RehabMan" notifications@github.com wrote:

So far, I've been trying to avoid using the status from the AC Adapter. Not all DSDTs have working AC adapters and I don't want to break computers that are already working. Given that 99.9% of laptops have only one battery, I don't want to risk it.

But I could look into only trusting the AC adapter when there are multiple batteries. Then it is up to DSDT patch authors, where multiple batteries (the 0.1%) are possible, to insure their AC adapter DSDT code is working properly. For the single battery case, I can continue to use only charging/discharging status as a way to infer the status of the AC adapter connection.

Will consider the above after I have a chance to review the code.

— Reply to this email directly or view it on GitHubhttps://github.com/RehabMan/OS-X-ACPI-Battery-Driver/pull/2#issuecomment-33343152 .

RehabMan commented 10 years ago

The only purpose of the disconnect/connect done by the AC adapter is to the system (controls things like the backlight) and it triggers battery objects to poll for new information after a change. Currently, In the event of a non-working AC adapter (it is relatively common), the AC status will not be shown in the battery menu for an average of another ~15 seconds (polling is done every 30, so at some random point in time, the next one is average of 15 sec away), and things like automatic brightness up/down (system notification of AC status) will not work.

I'd rather not make more things break in the case of a non-working AC adapter for the single battery case. But I'm willing to do it if the system has multiple batteries.

It is not about trust of the battery driver itself and its AC adapter code. It is about trust of DSDT code associated with the AC adapter.