commit 65e1f21f13102b076156e2e6a07486c06f5b25d1
author Vitaly Buka <vitalybuka@google.com> Thu Nov 05 15:54:05 2015 -0800
committer Vitaly Buka <vitalybuka@google.com> Fri Nov 06 07:03:29 2015 +0000
tree 736cf3794df91f0d6e78b430de07241a9ece115a
parent d7c0fb8285e62e33b80623e9a48ef729905bafdf

Fix behavior of bootstrap manager after monitoring timeout was reached

StartMonitoring resets monitor_until_ member always.
ContinueMonitoring resets monitor_until_ only for online state as we
don't have timeout in this state.

Removed check for last_configured_ssid.empty() from
UpdateConnectionState because we should shutdown bootstrapping if device
is connected using other interface.

BUG:25463084

Change-Id: I0afd943f4a3ca797b65a51236103ea3d345828d2
Reviewed-on: https://weave-review.googlesource.com/1473
Reviewed-by: Paul Westbrook <pwestbro@google.com>

src/privet/wifi_bootstrap_manager.cc

diff --git a/src/privet/wifi_bootstrap_manager.cc b/src/privet/wifi_bootstrap_manager.cc
index 4f46ea4..292622d 100644
--- a/src/privet/wifi_bootstrap_manager.cc
+++ b/src/privet/wifi_bootstrap_manager.cc
@@ -20,7 +20,10 @@
 
 namespace {
 
+const int kMonitoringWithSsidTimeoutSeconds = 15;
 const int kMonitoringTimeoutSeconds = 120;
+const int kBootstrapTimeoutSeconds = 600;
+const int kConnectingTimeoutSeconds = 180;
 
 const EnumToStringMap<WifiBootstrapManager::State>::Map kWifiSetupStateMap[] = {
     {WifiBootstrapManager::State::kDisabled, "disabled"},
@@ -55,7 +58,8 @@
                  lifetime_weak_factory_.GetWeakPtr()));
   if (config_->GetSettings().last_configured_ssid.empty()) {
     // Give implementation some time to figure out state.
-    StartMonitoring(base::TimeDelta::FromSeconds(15));
+    StartMonitoring(
+        base::TimeDelta::FromSeconds(kMonitoringWithSsidTimeoutSeconds));
   } else {
     StartMonitoring(base::TimeDelta::FromSeconds(kMonitoringTimeoutSeconds));
   }
@@ -80,28 +84,30 @@
     task_runner_->PostDelayedTask(
         FROM_HERE, base::Bind(&WifiBootstrapManager::OnBootstrapTimeout,
                               tasks_weak_factory_.GetWeakPtr()),
-        base::TimeDelta::FromMinutes(10));
+        base::TimeDelta::FromSeconds(kBootstrapTimeoutSeconds));
   }
   // TODO(vitalybuka): Add SSID probing.
   privet_ssid_ = GenerateSsid();
   CHECK(!privet_ssid_.empty());
+
+  VLOG(1) << "Starting AP with SSID: " << privet_ssid_;
   wifi_->StartAccessPoint(privet_ssid_);
 }
 
 void WifiBootstrapManager::EndBootstrapping() {
+  VLOG(1) << "Stopping AP";
   wifi_->StopAccessPoint();
   privet_ssid_.clear();
 }
 
 void WifiBootstrapManager::StartConnecting(const std::string& ssid,
                                            const std::string& passphrase) {
-  VLOG(1) << "WiFi is attempting to connect. (ssid=" << ssid
-          << ", pass=" << passphrase << ").";
+  VLOG(1) << "Attempting connect to SSID:" << ssid;
   UpdateState(State::kConnecting);
   task_runner_->PostDelayedTask(
       FROM_HERE, base::Bind(&WifiBootstrapManager::OnConnectTimeout,
                             tasks_weak_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMinutes(3));
+      base::TimeDelta::FromSeconds(kConnectingTimeoutSeconds));
   wifi_->Connect(ssid, passphrase,
                  base::Bind(&WifiBootstrapManager::OnConnectDone,
                             tasks_weak_factory_.GetWeakPtr(), ssid));
@@ -110,6 +116,11 @@
 void WifiBootstrapManager::EndConnecting() {}
 
 void WifiBootstrapManager::StartMonitoring(const base::TimeDelta& timeout) {
+  monitor_until_ = {};
+  ContinueMonitoring(timeout);
+}
+
+void WifiBootstrapManager::ContinueMonitoring(const base::TimeDelta& timeout) {
   VLOG(1) << "Monitoring connectivity.";
   // We already have a callback in place with |network_| to update our
   // connectivity state.  See OnConnectivityChange().
@@ -134,8 +145,8 @@
 void WifiBootstrapManager::EndMonitoring() {}
 
 void WifiBootstrapManager::UpdateState(State new_state) {
-  VLOG(3) << "Switching state from " << static_cast<int>(state_) << " to "
-          << static_cast<int>(new_state);
+  VLOG(3) << "Switching state from " << EnumToString(state_) << " to "
+          << EnumToString(new_state);
   // Abort irrelevant tasks.
   tasks_weak_factory_.InvalidateWeakPtrs();
 
@@ -227,29 +238,26 @@
 }
 
 void WifiBootstrapManager::OnConnectivityChange() {
-  VLOG(3) << "ConnectivityChanged: "
-          << EnumToString(network_->GetConnectionState());
   UpdateConnectionState();
 
-  if (state_ == State::kMonitoring ||  // Reset monitoring timeout.
+  if (state_ == State::kMonitoring ||
       (state_ != State::kDisabled &&
        network_->GetConnectionState() == Network::State::kOnline)) {
-    StartMonitoring(base::TimeDelta::FromSeconds(kMonitoringTimeoutSeconds));
+    ContinueMonitoring(base::TimeDelta::FromSeconds(kMonitoringTimeoutSeconds));
   }
 }
 
 void WifiBootstrapManager::OnMonitorTimeout() {
-  VLOG(1) << "Spent too long offline.  Entering bootstrap mode.";
+  VLOG(1) << "Spent too long offline. Entering bootstrap mode.";
   // TODO(wiley) Retrieve relevant errors from shill.
   StartBootstrapping();
 }
 
 void WifiBootstrapManager::UpdateConnectionState() {
   connection_state_ = ConnectionState{ConnectionState::kUnconfigured};
-  if (config_->GetSettings().last_configured_ssid.empty())
-    return;
 
   Network::State service_state{network_->GetConnectionState()};
+  VLOG(3) << "New network state: " << EnumToString(service_state);
   switch (service_state) {
     case Network::State::kOffline:
       connection_state_ = ConnectionState{ConnectionState::kOffline};