D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-12 Thread Nathaniel Graham
ngraham created this revision. ngraham added reviewers: Plasma, jgrulich. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. Herald added 1 blocking reviewer(s): jgrulich. ngraham requested review of this revision. REVISION SUMMARY Right now Bluetooth is unconditionally ena

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-13 Thread Jan Grulich
jgrulich added a comment. There is already a method to enable/disable BT, why don't you reuse it? REPOSITORY R116 Plasma Network Management Applet REVISION DETAIL https://phabricator.kde.org/D25279 To: ngraham, #plasma, jgrulich Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fb

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-13 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > handler.cpp:366-369 > +Handler::updatePreviousBluetoothStatus(); > m_tmpWirelessEnabled = NetworkManager::isWirelessEnabled(); > m_tmpWwanEnabled = NetworkManager::isWwanEnabled(); > enableBluetooth(false);

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-13 Thread Nathaniel Graham
ngraham added a comment. In D25279#561853 , @jgrulich wrote: > There is already a method to enable/disable BT, why don't you reuse it? Nothing checks the previous BT status, so by the time we get to the function to enable BT, it can't kno

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-13 Thread Jan Grulich
jgrulich added a comment. You are right, I didn't properly read what you do. I still think we should modify the already existing function to additionaly save the previous state, because most of the code is same. The only difference is "Set" vs "Get" method on DBus. REPOSITORY R116 Plasma

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-14 Thread Nathaniel Graham
ngraham updated this revision to Diff 69753. ngraham added a comment. --boilerplate REPOSITORY R116 Plasma Network Management Applet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25279?vs=69653&id=69753 BRANCH only-conditionally-re-enable-bluetooth-after-airplane-mode (branche

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-15 Thread Jan Grulich
jgrulich added a comment. We were actually saving already the value of current BT state, it was just not used. Your code assumes there is just one BT adapter, while in theory there might be more of them. What I would just is just simply not add property like m_tmpBluetoothEnabled and use m_b

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-15 Thread Nathaniel Graham
ngraham added a comment. Hmm, reading the code more closely, it looks like it should already work. In fact, when I remove all my changes... it does work! I could have sworn that it didn't work before though. Does this make any sense? Could it have been fixed by 7dd740aa963057c255fbbe833

D25279: [Applet] Only re-enable BT after disabling airplane mode if it was on before

2019-11-18 Thread Jan Grulich
jgrulich added a comment. In D25279#562998 , @ngraham wrote: > Hmm, reading the code more closely, it looks like it should already work. In fact, when I remove all my changes... it does work! I could have sworn that it didn't work before though.