<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/109418/">http://git.reviewboard.kde.org/r/109418/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 11th, 2013, 4:19 p.m. CET, <b>Dan Vrátil</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The patch looks fine in general.
Could you however look whether following solution would work? I think that cleaner approach would be if the backend would emit changed() when a new interface is added or removed. This way, if signals are first deliver to Manager (which therefore passes "outdated" Device to user) would not be a problem. Later, when the signals are delivered to the DeviceBackend, it would emit changed() and UI would be updated with the new values.
</pre>
</blockquote>
<p>On March 11th, 2013, 7:41 p.m. CET, <b>Alexander Mezin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It won't work. If signals are delivered to Manager first, then deviceAdded will be emitted even before InterfaceAdded will be delivered to Backend. And receivers of deviceAdded will read old property values before Backend emits changed(). Or maybe I didn't understand you.
Clean approach is reworking Manager/Backend in a way that only Manager or only Backend receives signals from D-Bus and propagates them to other. I consider this patch only as temporary workaround.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ok, I hope this won't become a permanent temporary solution :-) I'll try to come with a proper fix next week.</pre>
<br />
<p>- Dan</p>
<br />
<p>On March 11th, 2013, 7:50 p.m. CET, Alexander Mezin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid and Lukáš Tinkl.</div>
<div>By Alexander Mezin.</div>
<p style="color: grey;"><i>Updated March 11, 2013, 7:50 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">DeviceManager and DeviceBackends are subscribed to the same signals. DeviceManager's slots can be executed earlier than DeviceBackend's, so when deviceAdded signals are emitted by DeviceManager, DeviceBackend could contain non-updated data.
Usually the problem appears with optical disks: device notifier shows inserted disks as generic storage volumes or shows wrong label.
Possible workaround is forcing refresh of DeviceBackend's property cache before emitting deviceAdded signal. For optical disks drive's properties should also be updated, because OpticalDisc reads drive's properties</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Tested on KDE 4.10.1</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=314778">314778</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>solid/solid/backends/udisks2/udisksdevicebackend.h <span style="color: grey">(829fa41)</span></li>
<li>solid/solid/backends/udisks2/udisksdevicebackend.cpp <span style="color: grey">(2a01b86)</span></li>
<li>solid/solid/backends/udisks2/udisksmanager.h <span style="color: grey">(fb929ce)</span></li>
<li>solid/solid/backends/udisks2/udisksmanager.cpp <span style="color: grey">(71b9e3e)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109418/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>