D16170: [Device Notifier] Restore busy indicator
This revision was automatically updated to reflect the committed changes. Closed by commit R120:a7b2ecdb12b8: [Device Notifier] Restore busy indicator (authored by thsurrel). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16170?vs=46093&id=49060 REVISION DETAIL https://phabricator.kde.org/D16170 AFFECTED FILES applets/devicenotifier/package/contents/ui/DeviceItem.qml applets/devicenotifier/package/contents/ui/devicenotifier.qml To: thsurrel, #plasma, #vdg, broulik, bruns, ngraham, davidedmundson Cc: mart, cfeck, ngraham, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D16170: [Device Notifier] Restore busy indicator
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Thanks, this works great. REPOSITORY R120 Plasma Workspace BRANCH arc_busyindicator (branched from master) REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns, ngraham Cc: mart, cfeck, ngraham, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D16170: [Device Notifier] Restore busy indicator
thsurrel updated this revision to Diff 46093. thsurrel added a comment. Update the storage size every 5 seconds Thank you for catching this one @mart ! REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D16170?vs=43513&id=46093 BRANCH arc_busyindicator (branched from master) REVISION DETAIL https://phabricator.kde.org/D16170 AFFECTED FILES applets/devicenotifier/package/contents/ui/DeviceItem.qml applets/devicenotifier/package/contents/ui/devicenotifier.qml To: thsurrel, #plasma, #vdg, broulik, bruns Cc: mart, cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D16170: [Device Notifier] Restore busy indicator
mart added a comment. Most of the solidevice engine should not relay on polling.. after a quick glance at its code: can you test if the free space bar is still updating correctly after copying big files from and to? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: mart, cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
D16170: [Device Notifier] Restore busy indicator
thsurrel added a comment. Ping ! Anyone could have a look at this patch ? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
thsurrel added a comment. It is awaiting a review. As I said in an older comment, I don't know if there are some drawbacks to not do polling, but i have been using this patch for a while and it works well. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
cfeck added a comment. What is the status of this patch? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
ngraham added a comment. FWIW, I can confirm that https://bugs.kde.org/show_bug.cgi?id=399986 is fixed with this. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
ngraham added reviewers: broulik, bruns. ngraham added a comment. I have no understanding of this code, but am adding some reviewers who do. :) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg, broulik, bruns Cc: ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
thsurrel added a comment. The bad news with this patch is that I don't understand it fully. Why were we doing polling before ? Why was it breaking the 'state' propagation ? What does it change not to do it anymore (performance hit ? something else ?) ? REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D16170 To: thsurrel, #plasma, #vdg Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D16170: [Device Notifier] Restore busy indicator
thsurrel created this revision. thsurrel added reviewers: Plasma, VDG. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. thsurrel requested review of this revision. REVISION SUMMARY This patch fixes two issues: - the 'state' of the device now propagates correctly, that makes the busy indicator be displayed again - there is no more delay between the moment when the device is actually mounted and the moment the device notifier says it is. The worst case used to be 5 seconds. BUG: 354321 REPOSITORY R120 Plasma Workspace BRANCH arc_busyindicator (branched from master) REVISION DETAIL https://phabricator.kde.org/D16170 AFFECTED FILES applets/devicenotifier/package/contents/ui/DeviceItem.qml applets/devicenotifier/package/contents/ui/devicenotifier.qml To: thsurrel, #plasma, #vdg Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
