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
