Load state and commands definition by calling methods of device This allows to have dynamic state and commands expansion and calls can be places near command handlers code. BUG:24267885 Change-Id: I1777b321cface7c081258fa62eec272a8a173dc8 Reviewed-on: https://weave-review.googlesource.com/1252 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/libweave/examples/ubuntu/file_config_store.cc b/libweave/examples/ubuntu/file_config_store.cc index c59de56..b1bb17a 100644 --- a/libweave/examples/ubuntu/file_config_store.cc +++ b/libweave/examples/ubuntu/file_config_store.cc
@@ -60,48 +60,5 @@ str << settings; } -std::vector<std::string> FileConfigStore::LoadCommandDefs() { - return {R"({ - "base": { - "updateBaseConfiguration": {}, - "identify": {}, - "updateDeviceInfo": {} - }, - "_greeter": { - "_greet": { - "minimalRole": "user", - "parameters": { "_name": "string"}, - "results": { "_greeting": "string" } - } - }, - "_ledflasher": { - "_set":{ - "parameters": { - "_led": {"minimum": 1, "maximum": 3}, - "_on": "boolean" - } - }, - "_toggle":{ - "parameters": { - "_led": {"minimum": 1, "maximum": 3} - } - } - } - })"}; -} -std::vector<std::string> FileConfigStore::LoadStateDefs() { - return {R"({ - "_greeter": {"_greetings_counter":"integer"}, - "_ledflasher": {"_leds": {"items": "boolean"}} - })"}; -} - -std::vector<std::string> FileConfigStore::LoadStateDefaults() { - return {R"({ - "_greeter": {"_greetings_counter": 0}, - "_ledflasher":{"_leds": [false, false, false]} - })"}; -} - } // namespace examples } // namespace weave
diff --git a/libweave/examples/ubuntu/file_config_store.h b/libweave/examples/ubuntu/file_config_store.h index 926576f..25431e3 100644 --- a/libweave/examples/ubuntu/file_config_store.h +++ b/libweave/examples/ubuntu/file_config_store.h
@@ -21,9 +21,6 @@ bool LoadDefaults(Settings* settings) override; std::string LoadSettings() override; void SaveSettings(const std::string& settings) override; - std::vector<std::string> LoadCommandDefs() override; - std::vector<std::string> LoadStateDefs() override; - std::vector<std::string> LoadStateDefaults() override; private: bool disable_security_{false};
diff --git a/libweave/examples/ubuntu/main.cc b/libweave/examples/ubuntu/main.cc index c28d164..b53e6c5 100644 --- a/libweave/examples/ubuntu/main.cc +++ b/libweave/examples/ubuntu/main.cc
@@ -34,6 +34,45 @@ class CommandHandler { public: explicit CommandHandler(weave::Device* device) : device_{device} { + device->AddCommandDefinitions(R"({ + "base": { + "updateBaseConfiguration": {}, + "identify": {}, + "updateDeviceInfo": {} + }, + "_greeter": { + "_greet": { + "minimalRole": "user", + "parameters": { "_name": "string"}, + "results": { "_greeting": "string" } + } + }, + "_ledflasher": { + "_set":{ + "parameters": { + "_led": {"minimum": 1, "maximum": 3}, + "_on": "boolean" + } + }, + "_toggle":{ + "parameters": { + "_led": {"minimum": 1, "maximum": 3} + } + } + } + })"); + + device->AddStateDefinitions(R"({ + "_greeter": {"_greetings_counter":"integer"}, + "_ledflasher": {"_leds": {"items": "boolean"}} + })"); + + device->SetState(R"({ + "_greeter": {"_greetings_counter": 0}, + "_ledflasher":{"_leds": [false, false, false]} + })", + nullptr); + device->AddCommandHandler("_greeter._greet", base::Bind(&CommandHandler::OnGreetCommand, weak_ptr_factory_.GetWeakPtr()));
diff --git a/libweave/include/weave/provider/config_store.h b/libweave/include/weave/provider/config_store.h index aa7c5e5..7bc5d31 100644 --- a/libweave/include/weave/provider/config_store.h +++ b/libweave/include/weave/provider/config_store.h
@@ -35,15 +35,6 @@ // recommended to protect data, e.g. using encryption. virtual void SaveSettings(const std::string& settings) = 0; - // Returns command definitions as array of JSONs. - virtual std::vector<std::string> LoadCommandDefs() = 0; - - // Returns device state definitions as array of JSONs. - virtual std::vector<std::string> LoadStateDefs() = 0; - - // Returns device state defaults as array of JSONs. - virtual std::vector<std::string> LoadStateDefaults() = 0; - protected: virtual ~ConfigStore() = default; };
diff --git a/libweave/include/weave/provider/test/mock_config_store.h b/libweave/include/weave/provider/test/mock_config_store.h index 6eab802..4aef17e 100644 --- a/libweave/include/weave/provider/test/mock_config_store.h +++ b/libweave/include/weave/provider/test/mock_config_store.h
@@ -41,10 +41,6 @@ MOCK_METHOD1(LoadDefaults, bool(Settings*)); MOCK_METHOD0(LoadSettings, std::string()); MOCK_METHOD1(SaveSettings, void(const std::string&)); - - MOCK_METHOD0(LoadCommandDefs, std::vector<std::string>()); - MOCK_METHOD0(LoadStateDefs, std::vector<std::string>()); - MOCK_METHOD0(LoadStateDefaults, std::vector<std::string>()); }; } // namespace test
diff --git a/libweave/src/commands/command_manager.cc b/libweave/src/commands/command_manager.cc index 9aa3385..efca5ff 100644 --- a/libweave/src/commands/command_manager.cc +++ b/libweave/src/commands/command_manager.cc
@@ -7,7 +7,6 @@ #include <base/values.h> #include <weave/enum_to_string.h> #include <weave/error.h> -#include <weave/provider/config_store.h> #include "src/commands/schema_constants.h" #include "src/utils.h" @@ -93,15 +92,11 @@ return LoadCommands(*dict, error); } -void CommandManager::Startup(provider::ConfigStore* config_store) { +void CommandManager::Startup() { LOG(INFO) << "Initializing CommandManager."; // Load global standard GCD command dictionary. CHECK(LoadBaseCommands(kBaseCommandDefs, nullptr)); - - // Loading the rest of commands. - for (const auto& defs : config_store->LoadCommandDefs()) - CHECK(this->LoadCommands(defs, nullptr)); } void CommandManager::AddCommand(
diff --git a/libweave/src/commands/command_manager.h b/libweave/src/commands/command_manager.h index 97fc73f..f34a907 100644 --- a/libweave/src/commands/command_manager.h +++ b/libweave/src/commands/command_manager.h
@@ -20,10 +20,6 @@ class CommandInstance; -namespace provider { -class ConfigStore; -} - // CommandManager class that will have a list of all the device command // schemas as well as the live command queue of pending command instances // dispatched to the device. @@ -71,10 +67,8 @@ ErrorPtr* error); // Startup method to be called by buffet daemon at startup. - // Initializes the object and loads: - // 1) the standard GCD command dictionary - // 2) static vendor-provided command definitions - void Startup(weave::provider::ConfigStore* config_store); + // Initializes standard GCD command dictionary. + void Startup(); // Adds a new command to the command queue. void AddCommand(std::unique_ptr<CommandInstance> command_instance);
diff --git a/libweave/src/commands/command_manager_unittest.cc b/libweave/src/commands/command_manager_unittest.cc index 3e0d383..5101b4e 100644 --- a/libweave/src/commands/command_manager_unittest.cc +++ b/libweave/src/commands/command_manager_unittest.cc
@@ -115,11 +115,9 @@ TEST(CommandManager, ShouldLoadStandardAndTestDefinitions) { CommandManager manager; - provider::test::MockConfigStore config_store; - EXPECT_CALL(config_store, LoadCommandDefs()) - .WillOnce(Return( - std::vector<std::string>{kTestVendorCommands, kTestTestCommands})); - manager.Startup(&config_store); + manager.Startup(); + ASSERT_TRUE(manager.LoadCommands(kTestVendorCommands, nullptr)); + ASSERT_TRUE(manager.LoadCommands(kTestTestCommands, nullptr)); EXPECT_EQ(3, manager.GetCommandDictionary().GetSize()); EXPECT_NE(nullptr, manager.GetCommandDictionary().FindCommand("robot._jump")); EXPECT_NE(nullptr,
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc index 9c52912..865825f 100644 --- a/libweave/src/device_manager.cc +++ b/libweave/src/device_manager.cc
@@ -34,10 +34,10 @@ provider::Wifi* wifi, provider::Bluetooth* bluetooth) { command_manager_ = std::make_shared<CommandManager>(); - command_manager_->Startup(config_store); + command_manager_->Startup(); state_change_queue_.reset(new StateChangeQueue(kMaxStateChangeQueueSize)); state_manager_ = std::make_shared<StateManager>(state_change_queue_.get()); - state_manager_->Startup(config_store); + state_manager_->Startup(); std::unique_ptr<Config> config{new Config{config_store}}; config->Load();
diff --git a/libweave/src/states/state_manager.cc b/libweave/src/states/state_manager.cc index cd94736..b0dd802 100644 --- a/libweave/src/states/state_manager.cc +++ b/libweave/src/states/state_manager.cc
@@ -6,7 +6,6 @@ #include <base/logging.h> #include <base/values.h> -#include <weave/provider/config_store.h> #include "src/json_error_codes.h" #include "src/states/error_codes.h" @@ -54,23 +53,15 @@ callback.Run(); // Force to read current state. } -void StateManager::Startup(provider::ConfigStore* config_store) { +void StateManager::Startup() { LOG(INFO) << "Initializing StateManager."; // Load standard device state definition. CHECK(LoadBaseStateDefinition(kBaseStateDefs, nullptr)); - // Load component-specific device state definitions. - for (const auto& state_def : config_store->LoadStateDefs()) - CHECK(LoadStateDefinition(state_def, nullptr)); - // Load standard device state defaults. CHECK(LoadStateDefaults(kBaseStateDefaults, nullptr)); - // Load component-specific device state defaults. - for (const auto& json : config_store->LoadStateDefaults()) - CHECK(LoadStateDefaults(json, nullptr)); - for (const auto& cb : on_changed_) cb.Run(); } @@ -176,8 +167,8 @@ bool StateManager::LoadStateDefinition(const base::DictionaryValue& dict, ErrorPtr* error) { - base::DictionaryValue::Iterator iter(dict); - while (!iter.IsAtEnd()) { + for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd(); + iter.Advance()) { std::string package_name = iter.key(); if (package_name.empty()) { Error::AddTo(error, FROM_HERE, errors::kErrorDomain, @@ -196,7 +187,6 @@ CHECK(package) << "Unable to create state package " << package_name; if (!package->AddSchemaFromJson(package_dict, error)) return false; - iter.Advance(); } return true; @@ -232,35 +222,39 @@ bool StateManager::LoadStateDefaults(const base::DictionaryValue& dict, ErrorPtr* error) { - base::DictionaryValue::Iterator iter(dict); - while (!iter.IsAtEnd()) { - std::string package_name = iter.key(); - if (package_name.empty()) { + base::Time timestamp = base::Time::Now(); + bool all_success = true; + for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd(); + iter.Advance()) { + if (iter.key().empty()) { Error::AddTo(error, FROM_HERE, errors::kErrorDomain, errors::kInvalidPackageError, "State package name is empty"); - return false; + all_success = false; + continue; } + const base::DictionaryValue* package_dict = nullptr; if (!iter.value().GetAsDictionary(&package_dict)) { Error::AddToPrintf(error, FROM_HERE, errors::json::kDomain, errors::json::kObjectExpected, "State package '%s' must be an object", - package_name.c_str()); - return false; + iter.key().c_str()); + all_success = false; + continue; } - StatePackage* package = FindPackage(package_name); - if (package == nullptr) { - Error::AddToPrintf(error, FROM_HERE, errors::json::kDomain, - errors::json::kObjectExpected, - "Providing values for undefined state package '%s'", - package_name.c_str()); - return false; + + for (base::DictionaryValue::Iterator it_prop(*package_dict); + !it_prop.IsAtEnd(); it_prop.Advance()) { + if (!SetPropertyValue(iter.key() + "." + it_prop.key(), it_prop.value(), + timestamp, error)) { + all_success = false; + continue; + } } - if (!package->AddValuesFromJson(package_dict, error)) - return false; - iter.Advance(); } - return true; + for (const auto& cb : on_changed_) + cb.Run(); + return all_success; } bool StateManager::LoadStateDefaults(const std::string& json, ErrorPtr* error) {
diff --git a/libweave/src/states/state_manager.h b/libweave/src/states/state_manager.h index 3dbd0d5..72f9ddd 100644 --- a/libweave/src/states/state_manager.h +++ b/libweave/src/states/state_manager.h
@@ -26,10 +26,6 @@ namespace weave { -namespace provider { -class ConfigStore; -} - // StateManager is the class that aggregates the device state fragments // provided by device daemons and makes the aggregate device state available // to the GCD cloud server and local clients. @@ -53,7 +49,7 @@ // Initializes the state manager and load device state fragments. // Called by Buffet daemon at startup. - void Startup(provider::ConfigStore* config_store); + void Startup(); // Returns the recorded state changes since last time this method has been // called.
diff --git a/libweave/src/states/state_manager_unittest.cc b/libweave/src/states/state_manager_unittest.cc index 522e0e6..3273a16 100644 --- a/libweave/src/states/state_manager_unittest.cc +++ b/libweave/src/states/state_manager_unittest.cc
@@ -59,12 +59,12 @@ // Initial expectations. EXPECT_CALL(mock_state_change_queue_, IsEmpty()).Times(0); EXPECT_CALL(mock_state_change_queue_, NotifyPropertiesUpdated(_, _)) - .Times(0); + .WillRepeatedly(Return(true)); EXPECT_CALL(mock_state_change_queue_, GetAndClearRecordedStateChanges()) .Times(0); mgr_.reset(new StateManager(&mock_state_change_queue_)); - EXPECT_CALL(*this, OnStateChanged()).Times(1); + EXPECT_CALL(*this, OnStateChanged()).Times(2); mgr_->AddChangedCallback( base::Bind(&StateManagerTest::OnStateChanged, base::Unretained(this))); @@ -133,18 +133,13 @@ } TEST_F(StateManagerTest, Startup) { - provider::test::MockConfigStore config_store; StateManager manager(&mock_state_change_queue_); - EXPECT_CALL(config_store, LoadStateDefs()) - .WillOnce(Return(std::vector<std::string>{ - {R"({"power": {"battery_level":"integer"}})"}})); - - EXPECT_CALL(config_store, LoadStateDefaults()) - .WillOnce(Return( - std::vector<std::string>{{R"({"power": {"battery_level":44}})"}})); - - manager.Startup(&config_store); + manager.Startup(); + ASSERT_TRUE(manager.LoadStateDefinition( + R"({"power": {"battery_level":"integer"}})", nullptr)); + ASSERT_TRUE( + manager.LoadStateDefaults(R"({"power": {"battery_level":44}})", nullptr)); auto expected = R"({ 'base': {
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc index f0f8a58..b794fd6 100644 --- a/libweave/src/weave_unittest.cc +++ b/libweave/src/weave_unittest.cc
@@ -157,15 +157,6 @@ void InitConfigStore() { EXPECT_CALL(config_store_, SaveSettings("")).WillRepeatedly(Return()); - - EXPECT_CALL(config_store_, LoadCommandDefs()) - .WillOnce(Return(std::vector<std::string>{kCommandDefs})); - - EXPECT_CALL(config_store_, LoadStateDefs()) - .WillOnce(Return(std::vector<std::string>{kStateDefs})); - - EXPECT_CALL(config_store_, LoadStateDefaults()) - .WillOnce(Return(std::vector<std::string>{kStateDefaults})); } void InitNetwork() { @@ -236,6 +227,10 @@ &http_client_, &network_, &dns_sd_, &http_server_, &wifi_, &bluetooth_); + device_->AddCommandDefinitions(kCommandDefs); + device_->AddStateDefinitions(kStateDefs); + device_->SetState(kStateDefaults, nullptr); + for (const auto& cb : http_server_changed_cb_) cb.Run(http_server_); @@ -290,6 +285,7 @@ device_ = weave::Device::Create(&config_store_, &task_runner_, &http_client_, &network_, &dns_sd_, &http_server_, nullptr, &bluetooth_); + device_->AddCommandDefinitions(kCommandDefs); for (const auto& cb : http_server_changed_cb_) cb.Run(http_server_);