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};