Simplify code by removing error from ::GetModelId and ::GetName
Error where used when privet lived in separate process and was
possibility of request before config was available.
Change-Id: I48da2898ad4b1b703d0db2c2cdf9fcf3269b2f88
diff --git a/libweave/src/privet/cloud_delegate.cc b/libweave/src/privet/cloud_delegate.cc
index cce7994..1db4228 100644
--- a/libweave/src/privet/cloud_delegate.cc
+++ b/libweave/src/privet/cloud_delegate.cc
@@ -65,22 +65,12 @@
~CloudDelegateImpl() override = default;
- bool GetModelId(std::string* id, ErrorPtr* error) const override {
- if (device_->GetConfig().model_id().size() != 5) {
- Error::AddToPrintf(error, FROM_HERE, errors::kDomain,
- errors::kInvalidState, "Model ID is invalid: %s",
- device_->GetConfig().model_id().c_str());
- return false;
- }
- *id = device_->GetConfig().model_id();
- return true;
+ std::string GetModelId() const override {
+ CHECK_EQ(5, device_->GetConfig().model_id().size());
+ return device_->GetConfig().model_id();
}
- // TODO(vitalybuka): Remove error.
- bool GetName(std::string* name, ErrorPtr* error) const override {
- *name = device_->GetConfig().name();
- return true;
- }
+ std::string GetName() const override { return device_->GetConfig().name(); }
std::string GetDescription() const override {
return device_->GetConfig().description();
diff --git a/libweave/src/privet/cloud_delegate.h b/libweave/src/privet/cloud_delegate.h
index 5f79a6d..eb87e67 100644
--- a/libweave/src/privet/cloud_delegate.h
+++ b/libweave/src/privet/cloud_delegate.h
@@ -49,10 +49,10 @@
};
// Returns the model ID of the device.
- virtual bool GetModelId(std::string* id, ErrorPtr* error) const = 0;
+ virtual std::string GetModelId() const = 0;
// Returns the name of device.
- virtual bool GetName(std::string* name, ErrorPtr* error) const = 0;
+ virtual std::string GetName() const = 0;
// Returns the description of the device.
virtual std::string GetDescription() const = 0;
diff --git a/libweave/src/privet/mock_delegates.h b/libweave/src/privet/mock_delegates.h
index 94686c3..552640e 100644
--- a/libweave/src/privet/mock_delegates.h
+++ b/libweave/src/privet/mock_delegates.h
@@ -139,8 +139,8 @@
class MockCloudDelegate : public CloudDelegate {
public:
- MOCK_CONST_METHOD2(GetModelId, bool(std::string*, ErrorPtr*));
- MOCK_CONST_METHOD2(GetName, bool(std::string*, ErrorPtr*));
+ MOCK_CONST_METHOD0(GetModelId, std::string());
+ MOCK_CONST_METHOD0(GetName, std::string());
MOCK_CONST_METHOD0(GetDescription, std::string());
MOCK_CONST_METHOD0(GetLocation, std::string());
MOCK_METHOD5(UpdateDeviceInfo,
@@ -180,10 +180,8 @@
const ErrorCallback&));
MockCloudDelegate() {
- EXPECT_CALL(*this, GetModelId(_, _))
- .WillRepeatedly(DoAll(SetArgPointee<0>("ABMID"), Return(true)));
- EXPECT_CALL(*this, GetName(_, _))
- .WillRepeatedly(DoAll(SetArgPointee<0>("TestDevice"), Return(true)));
+ EXPECT_CALL(*this, GetModelId()).WillRepeatedly(Return("ABMID"));
+ EXPECT_CALL(*this, GetName()).WillRepeatedly(Return("TestDevice"));
EXPECT_CALL(*this, GetDescription()).WillRepeatedly(Return(""));
EXPECT_CALL(*this, GetLocation()).WillRepeatedly(Return(""));
EXPECT_CALL(*this, UpdateDeviceInfo(_, _, _, _, _))
diff --git a/libweave/src/privet/privet_handler.cc b/libweave/src/privet/privet_handler.cc
index 97a8d6a..0e1b2e0 100644
--- a/libweave/src/privet/privet_handler.cc
+++ b/libweave/src/privet/privet_handler.cc
@@ -476,14 +476,8 @@
const RequestCallback& callback) {
base::DictionaryValue output;
- ErrorPtr error;
-
- std::string name;
- std::string model_id;
- if (!cloud_->GetName(&name, &error) ||
- !cloud_->GetModelId(&model_id, &error)) {
- return ReturnError(*error, callback);
- }
+ std::string name = cloud_->GetName();
+ std::string model_id = cloud_->GetModelId();
output.SetString(kInfoVersionKey, kInfoVersionValue);
output.SetString(kInfoIdKey, identity_->GetId());
@@ -660,10 +654,7 @@
void PrivetHandler::HandleSetupStart(const base::DictionaryValue& input,
const UserInfo& user_info,
const RequestCallback& callback) {
- std::string name;
- ErrorPtr error;
- if (!cloud_->GetName(&name, &error))
- return ReturnError(*error, callback);
+ std::string name{cloud_->GetName()};
input.GetString(kNameKey, &name);
std::string description{cloud_->GetDescription()};
@@ -680,12 +671,14 @@
const base::DictionaryValue* wifi = nullptr;
if (input.GetDictionary(kWifiKey, &wifi)) {
if (!wifi_ || wifi_->GetTypes().empty()) {
+ ErrorPtr error;
Error::AddTo(&error, FROM_HERE, errors::kDomain,
errors::kSetupUnavailable, "WiFi setup unavailible");
return ReturnError(*error, callback);
}
wifi->GetString(kSetupStartSsidKey, &ssid);
if (ssid.empty()) {
+ ErrorPtr error;
Error::AddToPrintf(&error, FROM_HERE, errors::kDomain,
errors::kInvalidParams, kInvalidParamValueFormat,
kSetupStartSsidKey, "");
@@ -698,6 +691,7 @@
if (input.GetDictionary(kGcdKey, ®istration)) {
registration->GetString(kSetupStartTicketIdKey, &ticket);
if (ticket.empty()) {
+ ErrorPtr error;
Error::AddToPrintf(&error, FROM_HERE, errors::kDomain,
errors::kInvalidParams, kInvalidParamValueFormat,
kSetupStartTicketIdKey, "");
diff --git a/libweave/src/privet/publisher.cc b/libweave/src/privet/publisher.cc
index e4accb1..a2595f5 100644
--- a/libweave/src/privet/publisher.cc
+++ b/libweave/src/privet/publisher.cc
@@ -50,12 +50,8 @@
}
void Publisher::ExposeService() {
- std::string name;
- std::string model_id;
- if (!cloud_->GetName(&name, nullptr) ||
- !cloud_->GetModelId(&model_id, nullptr)) {
- return;
- }
+ std::string name{cloud_->GetName()};
+ std::string model_id{cloud_->GetModelId()};
DCHECK_EQ(model_id.size(), 5U);
VLOG(1) << "Starting peerd advertising.";
diff --git a/libweave/src/privet/wifi_bootstrap_manager.cc b/libweave/src/privet/wifi_bootstrap_manager.cc
index 6dd044d..9948301 100644
--- a/libweave/src/privet/wifi_bootstrap_manager.cc
+++ b/libweave/src/privet/wifi_bootstrap_manager.cc
@@ -29,14 +29,9 @@
last_configured_ssid_{last_configured_ssid},
test_privet_ssid_{test_privet_ssid},
ble_setup_enabled_{ble_setup_enabled} {
- cloud_observer_.Add(gcd);
}
void WifiBootstrapManager::Init() {
- CHECK(!is_initialized_);
- std::string ssid = GenerateSsid();
- if (ssid.empty())
- return; // Delay initialization until ssid_generator_ is ready.
UpdateConnectionState();
network_->AddOnConnectionChangedCallback(
base::Bind(&WifiBootstrapManager::OnConnectivityChange,
@@ -46,7 +41,6 @@
} else {
StartMonitoring();
}
- is_initialized_ = true;
}
void WifiBootstrapManager::RegisterStateListener(
@@ -210,12 +204,6 @@
return {WifiType::kWifi24};
}
-void WifiBootstrapManager::OnDeviceInfoChanged() {
- // Initialization was delayed until dependencies are ready.
- if (!is_initialized_)
- Init();
-}
-
void WifiBootstrapManager::OnConnectSuccess(const std::string& ssid) {
VLOG(1) << "Wifi was connected successfully";
last_configured_ssid_ = ssid;
diff --git a/libweave/src/privet/wifi_bootstrap_manager.h b/libweave/src/privet/wifi_bootstrap_manager.h
index c0052a0..f32340d 100644
--- a/libweave/src/privet/wifi_bootstrap_manager.h
+++ b/libweave/src/privet/wifi_bootstrap_manager.h
@@ -13,8 +13,9 @@
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
#include <base/scoped_observer.h>
+#include <base/time/time.h>
+#include <weave/privet.h>
-#include "libweave/src/privet/cloud_delegate.h"
#include "libweave/src/privet/privet_types.h"
#include "libweave/src/privet/wifi_delegate.h"
#include "libweave/src/privet/wifi_ssid_generator.h"
@@ -22,14 +23,14 @@
namespace weave {
class Network;
+class TaskRunner;
namespace privet {
class CloudDelegate;
class DeviceDelegate;
-class WifiBootstrapManager : public WifiDelegate,
- public CloudDelegate::Observer {
+class WifiBootstrapManager : public WifiDelegate {
public:
using State = WifiSetupState;
@@ -55,9 +56,6 @@
std::string GetHostedSsid() const override;
std::set<WifiType> GetTypes() const override;
- // Overrides from CloudDelegate::Observer.
- void OnDeviceInfoChanged() override;
-
private:
// These Start* tasks:
// 1) Do state appropriate work for entering the indicated state.
@@ -91,8 +89,6 @@
void OnMonitorTimeout();
void UpdateConnectionState();
- // Initialization could be delayed if ssid_generator_ is not ready.
- bool is_initialized_{false};
State state_{State::kDisabled};
// Setup state is the temporal state of the most recent bootstrapping attempt.
// It is not persisted to disk.
@@ -110,8 +106,6 @@
std::string privet_ssid_;
bool ble_setup_enabled_{false};
- ScopedObserver<CloudDelegate, CloudDelegate::Observer> cloud_observer_{this};
-
// Helps to reset irrelevant tasks switching state.
base::WeakPtrFactory<WifiBootstrapManager> tasks_weak_factory_{this};
diff --git a/libweave/src/privet/wifi_ssid_generator.cc b/libweave/src/privet/wifi_ssid_generator.cc
index c1b7ff0..280fcb9 100644
--- a/libweave/src/privet/wifi_ssid_generator.cc
+++ b/libweave/src/privet/wifi_ssid_generator.cc
@@ -69,13 +69,9 @@
}
std::string WifiSsidGenerator::GenerateSsid() const {
- std::string name;
- std::string model_id;
- if (!gcd_ || !gcd_->GetName(&name, nullptr) ||
- !gcd_->GetModelId(&model_id, nullptr)) {
- return std::string();
- }
- std::string idx{base::IntToString(get_random_.Run())};
+ std::string name = gcd_->GetName();
+ std::string model_id = gcd_->GetModelId();
+ std::string idx = base::IntToString(get_random_.Run());
name = name.substr(0, kDeviceNameSize - idx.size() - 1);
CHECK_EQ(5u, model_id.size());
diff --git a/libweave/src/privet/wifi_ssid_generator_unittest.cc b/libweave/src/privet/wifi_ssid_generator_unittest.cc
index b30398b..eb7b94e 100644
--- a/libweave/src/privet/wifi_ssid_generator_unittest.cc
+++ b/libweave/src/privet/wifi_ssid_generator_unittest.cc
@@ -53,17 +53,9 @@
TEST_F(WifiSsidGeneratorTest, GenerateSsidLongName) {
SetRandomForTests(99);
- EXPECT_CALL(gcd_, GetName(_, _))
- .WillRepeatedly(
- DoAll(SetArgPointee<0>("Very Long Device Name"), Return(true)));
+ EXPECT_CALL(gcd_, GetName()).WillRepeatedly(Return("Very Long Device Name"));
EXPECT_EQ("Very Long Device 99.ABMIDABprv", ssid_generator_.GenerateSsid());
}
-TEST_F(WifiSsidGeneratorTest, GenerateSsidNoName) {
- SetRandomForTests(99);
- EXPECT_CALL(gcd_, GetName(_, _)).WillRepeatedly(Return(false));
- EXPECT_EQ("", ssid_generator_.GenerateSsid());
-}
-
} // namespace privet
} // namespace weave
