Odd.. Can you attach a lshal, and a description of what you think the icon should be, when the icon is "incorrect". Also, you might be able to get a trace of the initial "wrong" icon if you set g-p-m to log to a file when it is started up, which depends how your distro chooses to set this up.

Created attachment 61086 [details]
lshal output

Created attachment 61087 [details]
gpm debug output

OK, here are the attachments (I simply started gpm manually to get the debug output).

I noticed the tooltip is also wrong, as it says:
"Running on battery
laptop battery - fully charged (93%)"

The icon displayed when debugging was http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/battery-charged.png?rev=1.1
whereas it should have been: http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/battery-discharging-100.png?rev=1.1

(depending on the remaining battery power of course)

I was also wrong in the first message as killing and reloading gpm doesn't fix the issue (plugging the AC adapter does). I guess it is a HAL issue, what do you think?

Cheers,

Julien

Oh, forgot to mention I'm running gpm 2.13.93 with hal 0.5.7 on Debian testing/unstable

I'm a bit confused. The battery "full" icon only appears when !charging && !discharging && percentage_charge > 90, and your lshal clearly states that battery.rechargeable.is_discharging = true.

"is_discharging = true" would explain the "Running on battery power" tooltip, but not the icon.

And I'm more confused as:

gpm-power.c:374 (14:16:22):	 charge     0	rate       0
gpm-power.c:376 (14:16:22):	 discharge  0	remaining  0

g-p-m seems to think that you are not discharging at all, but hal says you are... Maybe the dbus struct issue you are running into, but I thought that was solved in 2.13.93.

You've not compiled g-p-m from source, and then updated dbus 0.31 have you? Or some other weird update combination? To test this, can you do:

lshal -m

and then plug the ac_adapter in and out 3 times after about 30 seconds delay in each state.

We should get lots of battery.rechargeable.is_discharging type changes if dbus is okay, but none if not.

Thanks Julien.

To answer shortly, I run dbus 0.60 and built a Debian package from gpm sources.

Here is the results of the test you proposed (starting with AC unplugged, thus the first change is when plugging the adapter):

julien@hera:~$ lshal -m

Start monitoring devicelist:
-------------------------------------------------
acpi_ACAD property ac_adapter.present = true
acpi_BAT1 property battery.charge_level.percentage = 100 (0x64)
acpi_BAT1 property battery.remaining_time removed
acpi_BAT1 property battery.charge_level.current = 48840000 (0x2e93d40)
acpi_BAT1 property battery.reporting.current = 4400 (0x1130)
acpi_BAT1 property battery.rechargeable.is_discharging = false
acpi_ACAD property ac_adapter.present = false
acpi_BAT1 property battery.charge_level.percentage = 98 (0x62)
acpi_BAT1 property battery.remaining_time = 3541 (0xdd5) (new)
acpi_BAT1 property battery.charge_level.current = 47863200 (0x2da55a0)
acpi_BAT1 property battery.reporting.current = 4312 (0x10d8)
acpi_BAT1 property battery.rechargeable.is_discharging = true
acpi_ACAD property ac_adapter.present = true
acpi_BAT1 property battery.charge_level.percentage = 100 (0x64)
acpi_BAT1 property battery.remaining_time removed
acpi_BAT1 property battery.charge_level.current = 48840000 (0x2e93d40)
acpi_BAT1 property battery.reporting.current = 4400 (0x1130)
acpi_BAT1 property battery.rechargeable.is_discharging = false

Cheers,
Julien

Okay... so it's not DBUS at fault. Thanks for the quick reply btw.
I think this may be a coldplug issue, with g-p-m at fault, but I've still not ruled out the possibility of a HAL problem. When your battery is fully charged, what it its charge? 100%?

yes:
  battery.charge_level.percentage = 100 (0x64)   (int)

And in gpm tooltip: "laptop battery fully charged (100%)"

Cheers,
Julien

Does this stil happen with 2.14.0?

Yes, exactly the same issue with 2.14

Cheers,
Julien

