Closed Bug 727287 Opened 8 years ago Closed 8 years ago

Battery support for akami devices

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: fabrice, Assigned: fabrice)

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → fabrice
Attached patch patch (obsolete) — Splinter Review
use battery/status instead of battery/charging_source on these devices.
Attachment #597259 - Flags: feedback?(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().
Attachment #597259 - Flags: feedback?(mounir)
Attached patch patch v2Splinter Review
Reorganised the code a bit to make it easier to add other fallbacks, and addressed comments.
Attachment #597259 - Attachment is obsolete: true
Attachment #597935 - Flags: review?(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?
Attachment #597935 - Flags: review?(mounir) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/956fe683561c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.