[Merge] lp:~phablet-team/messaging-framework/detect-network-switch into lp:messaging-framework

Gustavo Pichorim Boiko gustavo.boiko at canonical.com
Fri Jul 15 20:00:46 UTC 2016


Review: Needs Fixing

Some things to look at.

Diff comments:

> === added file 'include/messaging/qt/network_monitor.h'
> --- include/messaging/qt/network_monitor.h	1970-01-01 00:00:00 +0000
> +++ include/messaging/qt/network_monitor.h	2016-07-15 09:31:24 +0000
> @@ -0,0 +1,68 @@
> +/*
> + * Copyright 2014 Canonical Ltd.

Probably needs 2014-2016 here.

> + *
> + * This file and its implementation is adaptation of sync-monitor.
> + *
> + * sync-monitor is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 3.
> + *
> + * contact-service-app is distributed in the hope that it will be useful,

Probably should be changed by messaging-framework, or just by "This software"

> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef MESSAGING_QT_NETWORK_MONITOR
> +#define MESSAGING_QT_NETWORK_MONITOR
> +
> +#include <QtNetwork/QNetworkConfigurationManager>
> +
> +#include <memory>
> +
> +namespace messaging
> +{
> +
> +//Handy fw declaration
> +class Connection;
> +
> +namespace qt
> +{
> +
> +class NetworkMonitor : public QObject
> +{
> +    Q_OBJECT
> +public:
> +    enum NetworkState {
> +        NetworkOffline = 0,
> +        NetworkPartialOnline,
> +        NetworkOnline
> +    };
> +    NetworkMonitor(const std::weak_ptr<Connection> &connection, QObject *parent=0);
> +    virtual ~NetworkMonitor();
> +
> +    void stop_monitoring();
> +    void set_connection_ready(bool ready);
> +
> +private Q_SLOTS:
> +    void refresh();
> +
> +private:
> +    QNetworkConfigurationManager network_config_manager_;
> +    std::weak_ptr<Connection> connection_;
> +    std::string current_ssid_;
> +    bool is_connection_ready_;
> +
> +    void idle_refresh();
> +    void start_monitoring();
> +    void dump(const QNetworkConfiguration &config);
> +};
> +
> +}
> +}
> +
> +#endif // MESSAGING_QT_NETWORK_MONITOR
> +
> 
> === added file 'src/messaging/qt/network_monitor.cpp'
> --- src/messaging/qt/network_monitor.cpp	1970-01-01 00:00:00 +0000
> +++ src/messaging/qt/network_monitor.cpp	2016-07-15 09:31:24 +0000
> @@ -0,0 +1,139 @@
> +#include <messaging/qt/network_monitor.h>
> +
> +#include <messaging/connection.h>
> +
> +#include <QtNetwork/QNetworkConfigurationManager>
> +
> +#include <glog/logging.h>
> +
> +#include <thread>
> +
> +namespace mq = messaging::qt;
> +
> +mq::NetworkMonitor::NetworkMonitor(const std::weak_ptr<Connection> &connection, QObject *parent)
> +    : QObject(parent)
> +    , network_config_manager_{this}
> +    , connection_{connection}
> +    , current_ssid_{}
> +    , is_connection_ready_{false}
> +{
> +    start_monitoring();
> +}
> +
> +mq::NetworkMonitor::~NetworkMonitor()
> +{
> +}
> +
> +/*!
> + * \brief starts monitoring current connection. The monitoring will restart in case a new
> + * connection is created but this one won't be monitored anymore
> + */
> +void mq::NetworkMonitor::start_monitoring()
> +{
> +    connect(&network_config_manager_,
> +            SIGNAL(onlineStateChanged(bool)),
> +            SLOT(refresh()), Qt::QueuedConnection);
> +    connect(&network_config_manager_,
> +            SIGNAL(configurationAdded(QNetworkConfiguration)),
> +            SLOT(refresh()), Qt::QueuedConnection);
> +    connect(&network_config_manager_,
> +            SIGNAL(configurationChanged(QNetworkConfiguration)),
> +            SLOT(refresh()), Qt::QueuedConnection);
> +    connect(&network_config_manager_,
> +            SIGNAL(configurationRemoved(QNetworkConfiguration)),
> +            SLOT(refresh()), Qt::QueuedConnection);
> +    connect(&network_config_manager_,
> +            SIGNAL(updateCompleted()),
> +            SLOT(refresh()), Qt::QueuedConnection);
> +}
> +
> +/*!
> + * \brief stops monitoring network switch in this connection
> + */
> +void mq::NetworkMonitor::stop_monitoring()
> +{
> +    disconnect(&network_config_manager_,
> +            SIGNAL(onlineStateChanged(bool)),
> +            this,
> +            SLOT(refresh()));
> +    disconnect(&network_config_manager_,
> +            SIGNAL(configurationAdded(QNetworkConfiguration)),
> +            this,
> +            SLOT(refresh()));
> +    disconnect(&network_config_manager_,
> +            SIGNAL(configurationChanged(QNetworkConfiguration)),
> +            this,
> +            SLOT(refresh()));
> +    disconnect(&network_config_manager_,
> +            SIGNAL(configurationRemoved(QNetworkConfiguration)),
> +            this,
> +            SLOT(refresh()));
> +    disconnect(&network_config_manager_,
> +            SIGNAL(updateCompleted()),
> +            this,
> +            SLOT(refresh()));
> +}
> +
> +/*!
> + * \brief flag needed to know when the connection object has become effectively online.
> + * This is needed to determinate if a received ssid different from the previous one should be taken
> + * as the trigger
> + */
> +void mq::NetworkMonitor::set_connection_ready(bool ready)
> +{
> +    is_connection_ready_ = ready;
> +}
> +
> +void mq::NetworkMonitor::refresh()
> +{  
> +    std::thread([this]() {
> +        std::this_thread::sleep_for(std::chrono::milliseconds(3000));
> +        idle_refresh();

I think instead of using threads, we could use the Qt event loop here (a QTimer::singleShot or even a QObject::startTimer) to make sure the idle_refresh() call is called within the same thread as the other methods (to avoid having to sincronize, etc).

> +    }).detach();
> +}
> +
> +void mq::NetworkMonitor::idle_refresh()
> +{
> +    // Check if is online
> +    QList<QNetworkConfiguration> active_configs = network_config_manager_.allConfigurations(QNetworkConfiguration::Active);
> +    bool is_online = active_configs.size() > 0;
> +    if (is_online) {
> +        // Check if the connection is wifi or ethernet
> +        QNetworkConfiguration default_config = network_config_manager_.defaultConfiguration();
> +
> +        // while connecting, we admit updating over an empty current_ssid. In the case
> +        // of being effectively connected, that means a network switch
> +        if (not is_connection_ready_)
> +        {
> +            if (current_ssid_.empty())
> +            {
> +                dump(default_config);
> +                current_ssid_ = default_config.name().toStdString();
> +            }
> +        }
> +
> +        if (current_ssid_ != default_config.name().toStdString())
> +        {
> +            // network is switched at this point, so desconnect the registered connection in case is valid.
> +            LOG(INFO) << "detected network switch";
> +            auto sp = connection_.lock();
> +            if (sp)
> +            {
> +                sp->disconnect();
> +                stop_monitoring();
> +            }
> +        }
> +
> +    } else {
> +        LOG(INFO) << "Network is offline";
> +    }
> +}
> +
> +void mq::NetworkMonitor::dump(const QNetworkConfiguration &config)
> +{
> +    LOG(INFO) << "New network connection:\nType: " << config.bearerTypeName().toStdString()
> +              << "\nId: " << config.identifier().toStdString()
> +              << "\nName: " << config.name().toStdString()
> +              << "\nIsValid: " << config.isValid()
> +              << "\nRoamingAvailable: " << config.isRoamingAvailable();
> +}


-- 
https://code.launchpad.net/~phablet-team/messaging-framework/detect-network-switch/+merge/300164
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/messaging-framework/avoid-removing-tp-connection-after-disconnect.



More information about the Ubuntu-reviews mailing list