Some system info: my laptop is Dell Latitude C600 with Debian Unstable

please attach output of 'grep . /proc/acpi/battery/*/*' and the output for the 
battery from lshal

Created attachment 4675 [details]
acpi output

Created attachment 4676 [details]
lshal output

You see here with BAT0 that current_charge equals the last_full_charge but
charging still equals true

Sorry I meant with BAT1 you see it

>You see here with BAT0 that current_charge equals the last_full_charge
> but charging still equals true

So we can add a condition in the battery refresh handler:

if (current_charge >= last_full_charge) {
        is_charging = FALSE;
}

and that would fix the problem, right?

(In reply to comment #6)
> >You see here with BAT0 that current_charge equals the last_full_charge
> > but charging still equals true
> 
> So we can add a condition in the battery refresh handler:
> 
> if (current_charge >= last_full_charge) {
>         is_charging = FALSE;
> }
> 
> and that would fix the problem, right?

I think so. Make sure you add a comment and a reference to this bug in the code
otherwise somebody else might rip it out again

Okay, I'll knock a patch up. It'll need some infrastructure work first, but
you'll see what I mean in a minute. I'll post to hal-devel list, and I'll cc you
Jaap.

Created attachment 4685 [details] [review]
patch which cleansup acpi.c and util.c so we can code a suitable fix

Jaap, can you try the attached patch please? It's against CVS, but should apply
against the last release.

The idea to do this:

if (current_charge >= last_full_charge) {
        is_charging = FALSE;
}

is not correct. This maybe fix this machine, but this is wrong for other acpi 
machines. 

If e.g. last_full_charge is not correct set and the machine really charge the 
battery this presumption is complete wrong. This maybe also fails on machines 
which not set the last full capacity. 

You need to accept, that there are some machines which reports this in this 
way. There is maybe a reason why the battery report this. You can workaround 
this in your application, but not in HAL because you can't presume this in 
every case.

Valid point, but wouldn't:

if (last_full_charge > 0 && current_charge >= last_full_charge) {
        is_charging = FALSE;
}

cover the not setting of last_full_charge?

I would argue that this *does* belong in HAL for exactly the oppose reason, that
the user-programs should not have to indervidually handle errata and hardware
bugs, but the solution is in one place, properly documented.

Richard.

No, IMO not. I also have here machines which definitely charing if they already 
arrived the last_full_charge. They are not full and they charge. 

One example is a DELL D600 which has as last full 44810 mWh and as current 
53000 mWh. This battery is over the last full but is charging. In the best case 
your code would only be some seconds wrong, but in the worst case a couple of 
minutes. IMO not the best idea. 

Btw. an other example would be a machine which reports a too low last full 
(maybe the battery is broken, maybe the ACPI implementation or the value is 
never correct set and never changed). In this case the battery maybe charge the 
next 30-60 and hal already reports the machine as full loaded.

Btw. In this special case the battery looks a littlebit old. Unlike to battery 
0 this battery has a lower last full value than the designed value. This is 
maybe the reason why the battery reports as charging. What say cat /proc/acpi/
battery/BAT0/* and what happens if you switch the both batteries?

>One example is a DELL D600 which has as last full 44810 mWh and as current
>53000 mWh. This battery is over the last full but is charging. In the best case 
>your code would only be some seconds wrong, but in the worst case a couple of 
>minutes. IMO not the best idea. 

Then this is a broken battery in itself.

Current charge should never be greater than the last full capacity, logically.

To fix this battery we can correct for the value by saying:

If (current_charge > last_full)
  last_full = current_charge;

The permutations of broken stuff mean all this should be worked around in *one
place* rather than dozens of programs that trust the values from HAL.

Richard.

Bugzilla Upgrade Mass Bug Change

NEEDSINFO state was removed in Bugzilla 3.x, reopening any bugs previously listed as NEEDSINFO.

  - benjsc
    fd.o Wrangler

Close really old bug, no activity since more than 2 years. Feel free to reopen if the problem is still present in latest HAL release or git master.