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>
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};
diff --git a/src/privet/wifi_bootstrap_manager.h b/src/privet/wifi_bootstrap_manager.h index 71dbb49..62a77c2 100644 --- a/src/privet/wifi_bootstrap_manager.h +++ b/src/privet/wifi_bootstrap_manager.h
@@ -74,6 +74,7 @@ void EndConnecting(); void StartMonitoring(const base::TimeDelta& timeout); + void ContinueMonitoring(const base::TimeDelta& timeout); void EndMonitoring(); // Update the current state, post tasks to notify listeners accordingly to
diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc index 6ff3f6d..fb00cc9 100644 --- a/src/weave_unittest.cc +++ b/src/weave_unittest.cc
@@ -21,13 +21,14 @@ #include "src/bind_lambda.h" using testing::_; +using testing::AtLeast; using testing::AtMost; using testing::HasSubstr; +using testing::InSequence; using testing::Invoke; using testing::InvokeWithoutArgs; using testing::MatchesRegex; using testing::Mock; -using testing::AtLeast; using testing::Return; using testing::ReturnRefOfCopy; using testing::StartsWith; @@ -266,10 +267,13 @@ void NotifyNetworkChanged(provider::Network::State state, base::TimeDelta delay) { - EXPECT_CALL(network_, GetConnectionState()).WillRepeatedly(Return(state)); - for (const auto& cb : network_callbacks_) { - task_runner_.PostDelayedTask(FROM_HERE, cb, delay); - } + auto task = [this, state] { + EXPECT_CALL(network_, GetConnectionState()).WillRepeatedly(Return(state)); + for (const auto& cb : network_callbacks_) + cb.Run(); + }; + + task_runner_.PostDelayedTask(FROM_HERE, base::Bind(task), delay); } std::map<std::string, provider::HttpServer::RequestHandlerCallback> @@ -460,4 +464,67 @@ StartDevice(); } +TEST_F(WeaveWiFiSetupTest, OfflineLongTimeWithNoSsid) { + EXPECT_CALL(network_, GetConnectionState()) + .WillRepeatedly(Return(Network::State::kOffline)); + NotifyNetworkChanged(provider::Network::State::kOnline, + base::TimeDelta::FromHours(15)); + + { + InSequence s; + auto time_stamp = task_runner_.GetClock()->Now(); + + EXPECT_CALL(wifi_, StartAccessPoint(MatchesRegex("TEST_NAME.*prv"))) + .WillOnce(InvokeWithoutArgs([this, &time_stamp]() { + EXPECT_LE(task_runner_.GetClock()->Now() - time_stamp, + base::TimeDelta::FromMinutes(1)); + time_stamp = task_runner_.GetClock()->Now(); + })); + + EXPECT_CALL(wifi_, StopAccessPoint()) + .WillOnce(InvokeWithoutArgs([this, &time_stamp]() { + EXPECT_GT(task_runner_.GetClock()->Now() - time_stamp, + base::TimeDelta::FromMinutes(5)); + time_stamp = task_runner_.GetClock()->Now(); + task_runner_.Break(); + })); + } + + StartDevice(); +} + +TEST_F(WeaveWiFiSetupTest, OfflineLongTimeWithSsid) { + EXPECT_CALL(config_store_, LoadSettings()) + .WillRepeatedly(Return(R"({"last_configured_ssid": "TEST_ssid"})")); + EXPECT_CALL(network_, GetConnectionState()) + .WillRepeatedly(Return(Network::State::kOffline)); + NotifyNetworkChanged(provider::Network::State::kOnline, + base::TimeDelta::FromHours(15)); + + { + InSequence s; + auto time_stamp = task_runner_.GetClock()->Now(); + for (size_t i = 0; i < 10; ++i) { + EXPECT_CALL(wifi_, StartAccessPoint(MatchesRegex("TEST_NAME.*prv"))) + .WillOnce(InvokeWithoutArgs([this, &time_stamp]() { + EXPECT_GT(task_runner_.GetClock()->Now() - time_stamp, + base::TimeDelta::FromMinutes(1)); + time_stamp = task_runner_.GetClock()->Now(); + })); + + EXPECT_CALL(wifi_, StopAccessPoint()) + .WillOnce(InvokeWithoutArgs([this, &time_stamp]() { + EXPECT_GT(task_runner_.GetClock()->Now() - time_stamp, + base::TimeDelta::FromMinutes(5)); + time_stamp = task_runner_.GetClock()->Now(); + })); + } + + EXPECT_CALL(wifi_, StartAccessPoint(MatchesRegex("TEST_NAME.*prv"))) + .WillOnce(InvokeWithoutArgs([this]() { task_runner_.Break(); })); + } + + StartDevice(); +} + } // namespace weave