844974 - B2G mis-interpretes absence of battery as a low-battery condition

Status: RESOLVED WONTFIX

(Core :: Hardware Abstraction Layer (HAL), defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

STR:

- take an Otoro phone
- remove battery
- boot, running on USB power
- it will reboot/poweroff due to what it mis-interpretes as a low-battery condition.

JS stack:

I/Gecko   (  329): 0 sm_startPowerOff() ["app://system.gaiamobile.org/js/sleep_menu.js":219]
I/Gecko   (  329):     this = [object Object]
I/Gecko   (  329): 1 bm_checkBatteryDrainage() ["app://system.gaiamobile.org/js/battery_manager.js":29]
I/Gecko   (  329):     this = [object Object]
I/Gecko   (  329): 2 bm_init() ["app://system.gaiamobile.org/js/battery_manager.js":38]
I/Gecko   (  329):     this = [object Object]
I/Gecko   (  329): 3 <TOP LEVEL> ["app://system.gaiamobile.org/js/battery_manager.js":277]
I/Gecko   (  329):     this = [object Window]
I/Gecko   (  329):

See bug 839452 for context.

Comment 1

[Mounir Lamouri (:mounir)]

What does /sys/class/power_supply/battery/status return when the phone has no battery?

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Removed battery, rebooted (adb reboot), ./run-gdb.sh attach. When it broke:

$ adb shell cat /sys/class/power_supply/battery/status
Charging

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

...and same result starting with a completely unplugged phone.

Comment 4

[Mounir Lamouri (:mounir)]

And /sys/class/power_supply/battery/capacity ?
I wouldn't be surprised if that's 0.

FWIW, the author of the Gonk backend for the Battery API never cared about the case of no battery. The backend never check that and I was hoping that somehow "/sys/class/power_supply/battery/" wouldn't exist. It seems that "/sys/class/power_supply/battery/present" could be used but doing a hot unplug of my battery didn't update the value of that file. I can't boot the phone without a battery for the moment. If it seems to work, I could try to write a patch.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

$ adb shell cat /sys/class/power_supply/battery/capacity
1

Comment 6

[Mounir Lamouri (:mounir)]

(In reply to Benoit Jacob [:bjacob] from comment #5)
> $ adb shell cat /sys/class/power_supply/battery/capacity
> 1

1 or 0 is the same given that it is likely low enough to get the system to shutdown.

Did you check "/sys/class/power_supply/battery/present"?

Comment 7

[Thomas Zimmermann [:tzimmermann] [:tdz]]

On my otoro, /sys/class/power_supply/battery/present is always 1, even with the battery removed. Same on unagi.

Comment 8

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Check presence of battery

Here is a patch for testing the presence of the battery. As I mentioned, the respective value is always 1, so it doesn't really solve the problem. But it might work in the future.

Comment 9

[Mounir Lamouri (:mounir)]

Comment on attachment 718960 [details] [diff] [review]
Check presence of battery

We shouldn't land something that we know doesn't work, this is going to mislead developers.

Could that problem be on a lower level? Who is responsible to update the /present file? The kernel?

Comment 10

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Yes. And the kernel comes from the phone vendors. CC'ing cjones, maybe he can help.

Comment 11

[Ben Turner (not reading bugmail, use the needinfo flag!)]

The Michaels may be able to offer advice too.

Comment 12

[Marco Chen [:mchen]]

(In reply to Thomas Zimmermann [:tzimmermann] from comment #10)
> Yes. And the kernel comes from the phone vendors. CC'ing cjones, maybe he
> can help.

As I knew that battery present is dependent on detecting of battery ID and it is done by kernel driver or AMSS from modem processor (report to linux kernel via RPC)

Comment 13

[Michael Vines [:m1] [:evilmachines]]

For v1.0.1 devices, iirc the lack of a battery really isn't a valid operating mode.   Ideally the bootloader would just block this from occurring.    That's why we need to power down the device when the battery is low even with USB power applied.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Mass closing as we are no longer working on b2g/firefox os.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Mass closing as we are no longer working on b2g/firefox os.