<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/121606/">https://git.reviewboard.kde.org/r/121606/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 20th, 2014, 8:25 a.m. UTC, <b>Jan Grulich</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;">I definitely don't want to drop support for Bluez 4, it will take some time to get distributions use NetworkManager 1.0.0 so your patch would break it for those using older NM versions.</p></pre>
</blockquote>
<p>On December 20th, 2014, 8:34 a.m. UTC, <b>Jan Grulich</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;">I mean we have to support older NM versions which requires also ModemManager to make this work. Even libnm-qt supports older NM versions. I'll take a look properly on your patch when I have more time.</p></pre>
</blockquote>
<p>On December 20th, 2014, 5:58 p.m. UTC, <b>Xuetian Weng</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;">But think about it:
1. network manager prior to 0.9.8 doesn't support bluez 5 dun connection. (Fedora patches NM for 0.9.8)
2. bluedevil in plasma 5 only works with bluez 5. (Though there's no release yet, latest bluedevil 2.0 release can also only work with bluez 5).
3. that function is only called from bluedevil user interface (which is currently dropped in bluedevil because nm doesn't support it).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Distribution who wants to use bluedevil in kf5 must use bluez 5.
opensuse use bluez 5 since 13.1
fedora use bluez 5 since fedora 20.
arch is using bluez 5 since 2013.5.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">which means in those distribution, bluetooth dun doesn't work with NM at all.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The only distribution left is ubuntu, which might also use bluez 5 for their next release http://www.phoronix.com/scan.php?page=news_item&px=MTgzNzM .</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is it possible to use old bluedevil that work with bluez 4 and bluez 4 on a distribution? That means, bluedevil is not ported to kde5, and it still uses kded4, the bluedevil kded module is not even loaded in non-kde4 environment. Why would they package plasma 5 with such old bluez then?</p></pre>
</blockquote>
<p>On December 20th, 2014, 6:56 p.m. UTC, <b>Xuetian Weng</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;">Oh sorry, there's one small mistake: fedora's patch is for nm 0.9.10, here's the report https://bugzilla.redhat.com/show_bug.cgi?id=1055628.</p></pre>
</blockquote>
<p>On December 23rd, 2014, 12:32 p.m. UTC, <b>Jan Grulich</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;">Add please Lukáš Tinkl and Lamarque Souza to reviewers.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Weng has a point. Only Bluedevil uses this code, if there is a Bluez 5 and Plasma 5 version of Bluedevil available then it makes sense to drop support for Bluez 4. Let's face it this code is a kind of horrible, it needs a rewrite even. It does not support more than one bluetooth controller either, luckily for us that is not a common setup :-) Plasma 5 is not mainstream yet, there will be time for distributions to add Bluez 5 to their package suites, if they have not done so yet.</p></pre>
<br />
<p>- Lamarque</p>
<br />
<p>On December 23rd, 2014, 4:26 p.m. UTC, Xuetian Weng 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 Xuetian Weng.</div>
<p style="color: grey;"><i>Updated Dec. 23, 2014, 4:26 p.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;">bluez 5 + networkmanager 1.0.0 suppose to support dun connection, bluedevil for kf5 IMHO will support bluez 5. So this patch also drops bluez4 support completely.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">nm 1.0.0's dun support doesn't require modemmanager AFAIK, so that part is dropped in this patch. (Not quite sure if nap is supported or not)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">bluetoothdbustype.cpp is used because there's metatype conflict with libnm-qt, so declare the type in a separate file to avoid this.</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;">qdbus org.kde.plasmanetworkmanagement /org/kde/plasmanetworkmanagement org.kde.plasmanetworkmanagement.addBluetoothConnection [macaddress] dun can now create 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/CMakeLists.txt <span style="color: grey">(910f5fa)</span></li>
<li>kded/bluetoothdbustype.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kded/bluetoothdbustype.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kded/bluetoothmonitor.h <span style="color: grey">(5f43369)</span></li>
<li>kded/bluetoothmonitor.cpp <span style="color: grey">(3aaf701)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/121606/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>