Closed Bug 1127226 Opened 7 years ago Closed 7 years ago

[Wifi] Modify wifi power saving mode on gaia.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-master fixed)

RESOLVED FIXED
2.2 S8 (20mar)
Tracking Status
b2g-master --- fixed

People

(Reporter: amchung, Assigned: amchung, Mentored)

Details

Attachments

(1 file, 1 obsolete file)

Base on Bug 1084156 [WIFI] While screen on WiFi do not enter power save mode when there is no traffic, already added power saving when wifi init.
So it doesn't need to turn on power saving mode when screen off and unplugged.
Attachment #8556350 - Attachment is obsolete: true
Dear Seinlin,
I have removed the power saving mode when screen off and unplugged, and modified unit test case.
Please help me to confirm this modification.
Flags: needinfo?(kli)
Attachment #8568438 - Flags: feedback?(kli)
I left comments on PR, please have a look.
Flags: needinfo?(kli)
Dear Seinlin,
I have removed the power saving mode when wifi wake lock, and modified unit test case.
Please help me to confirm this modification.

Thanks!
Flags: needinfo?(kli)
Amy, I notice the following two points:

1. I think 'Test wifi should be turned on and in power saving mode' should be modified only. It should not be removed entirely.

2. Please also modify 'Test wifi should stay disabled', as comment on PR.

Could you also go through other test cases with similar conditions?
Flags: needinfo?(kli)
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

Cancel the request. Ping me again when you finished as my comment.
Attachment #8568438 - Flags: feedback?(kli)
Dear Seinlin,
I modified the unit test case and sleep function in wifi.js.
Please help me to review it.
Thanks!
Flags: needinfo?(kli)
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

Arthur, In bug 1084156, wifi power save mode is controlled in gecko. So I think the control in system app should be removed.

Could you review this patch? Thanks!
Flags: needinfo?(kli)
Attachment #8568438 - Flags: review?(arthur.chen)
Attachment #8568438 - Flags: feedback+
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

It would be better to have system owner/peer review this patch. Redirect the request to Alive.
Attachment #8568438 - Flags: review?(arthur.chen) → review?(alive)
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

There is one testing failure and several linting errors, please fix them.
The unit test failure is because you removed the declaration of 'request' but used it at https://github.com/tefn3849/gaia/blob/wifi_1127226/apps/system/js/wifi.js#L284
Attachment #8568438 - Flags: review?(alive)
Dear Seinlin,
I have modified it, please help me to confirm it.
Thanks!
Flags: needinfo?(kli)
From try result, unit test is passed. But there is a gjslint error
--
TEST-UNEXPECTED-FAIL | apps/system/test/unit/wifi_test.js | Line 443, E:0121: Illegal comma at end of object literal
--

You should fix this and then request for review from alive.
Flags: needinfo?(kli)
Dear Seinlin,
I have modified it, please help me to confirm it.
Thanks!
Flags: needinfo?(kli)
Test result looks good.
Flags: needinfo?(kli)
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

Dear Alive,
Please help me to review this patch.
Thanks!
Attachment #8568438 - Flags: review?(alive)
Comment on attachment 8568438 [details] [review]
Modified power saving mode when screen off and unplugged.

r=me with nits addressed: remove unwanted stub and restore.
Attachment #8568438 - Flags: review?(alive) → review+
Keywords: checkin-needed
Master: https://github.com/mozilla-b2g/gaia/commit/8085188e5c723c43d76d386a626e03a713748d98
Assignee: nobody → amchung
Status: NEW → RESOLVED
Closed: 7 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S8 (20mar)
You need to log in before you can comment on or make changes to this bug.