buffet: Remove unnecessary config options
We don't use them, we can re-add then when needed.
BUG=brillo:1213
CQ-DEPEND=CL:281024
TEST=`FEATURES=test emerge-gizmo buffet`
Change-Id: Ib1f8c410f82f9540e939b20325ff64b6dba900df
Reviewed-on: https://chromium-review.googlesource.com/281017
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/privet/cloud_delegate.cc b/buffet/privet/cloud_delegate.cc
index 7c9adc5..3c0449c 100644
--- a/buffet/privet/cloud_delegate.cc
+++ b/buffet/privet/cloud_delegate.cc
@@ -45,12 +45,10 @@
class CloudDelegateImpl : public CloudDelegate {
public:
- CloudDelegateImpl(bool is_gcd_setup_enabled,
- buffet::DeviceRegistrationInfo* device,
+ CloudDelegateImpl(buffet::DeviceRegistrationInfo* device,
buffet::CommandManager* command_manager,
buffet::StateManager* state_manager)
- : is_gcd_setup_enabled_(is_gcd_setup_enabled),
- device_{device},
+ : device_{device},
command_manager_{command_manager},
state_manager_{state_manager} {
device_->AddOnConfigChangedCallback(base::Bind(
@@ -141,12 +139,6 @@
bool Setup(const std::string& ticket_id,
const std::string& user,
chromeos::ErrorPtr* error) override {
- if (!is_gcd_setup_enabled_) {
- chromeos::Error::AddTo(error, FROM_HERE, errors::kDomain,
- errors::kSetupUnavailable,
- "GCD setup unavailable");
- return false;
- }
if (setup_state_.IsStatusEqual(SetupState::kInProgress)) {
chromeos::Error::AddTo(error, FROM_HERE, errors::kDomain,
errors::kDeviceBusy, "Setup in progress");
@@ -358,8 +350,6 @@
return false;
}
- bool is_gcd_setup_enabled_{false};
-
buffet::DeviceRegistrationInfo* device_{nullptr};
buffet::CommandManager* command_manager_{nullptr};
buffet::StateManager* state_manager_{nullptr};
@@ -396,12 +386,11 @@
// static
std::unique_ptr<CloudDelegate> CloudDelegate::CreateDefault(
- bool is_gcd_setup_enabled,
buffet::DeviceRegistrationInfo* device,
buffet::CommandManager* command_manager,
buffet::StateManager* state_manager) {
- return std::unique_ptr<CloudDelegateImpl>{new CloudDelegateImpl{
- is_gcd_setup_enabled, device, command_manager, state_manager}};
+ return std::unique_ptr<CloudDelegateImpl>{
+ new CloudDelegateImpl{device, command_manager, state_manager}};
}
void CloudDelegate::NotifyOnDeviceInfoChanged() {
diff --git a/buffet/privet/cloud_delegate.h b/buffet/privet/cloud_delegate.h
index 95bcca8..49dfd7d 100644
--- a/buffet/privet/cloud_delegate.h
+++ b/buffet/privet/cloud_delegate.h
@@ -133,7 +133,6 @@
// Create default instance.
static std::unique_ptr<CloudDelegate> CreateDefault(
- bool is_gcd_setup_enabled,
buffet::DeviceRegistrationInfo* device,
buffet::CommandManager* command_manager,
buffet::StateManager* state_manager);
diff --git a/buffet/privet/privet_manager.cc b/buffet/privet/privet_manager.cc
index 2ae9c8a..0365091 100644
--- a/buffet/privet/privet_manager.cc
+++ b/buffet/privet/privet_manager.cc
@@ -87,40 +87,22 @@
}
state_store_->Init();
- std::set<std::string> device_whitelist{options.device_whitelist};
- // This state store key doesn't exist naturally, but developers
- // sometime put it in their state store to cause the device to bring
- // up WiFi bootstrapping while being connected to an ethernet interface.
- std::string test_device_whitelist;
- if (device_whitelist.empty() &&
- state_store_->GetString(kWiFiBootstrapInterfaces,
- &test_device_whitelist)) {
- auto interfaces =
- chromeos::string_utils::Split(test_device_whitelist, ",", true, true);
- device_whitelist.insert(interfaces.begin(), interfaces.end());
- }
device_ = DeviceDelegate::CreateDefault();
- cloud_ = CloudDelegate::CreateDefault(
- parser_->gcd_bootstrap_mode() != GcdBootstrapMode::kDisabled, device,
- command_manager, state_manager);
+ cloud_ = CloudDelegate::CreateDefault(device, command_manager, state_manager);
cloud_observer_.Add(cloud_.get());
security_.reset(new SecurityManager(parser_->pairing_modes(),
parser_->embedded_code_path(),
disable_security_));
- shill_client_.reset(new ShillClient(
- bus, device_whitelist.empty() ? parser_->automatic_wifi_interfaces()
- : device_whitelist));
+ shill_client_.reset(new ShillClient(bus, options.device_whitelist));
shill_client_->RegisterConnectivityListener(
base::Bind(&Manager::OnConnectivityChanged, base::Unretained(this)));
ap_manager_client_.reset(new ApManagerClient(bus));
- if (parser_->wifi_bootstrap_mode() != WiFiBootstrapMode::kDisabled) {
+ if (parser_->wifi_auto_setup_enabled()) {
VLOG(1) << "Enabling WiFi bootstrapping.";
- wifi_bootstrap_manager_.reset(new WifiBootstrapManager(
- state_store_.get(), shill_client_.get(), ap_manager_client_.get(),
- cloud_.get(), parser_->connect_timeout_seconds(),
- parser_->bootstrap_timeout_seconds(),
- parser_->monitor_timeout_seconds()));
+ wifi_bootstrap_manager_.reset(
+ new WifiBootstrapManager(state_store_.get(), shill_client_.get(),
+ ap_manager_client_.get(), cloud_.get()));
wifi_bootstrap_manager_->Init();
}
diff --git a/buffet/privet/privetd_conf_parser.cc b/buffet/privet/privetd_conf_parser.cc
index 6ebe155..ca08ec8 100644
--- a/buffet/privet/privetd_conf_parser.cc
+++ b/buffet/privet/privetd_conf_parser.cc
@@ -15,105 +15,21 @@
namespace {
const char kWiFiBootstrapMode[] = "wifi_bootstrapping_mode";
-const char kGcdBootstrapMode[] = "gcd_bootstrapping_mode";
-const char kConnectTimeout[] = "connect_timeout_seconds";
-const char kBootstrapTimeout[] = "bootstrap_timeout_seconds";
-const char kMonitorTimeout[] = "monitor_timeout_seconds";
const char kPairingModes[] = "pairing_modes";
const char kEmbeddedCodePath[] = "embedded_code_path";
const char kBootstrapModeOff[] = "off";
-const char kBootstrapModeAutomatic[] = "automatic";
-const char kBootstrapModeManual[] = "manual";
} // namespace
-const char kWiFiBootstrapInterfaces[] = "automatic_mode_interfaces";
-
PrivetdConfigParser::PrivetdConfigParser()
- : wifi_bootstrap_mode_{WiFiBootstrapMode::kDisabled},
- gcd_bootstrap_mode_{GcdBootstrapMode::kDisabled},
- connect_timeout_seconds_{60u},
- bootstrap_timeout_seconds_{600u},
- monitor_timeout_seconds_{120u},
- pairing_modes_{PairingType::kPinCode} {
-}
+ : wifi_auto_setup_enabled_{true}, pairing_modes_{PairingType::kPinCode} {}
bool PrivetdConfigParser::Parse(const chromeos::KeyValueStore& config_store) {
std::string wifi_bootstrap_mode_str;
+ // TODO(vitalybuka): switch to boolean.
if (config_store.GetString(kWiFiBootstrapMode, &wifi_bootstrap_mode_str)) {
- if (wifi_bootstrap_mode_str.compare(kBootstrapModeOff) == 0) {
- wifi_bootstrap_mode_ = WiFiBootstrapMode::kDisabled;
- } else if (wifi_bootstrap_mode_str.compare(kBootstrapModeAutomatic) == 0) {
- wifi_bootstrap_mode_ = WiFiBootstrapMode::kAutomatic;
- } else if (wifi_bootstrap_mode_str.compare(kBootstrapModeManual) == 0) {
- LOG(ERROR) << "Manual WiFi bootstrapping mode is unsupported.";
- return false;
- } else {
- LOG(ERROR) << "Unrecognized WiFi bootstrapping mode: "
- << wifi_bootstrap_mode_str;
- return false;
- }
- }
-
- std::string gcd_bootstrap_mode_str;
- if (config_store.GetString(kGcdBootstrapMode, &gcd_bootstrap_mode_str)) {
- if (gcd_bootstrap_mode_str.compare(kBootstrapModeOff) == 0) {
- gcd_bootstrap_mode_ = GcdBootstrapMode::kDisabled;
- } else if (gcd_bootstrap_mode_str.compare(kBootstrapModeAutomatic) == 0) {
- gcd_bootstrap_mode_ = GcdBootstrapMode::kAutomatic;
- } else if (gcd_bootstrap_mode_str.compare(kBootstrapModeManual) == 0) {
- LOG(ERROR) << "Manual GCD bootstrapping mode is unsupported.";
- return false;
- } else {
- LOG(ERROR) << "Unrecognized GCD bootstrapping mode: "
- << gcd_bootstrap_mode_str;
- return false;
- }
- }
-
- std::string wifi_interface_list_str;
- if (config_store.GetString(kWiFiBootstrapInterfaces,
- &wifi_interface_list_str)) {
- auto interfaces =
- chromeos::string_utils::Split(wifi_interface_list_str, ",", true, true);
- automatic_wifi_interfaces_.insert(interfaces.begin(), interfaces.end());
- }
-
- auto parse_timeout = [](const std::string& input, uint32_t* parsed_value) {
- uint32_t temp{0};
- // Tragically, this will set |temp| even for invalid parsings.
- if (base::StringToUint(input, &temp)) {
- *parsed_value = temp; // Parse worked, use that value.
- return true;
- }
- return false;
- };
-
- std::string connect_timeout_seconds_str;
- if (config_store.GetString(kConnectTimeout, &connect_timeout_seconds_str) &&
- !parse_timeout(connect_timeout_seconds_str, &connect_timeout_seconds_)) {
- LOG(ERROR) << "Invalid string given for connect timeout: "
- << connect_timeout_seconds_str;
- return false;
- }
-
- std::string bootstrap_timeout_seconds_str;
- if (config_store.GetString(kBootstrapTimeout,
- &bootstrap_timeout_seconds_str) &&
- !parse_timeout(bootstrap_timeout_seconds_str,
- &bootstrap_timeout_seconds_)) {
- LOG(ERROR) << "Invalid string given for bootstrap timeout: "
- << bootstrap_timeout_seconds_str;
- return false;
- }
-
- std::string monitor_timeout_seconds_str;
- if (config_store.GetString(kMonitorTimeout, &monitor_timeout_seconds_str) &&
- !parse_timeout(monitor_timeout_seconds_str, &monitor_timeout_seconds_)) {
- LOG(ERROR) << "Invalid string given for monitor timeout: "
- << monitor_timeout_seconds_str;
- return false;
+ wifi_auto_setup_enabled_ = (wifi_bootstrap_mode_str != kBootstrapModeOff);
}
std::set<PairingType> pairing_modes;
diff --git a/buffet/privet/privetd_conf_parser.h b/buffet/privet/privetd_conf_parser.h
index 846342f..db3e6ec 100644
--- a/buffet/privet/privetd_conf_parser.h
+++ b/buffet/privet/privetd_conf_parser.h
@@ -12,20 +12,6 @@
namespace privetd {
-extern const char kWiFiBootstrapInterfaces[];
-
-enum class WiFiBootstrapMode {
- kDisabled,
- kManual,
- kAutomatic,
-};
-
-enum class GcdBootstrapMode {
- kDisabled,
- kManual,
- kAutomatic,
-};
-
enum class PairingType;
class PrivetdConfigParser final {
@@ -34,28 +20,14 @@
bool Parse(const chromeos::KeyValueStore& config_store);
- WiFiBootstrapMode wifi_bootstrap_mode() const { return wifi_bootstrap_mode_; }
- GcdBootstrapMode gcd_bootstrap_mode() const { return gcd_bootstrap_mode_; }
- const std::set<std::string>& automatic_wifi_interfaces() const {
- return automatic_wifi_interfaces_;
- }
- uint32_t connect_timeout_seconds() const { return connect_timeout_seconds_; }
- uint32_t bootstrap_timeout_seconds() const {
- return bootstrap_timeout_seconds_;
- }
- uint32_t monitor_timeout_seconds() const { return monitor_timeout_seconds_; }
+ bool wifi_auto_setup_enabled() const { return wifi_auto_setup_enabled_; }
const std::set<PairingType>& pairing_modes() { return pairing_modes_; }
const base::FilePath& embedded_code_path() const {
return embedded_code_path_;
}
private:
- WiFiBootstrapMode wifi_bootstrap_mode_;
- GcdBootstrapMode gcd_bootstrap_mode_;
- std::set<std::string> automatic_wifi_interfaces_;
- uint32_t connect_timeout_seconds_;
- uint32_t bootstrap_timeout_seconds_;
- uint32_t monitor_timeout_seconds_;
+ bool wifi_auto_setup_enabled_;
std::set<PairingType> pairing_modes_;
base::FilePath embedded_code_path_;
};
diff --git a/buffet/privet/privetd_conf_parser_unittest.cc b/buffet/privet/privetd_conf_parser_unittest.cc
index 5dfde4c..86f0322 100644
--- a/buffet/privet/privetd_conf_parser_unittest.cc
+++ b/buffet/privet/privetd_conf_parser_unittest.cc
@@ -27,10 +27,6 @@
namespace {
const char kWiFiBootstrapMode[] = "wifi_bootstrapping_mode";
-const char kGcdBootstrapMode[] = "gcd_bootstrapping_mode";
-const char kConnectTimeout[] = "connect_timeout_seconds";
-const char kBootstrapTimeout[] = "bootstrap_timeout_seconds";
-const char kMonitorTimeout[] = "monitor_timeout_seconds";
const char kPairingModes[] = "pairing_modes";
const char kEmbeddedCodePath[] = "embedded_code_path";
@@ -45,13 +41,6 @@
temp_file_ = temp_dir_.path().Append("temp.conf");
}
- bool IsValid(const ConfDict& conf_dict) {
- KeyValueStore store;
- FillKeyValueStore(conf_dict, &store);
- PrivetdConfigParser config;
- return config.Parse(store);
- }
-
void FillKeyValueStore(const ConfDict& conf_dict, KeyValueStore* store) {
std::vector<string> file_pieces;
for (const auto& it : conf_dict) {
@@ -69,42 +58,12 @@
base::ScopedTempDir temp_dir_;
};
-TEST_F(PrivetdConfParserTest, ShouldRejectInvalidTimeouts) {
- EXPECT_FALSE(IsValid({{kConnectTimeout, "-1"}}));
- EXPECT_FALSE(IsValid({{kConnectTimeout, "a"}}));
- EXPECT_FALSE(IsValid({{kConnectTimeout, ""}}));
- EXPECT_FALSE(IsValid({{kConnectTimeout, "30 430"}}));
-}
-
-TEST_F(PrivetdConfParserTest, ShouldRejectInvalidWiFiBootstrapModes) {
- EXPECT_FALSE(IsValid({{kWiFiBootstrapMode, ""}}));
- EXPECT_FALSE(IsValid({{kWiFiBootstrapMode, "clown_shoes"}}));
- EXPECT_FALSE(IsValid({{kWiFiBootstrapMode, "off is invalid"}}));
- EXPECT_FALSE(IsValid({{kWiFiBootstrapMode, "30"}}));
-}
-
-TEST_F(PrivetdConfParserTest, ShouldRejectInvalidGcdBootstrapModes) {
- EXPECT_FALSE(IsValid({{kGcdBootstrapMode, ""}}));
- EXPECT_FALSE(IsValid({{kGcdBootstrapMode, "clown_shoes"}}));
- EXPECT_FALSE(IsValid({{kGcdBootstrapMode, "off is invalid"}}));
- EXPECT_FALSE(IsValid({{kGcdBootstrapMode, "30"}}));
-}
-
TEST_F(PrivetdConfParserTest, ShouldParseSettings) {
- const std::set<std::string> kExpectedWiFiInterfaces{"eth1", "clown shoes"};
- const uint32_t kExpectedConnectTimeout{1};
- const uint32_t kExpectedBootstrapTimeout{2};
- const uint32_t kExpectedMonitorTimeout{3};
const std::set<PairingType> kExpectedPairingModes{PairingType::kEmbeddedCode,
PairingType::kPinCode};
static const char kExpectedEmbeddedCodePath[]{"123ABC"};
const ConfDict conf_dict{
{kWiFiBootstrapMode, "automatic"},
- {kGcdBootstrapMode, "automatic"},
- {kWiFiBootstrapInterfaces, Join(",", kExpectedWiFiInterfaces)},
- {kConnectTimeout, std::to_string(kExpectedConnectTimeout)},
- {kBootstrapTimeout, std::to_string(kExpectedBootstrapTimeout)},
- {kMonitorTimeout, std::to_string(kExpectedMonitorTimeout)},
{kPairingModes, "pinCode"},
{kEmbeddedCodePath, kExpectedEmbeddedCodePath},
};
@@ -112,23 +71,14 @@
FillKeyValueStore(conf_dict, &store);
PrivetdConfigParser parser;
EXPECT_TRUE(parser.Parse(store));
- EXPECT_EQ(WiFiBootstrapMode::kAutomatic, parser.wifi_bootstrap_mode());
- EXPECT_EQ(GcdBootstrapMode::kAutomatic, parser.gcd_bootstrap_mode());
- EXPECT_EQ(kExpectedWiFiInterfaces, parser.automatic_wifi_interfaces());
- EXPECT_EQ(kExpectedConnectTimeout, parser.connect_timeout_seconds());
- EXPECT_EQ(kExpectedBootstrapTimeout, parser.bootstrap_timeout_seconds());
- EXPECT_EQ(kExpectedMonitorTimeout, parser.monitor_timeout_seconds());
+ EXPECT_TRUE(parser.wifi_auto_setup_enabled());
EXPECT_EQ(kExpectedPairingModes, parser.pairing_modes());
EXPECT_EQ(kExpectedEmbeddedCodePath, parser.embedded_code_path().value());
}
TEST_F(PrivetdConfParserTest, CriticalDefaults) {
PrivetdConfigParser parser;
- EXPECT_EQ(WiFiBootstrapMode::kDisabled, parser.wifi_bootstrap_mode());
- EXPECT_EQ(GcdBootstrapMode::kDisabled, parser.gcd_bootstrap_mode());
- EXPECT_GT(parser.connect_timeout_seconds(), 0);
- EXPECT_GT(parser.bootstrap_timeout_seconds(), 0);
- EXPECT_GT(parser.monitor_timeout_seconds(), 0);
+ EXPECT_TRUE(parser.wifi_auto_setup_enabled());
EXPECT_EQ(std::set<PairingType>{PairingType::kPinCode},
parser.pairing_modes());
EXPECT_TRUE(parser.embedded_code_path().empty());
diff --git a/buffet/privet/wifi_bootstrap_manager.cc b/buffet/privet/wifi_bootstrap_manager.cc
index fc3117c..da91a11 100644
--- a/buffet/privet/wifi_bootstrap_manager.cc
+++ b/buffet/privet/wifi_bootstrap_manager.cc
@@ -16,20 +16,20 @@
namespace privetd {
+namespace {
+const int kConnectTimeoutSeconds = 60;
+const int kBootstrapTimeoutSeconds = 600;
+const int kMonitorTimeoutSeconds = 120;
+}
+
WifiBootstrapManager::WifiBootstrapManager(DaemonState* state_store,
ShillClient* shill_client,
ApManagerClient* ap_manager_client,
- CloudDelegate* gcd,
- uint32_t connect_timeout_seconds,
- uint32_t bootstrap_timeout_seconds,
- uint32_t monitor_timeout_seconds)
+ CloudDelegate* gcd)
: state_store_(state_store),
shill_client_(shill_client),
ap_manager_client_(ap_manager_client),
- ssid_generator_(gcd, this),
- connect_timeout_seconds_(connect_timeout_seconds),
- bootstrap_timeout_seconds_(bootstrap_timeout_seconds),
- monitor_timeout_seconds_(monitor_timeout_seconds) {
+ ssid_generator_(gcd, this) {
cloud_observer_.Add(gcd);
}
@@ -83,7 +83,7 @@
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, base::Bind(&WifiBootstrapManager::OnBootstrapTimeout,
tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromSeconds(bootstrap_timeout_seconds_));
+ base::TimeDelta::FromSeconds(kBootstrapTimeoutSeconds));
}
// TODO(vitalybuka): Add SSID probing.
std::string ssid = ssid_generator_.GenerateSsid();
@@ -103,7 +103,7 @@
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, base::Bind(&WifiBootstrapManager::OnConnectTimeout,
tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromSeconds(connect_timeout_seconds_));
+ base::TimeDelta::FromSeconds(kConnectTimeoutSeconds));
shill_client_->ConnectToService(
ssid, passphrase, base::Bind(&WifiBootstrapManager::OnConnectSuccess,
tasks_weak_factory_.GetWeakPtr(), ssid),
@@ -246,7 +246,7 @@
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE, base::Bind(&WifiBootstrapManager::OnMonitorTimeout,
tasks_weak_factory_.GetWeakPtr()),
- base::TimeDelta::FromSeconds(monitor_timeout_seconds_));
+ base::TimeDelta::FromSeconds(kMonitorTimeoutSeconds));
}
}
}
diff --git a/buffet/privet/wifi_bootstrap_manager.h b/buffet/privet/wifi_bootstrap_manager.h
index bb7d3ea..81812ef 100644
--- a/buffet/privet/wifi_bootstrap_manager.h
+++ b/buffet/privet/wifi_bootstrap_manager.h
@@ -43,10 +43,7 @@
WifiBootstrapManager(DaemonState* state_store,
ShillClient* shill_client,
ApManagerClient* ap_manager_client,
- CloudDelegate* gcd,
- uint32_t connect_timeout_seconds,
- uint32_t bootstrap_timeout_seconds,
- uint32_t monitor_timeout_seconds);
+ CloudDelegate* gcd);
~WifiBootstrapManager() override = default;
virtual void Init();
void RegisterStateListener(const StateListener& listener);
@@ -107,9 +104,6 @@
ApManagerClient* ap_manager_client_;
WifiSsidGenerator ssid_generator_;
- uint32_t connect_timeout_seconds_;
- uint32_t bootstrap_timeout_seconds_;
- uint32_t monitor_timeout_seconds_;
std::vector<StateListener> state_listeners_;
bool have_ever_been_bootstrapped_{false};
bool currently_online_{false};
