Remove notifications from WifiBootstrapManager Manager can change Config directly. BUG:24267885 Change-Id: I413479d4614eb84b4257be9f46ef3290693985f7 Reviewed-on: https://weave-review.googlesource.com/1176 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/include/weave/privet.h b/libweave/include/weave/privet.h index 60acbeb..a50548a 100644 --- a/libweave/include/weave/privet.h +++ b/libweave/include/weave/privet.h
@@ -36,10 +36,6 @@ using OnPairingEndedCallback = base::Callback<void(const std::string& session_id)>; - // Sets callback which is called when WiFi state is changed. - virtual void AddOnWifiSetupChangedCallback( - const OnWifiSetupChangedCallback& callback) = 0; - virtual void AddOnPairingChangedCallbacks( const OnPairingStartedCallback& on_start, const OnPairingEndedCallback& on_end) = 0;
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc index f8c4ef6..1180488 100644 --- a/libweave/src/device_manager.cc +++ b/libweave/src/device_manager.cc
@@ -97,19 +97,6 @@ privet_->Start(options, task_runner, network, dns_sd, http_server, wifi, device_info_.get(), command_manager_.get(), state_manager_.get()); - - privet_->AddOnWifiSetupChangedCallback( - base::Bind(&DeviceManager::OnWiFiBootstrapStateChanged, - weak_ptr_factory_.GetWeakPtr())); -} - -void DeviceManager::OnWiFiBootstrapStateChanged( - weave::privet::WifiBootstrapManager::State state) { - const std::string& ssid = privet_->GetCurrentlyConnectedSsid(); - if (ssid != device_info_->GetSettings().last_configured_ssid) { - weave::Config::Transaction change{device_info_->GetMutableConfig()}; - change.set_last_configured_ssid(ssid); - } } std::unique_ptr<Device> Device::Create() {
diff --git a/libweave/src/device_manager.h b/libweave/src/device_manager.h index 9a603fd..41d538e 100644 --- a/libweave/src/device_manager.h +++ b/libweave/src/device_manager.h
@@ -52,8 +52,6 @@ provider::Wifi* wifi, provider::Bluetooth* bluetooth); - void OnWiFiBootstrapStateChanged(weave::WifiSetupState state); - std::shared_ptr<CommandManager> command_manager_; std::unique_ptr<StateChangeQueue> state_change_queue_; std::shared_ptr<StateManager> state_manager_;
diff --git a/libweave/src/privet/privet_manager.cc b/libweave/src/privet/privet_manager.cc index 52a2657..5b72a2a 100644 --- a/libweave/src/privet/privet_manager.cc +++ b/libweave/src/privet/privet_manager.cc
@@ -63,9 +63,8 @@ if (wifi && device->GetSettings().wifi_auto_setup_enabled) { VLOG(1) << "Enabling WiFi bootstrapping."; wifi_bootstrap_manager_.reset(new WifiBootstrapManager( - device->GetSettings().last_configured_ssid, options.test_privet_ssid, - device->GetSettings().ble_setup_enabled, task_runner, network, wifi, - cloud_.get())); + options.test_privet_ssid, device->GetMutableConfig(), task_runner, + network, wifi, cloud_.get())); wifi_bootstrap_manager_->Init(); } @@ -94,14 +93,6 @@ : ""; } -void Manager::AddOnWifiSetupChangedCallback( - const WifiBootstrapManager::StateListener& callback) { - if (wifi_bootstrap_manager_) - wifi_bootstrap_manager_->RegisterStateListener(callback); - else - callback.Run(WifiSetupState::kDisabled); -} - void Manager::AddOnPairingChangedCallbacks( const SecurityManager::PairingStartListener& on_start, const SecurityManager::PairingEndListener& on_end) {
diff --git a/libweave/src/privet/privet_manager.h b/libweave/src/privet/privet_manager.h index 1c635da..5e3df3f 100644 --- a/libweave/src/privet/privet_manager.h +++ b/libweave/src/privet/privet_manager.h
@@ -59,9 +59,6 @@ std::string GetCurrentlyConnectedSsid() const; - void AddOnWifiSetupChangedCallback( - const OnWifiSetupChangedCallback& callback) override; - void AddOnPairingChangedCallbacks( const OnPairingStartedCallback& on_start, const OnPairingEndedCallback& on_end) override;
diff --git a/libweave/src/privet/wifi_bootstrap_manager.cc b/libweave/src/privet/wifi_bootstrap_manager.cc index 144fad6..924835a 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.cc +++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -13,27 +13,25 @@ #include "libweave/src/bind_lambda.h" #include "libweave/src/privet/constants.h" +#include "libweave/src/config.h" namespace weave { namespace privet { using provider::Network; -WifiBootstrapManager::WifiBootstrapManager( - const std::string& last_configured_ssid, - const std::string& test_privet_ssid, - bool ble_setup_enabled, - provider::TaskRunner* task_runner, - provider::Network* network, - provider::Wifi* wifi, - CloudDelegate* gcd) - : task_runner_{task_runner}, +WifiBootstrapManager::WifiBootstrapManager(const std::string& test_privet_ssid, + Config* config, + provider::TaskRunner* task_runner, + provider::Network* network, + provider::Wifi* wifi, + CloudDelegate* gcd) + : config_{config}, + task_runner_{task_runner}, network_{network}, wifi_{wifi}, ssid_generator_{gcd, this}, - last_configured_ssid_{last_configured_ssid}, - test_privet_ssid_{test_privet_ssid}, - ble_setup_enabled_{ble_setup_enabled} { + test_privet_ssid_{test_privet_ssid} { CHECK(network_); CHECK(task_runner_); CHECK(wifi_); @@ -44,20 +42,13 @@ network_->AddConnectionChangedCallback( base::Bind(&WifiBootstrapManager::OnConnectivityChange, lifetime_weak_factory_.GetWeakPtr())); - if (last_configured_ssid_.empty()) { + if (config_->GetSettings().last_configured_ssid.empty()) { StartBootstrapping(); } else { StartMonitoring(); } } -void WifiBootstrapManager::RegisterStateListener( - const StateListener& listener) { - // Notify about current state. - listener.Run(state_); - state_listeners_.push_back(listener); -} - void WifiBootstrapManager::StartBootstrapping() { if (network_->GetConnectionState() == Network::State::kConnected) { // If one of the devices we monitor for connectivity is online, we need not @@ -70,7 +61,7 @@ } UpdateState(State::kBootstrapping); - if (!last_configured_ssid_.empty()) { + if (!config_->GetSettings().last_configured_ssid.empty()) { // If we have been configured before, we'd like to periodically take down // our AP and find out if we can connect again. Many kinds of failures are // transient, and having an AP up prohibits us from connecting as a client. @@ -83,11 +74,13 @@ privet_ssid_ = GenerateSsid(); CHECK(!privet_ssid_.empty()); wifi_->StartAccessPoint(privet_ssid_); - LOG_IF(INFO, ble_setup_enabled_) << "BLE Bootstrap start: not implemented."; + LOG_IF(INFO, config_->GetSettings().ble_setup_enabled) + << "BLE Bootstrap start: not implemented."; } void WifiBootstrapManager::EndBootstrapping() { - LOG_IF(INFO, ble_setup_enabled_) << "BLE Bootstrap stop: not implemented."; + LOG_IF(INFO, config_->GetSettings().ble_setup_enabled) + << "BLE Bootstrap stop: not implemented."; wifi_->StopAccessPoint(); privet_ssid_.clear(); } @@ -162,22 +155,7 @@ break; } - if (new_state != state_) { - state_ = new_state; - // Post with weak ptr to avoid notification after this object destroyed. - task_runner_->PostDelayedTask( - FROM_HERE, base::Bind(&WifiBootstrapManager::NotifyStateListeners, - lifetime_weak_factory_.GetWeakPtr(), new_state), - {}); - } else { - VLOG(3) << "Not notifying listeners of state change, " - << "because the states are the same."; - } -} - -void WifiBootstrapManager::NotifyStateListeners(State new_state) const { - for (const StateListener& listener : state_listeners_) - listener.Run(new_state); + state_ = new_state; } std::string WifiBootstrapManager::GenerateSsid() const { @@ -209,7 +187,7 @@ std::string WifiBootstrapManager::GetCurrentlyConnectedSsid() const { // TODO(vitalybuka): Get from shill, if possible. - return last_configured_ssid_; + return config_->GetSettings().last_configured_ssid; } std::string WifiBootstrapManager::GetHostedSsid() const { @@ -223,7 +201,9 @@ void WifiBootstrapManager::OnConnectSuccess(const std::string& ssid) { VLOG(1) << "Wifi was connected successfully"; - last_configured_ssid_ = ssid; + Config::Transaction change{config_}; + change.set_last_configured_ssid(ssid); + change.Commit(); setup_state_ = SetupState{SetupState::kSuccess}; StartMonitoring(); } @@ -253,7 +233,7 @@ void WifiBootstrapManager::UpdateConnectionState() { connection_state_ = ConnectionState{ConnectionState::kUnconfigured}; - if (last_configured_ssid_.empty()) + if (config_->GetSettings().last_configured_ssid.empty()) return; Network::State service_state{network_->GetConnectionState()};
diff --git a/libweave/src/privet/wifi_bootstrap_manager.h b/libweave/src/privet/wifi_bootstrap_manager.h index 8174cac..fb6a517 100644 --- a/libweave/src/privet/wifi_bootstrap_manager.h +++ b/libweave/src/privet/wifi_bootstrap_manager.h
@@ -22,6 +22,8 @@ namespace weave { +class Config; + namespace provider { class Network; class TaskRunner; @@ -37,18 +39,14 @@ public: using State = WifiSetupState; - using StateListener = base::Callback<void(State)>; - - WifiBootstrapManager(const std::string& last_configured_ssid, - const std::string& test_privet_ssid, - bool wifi_setup_enabled, + WifiBootstrapManager(const std::string& test_privet_ssid, + Config* config, provider::TaskRunner* task_runner, provider::Network* shill_client, provider::Wifi* wifi, CloudDelegate* gcd); ~WifiBootstrapManager() override = default; virtual void Init(); - void RegisterStateListener(const StateListener& listener); // Overrides from WifiDelegate. const ConnectionState& GetConnectionState() const override; @@ -78,7 +76,6 @@ // Update the current state, post tasks to notify listeners accordingly to // the MessageLoop. void UpdateState(State new_state); - void NotifyStateListeners(State new_state) const; std::string GenerateSsid() const; @@ -98,18 +95,16 @@ // It is not persisted to disk. SetupState setup_state_{SetupState::kNone}; ConnectionState connection_state_{ConnectionState::kDisabled}; + Config* config_{nullptr}; provider::TaskRunner* task_runner_{nullptr}; provider::Network* network_{nullptr}; provider::Wifi* wifi_{nullptr}; WifiSsidGenerator ssid_generator_; base::Time monitor_until_; - std::vector<StateListener> state_listeners_; bool currently_online_{false}; - std::string last_configured_ssid_; std::string test_privet_ssid_; std::string privet_ssid_; - bool ble_setup_enabled_{false}; // Helps to reset irrelevant tasks switching state. base::WeakPtrFactory<WifiBootstrapManager> tasks_weak_factory_{this};