tp_smapi does not report status correctly when using battery charge control features

[Darren Hoo]

On Mon, Jul 6, 2009 at 4:04 PM, Richard Hughes<hughsient@...> wrote:

> 2009/7/6 Darren Hoo <darren.hoo@...>:
>> I've got a thinkpad T60 and have battery charge control features enabled:
>>    # echo 40 > /sys/devices/platform/smapi/BAT0/start_charge_thresh
>>    # echo 100 > /sys/devices/platform/smapi/BAT0/stop_charge_thresh
>> upon this setup,when the battery  capacity is about say 80% and AC power
>> is on, the battery is in a state of   not charging nor fully charged.thus
>> g-p-m applet shows an warning icon.This is not as g-p-m 2.24 has worked.
>>
>> line 131 of  src/gpm-devicekit.c says:
>>                } else if (state == DKP_DEVICE_STATE_UNKNOWN && percentage > 95.0f) {
>>                        egg_warning ("fixing up unknown %f", percentage);
>>                        filename = g_strdup_printf ("gpm-%s-charged", prefix);
>>
>> I think here the percentage > 95.0f  here is too strict and unnecessary.
>
> Well, the battery state is unknown, so g-p-m can't really do anything
> sensible here. I think part of the problem is that ACPI exposes two
> values, is_charging and is_discharging, and so it's tricky to map them
> to a sane number of states.
>
> What's the value of state in the correct device in
> /sys/class/power_supply? If this is fixed in the kernel, it'll just
> work in g-p-m. Just lowering the threshold is not a good idea at all,
> and I'll probably remove that hack from g-p-m altogether.
>

Hi,Richard and tp_smapi developers,
Thanks,

$ cat /sys/class/power_supply/BAT0/status
Unknown

So the status is unknown.The kernel module I use is tp_smapi which is
not in the  mainstream kernel yet .
If you don't mind I've cc'ed this mail to tm_smapi developer's mailing-list.
I would like to see what are their  opinions on this.
_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

[Henrique de Moraes Holschuh-2]

On Mon, 06 Jul 2009, Darren Hoo wrote:
> >> I've got a thinkpad T60 and have battery charge control features enabled:
> >>    # echo 40 > /sys/devices/platform/smapi/BAT0/start_charge_thresh
> >>    # echo 100 > /sys/devices/platform/smapi/BAT0/stop_charge_thresh

So the battery will stop charging before it is full...

BTW, /sys/class/power_supply doesn't come from tp_smapi, it comes from ACPI.

Well, we all know just how beatifully complete the ACPI battery state
reporting is, don't we?  Detecting FULL is possible ONLY if you can trust
the battery reporting...

I think the kernel nowadays tries its best to do it properly, and any quirks
for broken platforms should be added there...  Please take a look on 2.6.29
or 2.6.30's drivers/acpi/battery.c to check what it does to try to give
userspace some decent state reports.

> >> line 131 of  src/gpm-devicekit.c says:
> >>                } else if (state == DKP_DEVICE_STATE_UNKNOWN && percentage > 95.0f) {

Well, that just plain can't work if the laptop stopped charging the battery
before it was full, like almost every thinkpad made in the last 10 years can
do (and it is not just thinkpads that can do it, either).

Yes, I know there are shitty laptops that have very broken battery status
reporting, but IMHO we should just cripple them instead of crippling the
support for platforms that do it right.

> > Well, the battery state is unknown, so g-p-m can't really do anything
> > sensible here. I think part of the problem is that ACPI exposes two
> > values, is_charging and is_discharging, and so it's tricky to map them
> > to a sane number of states.

AFAIK:

charge  discharge       state
  0        0            idle
  0        1            discharging
  1        0            charging
  1        1            invalid

If it is invalid, refuse to give battery readouts.  You may want to check
the table above, but that's what I recall from the ACPI and SBS
specifications.

> > What's the value of state in the correct device in
> > /sys/class/power_supply? If this is fixed in the kernel, it'll just
> > work in g-p-m. Just lowering the threshold is not a good idea at all,
> > and I'll probably remove that hack from g-p-m altogether.

Ah, good.  Thanks for cleaning up the mess.

We had a good thread in LKML (or linux-acpi? I forget) when fixing the
battery.c drivers.  You might want to hunt it down in the archives,
gmane.org probably has it.

> So the status is unknown.The kernel module I use is tp_smapi which is
> not in the  mainstream kernel yet .

And it will _never_ be.  But that doesn't matter much, you're getting stuff
from the generic ACPI battery module, as I said ;-)

If you want to use tp_smapi's battery readout instead of ACPI's, you have to
check for /sys/bus/platform/devices/smapi/* and use stuff from there when it
is present, instead of using the power_supply class.

If you're going to support tp_smapi, please remember that reading/writing
values through SMAPI is *expensive*, so don't do it for frivoulous reasons
(i.e. there is no point in reading battery state at more than 0.5Hz, don't
reprogram the battery charge thresholds at every step in a slider but rather
give the user a "apply" button, etc).   In fact, that holds true for ACPI as
well.

Maybe someday tp_smapi will be ported to the power_supply class, but that
has its own problems as well (one of which is the fact that you will get two
power_supply instances for everything in a thinkpad...)

--
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh
_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

[Darren Hoo]

Thanks Henrique a lot for your in-depth clarification.
And I am sorry for my misunderstandings about SMAPI and kernel ACPI
that may have led to confusion.

On Mon, Jul 6, 2009 at 11:16 PM, Henrique de Moraes
Holschuh<hmh@...> wrote:
> If you're going to support tp_smapi, please remember that reading/writing
> values through SMAPI is *expensive*, so don't do it for frivoulous reasons
> (i.e. there is no point in reading battery state at more than 0.5Hz, don't
> reprogram the battery charge thresholds at every step in a slider but rather
> give the user a "apply" button, etc).   In fact, that holds true for ACPI as
> well.
I think gnome-power-manager will never be dependent on SMAPI,otherwise
it will be tied to a specific platform.g-p-m should be universal.Richard,correct
me on this if I am wrong.
_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list

[Richard Hughes-6]

2009/7/6 Darren Hoo <darren.hoo@...>:
> I think gnome-power-manager will never be dependent on SMAPI,otherwise
> it will be tied to a specific platform.g-p-m should be universal.Richard,correct
> me on this if I am wrong.

Sure, the g-p-m front-end isn't Linux specific. DeviceKit power
provides the level of OS glue.

Richard.
_______________________________________________
gnome-power-manager-list mailing list
gnome-power-manager-list@...
http://mail.gnome.org/mailman/listinfo/gnome-power-manager-list