1066243 - [Status Bar][Battery] Battery indicator not always properly updating when charging/disconnecting

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Josh Schmitt [Joshs]]

Attachment: log.txt

Description:
Battery indicator on the status bar does not update in a timely matter when connecting/disconnecting to a wall outlet or a pc.
   
Repro Steps:
1) Update a Flame device to BuildID: 20140911064110
2) Open the Settings app
3) Connect the Phone to pc or wall outlet
4) Disconnect the phone
5) Reconnect the phone
  
Actual:
The Battery indicator does not update or does not update in a timely matter.
  
Expected: 
The Battery indicator updates immediately upon connecting to a power source.
  
Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140911064110
Gaia: e3b9d0d6516177636965d97c63c60981a24a0662
Gecko: 98ea98c8191a
Version: 35.0a1 (2.2 Master)
Firmware: V123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0
  
Notes:
Repro frequency: 3/5
See attached: logcat

Comment 1

[Peter Bylenga [:PBylenga]]

qawanted for branch checks.

Comment 2

[Cody Roesch [:croesch]]

This bug repro's on Flame KK builds: Flame 2.2, Flame 2.1 and OpenC 2.2

Actual Results: Lightning bolt in the battery indicator in the status bar is not updating timely when plugging or unplugging the phone from USB or wall.

Repro Rate: 10/10

Device: Flame 2.2
BuildID: 20140912061053
Gaia: b72909030e214175144342f7e5df7e88a2b52fd4
Gecko: 59d4326311e0
Version: 35.0a1 (2.2)
Firmware: v165
------------------------------------------------
Device: Flame 2.1
BuildID: 20140912081053
Gaia: 59e5c2467b7b8219ed194a0d0a94c6ed59af95be
Gecko: b09d2857b74e
Version: 34.0a2 (2.1)
Firmware: v165
------------------------------------------------
Device: Open_C 2.2
BuildID: 20140912061053
Gaia: b72909030e214175144342f7e5df7e88a2b52fd4
Gecko: 59d4326311e0
Version: 35.0a1 (2.2)
Firmware: P821A10v1.0.0B06_LOG_DL

------------------------------------------------
------------------------------------------------

This bug does NOT repro on Flame kk build: Flame 2.0, Flame 1.4 Base

Actual Result: Charging icon in the status bar correctly reflects the user having the device connected and charging or not.

Repro Rate: 0/5 attempts

Environmental Variables:
Device: Flame 2.0
BuildID: 20140911220254
Gaia: 91dd0e596aa7c124dd968e1474b23e7992dc35a1
Gecko: a66168598533
Version: 32.0 (2.0) 
Firmware Version: v165
-------------------------------------------------
Device: Flame 1.4
BuildID: 20140814202332
Gaia: 129211661489feb60bbd6772a44081d23b374f17
Gecko: 
Version: 30.0 (1.4)
Firmware: v165

Comment 3

[Joshua Mitchell (Inactive)]

triage - not nomming - moderate repro rate, graphical issue, additionally - there is also a LED light next to the camera shutter that comes on when you plug in to indicate charging and this comes on immediately when plugging in independent of the icon.

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

[Blocking Requested - why for this release]: regresssion

This is an annoying regression and can easily be a partner blocker.
We also can't depend on that LED light which is specific to Flame hardware design.

Flagging for QA to find the regressed bug. Thanks!

Comment 6

[Guillaume Marty [:gmarty]]

I see where is the issue. Let me work on a fix.

Comment 7

[Guillaume Marty [:gmarty]]

Attachment: Github PR

Etienne, here is a patch. Can you review it when you get a chance?

Comment 8

[Gregor Wagner [:gwagner]]

We need battery indicator working. Not sure if this is a regression though.

Comment 9

[Joshua Mitchell (Inactive)]

Removing regression window tag because a patch is already attached (awaiting review) and finding a window might waste resources and be unnecessary. If some reason the window is still needed or needed later, please re-tag it.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Gregor Wagner [:gwagner] from comment #8)
> We need battery indicator working. Not sure if this is a regression though.

This is clearly a regression per comment 2.

It's better if we could find the regressed bug and link it -- :gmarty should be able to help or we would have to re-flag regressionwindow-wanted. Clearly constructing the feature-regression relationship graph on Bugzilla will help us make better feature & engineering planning in the future.

Comment 11

[Michael Henretty [:mikehenrty][:mhenretty]]

Reflagging for regression window per comment 10.

Comment 12

[Etienne Segonzac (:etienne)]

Comment on attachment 8520696 [details] [review]
Github PR

Just a question on github but f+ otherwise.
Forwarding the final review to Alberto to get more eyes on the Statusbar code.

Comment 13

[Guillaume Marty [:gmarty]]

This bug is caused by Bug 1042105 so the regression window should incriminate commit https://github.com/mozilla-b2g/gaia/commit/6d02931dc9248edf66207066a0c01ebd3e279f8d.

Comment 14

[Alberto Pastor [:albertopq]]

Comment on attachment 8520696 [details] [review]
Github PR

It looks good to me. Thanks!

Comment 15

[Guillaume Marty [:gmarty]]

Landed in master in https://github.com/mozilla-b2g/gaia/commit/d13f29ef7b1656dbb555730dc161f89a1d25ce48

Comment 16

[Guillaume Marty [:gmarty]]

Comment on attachment 8520696 [details] [review]
Github PR

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Status bar icons prioritisation.
[User impact] if declined: There can be a lag between the time the battery state changes and the time this change is reflected in the minimised status bar.
[Testing completed]: This fix has a fairly good coverage in terms of unit tests, manual testing would be good too
[Risk to taking this patch] (and alternatives if risky): Very low, the change only affects a single function.
[String changes made]: None.

Comment 17

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue occurs on the earliest available Flame build so no regression window can be found.

Environmental Variables:
Device: Flame 2.0
BuildID: 20140418120005
Gaia: 76c94ac5dc3b8e17cc23d9cc3e2662b0d5d28b2e
Gecko: 45ba19361b97
Version: 31.0a1 (Master) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:31.0) Gecko/31.0 Firefox/31.0

Comment 18

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on 2.2.

Result: The battery indicator is displayed properly when the device is connected to a power source.

Device: Flame 2.2 (319mb, KK, Shallow Flash)
BuildID: 20141113040205
Gaia: be8b0151d2f9a4c41fc63952128e0b723cd1161d
Gecko: ab137ddd3746
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
============================================

Leaving verifyme for 2.1 patch uplift.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.1 uplift.

Comment 20

[Guillaume Marty [:gmarty]]

The rebased PR for v2.1 is here: https://github.com/mozilla-b2g/gaia/pull/26152

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/ce54015ff567630ccb63f96a9559b26e293564a4

Comment 23

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on 2.1.

Result: The battery indicator is displayed properly when the device is connected to a power source.

Device: Flame 2.1 (319mb, KK, Shallow Flash)
BuildID: 20141117001201
Gaia: 81160ad79e5b4c21967418dd63f1a1d08d77924e
Gecko: 3572aa3e6766
Version: 34.0 (2.1) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0