libweave: Extract weave::Network interface Implementation (ShillClient) will be moved out of libweave. BUG=brillo:1249 TEST='FEATURES=test emerge-gizmo buffet' Change-Id: I9d35df799fcaf607b76db478b1a12d6ebe8e727e Reviewed-on: https://chromium-review.googlesource.com/289929 Reviewed-by: Vitaly Buka <vitalybuka@chromium.org> Commit-Queue: Vitaly Buka <vitalybuka@chromium.org> Tested-by: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/libweave/include/weave/network.h b/libweave/include/weave/network.h new file mode 100644 index 0000000..cd27e2c --- /dev/null +++ b/libweave/include/weave/network.h
@@ -0,0 +1,48 @@ +// Copyright 2015 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIBWEAVE_INCLUDE_WEAVE_NETWORK_H_ +#define LIBWEAVE_INCLUDE_WEAVE_NETWORK_H_ + +#include <map> +#include <string> + +#include <base/callback.h> + +namespace weave { + +enum class NetworkState { + kOffline = 0, + kFailure, + kConnecting, + kConnected, +}; + +class Network { + public: + // A callback that interested parties can register to be notified of + // transitions from online to offline and vice versa. The boolean + // parameter will be true if we're online, and false if we're offline. + using OnConnectionChangedCallback = base::Callback<void(bool)>; + + virtual void AddOnConnectionChangedCallback( + const OnConnectionChangedCallback& listener) = 0; + + // Implementation should attempt to connect to the given network with the + // given passphrase. This is accomplished by: + // Returns false on immediate failures with some descriptive codes in |error|. + virtual bool ConnectToService(const std::string& ssid, + const std::string& passphrase, + const base::Closure& on_success, + chromeos::ErrorPtr* error) = 0; + + virtual NetworkState GetConnectionState() const = 0; + + protected: + virtual ~Network() = default; +}; + +} // namespace weave + +#endif // LIBWEAVE_INCLUDE_WEAVE_NETWORK_H_
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc index e6d90bb..85d77ef 100644 --- a/libweave/src/device_registration_info.cc +++ b/libweave/src/device_registration_info.cc
@@ -28,6 +28,7 @@ #include "libweave/src/commands/command_manager.h" #include "libweave/src/commands/schema_constants.h" #include "libweave/src/notification/xmpp_channel.h" +#include "libweave/src/privet/shill_client.h" #include "libweave/src/states/state_manager.h" #include "libweave/src/utils.h"
diff --git a/libweave/src/notification/xmpp_channel.cc b/libweave/src/notification/xmpp_channel.cc index 2c6ac61..77debd8 100644 --- a/libweave/src/notification/xmpp_channel.cc +++ b/libweave/src/notification/xmpp_channel.cc
@@ -17,6 +17,7 @@ #include "libweave/src/notification/xml_node.h" #include "libweave/src/privet/shill_client.h" #include "libweave/src/utils.h" +#include "weave/network.h" namespace weave { @@ -93,15 +94,15 @@ const std::string& account, const std::string& access_token, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - privet::ShillClient* shill) + Network* network) : account_{account}, access_token_{access_token}, backoff_entry_{&kDefaultBackoffPolicy}, task_runner_{task_runner}, iq_stanza_handler_{new IqStanzaHandler{this, task_runner}} { read_socket_data_.resize(4096); - if (shill) { - shill->RegisterConnectivityListener(base::Bind( + if (network) { + network->AddOnConnectionChangedCallback(base::Bind( &XmppChannel::OnConnectivityChanged, weak_ptr_factory_.GetWeakPtr())); } }
diff --git a/libweave/src/notification/xmpp_channel.h b/libweave/src/notification/xmpp_channel.h index d25669f..57f86ab 100644 --- a/libweave/src/notification/xmpp_channel.h +++ b/libweave/src/notification/xmpp_channel.h
@@ -23,6 +23,8 @@ namespace weave { +class Network; + namespace privet { class ShillClient; } @@ -46,7 +48,7 @@ XmppChannel(const std::string& account, const std::string& access_token, const scoped_refptr<base::SingleThreadTaskRunner>& task_runner, - privet::ShillClient* shill); + Network* network); ~XmppChannel() override = default; // Overrides from NotificationChannel.
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index 917bcea..4523337 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -35,7 +35,7 @@ #include "libweave/src/privet/device_delegate.h" #include "libweave/src/privet/privet_handler.h" #include "libweave/src/privet/publisher.h" -#include "libweave/src/privet/shill_client.h" +#include "weave/network.h" namespace weave { namespace privet { @@ -62,7 +62,7 @@ void Manager::Start(const Device::Options& options, const scoped_refptr<dbus::Bus>& bus, - ShillClient* shill_client, + Network* network, DeviceRegistrationInfo* device, CommandManager* command_manager, StateManager* state_manager, @@ -76,7 +76,7 @@ security_.reset(new SecurityManager(device->GetConfig().pairing_modes(), device->GetConfig().embedded_code_path(), disable_security_)); - shill_client->RegisterConnectivityListener( + network->AddOnConnectionChangedCallback( base::Bind(&Manager::OnConnectivityChanged, base::Unretained(this))); ap_manager_client_.reset(new ApManagerClient(bus)); @@ -85,7 +85,7 @@ wifi_bootstrap_manager_.reset(new WifiBootstrapManager( device->GetConfig().last_configured_ssid(), options.test_privet_ssid, device->GetConfig().ble_setup_enabled(), - shill_client, ap_manager_client_.get(), cloud_.get())); + network, ap_manager_client_.get(), cloud_.get())); wifi_bootstrap_manager_->Init(); }
diff --git a/libweave/src/privet/privet_manager.h b/libweave/src/privet/privet_manager.h index 98ded7d..06d3bae 100644 --- a/libweave/src/privet/privet_manager.h +++ b/libweave/src/privet/privet_manager.h
@@ -36,6 +36,7 @@ class CommandManager; class DeviceRegistrationInfo; class Mdns; +class Network; class StateManager; namespace privet { @@ -47,7 +48,6 @@ class PrivetHandler; class Publisher; class SecurityManager; -class ShillClient; class Manager : public Privet, public CloudDelegate::Observer { public: @@ -56,7 +56,7 @@ void Start(const weave::Device::Options& options, const scoped_refptr<dbus::Bus>& bus, - ShillClient* shill_client, + Network* network, DeviceRegistrationInfo* device, CommandManager* command_manager, StateManager* state_manager,
diff --git a/libweave/src/privet/privet_types.cc b/libweave/src/privet/privet_types.cc index dcf3209..cbe33da 100644 --- a/libweave/src/privet/privet_types.cc +++ b/libweave/src/privet/privet_types.cc
@@ -8,6 +8,7 @@ #include "weave/enum_to_string.h" #include "weave/privet.h" +#include "weave/network.h" namespace weave { @@ -65,6 +66,13 @@ {WifiSetupState::kConnecting, "connecting"}, }; +const EnumToStringMap<NetworkState>::Map kNetworkStateMap[] = { + {NetworkState::kOffline, "offline"}, + {NetworkState::kFailure, "failure"}, + {NetworkState::kConnecting, "connecting"}, + {NetworkState::kConnected, "connected"}, +}; + } // namespace template <> @@ -95,4 +103,8 @@ EnumToStringMap<WifiSetupState>::EnumToStringMap() : EnumToStringMap(kWifiSetupStateMap) {} +template <> +EnumToStringMap<NetworkState>::EnumToStringMap() + : EnumToStringMap(kNetworkStateMap) {} + } // namespace weave
diff --git a/libweave/src/privet/shill_client.cc b/libweave/src/privet/shill_client.cc index defed3f..9c81064 100644 --- a/libweave/src/privet/shill_client.cc +++ b/libweave/src/privet/shill_client.cc
@@ -13,6 +13,8 @@ #include <chromeos/errors/error.h> #include <chromeos/errors/error_codes.h> +#include "weave/enum_to_string.h" + using chromeos::Any; using chromeos::VariantDictionary; using dbus::ObjectPath; @@ -52,50 +54,35 @@ return true; } -ServiceState ShillServiceStateToServiceState(const string& state) { +NetworkState ShillNetworkStateToNetworkState(const string& state) { // TODO(wiley) What does "unconfigured" mean in a world with multiple sets // of WiFi credentials? // TODO(wiley) Detect disabled devices, update state appropriately. if ((state.compare(shill::kStateReady) == 0) || (state.compare(shill::kStatePortal) == 0) || (state.compare(shill::kStateOnline) == 0)) { - return ServiceState::kConnected; + return NetworkState::kConnected; } if ((state.compare(shill::kStateAssociation) == 0) || (state.compare(shill::kStateConfiguration) == 0)) { - return ServiceState::kConnecting; + return NetworkState::kConnecting; } if ((state.compare(shill::kStateFailure) == 0) || (state.compare(shill::kStateActivationFailure) == 0)) { // TODO(wiley) Get error information off the service object. - return ServiceState::kFailure; + return NetworkState::kFailure; } if ((state.compare(shill::kStateIdle) == 0) || (state.compare(shill::kStateOffline) == 0) || (state.compare(shill::kStateDisconnect) == 0)) { - return ServiceState::kOffline; + return NetworkState::kOffline; } LOG(WARNING) << "Unknown state found: '" << state << "'"; - return ServiceState::kOffline; + return NetworkState::kOffline; } } // namespace -std::string ServiceStateToString(ServiceState state) { - switch (state) { - case ServiceState::kOffline: - return "offline"; - case ServiceState::kFailure: - return "failure"; - case ServiceState::kConnecting: - return "connecting"; - case ServiceState::kConnected: - return "connected"; - } - LOG(ERROR) << "Unknown ServiceState value!"; - return "unknown"; -} - ShillClient::ShillClient(const scoped_refptr<dbus::Bus>& bus, const set<string>& device_whitelist) : bus_{bus}, @@ -116,7 +103,7 @@ VLOG(2) << "ShillClient::Init();"; CleanupConnectingService(false); devices_.clear(); - connectivity_state_ = ServiceState::kOffline; + connectivity_state_ = NetworkState::kOffline; VariantDictionary properties; if (!manager_proxy_.GetProperties(&properties, nullptr)) { LOG(ERROR) << "Unable to get properties from Manager, waiting for " @@ -159,16 +146,12 @@ return true; } -ServiceState ShillClient::GetConnectionState() const { +NetworkState ShillClient::GetConnectionState() const { return connectivity_state_; } -bool ShillClient::AmOnline() const { - return connectivity_state_ == ServiceState::kConnected; -} - -void ShillClient::RegisterConnectivityListener( - const ConnectivityListener& listener) { +void ShillClient::AddOnConnectionChangedCallback( + const OnConnectionChangedCallback& listener) { connectivity_listeners_.push_back(listener); } @@ -195,7 +178,7 @@ if (new_owner.empty()) { CleanupConnectingService(false); devices_.clear(); - connectivity_state_ = ServiceState::kOffline; + connectivity_state_ = NetworkState::kOffline; } else { Init(); // New service owner means shill reset! } @@ -314,7 +297,7 @@ return; // Spurious update? } device_state.selected_service.reset(); - device_state.service_state = ServiceState::kOffline; + device_state.service_state = NetworkState::kOffline; removed_old_service = true; } std::shared_ptr<ServiceProxy> new_service; @@ -329,7 +312,7 @@ // happened long in the past for the connecting service. string state; if (GetStateForService(new_service.get(), &state)) { - device_state.service_state = ShillServiceStateToServiceState(state); + device_state.service_state = ShillNetworkStateToNetworkState(state); } else { LOG(WARNING) << "Failed to read properties from existing service " "on selection."; @@ -414,7 +397,7 @@ const string& state) { if (!connecting_service_ || connecting_service_->GetObjectPath() != service_path || - ShillServiceStateToServiceState(state) != ServiceState::kConnected) { + ShillNetworkStateToNetworkState(state) != NetworkState::kConnected) { return; } connecting_service_reset_pending_ = true; @@ -449,7 +432,7 @@ if (kv.second.selected_service && kv.second.selected_service->GetObjectPath() == service_path) { VLOG(3) << "Updated cached connection state for selected service."; - kv.second.service_state = ShillServiceStateToServiceState(state); + kv.second.service_state = ShillNetworkStateToNetworkState(state); UpdateConnectivityState(); return; } @@ -459,15 +442,14 @@ void ShillClient::UpdateConnectivityState() { // Update the connectivity state of the device by picking the // state of the currently most connected selected service. - ServiceState new_connectivity_state{ServiceState::kOffline}; + NetworkState new_connectivity_state{NetworkState::kOffline}; for (const auto& kv : devices_) { if (kv.second.service_state > new_connectivity_state) { new_connectivity_state = kv.second.service_state; } } - VLOG(1) << "Connectivity changed: " - << ServiceStateToString(connectivity_state_) << " -> " - << ServiceStateToString(new_connectivity_state); + VLOG(1) << "Connectivity changed: " << EnumToString(connectivity_state_) + << " -> " << EnumToString(new_connectivity_state); // Notify listeners even if state changed to the same value. Listeners may // want to handle this event. connectivity_state_ = new_connectivity_state; @@ -478,7 +460,8 @@ // state. base::MessageLoop::current()->PostTask( FROM_HERE, base::Bind(&ShillClient::NotifyConnectivityListeners, - weak_factory_.GetWeakPtr(), AmOnline())); + weak_factory_.GetWeakPtr(), + GetConnectionState() == NetworkState::kConnected)); } void ShillClient::NotifyConnectivityListeners(bool am_online) {
diff --git a/libweave/src/privet/shill_client.h b/libweave/src/privet/shill_client.h index ba36154..7d9069f 100644 --- a/libweave/src/privet/shill_client.h +++ b/libweave/src/privet/shill_client.h
@@ -18,45 +18,27 @@ #include <dbus/bus.h> #include "shill/dbus-proxies.h" +#include "weave/network.h" namespace weave { namespace privet { -enum class ServiceState { - kOffline = 0, - kFailure, - kConnecting, - kConnected, -}; - -std::string ServiceStateToString(ServiceState state); - -class ShillClient final { +class ShillClient final : public Network { public: - // A callback that interested parties can register to be notified of - // transitions from online to offline and vice versa. The boolean - // parameter will be true if we're online, and false if we're offline. - using ConnectivityListener = base::Callback<void(bool)>; - ShillClient(const scoped_refptr<dbus::Bus>& bus, const std::set<std::string>& device_whitelist); ~ShillClient() = default; void Init(); - void RegisterConnectivityListener(const ConnectivityListener& listener); - // Causes shill to attempt to connect to the given network with the given - // passphrase. This is accomplished by: - // 1) Configuring a service through the Manager with the SSID and passphrase. - // 2) Calling Connect() on the service. - // 2) Monitoring the returned Service object until we reach an online state, - // an error state, or another call to ConnectToService() occurs. - // Returns false on immediate failures with some descriptive codes in |error|. + + // Network implementation. + void AddOnConnectionChangedCallback( + const OnConnectionChangedCallback& listener) override; bool ConnectToService(const std::string& ssid, const std::string& passphrase, const base::Closure& on_success, - chromeos::ErrorPtr* error); - ServiceState GetConnectionState() const; - bool AmOnline() const; + chromeos::ErrorPtr* error) override; + NetworkState GetConnectionState() const override; private: struct DeviceState { @@ -66,7 +48,7 @@ // service (for instance, in the period between configuring a WiFi service // with credentials, and when Connect() is called.) std::shared_ptr<org::chromium::flimflam::ServiceProxy> selected_service; - ServiceState service_state{ServiceState::kOffline}; + NetworkState service_state{NetworkState::kOffline}; }; bool IsMonitoredDevice(org::chromium::flimflam::DeviceProxy* device); @@ -112,7 +94,7 @@ // There is logic that assumes we will never change this device list // in OnManagerPropertyChange. Do not be tempted to remove this const. const std::set<std::string> device_whitelist_; - std::vector<ConnectivityListener> connectivity_listeners_; + std::vector<OnConnectionChangedCallback> connectivity_listeners_; // State for tracking where we are in our attempts to connect to a service. bool connecting_service_reset_pending_{false}; @@ -122,7 +104,7 @@ // State for tracking our online connectivity. std::map<dbus::ObjectPath, DeviceState> devices_; - ServiceState connectivity_state_{ServiceState::kOffline}; + NetworkState connectivity_state_{NetworkState::kOffline}; base::WeakPtrFactory<ShillClient> weak_factory_{this};
diff --git a/libweave/src/privet/wifi_bootstrap_manager.cc b/libweave/src/privet/wifi_bootstrap_manager.cc index d1bf380..4eeaa94 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.cc +++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -12,7 +12,8 @@ #include "libweave/src/privet/ap_manager_client.h" #include "libweave/src/privet/constants.h" -#include "libweave/src/privet/shill_client.h" +#include "weave/enum_to_string.h" +#include "weave/network.h" namespace weave { namespace privet { @@ -27,10 +28,10 @@ const std::string& last_configured_ssid, const std::string& test_privet_ssid, bool ble_setup_enabled, - ShillClient* shill_client, + Network* network, ApManagerClient* ap_manager_client, CloudDelegate* gcd) - : shill_client_{shill_client}, + : network_{network}, ap_manager_client_{ap_manager_client}, ssid_generator_{gcd, this}, last_configured_ssid_{last_configured_ssid}, @@ -45,7 +46,7 @@ if (ssid.empty()) return; // Delay initialization until ssid_generator_ is ready. UpdateConnectionState(); - shill_client_->RegisterConnectivityListener( + network_->AddOnConnectionChangedCallback( base::Bind(&WifiBootstrapManager::OnConnectivityChange, lifetime_weak_factory_.GetWeakPtr())); if (last_configured_ssid_.empty()) { @@ -64,7 +65,7 @@ } void WifiBootstrapManager::StartBootstrapping() { - if (shill_client_->AmOnline()) { + if (network_->GetConnectionState() == NetworkState::kConnected) { // If one of the devices we monitor for connectivity is online, we need not // start an AP. For most devices, this is a situation which happens in // testing when we have an ethernet connection. If you need to always @@ -105,10 +106,10 @@ FROM_HERE, base::Bind(&WifiBootstrapManager::OnConnectTimeout, tasks_weak_factory_.GetWeakPtr()), base::TimeDelta::FromSeconds(kConnectTimeoutSeconds)); - shill_client_->ConnectToService( - ssid, passphrase, base::Bind(&WifiBootstrapManager::OnConnectSuccess, - tasks_weak_factory_.GetWeakPtr(), ssid), - nullptr); + network_->ConnectToService(ssid, passphrase, + base::Bind(&WifiBootstrapManager::OnConnectSuccess, + tasks_weak_factory_.GetWeakPtr(), ssid), + nullptr); } void WifiBootstrapManager::EndConnecting() { @@ -116,7 +117,7 @@ void WifiBootstrapManager::StartMonitoring() { VLOG(1) << "Monitoring connectivity."; - // We already have a callback in place with |shill_client_| to update our + // We already have a callback in place with |network_| to update our // connectivity state. See OnConnectivityChange(). UpdateState(State::kMonitoring); } @@ -262,12 +263,12 @@ connection_state_ = ConnectionState{ConnectionState::kUnconfigured}; if (last_configured_ssid_.empty()) return; - ServiceState service_state{shill_client_->GetConnectionState()}; + NetworkState service_state{network_->GetConnectionState()}; switch (service_state) { - case ServiceState::kOffline: + case NetworkState::kOffline: connection_state_ = ConnectionState{ConnectionState::kOffline}; return; - case ServiceState::kFailure: { + case NetworkState::kFailure: { // TODO(wiley) Pull error information from somewhere. chromeos::ErrorPtr error; chromeos::Error::AddTo(&error, FROM_HERE, errors::kDomain, @@ -275,10 +276,10 @@ connection_state_ = ConnectionState{std::move(error)}; return; } - case ServiceState::kConnecting: + case NetworkState::kConnecting: connection_state_ = ConnectionState{ConnectionState::kConnecting}; return; - case ServiceState::kConnected: + case NetworkState::kConnected: connection_state_ = ConnectionState{ConnectionState::kOnline}; return; } @@ -286,7 +287,7 @@ chromeos::Error::AddToPrintf(&error, FROM_HERE, errors::kDomain, errors::kInvalidState, "Unknown state returned from ShillClient: %s", - ServiceStateToString(service_state).c_str()); + EnumToString(service_state).c_str()); connection_state_ = ConnectionState{std::move(error)}; }
diff --git a/libweave/src/privet/wifi_bootstrap_manager.h b/libweave/src/privet/wifi_bootstrap_manager.h index fbe89d0..ad87df1 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.h +++ b/libweave/src/privet/wifi_bootstrap_manager.h
@@ -21,12 +21,14 @@ #include "libweave/src/privet/wifi_ssid_generator.h" namespace weave { + +class Network; + namespace privet { class ApManagerClient; class CloudDelegate; class DeviceDelegate; -class ShillClient; class WifiBootstrapManager : public WifiDelegate, public CloudDelegate::Observer { @@ -38,7 +40,7 @@ WifiBootstrapManager(const std::string& last_configured_ssid, const std::string& test_privet_ssid, bool wifi_setup_enabled, - ShillClient* shill_client, + Network* shill_client, ApManagerClient* ap_manager_client, CloudDelegate* gcd); ~WifiBootstrapManager() override = default; @@ -98,7 +100,7 @@ // It is not persisted to disk. SetupState setup_state_{SetupState::kNone}; ConnectionState connection_state_{ConnectionState::kDisabled}; - ShillClient* shill_client_; + Network* network_; ApManagerClient* ap_manager_client_; WifiSsidGenerator ssid_generator_;