ubuntu-system-settings

Merge lp://staging/~ken-vandine/ubuntu-system-settings/lp1337200 into lp://staging/ubuntu-system-settings

  1. lp1337200
  2. Merge into trunk
Proposed by Ken VanDine
Status: Rejected
Rejected by: Sebastien Bacher
Proposed branch: lp://staging/~ken-vandine/ubuntu-system-settings/lp1337200
Merge into: lp://staging/ubuntu-system-settings
Diff against target: 138 lines (+58/-31)
1 file modified
plugins/battery/plugin/battery-plugin.cpp (+58/-31)
To merge this branch: bzr merge lp://staging/~ken-vandine/ubuntu-system-settings/lp1337200
Reviewer Review Type Date Requested Status
Sebastien Bacher (community) Disapprove
Iain Lane (community) Needs Information
PS Jenkins bot continuous-integration Approve
Review via email: mp+241747@code.staging.launchpad.net

Commit message

Disconnect from upower on suspend and connect on resume

Description of the change

Disconnect from upower on suspend and connect on resume

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1203
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1783/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/204
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/139
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/53
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/204
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/204/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15814
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid/118
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/143
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/143/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1783/rebuild

review: Approve (continuous-integration)
lp://staging/~ken-vandine/ubuntu-system-settings/lp1337200 updated
1204. By Ken VanDine

Moved up_client_enumerate_devices_sync to the constructor, we only need to call it once.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1204
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1787/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/210
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/144
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-vivid-i386-ci/57
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/188
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/210
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/210/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15822
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-vivid/122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/148
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/148/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1787/rebuild

review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

One inline comment.

If we disconnect when suspended, what happens if a battery is added or removed then?

Can you add a test for this (how do you programatically suspend an application?) and fix it (call the callback on resuming) if it's broken?

review: Needs Information
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks, can you explain why we need to disconnect from upower? With a suspend device not a lot should be happening, if there are events generated/queued while the device is sleeping and not getting any interaction then that seems to be the real bug to fix. Or do we plan to patch every single upower client to workaround the issue?

review: Needs Information
Revision history for this message
Sebastien Bacher (seb128) wrote :

(just read the bug comments, still it feels like wrong to have to workaround in every client)

Revision history for this message
Ken VanDine (ken-vandine) wrote :

It doesn't really fix the issue either, I think the only solution is to really fix upower.

Revision history for this message
Sebastien Bacher (seb128) wrote :

setting as rejected then

review: Disapprove

Unmerged revisions

1204. By Ken VanDine

Moved up_client_enumerate_devices_sync to the constructor, we only need to call it once.

1203. By Ken VanDine

Disconnect from upower on suspend and connect on resume

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches