Differential D11854 Diff 31078 plugins/telephony/telephonyplugin.cpp

Changeset View

Standalone View

View Options

plugins/telephony/telephonyplugin.cpp

Show All 39 Lines
40} 40​}
41 41​
42KNotification* TelephonyPlugin::createNotification(const NetworkPacket& np) 42​KNotification* TelephonyPlugin::createNotification(const NetworkPacket& np)
43{ 43​{
44 const QString event = np.get<QString>(QStringLiteral("event")); 44​ const QString event = np.get<QString>(QStringLiteral("event"));
45 const QString phoneNumber = np.get<QString>(QStringLiteral("phoneNumber"), i18n("unknown number")); 45​ const QString phoneNumber = np.get<QString>(QStringLiteral("phoneNumber"), i18n("unknown number"));
46 const QString contactName = np.get<QString>(QStringLiteral("contactName"), phoneNumber); 46​ const QString contactName = np.get<QString>(QStringLiteral("contactName"), phoneNumber);
47 const QByteArray phoneThumbnail = QByteArray::fromBase64(np.get<QByteArray>(QStringLiteral("phoneThumbnail"), "")); 47​ const QByteArray phoneThumbnail = QByteArray::fromBase64(np.get<QByteArray>(QStringLiteral("phoneThumbnail"), ""));
48 48​
apolUnsubmitted
Done

Instead of passing "" or QLatin1String(""), pass QString() or even {}.

apol: Instead of passing "" or QLatin1String(""), pass `QString()` or even `{}`.
49 // In case telepathy can handle the message, don't do anything else 49​ // In case telepathy can handle the message, don't do anything else
50 if (event == QLatin1String("sms") && m_telepathyInterface.isValid()) { 50​ if (event == QLatin1String("sms") && m_telepathyInterface.isValid()) {
51 qCDebug(KDECONNECT_PLUGIN_TELEPHONY) << "Passing a text message to the telepathy interface"; 51​ qCDebug(KDECONNECT_PLUGIN_TELEPHONY) << "Passing a text message to the telepathy interface";
52 connect(&m_telepathyInterface, SIGNAL(messageReceived(QString,QString)), SLOT(sendSms(QString,QString)), Qt::UniqueConnection); 52​ connect(&m_telepathyInterface, SIGNAL(messageReceived(QString,QString)), SLOT(sendSms(QString,QString)), Qt::UniqueConnection);
53 const QString messageBody = np.get<QString>(QStringLiteral("messageBody"),QLatin1String("")); 53​ const QString messageBody = np.get<QString>(QStringLiteral("messageBody"),QLatin1String(""));
54 QDBusReply<bool> reply = m_telepathyInterface.call(QStringLiteral("sendMessage"), phoneNumber, contactName, messageBody); 54​ QDBusReply<bool> reply = m_telepathyInterface.call(QStringLiteral("sendMessage"), phoneNumber, contactName, messageBody);
55 if (reply) { 55​ if (reply) {
56 return nullptr; 56​ return nullptr;
57 } else { 57​ } else {
58 qCDebug(KDECONNECT_PLUGIN_TELEPHONY) << "Telepathy failed, falling back to the default handling"; 58​ qCDebug(KDECONNECT_PLUGIN_TELEPHONY) << "Telepathy failed, falling back to the default handling";
apolUnsubmitted
Done

Shouldn't this be !m_telepathyInterface.isValid()?

apol: Shouldn't this be `!m_telepathyInterface.isValid()`?
sredmanAuthorUnsubmitted
Done

No, this is handling the case where the telepathy interface was valid. If the message has been passed to telepathy, it does not keep trying to process it using a notification

sredman: No, this is handling the case where the telepathy interface was valid. If the message has been…
59 } 59​ }
60 } 60​ }
61 61​
62 QString content, type, icon; 62​ QString content, type, icon;
63 KNotification::NotificationFlags flags = KNotification::CloseOnTimeout; 63​ KNotification::NotificationFlags flags = KNotification::CloseOnTimeout;
64 64​
65 const QString title = device()->name(); 65​ const QString title = device()->name();
66 66​
▲ Show 20 Lines • Show All 64 Lines • ▼ Show 20 Line(s)125{
131 131​
132 KNotification* n = createNotification(np); 132​ KNotification* n = createNotification(np);
133 if (n != nullptr) n->sendEvent(); 133​ if (n != nullptr) n->sendEvent();
134 134​
135 return true; 135​ return true;
136} 136​}
137 137​
138void TelephonyPlugin::sendMutePacket() 138​void TelephonyPlugin::sendMutePacket()
139{ 139​{
nicolasfellaUnsubmitted
Done

"->this" is weird, never seen it in a KDE Project

nicolasfella: "->this" is weird, never seen it in a KDE Project
140 NetworkPacket packet(PACKET_TYPE_TELEPHONY_REQUEST, {{"action", "mute"}}); 140​ NetworkPacket packet(PACKET_TYPE_TELEPHONY_REQUEST, {{"action", "mute"}});
141 sendPacket(packet); 141​ sendPacket(packet);
142} 142​}
nicolasfellaUnsubmitted
Done

Else if?

nicolasfella: Else if?
sredmanAuthorUnsubmitted
Done

At this revision I hadn't figured out what to do with old-style packets. I have decided to support them fully (meaning old Android apps would continue to function as they currently do) and return at the end of the if block. Does this work for you?

sredman: At this revision I hadn't figured out what to do with old-style packets. I have decided to…
143 143​
144void TelephonyPlugin::sendSms(const QString& phoneNumber, const QString& messageBody) 144​void TelephonyPlugin::sendSms(const QString& phoneNumber, const QString& messageBody)
145{ 145​{
146 NetworkPacket np(PACKET_TYPE_SMS_REQUEST, { 146​ NetworkPacket np(PACKET_TYPE_SMS_REQUEST, {
147 {"sendSms", true}, 147​ {"sendSms", true},
148 {"phoneNumber", phoneNumber}, 148​ {"phoneNumber", phoneNumber},
149 {"messageBody", messageBody} 149​ {"messageBody", messageBody}
150 }); 150​ });
151 qDebug() << "sending sms!"; 151​ qDebug() << "sending sms!";
152 sendPacket(np); 152​ sendPacket(np);
153} 153​}
154 154​
155void TelephonyPlugin::showSendSmsDialog() 155​void TelephonyPlugin::showSendSmsDialog()
156{ 156​{
157 QString phoneNumber = sender()->property("phoneNumber").toString(); 157​ QString phoneNumber = sender()->property("phoneNumber").toString();
158 QString contactName = sender()->property("contactName").toString(); 158​ QString contactName = sender()->property("contactName").toString();
159 QString originalMessage = sender()->property("originalMessage").toString(); 159​ QString originalMessage = sender()->property("originalMessage").toString();
160 SendReplyDialog* dialog = new SendReplyDialog(originalMessage, phoneNumber, contactName); 160​ SendReplyDialog* dialog = new SendReplyDialog(originalMessage, phoneNumber, contactName);
161 connect(dialog, &SendReplyDialog::sendReply, this, &TelephonyPlugin::sendSms); 161​ connect(dialog, &SendReplyDialog::sendReply, this, &TelephonyPlugin::sendSms);
162 dialog->show(); 162​ dialog->show();
163 dialog->raise(); 163​ dialog->raise();
164} 164​}
165 165​
166​void TelephonyPlugin::sendAllConversationsRequest()
167​{
168​ NetworkPacket np(PACKET_TYPE_TELEPHONY_REQUEST_CONVERSATIONS);
169​
170​ sendPacket(np);
171​}
172​
166QString TelephonyPlugin::dbusPath() const 173​QString TelephonyPlugin::dbusPath() const
167{ 174​{
168 return "/modules/kdeconnect/devices/" + device()->id() + "/telephony"; 175​ return "/modules/kdeconnect/devices/" + device()->id() + "/telephony";
169} 176​}
170 177​
171#include "telephonyplugin.moc" 178​#include "telephonyplugin.moc"
nicolasfellaUnsubmitted
Done

Use new syntax if possible

nicolasfella: Use new syntax if possible
sredmanAuthorUnsubmitted
Done

The messageReceived method being used as the sender of this connection is on the other side of the DBus interface (telepathy-kdeconnect/connection.h line 55). I don't know how or if the new syntax can be used in this case

sredman: The messageReceived method being used as the sender of this connection is on the other side of…
nicolasfellaUnsubmitted
Done

returning false here is not necessary

nicolasfella: returning false here is not necessary
apolUnsubmitted
Done

Shouldn't initialize if it's empty.

apol: Shouldn't initialize if it's empty.
apolUnsubmitted
Done

return reply?
Actually nobody seems to be reading the result of forwardToTelepathy, you can probablty just omit returning anything and call asynchronously.

apol: `return reply`? Actually nobody seems to be reading the result of `forwardToTelepathy`, you can…
apolUnsubmitted
Done

const QVariant &

apol: const QVariant &
apolUnsubmitted
Done

const

apol: const