- We do not need the return type. If a plugin declares it can handle the
packet it should do so. We don't have any fallback logic in place and
the packet types are namespaced with the plugin IDs anyway.
- Provide a default implementation with a warning, not all plugins need
to overwrite this
This is not needed, because only a symbol to create the KPluginFactory
instance must be exported. The KPluginFactory macros or rather the underlying
Q_PLUGIN_METADATA macro already take care of that.
In other plugin code of KDE, we also do not export classes.
The size of the generated plugin files is nearly identical, but removing
the macros avoids confusion for developers.
## Summary
This patch adds an interface to return only a specified window of messages, making loading the conversations history smooth, fast, and enjoyable.
The current implementation of the conversation interface loads all messages every time the conversation is requested. This is might be painfully slow to load in case the conversation is large or if there are a lot of MMS/RCS messages in the conversation (since those are wildly slower to load than SMS)
Requires https://invent.kde.org/kde/kdeconnect-android/merge_requests/122 to enable Android functionality
## Test Plan
- With new Android app and old Desktop app:
- The Android app will notice the missing fields and query for all messages as before.
- With old Android app and new Desktop app:
- The desktop will send fields for the new interface which will not be read and all messages will be returned.
- With new Android app and new Desktop app:
- The new interface is used and returns only a certain number of messages at a time.
While mostly theoretical it makes for a more expressive API since it communicates that it is a list instead of a single value. It also allows to get rid of some conversion code.
This is also preparation work for potentially using the custom type capabilities that QtDBus offers.
Summary:
After using the ConversationsDbusInterface for a little while, there can be significant (MBs) memory usage of cached messages. The QDBusAbstractAdaptor does not like to be manually deleted, but it looks like it is safe to do so after constructing a new one
This contradicts the comment in the BatteryPlugin and the BatteryDbusInterface, which says deletelater() is not safe. Has Qt been updated since then?
Test Plan:
- Run daemon
- Hopefully experience no crashes after many phone reconnects
Reviewers: #kde_connect, apol, albertvaka
Reviewed By: #kde_connect, apol, albertvaka
Subscribers: apol, kdeconnect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D16553
Summary:
The most serious change from this patch is to move the asynchronous replying to a request from the app for more messages to a newly-spawned, self-destructing thread. Within that thread, we block until the remote device replies with the requested messages.
All gotten messages are cached in the ConversationDbusInterface, so all future requests are fast and don't hit the remote device.
Test Plan: After applying this diff, the messaging app should show 10 messages every time it is opened
Reviewers: #kde_connect, nicolasfella, albertvaka
Reviewed By: #kde_connect, albertvaka
Subscribers: albertvaka, apol, nicolasfella, kdeconnect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D16475
Summary:
Add "event" field to ConversationMessage
Update packet type with proper field names and commenting
Future "proof" SmsPlugin against new event types
Test Plan:
- Install the corresponding Android-side patch D16600
- Verify that it is possible to synchronize messages the same as it was before
Reviewers: #kde_connect, apol
Reviewed By: #kde_connect, apol
Subscribers: apol, kdeconnect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D16599
Summary: Braces to start a method are on a newline, braces to begin an in-method block are on the same line
Test Plan: Pure source code cosmetic changes. Hopefully no functionality has changed!
Reviewers: #kde_connect, nicolasfella
Reviewed By: #kde_connect, nicolasfella
Subscribers: apol, nicolasfella, kdeconnect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D15978
Summary:
Drop support for creating notifications from the SMS plugin. The old usecase is better handled by the notifications plugin reply box anyway
Rename packets defined in SMS plugin from PACKET_TYPE_TELEPHONY_* to PACKET_TYPE_SMS_*
Update TELEPHONY plugin packet description to point to SMS plugin
Define TELEPHONY_REQUEST_MUTE packet to replace old TELEPHONY_REQUEST with mute event field
Please see Android-side patch here: D15544
Test Plan:
I see four test cases, based on the version of the app being used, where "old" means any version built with sources not containing this revision and "new" means any version built with sources using this revision:
- New Android vs. New Desktop
- Supported and works for me
- New Android vs. Old Desktop
- Supported and works for me
- Old Android vs. New Desktop
- Not supported - Download a new app from the Play store or F-Droid
- Old Android vs. Old Desktop
- If this is broken, it is not my fault :)
In the supported use cases:
- Test SMS reply
- Receive SMS (or MMS!) message
- Verify that the //notification// plugin forwards a replyable notification and that replying works
- Test incoming call ringer mute
- Enable ringer volume (not vibrate)
- Receive phone call
- Verify a desktop notification with a Mute button appears
- Verify that clicking the mute button causes the phone to stop making the ringer noise (vibration not affected)
Reviewers: #kde_connect, albertvaka, nicolasfella
Reviewed By: #kde_connect, albertvaka, nicolasfella
Subscribers: nicolasfella, kdeconnect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D15543
Summary:
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
Test Plan: Only supeficially tested. Receive an SMS (old way), Notification is shown
Reviewers: #kde_connect, sredman
Reviewed By: sredman
Subscribers: albertvaka, apol, sredman, kdeconnect, #kde_connect
Tags: #kde_connect
Differential Revision: https://phabricator.kde.org/D13594
