[Merge] lp:~tiagosh/telephony-service/refactor-ussd into lp:telephony-service

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Tue Apr 21 20:52:24 UTC 2015


Review: Needs Fixing

Code looks good, just a couple diff comments.

Diff comments:

> === modified file 'Ubuntu/Telephony/components.cpp'
> --- Ubuntu/Telephony/components.cpp	2014-08-25 14:49:53 +0000
> +++ Ubuntu/Telephony/components.cpp	2015-04-21 16:46:57 +0000
> @@ -54,7 +54,6 @@
>      mRootContext->setContextProperty("telepathyHelper", TelepathyHelper::instance());
>      mRootContext->setContextProperty("chatManager", ChatManager::instance());
>      mRootContext->setContextProperty("callManager", CallManager::instance());
> -    mRootContext->setContextProperty("ussdManager", USSDManager::instance());
>      mRootContext->setContextProperty("greeter", GreeterContacts::instance());
>  
>  }
> @@ -66,6 +65,7 @@
>      qmlRegisterUncreatableType<CallEntry>(uri, 0, 1, "CallEntry", "Objects of this type are created in CallManager and made available to QML for usage");
>      qmlRegisterUncreatableType<AudioOutput>(uri, 0, 1, "AudioOutput", "Objects of this type are created in CallEntry and made available to QML for usage");
>      qmlRegisterUncreatableType<AccountEntry>(uri, 0, 1, "AccountEntry", "Objects of this type are created in TelepathyHelper and made available to QML");
> +    qmlRegisterUncreatableType<USSDManager>(uri, 0, 1, "USSDManager", "Objects of this type are created in AccountEntry and made available to QML");
>      qmlRegisterType<ContactWatcher>(uri, 0, 1, "ContactWatcher");
>      qmlRegisterType<PhoneUtils>(uri, 0, 1, "PhoneUtils");
>  }
> 
> === modified file 'indicator/ussdindicator.cpp'
> --- indicator/ussdindicator.cpp	2014-03-31 19:37:40 +0000
> +++ indicator/ussdindicator.cpp	2015-04-21 16:46:57 +0000
> @@ -27,6 +27,8 @@
>  #include <libnotify/notify.h>
>  #include "ringtone.h"
>  #include "ussdindicator.h"
> +#include "ofonoaccountentry.h"
> +#include "telepathyhelper.h"
>  
>  USSDIndicator::USSDIndicator(QObject *parent)
>  : QObject(parent),
> @@ -35,35 +37,54 @@
>    m_notifications("org.freedesktop.Notifications",
>                    "/org/freedesktop/Notifications", QDBusConnection::sessionBus())
>  {
> -    connect(USSDManager::instance(), SIGNAL(notificationReceived(const QString &)), SLOT(onNotificationReceived(const QString &)));
> -    connect(USSDManager::instance(), SIGNAL(requestReceived(const QString &)), SLOT(onRequestReceived(const QString &)));
> -    connect(USSDManager::instance(), SIGNAL(initiateUSSDComplete(const QString &)), SLOT(onInitiateUSSDComplete(const QString &)));
> -    connect(USSDManager::instance(), SIGNAL(respondComplete(bool, const QString &)), SLOT(onRespondComplete(bool, const QString &)));
> -    connect(USSDManager::instance(), SIGNAL(stateChanged(const QString &)), SLOT(onStateChanged(const QString &)));
> -
> +    setupAccounts();
> +    connect(TelepathyHelper::instance(), SIGNAL(accountsChanged()), this, SLOT(setupAccounts()));
>      connect(&m_notifications, SIGNAL(ActionInvoked(uint, const QString &)), this, SLOT(actionInvoked(uint, const QString &)));
>      connect(&m_notifications, SIGNAL(NotificationClosed(uint, uint)), this, SLOT(notificationClosed(uint, uint)));
>  }
>  
> +void USSDIndicator::setupAccounts()
> +{
> +    Q_FOREACH(AccountEntry *account, TelepathyHelper::instance()->accounts()) {
> +        OfonoAccountEntry *ofonoAccount = qobject_cast<OfonoAccountEntry*>(account);
> +        if (!ofonoAccount) {
> +            continue;
> +        }
> +        
> +        connect(ofonoAccount->ussdManager(), SIGNAL(notificationReceived(const QString &)), SLOT(onNotificationReceived(const QString &)), Qt::UniqueConnection);
> +        connect(ofonoAccount->ussdManager(), SIGNAL(requestReceived(const QString &)), SLOT(onRequestReceived(const QString &)), Qt::UniqueConnection);
> +        connect(ofonoAccount->ussdManager(), SIGNAL(initiateUSSDComplete(const QString &)), SLOT(onInitiateUSSDComplete(const QString &)), Qt::UniqueConnection);
> +        connect(ofonoAccount->ussdManager(), SIGNAL(respondComplete(bool, const QString &)), SLOT(onRespondComplete(bool, const QString &)), Qt::UniqueConnection);
> +        connect(ofonoAccount->ussdManager(), SIGNAL(stateChanged(const QString &)), SLOT(onStateChanged(const QString &)), Qt::UniqueConnection);
> +    }
> +}
> +
>  void USSDIndicator::onNotificationReceived(const QString &message)
>  {
> -    showUSSDNotification(message, false);
> +    USSDManager *ussdManager = qobject_cast<USSDManager*>(sender());

Just to prevent undefined behaviors, can you check that the cast worked?

> +    showUSSDNotification(message, false, ussdManager);
>  }
>  
>  void USSDIndicator::onRequestReceived(const QString &message)
>  {
> -    showUSSDNotification(message, true);
> +    USSDManager *ussdManager = qobject_cast<USSDManager*>(sender());

Same here.

> +    showUSSDNotification(message, true, ussdManager);
>  }
>  
>  void USSDIndicator::onInitiateUSSDComplete(const QString &ussdResp)
>  {
> -    showUSSDNotification(ussdResp, (USSDManager::instance()->state() == "user-response"));
> +    USSDManager *ussdManager = qobject_cast<USSDManager*>(sender());
> +    if (!ussdManager) {
> +        return;
> +    }
> +    showUSSDNotification(ussdResp, (ussdManager->state() == "user-response"), ussdManager);
>  }
>  
>  void USSDIndicator::onRespondComplete(bool success, const QString &ussdResp)
>  {
> -    if (success) {
> -        showUSSDNotification(ussdResp, (USSDManager::instance()->state() == "user-response"));
> +    USSDManager *ussdManager = qobject_cast<USSDManager*>(sender());
> +    if (success && ussdManager) {
> +        showUSSDNotification(ussdResp, (ussdManager->state() == "user-response"), ussdManager);
>      }
>  }
>  
> @@ -72,7 +93,7 @@
>      // TODO: check if we should close notifications when the state is idle
>  }
>  
> -void USSDIndicator::showUSSDNotification(const QString &message, bool replyRequired)
> +void USSDIndicator::showUSSDNotification(const QString &message, bool replyRequired, USSDManager *ussdManager)
>  {
>      USSDMenu *menu = replyRequired ? &m_menuRequest : &m_menuNotification;
>      QString actionId = "ok_id";
> @@ -102,7 +123,7 @@
>                          "", message,
>                          QStringList() << actionId << actionLabel << "cancel_id"
>                                          << C::gettext("Cancel"), notificationHints, 0);
> -
> +    mUSSDRequests[m_notificationId] = ussdManager;
>  
>      Ringtone::instance()->playIncomingMessageSound();
>  }
> @@ -113,12 +134,17 @@
>          return;
>      }
>  
> +    USSDManager *ussdManager = mUSSDRequests.take(id);
> +    if (!ussdManager) {
> +        return;
> +    }
> +
>      m_notificationId = 0;
>  
>      if (actionKey == "reply_id") {
> -        USSDManager::instance()->respond(m_menuRequest.response(), USSDManager::instance()->activeAccountId());
> +        ussdManager->respond(m_menuRequest.response());
>      } else if (actionKey == "cancel_id") {
> -        USSDManager::instance()->cancel(USSDManager::instance()->activeAccountId());
> +        ussdManager->cancel();
>      }
>      m_menuRequest.clearResponse();
>  }
> 
> === modified file 'indicator/ussdindicator.h'
> --- indicator/ussdindicator.h	2014-03-31 19:37:40 +0000
> +++ indicator/ussdindicator.h	2015-04-21 16:46:57 +0000
> @@ -27,13 +27,14 @@
>  #include "indicator/NotificationsInterface.h"
>  #include "ussdmanager.h"
>  #include "ussdmenu.h"
> +#include "ofonoaccountentry.h"
>  
>  class USSDIndicator : public QObject
>  {
>      Q_OBJECT
>  public:
>      explicit USSDIndicator(QObject *parent = 0);
> -    void showUSSDNotification(const QString &message, bool replyRequired);
> +    void showUSSDNotification(const QString &message, bool replyRequired, USSDManager *ussdManager);
>  
>  public Q_SLOTS:
>      void onNotificationReceived(const QString &message);
> @@ -43,12 +44,17 @@
>      void onStateChanged(const QString &state);
>      void actionInvoked(uint id, const QString &actionKey);
>      void notificationClosed(uint id, uint reason);
> +private Q_SLOTS:
> +    void setupAccounts();
> +
>  private:
>      unsigned int m_notificationId;
>      USSDMenu m_menuRequest;
>      USSDMenu m_menuNotification;
>      QString mPendingMessage;
>      org::freedesktop::Notifications m_notifications;
> +    QMap<int, USSDManager*> mUSSDRequests;
> +    QList<OfonoAccountEntry*> mAccounts;
>  };
>  
>  #endif // USSDINDICATOR_H
> 
> === modified file 'libtelephonyservice/ofonoaccountentry.cpp'
> --- libtelephonyservice/ofonoaccountentry.cpp	2015-03-17 18:26:32 +0000
> +++ libtelephonyservice/ofonoaccountentry.cpp	2015-04-21 16:46:57 +0000
> @@ -37,6 +37,13 @@
>      connect(this,
>              SIGNAL(statusMessageChanged()),
>              SIGNAL(emergencyCallsAvailableChanged()));
> +
> +    mUssdManager = new USSDManager(this, this);
> +}
> +
> +USSDManager *OfonoAccountEntry::ussdManager() const
> +{
> +    return mUssdManager;
>  }
>  
>  QStringList OfonoAccountEntry::emergencyNumbers() const
> 
> === modified file 'libtelephonyservice/ofonoaccountentry.h'
> --- libtelephonyservice/ofonoaccountentry.h	2015-03-17 18:21:59 +0000
> +++ libtelephonyservice/ofonoaccountentry.h	2015-04-21 16:46:57 +0000
> @@ -23,6 +23,7 @@
>  #define OFONOACCOUNTENTRY_H
>  
>  #include "accountentry.h"
> +#include "ussdmanager.h"
>  
>  class OfonoAccountEntry : public AccountEntry
>  {
> @@ -35,6 +36,7 @@
>      Q_PROPERTY(bool emergencyCallsAvailable READ emergencyCallsAvailable NOTIFY emergencyCallsAvailableChanged)
>      Q_PROPERTY(bool simLocked READ simLocked NOTIFY simLockedChanged)
>      Q_PROPERTY(QString serial READ serial NOTIFY serialChanged)
> +    Q_PROPERTY(USSDManager* ussdManager READ ussdManager CONSTANT)
>      friend class AccountEntryFactory;
>  
>  public:
> @@ -46,6 +48,7 @@
>      bool emergencyCallsAvailable() const;
>      bool simLocked() const;
>      QString serial() const;
> +    USSDManager *ussdManager() const;
>  
>      // reimplemented from AccountEntry
>      virtual AccountEntry::AccountType type() const;
> @@ -81,6 +84,7 @@
>      uint mVoicemailCount;
>      bool mVoicemailIndicator;
>      QString mSerial;
> +    USSDManager *mUssdManager;
>  };
>  
>  #endif // OFONOACCOUNTENTRY_H
> 
> === modified file 'libtelephonyservice/telepathyhelper.cpp'
> --- libtelephonyservice/telepathyhelper.cpp	2015-03-26 21:52:43 +0000
> +++ libtelephonyservice/telepathyhelper.cpp	2015-04-21 16:46:57 +0000
> @@ -162,6 +162,22 @@
>      return activeAccountList;
>  }
>  
> +QList<AccountEntry*> TelepathyHelper::phoneAccounts() const
> +{
> +    QList<AccountEntry*> accountList;
> +    Q_FOREACH(AccountEntry *account, mAccounts) {
> +        if (account->type() == AccountEntry::PhoneAccount) {
> +            accountList << account;
> +        }
> +    }
> +    return accountList;
> +}
> +
> +QQmlListProperty<AccountEntry> TelepathyHelper::qmlPhoneAccounts()
> +{
> +    return QQmlListProperty<AccountEntry>(this, 0, phoneAccountsCount, phoneAccountAt);
> +}
> +
>  QQmlListProperty<AccountEntry> TelepathyHelper::qmlAccounts()
>  {
>      return QQmlListProperty<AccountEntry>(this, 0, accountsCount, accountAt);
> @@ -357,12 +373,24 @@
>      return spec;
>  }
>  
> +int TelepathyHelper::phoneAccountsCount(QQmlListProperty<AccountEntry> *p)
> +{
> +    Q_UNUSED(p)
> +    return TelepathyHelper::instance()->phoneAccounts().count();
> +}
> +
>  int TelepathyHelper::accountsCount(QQmlListProperty<AccountEntry> *p)
>  {
>      Q_UNUSED(p)
>      return TelepathyHelper::instance()->accounts().count();
>  }
>  
> +AccountEntry *TelepathyHelper::phoneAccountAt(QQmlListProperty<AccountEntry> *p, int index)
> +{
> +    Q_UNUSED(p)
> +    return TelepathyHelper::instance()->phoneAccounts()[index];
> +}
> +
>  AccountEntry *TelepathyHelper::accountAt(QQmlListProperty<AccountEntry> *p, int index)
>  {
>      Q_UNUSED(p)
> @@ -391,6 +419,7 @@
>  
>      Q_EMIT accountIdsChanged();
>      Q_EMIT accountsChanged();
> +    Q_EMIT phoneAccountsChanged();
>      Q_EMIT activeAccountsChanged();
>      onSettingsChanged("defaultSimForMessages");
>      onSettingsChanged("defaultSimForCalls");
> @@ -418,6 +447,7 @@
>  
>      Q_EMIT accountIdsChanged();
>      Q_EMIT accountsChanged();
> +    Q_EMIT phoneAccountsChanged();
>      Q_EMIT activeAccountsChanged();
>      onSettingsChanged("defaultSimForMessages");
>      onSettingsChanged("defaultSimForCalls");
> @@ -452,6 +482,7 @@
>  
>      Q_EMIT accountIdsChanged();
>      Q_EMIT accountsChanged();
> +    Q_EMIT phoneAccountsChanged();
>      Q_EMIT activeAccountsChanged();
>      onSettingsChanged("defaultSimForMessages");
>      onSettingsChanged("defaultSimForCalls");
> 
> === modified file 'libtelephonyservice/telepathyhelper.h'
> --- libtelephonyservice/telepathyhelper.h	2015-03-23 18:17:51 +0000
> +++ libtelephonyservice/telepathyhelper.h	2015-04-21 16:46:57 +0000
> @@ -48,6 +48,7 @@
>      Q_PROPERTY(bool connected READ connected NOTIFY connectedChanged)
>      Q_PROPERTY(QStringList accountIds READ accountIds NOTIFY accountIdsChanged)
>      Q_PROPERTY(QQmlListProperty<AccountEntry> accounts READ qmlAccounts NOTIFY accountsChanged)
> +    Q_PROPERTY(QQmlListProperty<AccountEntry> phoneAccounts READ qmlPhoneAccounts NOTIFY phoneAccountsChanged)
>      Q_PROPERTY(QQmlListProperty<AccountEntry> activeAccounts READ qmlActiveAccounts NOTIFY activeAccountsChanged)
>      Q_PROPERTY(AccountEntry *defaultMessagingAccount READ defaultMessagingAccount NOTIFY defaultMessagingAccountChanged)
>      Q_PROPERTY(AccountEntry *defaultCallAccount READ defaultCallAccount NOTIFY defaultCallAccountChanged)
> @@ -65,8 +66,10 @@
>  
>      static TelepathyHelper *instance();
>      QList<AccountEntry*> accounts() const;
> +    QList<AccountEntry*> phoneAccounts() const;
>      QList<AccountEntry*> activeAccounts() const;
>      QQmlListProperty<AccountEntry> qmlAccounts();
> +    QQmlListProperty<AccountEntry> qmlPhoneAccounts();
>      QQmlListProperty<AccountEntry> qmlActiveAccounts();
>      ChannelObserver *channelObserver() const;
>      QDBusInterface *handlerInterface() const;
> @@ -97,7 +100,9 @@
>      static AccountEntry *accountAt(QQmlListProperty<AccountEntry> *p, int index);
>      static int activeAccountsCount(QQmlListProperty<AccountEntry> *p);
>      static AccountEntry *activeAccountAt(QQmlListProperty<AccountEntry> *p, int index);
> -
> +    static int phoneAccountsCount(QQmlListProperty<AccountEntry> *p);
> +    static AccountEntry *phoneAccountAt(QQmlListProperty<AccountEntry> *p, int index);
> + 
>  Q_SIGNALS:
>      void channelObserverCreated(ChannelObserver *observer);
>      void channelObserverUnregistered();
> @@ -105,6 +110,7 @@
>      void connectedChanged();
>      void accountIdsChanged();
>      void accountsChanged();
> +    void phoneAccountsChanged();
>      void activeAccountsChanged();
>      void setupReady();
>      void defaultMessagingAccountChanged();
> 
> === modified file 'libtelephonyservice/ussdmanager.cpp'
> --- libtelephonyservice/ussdmanager.cpp	2015-02-03 17:30:41 +0000
> +++ libtelephonyservice/ussdmanager.cpp	2015-04-21 16:46:57 +0000
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2012 Canonical, Ltd.
> + * Copyright (C) 2012-2015 Canonical, Ltd.
>   *
>   * Authors:
>   *  Gustavo Pichorim Boiko <gustavo.boiko at canonical.com>
> @@ -30,198 +30,105 @@
>  typedef QMap<QString, QVariant> dbusQMap;
>  Q_DECLARE_METATYPE(dbusQMap)
>  
> -USSDManager *USSDManager::instance()
> -{
> -    static USSDManager *self = new USSDManager();
> -    return self;
> -}
> -
> -USSDManager::USSDManager(QObject *parent)
> -: QObject(parent)
> -{
> -    connect(TelepathyHelper::instance(), SIGNAL(accountsChanged()), SLOT(onAccountsChanged()));
> -    onAccountsChanged();
> -}
> -
> -Tp::ConnectionPtr USSDManager::connectionForAccountId(const QString &accountId)
> -{
> -    AccountEntry *accountEntry;
> -    if (accountId.isNull()) {
> -        accountEntry = TelepathyHelper::instance()->accounts()[0];
> -    } else {
> -        accountEntry = TelepathyHelper::instance()->accountForId(accountId);
> -    }
> -
> -    return accountEntry->account()->connection();
> -}
> -
> -void USSDManager::initiate(const QString &command, const QString &accountId)
> -{
> -    Tp::ConnectionPtr conn = connectionForAccountId(accountId);
> -    QString busName = conn->busName();
> -    QString objectPath = conn->objectPath();
> -    QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> +USSDManager::USSDManager(AccountEntry *account, QObject *parent)
> +: QObject(parent),
> +  mState("idle"),
> +  mAccount(account)
> +{
> +    connect(mAccount, SIGNAL(connectedChanged()), this, SLOT(onConnectionChanged()));
> +    onConnectionChanged();
> +}
> +
> +void USSDManager::initiate(const QString &command)
> +{
> +    QDBusInterface ussdIface(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE);
>      ussdIface.asyncCall("Initiate", command);
>  }
>  
> -void USSDManager::respond(const QString &reply, const QString &accountId)
> +void USSDManager::respond(const QString &reply)
>  {
> -    Tp::ConnectionPtr conn = connectionForAccountId(accountId);
> -    QString busName = conn->busName();
> -    QString objectPath = conn->objectPath();
> -    QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> +    QDBusInterface ussdIface(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE);
>      ussdIface.asyncCall("Respond", reply);
>  }
>  
> -void USSDManager::cancel(const QString &accountId)
> +void USSDManager::cancel()
>  {
> -    Tp::ConnectionPtr conn = connectionForAccountId(accountId);
> -    QString busName = conn->busName();
> -    QString objectPath = conn->objectPath();
> -    QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> +    QDBusInterface ussdIface(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE);
>      ussdIface.asyncCall("Cancel");
>  }
>  
> -void USSDManager::disconnectAllSignals(const Tp::ConnectionPtr& conn)
> -{
> -    if (conn.isNull()) {
> -        return;
> -    }
> -
> -    QString busName = conn->busName();
> -    QString objectPath = conn->objectPath();
> -
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "StateChanged", this, SLOT(onStateChanged(QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RequestReceived", this, SIGNAL(requestReceived(QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "NotificationReceived", this, SIGNAL(notificationReceived(QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateUSSDComplete", this, SIGNAL(initiateUSSDComplete(QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RespondComplete", this, SIGNAL(respondComplete(bool, QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "BarringComplete", this, SIGNAL(barringComplete(QString, QString, QVariantMap)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ForwardingComplete", this, SIGNAL(forwardingComplete(QString, QString, QVariantMap)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "WaitingComplete", this, SIGNAL(waitingComplete(QString, QVariantMap)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLinePresentationComplete", this, SIGNAL(callingLinePresentationComplete(QString, QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLineRestrictionComplete", this, SIGNAL(callingLineRestrictionComplete(QString, QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLineRestrictionComplete", this, SIGNAL(connectedLineRestrictionComplete(QString, QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLinePresentationComplete", this, SIGNAL(connectedLinePresentationComplete(QString, QString)));
> -    QDBusConnection::sessionBus().disconnect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateFailed", this, SIGNAL(initiateFailed()));
> -}
> -
> -void USSDManager::connectAllSignals(const Tp::ConnectionPtr& conn)
> -{
> -    if (conn.isNull()) {
> -        return;
> -    }
> -
> -    QString busName = conn->busName();
> -    QString objectPath = conn->objectPath();
> -
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "StateChanged", this, SLOT(onStateChanged(QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RequestReceived", this, SIGNAL(requestReceived(QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "NotificationReceived", this, SIGNAL(notificationReceived(QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateUSSDComplete", this, SIGNAL(initiateUSSDComplete(QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RespondComplete", this, SIGNAL(respondComplete(bool, QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "BarringComplete", this, SIGNAL(barringComplete(QString, QString, QVariantMap)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ForwardingComplete", this, SIGNAL(forwardingComplete(QString, QString, QVariantMap)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "WaitingComplete", this, SIGNAL(waitingComplete(QString, QVariantMap)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLinePresentationComplete", this, SIGNAL(callingLinePresentationComplete(QString, QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLineRestrictionComplete", this, SIGNAL(callingLineRestrictionComplete(QString, QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLineRestrictionComplete", this, SIGNAL(connectedLineRestrictionComplete(QString, QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLinePresentationComplete", this, SIGNAL(connectedLinePresentationComplete(QString, QString)));
> -    QDBusConnection::sessionBus().connect(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateFailed", this, SIGNAL(initiateFailed()));
> -}
> -
> -void USSDManager::accountConnectedChanged()
> -{
> -    AccountEntry *accountEntry = qobject_cast<AccountEntry*>(sender());
> -    if (!accountEntry) {
> -        return;
> -    }
> -    Tp::ConnectionPtr conn(accountEntry->account()->connection());
> -    disconnectAllSignals(conn);
> -
> -    if (accountEntry->connected()) {
> -        QString busName = conn->busName();
> -        QString objectPath = conn->objectPath();
> -
> -        connectAllSignals(conn);
> -
> -        QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> -        mStates[accountEntry->accountId()] = ussdIface.property("State").toString();
> -    }
> -}
> -
> -void USSDManager::onAccountsChanged()
> -{
> -    Q_FOREACH (AccountEntry *accountEntry, TelepathyHelper::instance()->accounts()) {
> -        QObject::disconnect(accountEntry, SIGNAL(connectedChanged()), this, SLOT(accountConnectedChanged()));
> -        QObject::connect(accountEntry, SIGNAL(connectedChanged()), this, SLOT(accountConnectedChanged()));
> -
> -        // disconnect all and reconnect only the online accounts
> -        Tp::ConnectionPtr conn(accountEntry->account()->connection());
> -        disconnectAllSignals(conn);
> -        mStates.remove(accountEntry->accountId());
> -
> -        if (accountEntry->connected()) {
> -            QString busName = conn->busName();
> -            QString objectPath = conn->objectPath();
> -
> -            connectAllSignals(conn);
> -
> -            QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> -            mStates[accountEntry->accountId()] = ussdIface.property("State").toString();
> -        }
> -    }
> -    Q_EMIT stateChanged(state());
> +void USSDManager::connectAllSignals()
> +{
> +    if (mBusName.isEmpty() || mObjectPath.isEmpty()) {
> +        return;
> +    }
> +
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "StateChanged", this, SLOT(onStateChanged(QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RequestReceived", this, SIGNAL(requestReceived(QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "NotificationReceived", this, SIGNAL(notificationReceived(QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateUSSDComplete", this, SIGNAL(initiateUSSDComplete(QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RespondComplete", this, SIGNAL(respondComplete(bool, QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "BarringComplete", this, SIGNAL(barringComplete(QString, QString, QVariantMap)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ForwardingComplete", this, SIGNAL(forwardingComplete(QString, QString, QVariantMap)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "WaitingComplete", this, SIGNAL(waitingComplete(QString, QVariantMap)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLinePresentationComplete", this, SIGNAL(callingLinePresentationComplete(QString, QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLineRestrictionComplete", this, SIGNAL(callingLineRestrictionComplete(QString, QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLineRestrictionComplete", this, SIGNAL(connectedLineRestrictionComplete(QString, QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLinePresentationComplete", this, SIGNAL(connectedLinePresentationComplete(QString, QString)));
> +    QDBusConnection::sessionBus().connect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateFailed", this, SIGNAL(initiateFailed()));
> +}
> +
> +void USSDManager::disconnectAllSignals()
> +{
> +    if (mBusName.isEmpty() || mObjectPath.isEmpty()) {
> +        return;
> +    }
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "StateChanged", this, SLOT(onStateChanged(QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RequestReceived", this, SIGNAL(requestReceived(QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "NotificationReceived", this, SIGNAL(notificationReceived(QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateUSSDComplete", this, SIGNAL(initiateUSSDComplete(QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "RespondComplete", this, SIGNAL(respondComplete(bool, QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "BarringComplete", this, SIGNAL(barringComplete(QString, QString, QVariantMap)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ForwardingComplete", this, SIGNAL(forwardingComplete(QString, QString, QVariantMap)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "WaitingComplete", this, SIGNAL(waitingComplete(QString, QVariantMap)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLinePresentationComplete", this, SIGNAL(callingLinePresentationComplete(QString, QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "CallingLineRestrictionComplete", this, SIGNAL(callingLineRestrictionComplete(QString, QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLineRestrictionComplete", this, SIGNAL(connectedLineRestrictionComplete(QString, QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "ConnectedLinePresentationComplete", this, SIGNAL(connectedLinePresentationComplete(QString, QString)));
> +    QDBusConnection::sessionBus().disconnect(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE, "InitiateFailed", this, SIGNAL(initiateFailed()));
> +}
> +
> +void USSDManager::onConnectionChanged()
> +{
> +    disconnectAllSignals();
> +
> +    if (mAccount->account()->connection().isNull()) {
> +        qDebug() << "USSDManager: Failed to connect signals";
> +        return;
> +    }
> +
> +    mBusName = mAccount->account()->connection()->busName();
> +    mObjectPath = mAccount->account()->connection()->objectPath();
> +
> +    QDBusInterface ussdIface(mBusName, mObjectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> +    mState = ussdIface.property("State").toString();
> +
> +    connectAllSignals();
> +}
> +
> +void USSDManager::onStateChanged(const QString &state)
> +{
> +    mState = state;
> +    Q_EMIT stateChanged(mState);
>      Q_EMIT activeChanged();
> -    Q_EMIT activeAccountIdChanged();
> -}
> -
> -void USSDManager::onStateChanged(const QString &)
> -{
> -    Q_FOREACH (AccountEntry *accountEntry, TelepathyHelper::instance()->accounts()) {
> -        Tp::ConnectionPtr conn(accountEntry->account()->connection());
> -        if (accountEntry->connected()) {
> -            QString busName = conn->busName();
> -            QString objectPath = conn->objectPath();
> -            QDBusInterface ussdIface(busName, objectPath, CANONICAL_TELEPHONY_USSD_IFACE);
> -            mStates[accountEntry->accountId()] = ussdIface.property("State").toString();
> -        }
> -    }
> -    Q_EMIT stateChanged(state());
>  }
>  
>  bool USSDManager::active() const
>  {
> -    QMap<QString, QString>::const_iterator i = mStates.constBegin();
> -    while (i != mStates.constEnd()) {
> -        if (i.value() != "idle") {
> -            return true;
> -        }
> -        ++i;
> -    }
> -    return false;
> -}
> -
> -QString USSDManager::activeAccountId() const
> -{
> -    QMap<QString, QString>::const_iterator i = mStates.constBegin();
> -    while (i != mStates.constEnd()) {
> -        if (i.value() != "idle") {
> -            return i.key();
> -        }
> -        ++i;
> -    }
> -    return QString::null;
> +    return mState != "idle";
>  }
>  
>  QString USSDManager::state() const
>  {
> -    QMap<QString, QString>::const_iterator i = mStates.constBegin();
> -    while (i != mStates.constEnd()) {
> -        if (i.value() != "idle") {
> -            return i.value();
> -        }
> -        ++i;
> -    }
> -    return "idle";
> +    return mState;
>  }
> 
> === modified file 'libtelephonyservice/ussdmanager.h'
> --- libtelephonyservice/ussdmanager.h	2015-02-03 17:30:41 +0000
> +++ libtelephonyservice/ussdmanager.h	2015-04-21 16:46:57 +0000
> @@ -28,6 +28,7 @@
>  #include <TelepathyQt/Connection>
>  
>  class TelepathyHelper;
> +class AccountEntry;
>  
>  class USSDManager : public QObject
>  {
> @@ -35,30 +36,23 @@
>      Q_PROPERTY(bool active 
>                 READ active
>                 NOTIFY activeChanged)
> -    Q_PROPERTY(QString activeAccountId
> -               READ activeAccountId
> -               NOTIFY activeAccountIdChanged)
>      Q_PROPERTY(QString state 
>                 READ state
>                 NOTIFY stateChanged)
>  public:
> -    static USSDManager *instance();
> -    Q_INVOKABLE void initiate(const QString &command, const QString &accountId = QString::null);
> -    Q_INVOKABLE void respond(const QString &reply, const QString &accountId = QString::null);
> -    Q_INVOKABLE void cancel(const QString &accountId = QString::null);
> +    explicit USSDManager(AccountEntry *account, QObject *parent = 0);
> +    Q_INVOKABLE void initiate(const QString &command);
> +    Q_INVOKABLE void respond(const QString &reply);
> +    Q_INVOKABLE void cancel();
>  
>      bool active() const;
> -    QString activeAccountId() const;
>      QString state() const;
>  
>  public Q_SLOTS:
> -    void onAccountsChanged();
>      void onStateChanged(const QString &state);
> -    void accountConnectedChanged();
>  
>  Q_SIGNALS:
>      void activeChanged();
> -    void activeAccountIdChanged();
>      void stateChanged(const QString &state);
>  
>      void notificationReceived(const QString &message);
> @@ -75,15 +69,17 @@
>      void connectedLineRestrictionComplete(const QString &ssOp, const QString &status);
>      void initiateFailed();
>  
> +private Q_SLOTS:
> +    void onConnectionChanged();
> +
>  private:
> -    explicit USSDManager(QObject *parent = 0);
> -
> -    Tp::ConnectionPtr connectionForAccountId(const QString &accountId = QString::null);
> -
> -    void disconnectAllSignals(const Tp::ConnectionPtr& conn);
> -    void connectAllSignals(const Tp::ConnectionPtr& conn);
> -
> -    QMap<QString, QString> mStates;
> +    void connectAllSignals();
> +    void disconnectAllSignals();
> +
> +    QString mState;
> +    QString mBusName;
> +    QString mObjectPath;
> +    AccountEntry *mAccount;
>  };
>  
>  #endif // USSDMANAGER_H
> 
> === modified file 'tests/libtelephonyservice/TelepathyHelperTest.cpp'
> --- tests/libtelephonyservice/TelepathyHelperTest.cpp	2015-04-16 02:51:08 +0000
> +++ tests/libtelephonyservice/TelepathyHelperTest.cpp	2015-04-21 16:46:57 +0000
> @@ -37,6 +37,7 @@
>      void cleanup();
>      void testConnected();
>      void testAccounts();
> +    void testPhoneAccounts();
>      void testAccountSorting();
>      void testAccountIds();
>      void testActiveAccounts();
> @@ -147,6 +148,37 @@
>      QCOMPARE(TelepathyHelper::instance()->accounts()[1]->accountId(), second->accountId());
>  }
>  
> +void TelepathyHelperTest::testPhoneAccounts()
> +{
> +    QCOMPARE(TelepathyHelper::instance()->phoneAccounts().count(), 1);
> +    AccountEntry *phoneAccount = TelepathyHelper::instance()->phoneAccounts()[0];
> +    QVERIFY(phoneAccount->accountId() == mPhoneTpAccount->uniqueIdentifier());
> +
> +    // now check that new phone accounts are captured
> +    QSignalSpy phoneAccountsChangedSpy(TelepathyHelper::instance(), SIGNAL(phoneAccountsChanged()));
> +    Tp::AccountPtr newAccount = addAccount("mock", "ofono", "extra");
> +    QVERIFY(!newAccount.isNull());
> +
> +    QTRY_COMPARE(phoneAccountsChangedSpy.count(), 1);
> +    QCOMPARE(TelepathyHelper::instance()->phoneAccounts().count(), 2);
> +
> +    bool accountFound = false;
> +    Q_FOREACH(AccountEntry *entry, TelepathyHelper::instance()->phoneAccounts()) {
> +        if (entry->accountId() == newAccount->uniqueIdentifier()) {
> +            accountFound = true;
> +            break;
> +        }
> +    }
> +    QVERIFY(accountFound);
> +
> +    // now remove the extra phone account and make sure it is properly removed
> +    phoneAccountsChangedSpy.clear();
> +    QVERIFY(removeAccount(newAccount));
> +    QTRY_COMPARE(phoneAccountsChangedSpy.count(), 1);
> +    QCOMPARE(TelepathyHelper::instance()->phoneAccounts().count(), 1);
> +    QCOMPARE(TelepathyHelper::instance()->phoneAccounts()[0]->accountId(), phoneAccount->accountId());
> +}
> +
>  void TelepathyHelperTest::testAccountSorting()
>  {
>      // create two accounts with modem-objpath parameters and make sure they are listed first
> 


-- 
https://code.launchpad.net/~tiagosh/telephony-service/refactor-ussd/+merge/256937
Your team Ubuntu Phablet Team is subscribed to branch lp:telephony-service.



More information about the Ubuntu-reviews mailing list