I've seen this problem as well. The issue is when the battery reports it is neither charging nor discharging (ie the ACPI info from the battery is crap), the icon defaults to empty but charging battery (gpm-manager.c line 300'ish). 

I've had this problem as well, and I wrote a patch to try to use whatever info that it's got that's valid (specifically, if the battery still returns the charge %). The patch is against 2.14.0 but I'm sure it wouldn't be hard to patch against HEAD. 

What is puzzling to me is that the tooltip is still correct, it says "plugged in / charging" or "discharging". I suspect the tooltip info is coming from ACPI/ac_adaptor? 

Created attachment 62071 [details] [review]
Handle broken batteries somewhat better

Sure, I don't mind adding bodges for this case as we get it for free, and if the user battery is broken then they shouldn't expect clever multi-battery charge calcuations to work anyway.

I would probably go one stage further, and check for the ac_adapter value to decide if the battery is (probably) charging.

And we should probably fix this in gpm-power.c, over-riding the value of status->is_discharging and status->is_charging where appropriate, and then the other parts of g-p-m (tooltip, icon, warnings, etc.) will just work without a bodge in each case.

Do you want to have a go at a patch to gpm-power.c or want me to have a go?

You can use 2.14 or head, although I would prefer the latter if you don't mind.

p.s. in HEAD we have:

	} else {
		/* We have a broken battery, not sure what to display here */
		gpm_debug ("BROKEN BATTERY...");
		filename = g_strdup_printf ("gpm-%s-broken", prefix);
	}

A specific "broken" icon -- but any fix may make this extra icon redundant.

Thanks for your patch,

Richard.

Sure, I'll see what I can do; shouldn't be too much of a fix

Hi,

I have tried the previous patch against 2.14.0 but it doesn't work for me.
When starting on battery, the icon displayed is still not the right one.

The tooltip says: 'Running on battery power - laptop battery fully charged (93%)'

At the same time, lshal is ok:
  battery.remaining_time = 2632  (0xa48)  (int)
  battery.charge_level.percentage = 94  (0x5e)  (int)
  battery.charge_level.rate = 0  (0x0)  (int)
  battery.charge_level.last_full = 48840000  (0x2e93d40)  (int)
  battery.charge_level.current = 45909600  (0x2bc8660)  (int)
[...]
  battery.charge_level.capacity_state = 'ok'  (string)
  battery.rechargeable.is_discharging = true  (bool)
  battery.rechargeable.is_charging = false  (bool)

Cheers,
Julien

What icon does it display? Also, the patch I attached isn't a complete fix, but it should at least get rid of the problem where it displays the empty-but-charging icon.

From the looks of your lshal, that patch won't be in effect; it's only when is_discharging and is_charging are both false (not this case). 

One thing I noticed that was going wrong when I was trying to debug this, try the following:

