Re: [Kde-hardware-devel] Review Request 121606: Port bluetooth connection support to bluez 5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121606/#review72500 --- kded/bluetoothmonitor.h https://git.reviewboard.kde.org/r/121606/#comment50535 Move this line to the class' private section. kded/bluetoothmonitor.h https://git.reviewboard.kde.org/r/121606/#comment50555 Code style: asterisk closer to variable name. kded/bluetoothmonitor.h https://git.reviewboard.kde.org/r/121606/#comment50556 Same here. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50557 I think this line can be removed now. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50536 Use camel case in oldowner and newowner. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50537 code style: asterisk is closer to the variable name, not the type. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50538 Same here. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50539 Code style: start comments with capital letter and end them with period. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50541 Code style: add one space between type and asterisk (QDBusPendingCallWatherspace*). kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50540 Code style: closer to the variable name. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50542 Code style: space after if. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50543 Code style: asterisk closer to variable name. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50544 Same here. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50545 Comment with inicial capital letter and period at the end. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50547 Code style: comments starting with capital letter and period at the end. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50548 Code style: capital letter at the start and period at the end. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50549 Code style: asterisk closer to variable name. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50550 Code style. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50551 Code style. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50552 Code style. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50553 Code style. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50558 When I implemented support for bluetooth connection I remember Alex Fiestas asking me to override if there was a connection with the same bluetooth address. That was decided like that because Bluetooth only presents the dialog to activate network service for the phone right after the pairing process. If we do not override the connection here the user will have to delete the connection in Connection Managerm unpair de bluetooth device and pair it again. By overriding we do not force the user to manually delete the connection. kded/bluetoothmonitor.cpp https://git.reviewboard.kde.org/r/121606/#comment50554 Code style: prepend new line here and move the closer to the variable name. - Lamarque Souza On Dec. 25, 2014, 2:58 a.m., Xuetian Weng wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121606/ --- (Updated Dec. 25, 2014, 2:58 a.m.) Review request for Solid, Jan Grulich, Lukáš Tinkl, and Lamarque Souza. Repository: plasma-nm Description --- 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. 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) bluetoothdbustype.cpp is used because there's metatype conflict with libnm-qt, so declare the type in a separate file to avoid this. Diffs - kded/bluetoothmonitor.cpp 3aaf701 kded/dbus/org.freedesktop.DBus.Properties.xml PRE-CREATION kded/dbus/org.freedesktop.DBus.ObjectManager.xml PRE-CREATION kded/bluetoothmonitor.h 5f43369 kded/bluetoothdbustype.cpp PRE-CREATION kded/CMakeLists.txt 910f5fa kded/bluetoothdbustype.h PRE-CREATION Diff:
Re: [Kde-hardware-devel] Review Request 121606: Port bluetooth connection support to bluez 5
On des. 20, 2014, 8:25 a.m., Jan Grulich wrote: 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. Jan Grulich wrote: 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. Xuetian Weng wrote: 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). 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. which means in those distribution, bluetooth dun doesn't work with NM at all. The only distribution left is ubuntu, which might also use bluez 5 for their next release http://www.phoronix.com/scan.php?page=news_itempx=MTgzNzM . 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? Xuetian Weng wrote: 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. Jan Grulich wrote: Add please Lukáš Tinkl and Lamarque Souza to reviewers. Lamarque Souza wrote: 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. There is (a bluedevil bluez5 plasma5) - Àlex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121606/#review72331 --- On des. 25, 2014, 2:58 a.m., Xuetian Weng wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121606/ --- (Updated des. 25, 2014, 2:58 a.m.) Review request for Solid, Jan Grulich, Lukáš Tinkl, and Lamarque Souza. Repository: plasma-nm Description --- 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. 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) bluetoothdbustype.cpp is used because there's metatype conflict with libnm-qt, so declare the type in a separate file to avoid this. Diffs - kded/bluetoothmonitor.cpp 3aaf701 kded/dbus/org.freedesktop.DBus.Properties.xml PRE-CREATION kded/dbus/org.freedesktop.DBus.ObjectManager.xml PRE-CREATION kded/bluetoothmonitor.h 5f43369 kded/bluetoothdbustype.cpp PRE-CREATION kded/CMakeLists.txt 910f5fa kded/bluetoothdbustype.h PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121606/diff/ Testing --- qdbus org.kde.plasmanetworkmanagement /org/kde/plasmanetworkmanagement org.kde.plasmanetworkmanagement.addBluetoothConnection [macaddress] dun can now create connection. Thanks, Xuetian Weng ___ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
[Kde-hardware-devel] Review Request 121673: Enable fstab and upower backends on FreeBSD
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121673/ --- Review request for Solid. Repository: solid Description --- Enable fstab and upower backends on FreeBSD Diffs - src/solid/devices/managerbase.cpp 9bfff0a Diff: https://git.reviewboard.kde.org/r/121673/diff/ Testing --- Ran solid-hardware, got my battery from UPower. Thanks, arrowdodger arrowdodger ___ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
Re: [Kde-hardware-devel] Review Request 121606: Port bluetooth connection support to bluez 5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121606/ --- (Updated Dec. 26, 2014, 7:21 a.m.) Review request for Solid, Jan Grulich, Lukáš Tinkl, and Lamarque Souza. Repository: plasma-nm Description --- 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. 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) bluetoothdbustype.cpp is used because there's metatype conflict with libnm-qt, so declare the type in a separate file to avoid this. Diffs (updated) - kded/CMakeLists.txt 910f5fa kded/bluetoothdbustype.h PRE-CREATION kded/bluetoothdbustype.cpp PRE-CREATION kded/bluetoothmonitor.h 5f43369 kded/bluetoothmonitor.cpp 3aaf701 kded/dbus/org.freedesktop.DBus.ObjectManager.xml PRE-CREATION kded/dbus/org.freedesktop.DBus.Properties.xml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/121606/diff/ Testing --- qdbus org.kde.plasmanetworkmanagement /org/kde/plasmanetworkmanagement org.kde.plasmanetworkmanagement.addBluetoothConnection [macaddress] dun can now create connection. Thanks, Xuetian Weng ___ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
[Kde-hardware-devel] Review Request 121677: Fix mobile connection wizard manual apn setting
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121677/ --- Review request for Solid and Lukáš Tinkl. Repository: plasma-nm Description --- Current problems: 1. if user selects manual provider, apn text is not editable. 2. if user select not listed in apn page, and go back to provider page and go next again, apn text will be editable while it shouldn't. 3. a redundant separator if there's on My plan is not listed This change reuses slotEnablePlanEditBox function to set mApnText to correct state, and remove a separator if it's manually set provider. Diffs - libs/editor/widgets/mobileconnectionwizard.cpp 29ce4f7 Diff: https://git.reviewboard.kde.org/r/121677/diff/ Testing --- problems above are fixed. Thanks, Xuetian Weng ___ Kde-hardware-devel mailing list Kde-hardware-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-hardware-devel
