727287 - Battery support for akami devices

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[[:fabrice] Fabrice Desré]

Battery support is broken on akami devices.

ls /sys/class/power_supply/battery :
uevent
subsystem
device
status
health
present
technology
voltage_max_design
voltage_min_design
voltage_now
capacity
type
power

I will take a look at fixing this.

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: patch

use battery/status instead of battery/charging_source on these devices.

Comment 2

[Mounir Lamouri (:mounir)]

Comment on attachment 597259 [details] [diff] [review]
patch

Review of attachment 597259 [details] [diff] [review]:
-----------------------------------------------------------------

Might be nicer to have this patch done on top of Ben Francis patch.

::: hal/gonk/GonkHal.cpp
@@ +270,5 @@
>      fclose(chargingFile);
> +  } else {
> +    // maguro & akami support
> +    chargingFile = fopen("/sys/class/power_supply/battery/status", "r");
> +    char status[16];

Wouldn't char status[8] be enough?

@@ +271,5 @@
> +  } else {
> +    // maguro & akami support
> +    chargingFile = fopen("/sys/class/power_supply/battery/status", "r");
> +    char status[16];
> +    fscanf(chargingFile, "%s", &status);

You might want to check that |chargingFile| isn't null.

@@ +273,5 @@
> +    chargingFile = fopen("/sys/class/power_supply/battery/status", "r");
> +    char status[16];
> +    fscanf(chargingFile, "%s", &status);
> +    if (strcmp(status, "Charging"))
> +      chargingSrc = 0;

Coding style:
if (foo) {
  bar;
}

@@ +278,1 @@
>    }

You forgot to fclose().

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: patch v2

Reorganised the code a bit to make it easier to add other fallbacks, and addressed comments.

Comment 4

[Mounir Lamouri (:mounir)]

Comment on attachment 597935 [details] [diff] [review]
patch v2

Review of attachment 597935 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with a reply to the comment.

::: hal/gonk/GonkHal.cpp
@@ +275,5 @@
> +  } else {
> +    // toro devices support
> +    chargingFile = fopen("/sys/class/power_supply/battery/status", "r");
> +    if (chargingFile) {
> +      char status[16];

So why not 8?

Comment 5

[[:fabrice] Fabrice Desré]

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #4)

> 
> So why not 8?

because there's a third state "Discharging" for which we need at least 12 chars.

Comment 6

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/956fe683561c

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/956fe683561c