1. Note the icon (correct/incorrect)
2. Switch the AC adaptor state (ie plug it in, or unplug it; whatever you didn't start on)
3. Wait a bit and see what the icon is
4. Switch the AC adaptor back
5. Wait a bit and see what the icon is again.
6. killall gnome-power-manager && gnome-power-manager --no-daemon --verbose
7. What does the icon look like now?

In my experience, the icon is correct from step 3-5, it looks like (for me anyways) that g-p-m is only incorrect when it starts. 

> What icon does it display?
full, plugged and not charging icon (http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/battery-charged.png?rev=1.1)

I also sometimes see the empty but charging icon (http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/gpm-battery-charging-000.png?rev=1.1), I think, but am far from sure, that below a certain charge level, it changes (I have to check this).

> 1. Note the icon (correct/incorrect)
> 2. Switch the AC adaptor state (ie plug it in, or unplug it; whatever you
> didn't start on)
I only have this problem when starting on battery power.

> 3. Wait a bit and see what the icon is
> 4. Switch the AC adaptor back
As already explained, the icon is ok once I plug the ac adapter, but reloading gpm does not change anything.

Cheers,
Julien

I've got the same issue on a bunch of older laptops (Toshiba's and Compaq's) with older batteries that will never charge to 100% due to age.

On battery power, the icons are correct and the tooltip reports sane percentages (90%, 80%, 60%, etc) when charging and discharging.

On AC, the icons are correct and the tooltip reports sane percentages when initially charging, but are incorrect once the battery becomes fully charged.

Without Paul's patch, the icon is always empty-but-on-AC
(http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/gpm-battery-charging-000.png?rev=1.1)
when on AC, but the tooltip says that the laptop is on AC and fully charged at anywhere from 82% to 95% (varies by laptop).

With it, the icon is the full-on-battery (http://cvs.gnome.org/viewcvs/*checkout*/gnome-power-manager/data/icons/24x24/gpm-battery-discharging-100.png?rev=1.1)
but the tooltip says "Computer is running on AC power, Laptop battery charging (xx%)"

Same problem in 2.13.93 (ubuntu dapper) and 2.14.0 (GARNOME on CentOS4)

Paul, how's it going? Want me to have a go?

I haven't had a chance to work on it, but I'll have a lot more time to work on it this week, I want to track down the actual problem more than just hacking something

Created attachment 62649 [details] [review]
Improved version of previous patch

I didn't include a changelog because CVS went a little weird on me

Created attachment 62651 [details] [review]
Improved patch with Changelog

Two things, when you do cvs diff, do "cvs diff -u" and you get the unified format. Second, what happens when the battery is actually charged on a non-broken system? You probably need to use the gpm_power_battery_is_charged () function in gpm-power.c

Otherwise the patch looks good. Do you want me to take the baton from here?

Ah! Good call, I forgot about that case. 

Paul, do you have an updated patch?

Yeah, I had this done awhile ago, I forgot to send in the patch though! Sorry

Created attachment 63596 [details] [review]
Handles case where battery is full

Hi,

FYI I have built the latest 2.14.3 release and the problem isn't fixed in it. I cannot try the 2.15 branch because hal and co. are not up-to-date in Debian testing for the moment.

Cheers,
Julien

First of all, I'm using Ubuntu Dapper with version 2.14.3. I applied the last patch (#28) with a little modification (not line 266 but 254), and it worked seamlessly. I don't really understand why this bug is labeled as needing more info or why this simple patch hasn't been applied upstream yet.

If it's fixed by 2.15 then just ignore this comment. I'm posting this bug to the Ubuntu Launchpad site too. I just felt I should report upstream too.

This patch does not work for me, at least, it seems it fixes the problem when starting on the battery, but the issue now happens when I (un)plug the power cable (which it did not before applying the patch).

After a few seconds (let's say 15 to 30s), the icon displays correctly.

I don't think it is gpm fault, as lshal displays the same information that gpm, but I think gpm should deal with such cases, or maybe Richard can talk to Hal developers so that they fix this (once again, I know they already refused some proposals).

Cheers,
Julien

(In reply to comment #31)

This is so because HAL apparently reports the percentage a few moments after (un)plugging the power cord. This is not really an issue. I think we can stand 15 seconds of the wrong icon. Don't you?

I do ;-)

But I also think it would be great if gpm could handle these cases - I would prefer it not to update the icon rather than seeing a wrong icon, what do you think?

Cheers,
Julien

Though I haven't tried to patch it yet, the bug on my laptop seems to be caused by  the FIXME on line 219 of gpm-power.c.  If it sees a charge rate of 0, it sets charging and discharging to 0, strangely.  When gpm starts up for me, it claims a broken battery since the charging and discharging booleans are both false.  Yet, lshal displays one as true and one as false as necessary.

Just to confirm, by commenting out the FIXMEs in gpm-power.c, gpm no longer claims my battery is broken and the icons show up as they should.

2006-09-08  Richard Hughes  <richard@hughsie.com>

	* src/gpm-power.c:
	Remove the FIXME to fix #334212. It's breaking more laptops that it
	was intended to fix and also breaks my (already broken) iBook.

Please reopen if not fixed. Thanks.

The patch in #36 fixed the issues on my computer; thanks for the help!