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

battery-applet: Look at upower slowness/weirdness (on N900 at least) #67

Closed
MerlijnWajer opened this issue Feb 14, 2018 · 26 comments
Assignees
Labels

Comments

@MerlijnWajer
Copy link
Member

@MerlijnWajer MerlijnWajer commented Feb 14, 2018

As a note, there are at least a few things to look at later:

  • mAh calculation (currently based on Wh + current voltage; so that's bad) - let's see if we can get what /sys gives us
  • upower is slow and inaccurate, let's figure out why (values in /sys respond fast and accurately)
  • charging/discharging detection is slow/bad, again because of upower.
  • TimeToEmpty and TimeToFull are often 0 or non sensical, but they are fine in /sys (again: wtf)
@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 14, 2018

The rx51_battery seems to contain more useful charging states in upower. Weird.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 15, 2018

Here is the upower code reading the info: https://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c#n533 - they seem to completely ignore the time_to_* sysfs values.

Their interpretating of charging is also funny: https://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c#n448

At least on the N900 the status seems to be 'Not charging' even when cable is not connected -- not 'Discharging'. That might explain the difference, and why UPower shows "Pending charging" all of the time. I think this also has an effect of it calculating the time-to-* values.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 15, 2018

After at least 30 minutes of discharging, the 'status' still says it's "Not charging", which apparently doesn't mean "Discharging". But the device is obviously discharding - capacity dropped 5 levels. Might be a kernel problem?

root@devuan:/sys/devices/platform/68000000.ocp/48072000.i2c/i2c-2/2-0055/power_supply/bq27200-0# cat capacity
94
root@devuan:/sys/devices/platform/68000000.ocp/48072000.i2c/i2c-2/2-0055/power_supply/bq27200-0# cat status
Not charging

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/bq27xxx_battery.c#n1663
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/power/supply/power_supply_core.c#n307

@MerlijnWajer

This comment has been minimized.

@android-808

This comment has been minimized.

Copy link

@android-808 android-808 commented Feb 16, 2018

Is Not Charging meant for when device is connected to charger but power supplied is not sufficient to charge the device, be it supply or rate of discharge.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 16, 2018

the other way around - battery is charged and device is powered from psu not from battery, afaiu - see mail thread as well :)

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 21, 2018

The charging issue is now solved with these three commits:

maemo-leste/n9xx-linux@32ae325
maemo-leste/n9xx-linux@093b8e4
maemo-leste/n9xx-linux@8c83dd8

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 21, 2018

So sysfs usually updates within 5 seconds with the right value; UPower picks it up much later. But for now, I think this is OK. Since the charge detecting fixes (in kernel) it also shows the time until empty (or time until full) properly on the N900.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 23, 2018

I've just added the fully charged state to the applet -- this was missing.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 24, 2018

I think TimeToEmpty and TimeToFull work OK now that the state is fixed in the kernel. Left now is 'slowness' (maybe poll more often, or react on specific select()able events in UPower?). And the proper energy is the other thing to look at (upower doesn't report them at all it seems)

@MerlijnWajer MerlijnWajer self-assigned this Feb 25, 2018
@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 27, 2018

I think I know why UPower updates slowly, and I've tried to explain it here: https://marc.info/?l=linux-omap&m=151977351627943&w=2

If people agree the approach is sensible, I'll write a patch to also send battery events on charging-status change.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 27, 2018

UPower not exporting the energy exposed in sysfs is something we'll have to patch in UPower, but this is lower priority for me.

@android-808

This comment has been minimized.

Copy link

@android-808 android-808 commented Feb 27, 2018

Even on CSSU-Testing version of Fremantle there is an issue with updating charge status uniformly. Battery applet on status bar updates instantly (almost) when disconnecting charger, where as status menu widget mAh figure often only shows what it was before starting to charge for a minute or so after disconnecting.

Obviously not upower but might be worth looking at to see if it can shed some more light.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 27, 2018

I see. Well, the big problem is that udev doesn't report on the battery charging status changes. This is the first issue that needs to be fixed, I will have a patch ready soon (Compiling now).

The second issue is that the kernel detects the charging status change only after a few seconds. But this is acceptable delay to me. I've mentioned it in the email still, though.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 28, 2018

This patch is supposed to improve the situation: maemo-leste/n9xx-linux@fd09cb9

It should fix the driver not sending updates on charge status, but it doesn't fix the delay in reporting in /sys in general.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 28, 2018

I am still left wondering if it really can't detect this way faster, but perhaps the poll interval is just too high at this point.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 28, 2018

An extreme value like this makes somewhat of a difference, but it still takes a while for the chip to realise it is being charged: echo 1 > /sys/module/bq27xxx_battery/parameters/poll_interval. Maybe this is normal and fremantle kernel did something else to detect this sooner.

@freemangordon

This comment has been minimized.

Copy link

@freemangordon freemangordon commented Feb 28, 2018

Could be that it is using isp1707 to detect wall chargers/USB cable

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Feb 28, 2018

Yes, those events trigger instantly. But UPower does not look at them.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Sep 8, 2018

Moving to beta.

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Sep 10, 2018

Perhaps we can detect the 'wall charger' 'battery' in UPower and use the on-battery status:

signal time=1536608284.619499 sender=:1.17 -> destination=(null destination) serial=46785 path=/org/freedesktop/UPower; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "org.freedesktop.UPower"
   array [
      dict entry(
         string "OnBattery"
         variant             boolean true
      )
   ]
   array [
   ]
@spinal84

This comment has been minimized.

Copy link

@spinal84 spinal84 commented Dec 4, 2018

bq27200-0 seems to be broken in the current maemo leste n900 image.
cat /sys/class/power_supply/bq27200-0/uevent

This command gives false POWER_SUPPLY_STATUS=Charging/Discharging

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Dec 4, 2018

bz27200-0 lags behind, a lot. It updates relatively slow, and upower even slower. This is why I suggested to combine the values with the usb power supply chip. Or are you saying it's wrong also after minutes of being connected?

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Dec 8, 2018

Some of this is being worked on here by @spinal84

@spinal84

This comment has been minimized.

Copy link

@spinal84 spinal84 commented Dec 22, 2018

Does something from the issue remain yet?

@MerlijnWajer

This comment has been minimized.

Copy link
Member Author

@MerlijnWajer MerlijnWajer commented Dec 22, 2018

No; it's all great and smooth now. Maybe even better than fremantle. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.