<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123388/">https://git.reviewboard.kde.org/r/123388/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 17th, 2015, 12:41 p.m. UTC, <b>Lamarque Souza</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/123388/diff/2/?file=361568#file361568line41" style="color: black; font-weight: bold; text-decoration: underline;">kded/bluetoothmonitor.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">41</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">38</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">addBluetoothConnection</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">bdAddr</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">service</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">connectionName</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is passing connectionName really necessary? The old code queries BlueZ to get the device name and use it as connectionName. That is something simular to using essid as connection name for wifi connections. Besides this particular change breaks compatibility with old Bluedevil versions. Is there a good reason for breaking compatibility here?</p></pre>
</blockquote>
<p>On April 17th, 2015, 12:53 p.m. UTC, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Old Bluedevil is using Bluez 4 and this code is for Bluez 5, so it's broken already.
I'd rather make sure that it will be called for valid device from Bluedevil, besides Network Manager won't add connection for invalid device.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I mean, using bluetooth device name for connection name is usefull, why just remove this feature alltogether instead of porting it to Bluez 5?</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 17th, 2015, 12:41 p.m. UTC, <b>Lamarque Souza</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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="https://git.reviewboard.kde.org/r/123388/diff/2/?file=361569#file361569line98" style="color: black; font-weight: bold; text-decoration: underline;">kded/bluetoothmonitor.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">82</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QRegExp</span> <span class="n">rx</span><span class="p">(</span><span class="s">"dun|rfcomm?|nap"</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">82</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="n">service</span> <span class="o">!=</span> <span class="n">QLatin1String</span><span class="p">(</span><span class="s">"dun"</span><span class="p">)</span> <span class="o">&&</span> <span class="n">service</span> <span class="o">!=</span> <span class="n">QLatin1String</span><span class="p">(</span><span class="s">"nap"</span><span class="p">))</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Are you sure regular expression "rfcomm?" does not appear in service variable anymore? Years ago when I tested this it used to appear sometimes.</p></pre>
</blockquote>
<p>On April 17th, 2015, 12:53 p.m. UTC, <b>David Rosca</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It does nothing for "rfcomm?" service. Only "dun" and "nap" is handled (before and after this patch).</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ok, the rfcomm part if from my original code and the code I wrote that used rfcomm is not in Plasma NM anymore.</p></pre>
<br />
<p>- Lamarque</p>
<br />
<p>On April 17th, 2015, 11:40 a.m. UTC, David Rosca wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Solid, Jan Grulich, Lukáš Tinkl, and Lamarque Souza.</div>
<div>By David Rosca.</div>
<p style="color: grey;"><i>Updated April 17, 2015, 11:40 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-nm
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch removes all Bluez related code from BluetoothMonitor.
It was used only to check if the device with specified address exists and if it supports nap/dun profile.
The addBluetoothConnection method is currently not used anywhere, so this shouldn't be an issue. If the check is important, we can do one call on Bluedevil DBus interface.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Bluedevil was using this method in Bluez 4 versions, but it got killed during porting to Bluez 5.
NAP connections are now automatically added on successful pairing with the device.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I plan to bring the functionality back in Bluedevil, so that user can manually add a network connection (DUN/NAP) for Bluetooth devices.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">lxr search: http://lxr.kde.org/search?v=kf5-qt5&_filestring=&_string=%22addBluetoothConnection%22</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Works as before, mobile connection wizard appears when trying to add dun connection.</p></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>kded/bluetoothdbustype.h <span style="color: grey">(1eeb3b2)</span></li>
<li>kded/bluetoothdbustype.cpp <span style="color: grey">(4eee8af)</span></li>
<li>kded/bluetoothmonitor.h <span style="color: grey">(f5e74ec)</span></li>
<li>kded/bluetoothmonitor.cpp <span style="color: grey">(9d833d2)</span></li>
<li>kded/dbus/org.freedesktop.DBus.ObjectManager.xml <span style="color: grey">(fa58c72)</span></li>
<li>kded/dbus/org.freedesktop.DBus.Properties.xml <span style="color: grey">(a71f409)</span></li>
<li>kded/CMakeLists.txt <span style="color: grey">(51aa370)</span></li>
<li>kded/monitor.h <span style="color: grey">(ae89d3e)</span></li>
<li>kded/monitor.cpp <span style="color: grey">(e33b918)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123388/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>