Telephony and SMS handling are quite distinct so they should be in separate plugins for better maintainability, given that @sredman has big plans with SMS.
This diff should be fully backwards compatible, but whether we really want to do that is up to discussion
Details
- Reviewers
sredman - Group Reviewers
KDE Connect - Commits
- R224:bcc9fb06dbf6: Split SMS and Telephony plugin on desktop
Only supeficially tested. Receive an SMS (old way), Notification is shown
Diff Detail
- Repository
- R224 KDE Connect
- Branch
- smsrefactor
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 141 Build 141: arc lint + arc unit
Yes, in a sense, but this branch is the next thing I intend to work on... Once the code is cleaned up, the path forward for improvements should be clearer
I am getting lots of printed warnings that the Telepathy interfaces does not seem to be working. I don't have telepathy set up right now so this is not a big surprise. The cause appears to be a missing check for whether the interface is valid (see inline comments)
Also, after rebasing or merging master the logic for connecting message addresses with KPeople seems to be broken so we don't get names or icons in the SMS gui list. I have not looked into why this is.
plugins/sms/smsplugin.cpp | ||
---|---|---|
72 | In the master branch, this check has been brought within the forwardToTelepathy method | |
185 | This line is causing an invalid call to the telepathy dbus interface since we have not asked if m_telepathyInterface.isValie() | |
plugins/telephony/telephonyplugin.cpp | ||
211 | This check should either be brought back within this method or bumped outside in all uses |
This does not appear to be due to this patch. My contacts are not synchronizing for some reason.
This has been working for me for a couple of weeks, so I think we're good. The tiny change I just pushed - to check the Telepathy interface before calling it - avoids an error and doesn't change any functionality (since it didn't work before anyway!)
plugins/sms/kdeconnect_sms.json | ||
---|---|---|
12 | Also perhaps we should find a new icon for this. How do I look at the list of available icons? |
plugins/sms/kdeconnect_sms.json | ||
---|---|---|
12 | cuttlefish is a tool for that |