<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/101270/">http://git.reviewboard.kde.org/r/101270/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">All ok to me, except for a very minor remark I missed in the previous review. Good job (and this includes also what you are doing in Powerdevil :) )</pre>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="http://git.reviewboard.kde.org/r/101270/diff/3/?file=15790#file15790line254" style="color: black; font-weight: bold; text-decoration: underline;">solid/solid/devicemanager.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">Solid::Ifaces::Device *Solid::DeviceManagerPrivate::createBackendObject(const QString &udi)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">250</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">udi</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="n">backend</span><span class="o">-></span><span class="n">udiPrefix</span><span class="p">()))</span> <span class="k"><span class="hl">continue</span></span><span class="p"><span class="hl">;</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">254</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">udi</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="n">backend</span><span class="o">-></span><span class="n">udiPrefix</span><span class="p">()))</span> <span class="p"><span class="hl">{</span></span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">255</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">backend</span><span class="o">-></span><span class="n">udiPrefix</span><span class="p">()</span> <span class="o">!=</span> <span class="n">QLatin1String</span><span class="p">(</span><span class="n">UDEV_UDI_PREFIX</span><span class="p">)</span> <span class="o">||</span> <span class="o">!</span><span class="n">QFile</span><span class="o">::</span><span class="n">exists</span><span class="p">(</span><span class="n">udi</span><span class="p">))</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">256</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">continue</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">257</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Remember to always use brackets (in the second if). Also, you might want to merge the two ifs into one, but not a big deal.</pre>
</div>
<br />
<p>- Dario</p>
<br />
<p>On May 2nd, 2011, 12:01 p.m., Lamarque Vieira Souza wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Solid, Kevin Ottens and Alex Fiestas.</div>
<div>By Lamarque Vieira Souza.</div>
<p style="color: grey;"><i>Updated May 2, 2011, 12:01 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;">solid-hardware does not report my 3G modem's vendor and model names. This patch fix that:
[lamarque@evolucao ~]$ solid-hardware details /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1
udi = '/sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1'
parent = '/org/kde/solid/udev' (string)
vendor = 'Sony Ericsson' (string)
product = 'Sony Ericsson MD300' (string)
description = '' (string)
Block.major = 189 (0xbd) (int)
Block.minor = 175 (0xaf) (int)
Block.device = '/dev/bus/usb/002/048' (string)
The patch basically tests if /sys/devices/pci0000:00/0000:00:1d.0/usb2/2-1/2-1.1 exists, if affirmative it creates a UDevDevice, which makes it possible to report the available device details instead of just ignoring them completely.
'solid-hardware list' still does not list the device. I am still not sure if I should fix that since it would give the false impression the device is supported.
The motivation for this change is Plasma NM. The patch allows me inform modem's vendor and model names
in Mobile Connection Wizard (when creating Gsm/Cdma connections) and in PinDialog (when requesting PIN unlock code from user). It is basically usability fix.</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;">I have been using kdelibs-4.6.2 compiled with the patch since yesterday, not problems so far.</pre>
</td>
</tr>
</table>
<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/udev/udevdevice.cpp <span style="color: grey">(4f34382)</span></li>
<li>solid/solid/backends/udev/udevmanager.cpp <span style="color: grey">(e08fcde)</span></li>
<li>solid/solid/devicemanager.cpp <span style="color: grey">(59f32a7)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101270/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>