D6116: Add new stateChanged() signal to active connection
jgrulich updated this revision to Diff 15566. jgrulich added a comment. Revert unwanted change REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6116?vs=15564&id=15566 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6116 AFFECTED FILES src/activeconnection.cpp src/activeconnection.h src/activeconnection_p.h src/fakenetwork/activeconnection.h src/fakenetwork/fakenetwork.cpp src/vpnconnection.h To: jgrulich, lvsouza Cc: anthonyfieroni, davidedmundson, #frameworks
D6116: Add new stateChanged() signal to active connection
jgrulich updated this revision to Diff 15564. jgrulich added a comment. Fix enum name and avoid ABI breakage with overriden signal REPOSITORY R282 NetworkManagerQt CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D6116?vs=15210&id=15564 BRANCH master REVISION DETAIL https://phabricator.kde.org/D6116 AFFECTED FILES CMakeLists.txt src/activeconnection.cpp src/activeconnection.h src/activeconnection_p.h src/fakenetwork/activeconnection.h src/fakenetwork/fakenetwork.cpp src/vpnconnection.h To: jgrulich, lvsouza Cc: anthonyfieroni, davidedmundson, #frameworks
D6116: Add new stateChanged() signal to active connection
jgrulich added a comment. In https://phabricator.kde.org/D6116#116560, @davidedmundson wrote: > > can you find a new signal name and emit both for compatibility? What would you suggest? I can think of stateChangedWithReason() or stateChangedWithKnownReason() or something like that. REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6116 To: jgrulich, lvsouza Cc: anthonyfieroni, davidedmundson, #frameworks
D6116: Add new stateChanged() signal to active connection
anthonyfieroni added inline comments. INLINE COMMENTS > activeconnection.h:77 > +DeviceRealizeFailedReason, /**< Could not create the software device > link. */ > +DeviceRemoved /** < The device this connection depended on > disappeared. */ > +}; All others are composed <-- real reason --> ## Reason, but this :) REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6116 To: jgrulich, lvsouza Cc: anthonyfieroni, davidedmundson, #frameworks
D6116: Add new stateChanged() signal to active connection
davidedmundson added a comment. > There is just one minor problem, with two stateChanged() signals you will need to specify to which signal you want > to connect to when using Qt5 syntax for signals/slots and without that you won't compile current code, i.e. of plasma-nm I think we're going to have to treat overloading signals as an API break. Current plasma needs to compile with new frameworks. There's a line in the Qt wiki page that implies they now have that policy. "… but we have been adding overloads in past minor releases of Qt because taking the address of a function was not a use case we support. But now this would be impossible without breaking the source compatibility. " can you find a new signal name and emit both for compatibility? REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6116 To: jgrulich, lvsouza Cc: davidedmundson, #frameworks
D6116: Add new stateChanged() signal to active connection
jgrulich created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY There is a new additional StateChanged() signal coming from NetworkManager with additional reason property so with that we can now check what was the reason for the changed state. Use this new signal when NM 1.8.0 is present and emit also the old one for backward compatibility. There is just one minor problem, with two stateChanged() signals you will need to specify to which signal you want to connect to when using Qt5 syntax for signals/slots and without that you won't compile current code, i.e. of plasma-nm REPOSITORY R282 NetworkManagerQt BRANCH master REVISION DETAIL https://phabricator.kde.org/D6116 AFFECTED FILES src/activeconnection.cpp src/activeconnection.h src/activeconnection_p.h src/fakenetwork/activeconnection.h src/fakenetwork/fakenetwork.cpp src/vpnconnection.h To: jgrulich Cc: #frameworks
D6116: Add new stateChanged() signal to active connection
jgrulich added a reviewer: lvsouza. REPOSITORY R282 NetworkManagerQt REVISION DETAIL https://phabricator.kde.org/D6116 To: jgrulich, lvsouza Cc: #frameworks