Add Device::SetCommandHandler BUG:24267885 Change-Id: Ibcc1af050ad0de4130dfc3349545f6ab1ddc3f49 Reviewed-on: https://weave-review.googlesource.com/1250 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/examples/ubuntu/main.cc b/libweave/examples/ubuntu/main.cc index 5c84d9c..c28d164 100644 --- a/libweave/examples/ubuntu/main.cc +++ b/libweave/examples/ubuntu/main.cc
@@ -34,66 +34,85 @@ class CommandHandler { public: explicit CommandHandler(weave::Device* device) : device_{device} { - device->AddCommandAddedCallback(base::Bind(&CommandHandler::OnNewCommand, - weak_ptr_factory_.GetWeakPtr())); + device->AddCommandHandler("_greeter._greet", + base::Bind(&CommandHandler::OnGreetCommand, + weak_ptr_factory_.GetWeakPtr())); + device->AddCommandHandler("_ledflasher._set", + base::Bind(&CommandHandler::OnFlasherSetCommand, + weak_ptr_factory_.GetWeakPtr())); + device->AddCommandHandler( + "_ledflasher._toggle", + base::Bind(&CommandHandler::OnFlasherToggleCommand, + weak_ptr_factory_.GetWeakPtr())); + device->AddCommandHandler("", + base::Bind(&CommandHandler::OnUnhandledCommand, + weak_ptr_factory_.GetWeakPtr())); } private: - void OnNewCommand(weave::Command* cmd) { + void OnGreetCommand(weave::Command* cmd) { LOG(INFO) << "received command: " << cmd->GetName(); - if (cmd->GetName() == "_greeter._greet") { - std::string name; - if (!cmd->GetParameters()->GetString("_name", &name)) - name = "anonymous"; + std::string name; + if (!cmd->GetParameters()->GetString("_name", &name)) + name = "anonymous"; - LOG(INFO) << cmd->GetName() << " command in progress"; - cmd->SetProgress(base::DictionaryValue{}, nullptr); + LOG(INFO) << cmd->GetName() << " command in progress"; + cmd->SetProgress(base::DictionaryValue{}, nullptr); - base::DictionaryValue result; - result.SetString("_greeting", "Hello " + name); - cmd->SetResults(result, nullptr); - LOG(INFO) << cmd->GetName() << " command finished: " << result; + base::DictionaryValue result; + result.SetString("_greeting", "Hello " + name); + cmd->SetResults(result, nullptr); + LOG(INFO) << cmd->GetName() << " command finished: " << result; - base::DictionaryValue state; - state.SetIntegerWithoutPathExpansion("_greeter._greetings_counter", - ++counter_); - device_->SetStateProperties(state, nullptr); + base::DictionaryValue state; + state.SetIntegerWithoutPathExpansion("_greeter._greetings_counter", + ++counter_); + device_->SetStateProperties(state, nullptr); - LOG(INFO) << "New state: " << *device_->GetState(); + LOG(INFO) << "New state: " << *device_->GetState(); - cmd->Done(); - } else if (cmd->GetName() == "_ledflasher._set") { - int32_t led_index; - bool cmd_value; - if (cmd->GetParameters()->GetInteger("_led", &led_index) && - cmd->GetParameters()->GetBoolean("_on", &cmd_value)) { - // Display this command in terminal - LOG(INFO) << cmd->GetName() << " _led: " << led_index - << ", _on: " << (cmd_value ? "true" : "false"); + cmd->Done(); + } - led_index--; - int new_state = cmd_value ? 1 : 0; - int cur_state = led_status_[led_index]; - led_status_[led_index] = new_state; + void OnFlasherSetCommand(weave::Command* cmd) { + LOG(INFO) << "received command: " << cmd->GetName(); + int32_t led_index; + bool cmd_value; + if (cmd->GetParameters()->GetInteger("_led", &led_index) && + cmd->GetParameters()->GetBoolean("_on", &cmd_value)) { + // Display this command in terminal + LOG(INFO) << cmd->GetName() << " _led: " << led_index + << ", _on: " << (cmd_value ? "true" : "false"); - if (cmd_value != cur_state) { - UpdateLedState(); - } - } - cmd->Done(); - } else if (cmd->GetName() == "_ledflasher._toggle") { - int32_t led_index; - if (cmd->GetParameters()->GetInteger("_led", &led_index)) { - LOG(INFO) << cmd->GetName() << " _led: " << led_index; - led_index--; - led_status_[led_index] = ~led_status_[led_index]; + led_index--; + int new_state = cmd_value ? 1 : 0; + int cur_state = led_status_[led_index]; + led_status_[led_index] = new_state; + if (cmd_value != cur_state) { UpdateLedState(); } - cmd->Done(); - } else { - LOG(INFO) << cmd->GetName() << " unimplemented command: ignored"; + return cmd->Done(); } + cmd->Abort(); + } + + void OnFlasherToggleCommand(weave::Command* cmd) { + LOG(INFO) << "received command: " << cmd->GetName(); + int32_t led_index; + if (cmd->GetParameters()->GetInteger("_led", &led_index)) { + LOG(INFO) << cmd->GetName() << " _led: " << led_index; + led_index--; + led_status_[led_index] = ~led_status_[led_index]; + + UpdateLedState(); + return cmd->Done(); + } + cmd->Abort(); + } + + void OnUnhandledCommand(weave::Command* cmd) { + LOG(INFO) << cmd->GetName() << " unimplemented command: ignored"; } void UpdateLedState(void) {
diff --git a/libweave/include/weave/device.h b/libweave/include/weave/device.h index ec8d4f5..d8b6963 100644 --- a/libweave/include/weave/device.h +++ b/libweave/include/weave/device.h
@@ -45,11 +45,16 @@ virtual void AddSettingsChangedCallback( const SettingsChangedCallback& callback) = 0; - // Callback type for AddCommandAddedCallback. - using CommandCallback = base::Callback<void(Command*)>; + // Callback type for AddCommandHandler. + using CommandHandlerCallback = base::Callback<void(Command*)>; - // Adds notification callback for a new command being added to the queue. - virtual void AddCommandAddedCallback(const CommandCallback& callback) = 0; + // Sets handler for new commands added to the queue. + // |command_name| is the full command name of the command to handle. e.g. + // "base.reboot". Each command can have no more than one handler. + // Empty |command_name| sets default handler for all unhanded commands. + // No new command handlers can be set after default handler was set. + virtual void AddCommandHandler(const std::string& command_name, + const CommandHandlerCallback& callback) = 0; // Adds a new command to the command queue. virtual bool AddCommand(const base::DictionaryValue& command,
diff --git a/libweave/include/weave/test/mock_device.h b/libweave/include/weave/test/mock_device.h index c289ca7..af149b4 100644 --- a/libweave/include/weave/test/mock_device.h +++ b/libweave/include/weave/test/mock_device.h
@@ -21,7 +21,8 @@ MOCK_CONST_METHOD0(GetSettings, const Settings&()); MOCK_METHOD1(AddSettingsChangedCallback, void(const SettingsChangedCallback& callback)); - MOCK_METHOD1(AddCommandAddedCallback, void(const CommandCallback&)); + MOCK_METHOD2(AddCommandHandler, + void(const std::string&, const CommandHandlerCallback&)); MOCK_METHOD3(AddCommand, bool(const base::DictionaryValue&, std::string*, ErrorPtr*)); MOCK_METHOD1(FindCommand, Command*(const std::string&));
diff --git a/libweave/src/base_api_handler.cc b/libweave/src/base_api_handler.cc index f5dccd7..e63d8cb 100644 --- a/libweave/src/base_api_handler.cc +++ b/libweave/src/base_api_handler.cc
@@ -30,22 +30,19 @@ settings.firmware_version); CHECK(device_->SetStateProperties(state, nullptr)); - device_->AddCommandAddedCallback(base::Bind(&BaseApiHandler::OnCommandAdded, - weak_ptr_factory_.GetWeakPtr())); -} + device_->AddCommandHandler( + "base.updateBaseConfiguration", + base::Bind(&BaseApiHandler::UpdateBaseConfiguration, + weak_ptr_factory_.GetWeakPtr())); -void BaseApiHandler::OnCommandAdded(Command* command) { - if (command->GetStatus() != CommandStatus::kQueued) - return; - - if (command->GetName() == "base.updateBaseConfiguration") - return UpdateBaseConfiguration(command); - - if (command->GetName() == "base.updateDeviceInfo") - return UpdateDeviceInfo(command); + device_->AddCommandHandler("base.updateDeviceInfo", + base::Bind(&BaseApiHandler::UpdateDeviceInfo, + weak_ptr_factory_.GetWeakPtr())); } void BaseApiHandler::UpdateBaseConfiguration(Command* command) { + CHECK(command->GetStatus() == CommandStatus::kQueued) + << EnumToString(command->GetStatus()); command->SetProgress(base::DictionaryValue{}, nullptr); const auto& settings = device_info_->GetSettings(); @@ -83,6 +80,8 @@ } void BaseApiHandler::UpdateDeviceInfo(Command* command) { + CHECK(command->GetStatus() == CommandStatus::kQueued) + << EnumToString(command->GetStatus()); command->SetProgress(base::DictionaryValue{}, nullptr); const auto& settings = device_info_->GetSettings();
diff --git a/libweave/src/base_api_handler.h b/libweave/src/base_api_handler.h index b8a8bd1..7712de1 100644 --- a/libweave/src/base_api_handler.h +++ b/libweave/src/base_api_handler.h
@@ -28,7 +28,6 @@ BaseApiHandler(DeviceRegistrationInfo* device_info, Device* device); private: - void OnCommandAdded(Command* command); void UpdateBaseConfiguration(Command* command); void UpdateDeviceInfo(Command* command); bool UpdateState(const std::string& anonymous_access_role,
diff --git a/libweave/src/base_api_handler_unittest.cc b/libweave/src/base_api_handler_unittest.cc index 2bd7741..8d30619 100644 --- a/libweave/src/base_api_handler_unittest.cc +++ b/libweave/src/base_api_handler_unittest.cc
@@ -19,6 +19,8 @@ #include "src/states/state_manager.h" using testing::_; +using testing::AnyOf; +using testing::Eq; using testing::Invoke; using testing::Return; using testing::StrictMock; @@ -37,9 +39,11 @@ EXPECT_CALL(device_, SetStateProperties(_, _)) .WillRepeatedly( Invoke(state_manager_.get(), &StateManager::SetProperties)); - EXPECT_CALL(device_, AddCommandAddedCallback(_)) - .WillRepeatedly(Invoke(command_manager_.get(), - &CommandManager::AddCommandAddedCallback)); + EXPECT_CALL(device_, AddCommandHandler(AnyOf("base.updateBaseConfiguration", + "base.updateDeviceInfo"), + _)) + .WillRepeatedly( + Invoke(command_manager_.get(), &CommandManager::AddCommandHandler)); auto state_definition = test::CreateDictionaryValue(R"({ 'base': { @@ -65,6 +69,40 @@ ASSERT_TRUE( state_manager_->LoadStateDefinition(*state_definition, nullptr)); ASSERT_TRUE(state_manager_->LoadStateDefaults(*state_defaults, nullptr)); + + auto base_commands = test::CreateDictionaryValue(R"({ + 'base': { + 'updateBaseConfiguration': { + 'parameters': { + 'localDiscoveryEnabled': 'boolean', + 'localAnonymousAccessMaxRole': [ 'none', 'viewer', 'user' ], + 'localPairingEnabled': 'boolean' + }, + 'results': {} + }, + 'updateDeviceInfo': { + 'parameters': { + 'description': 'string', + 'name': { + 'type': 'string', + 'minLength': 1 + }, + 'location': 'string' + }, + 'results': {} + } + } + })"); + EXPECT_TRUE(command_manager_->LoadBaseCommands(*base_commands, nullptr)); + + auto handled_commands = test::CreateDictionaryValue(R"({ + 'base': { + 'updateBaseConfiguration': {}, + 'updateDeviceInfo': {} + } + })"); + EXPECT_TRUE(command_manager_->LoadCommands(*handled_commands, nullptr)); + std::unique_ptr<Config> config{new Config{&config_store_}}; config->Load(); dev_reg_.reset(new DeviceRegistrationInfo(command_manager_, state_manager_, @@ -73,12 +111,6 @@ handler_.reset(new BaseApiHandler{dev_reg_.get(), &device_}); } - void LoadCommands(const std::string& command_definitions) { - auto json = test::CreateDictionaryValue(command_definitions.c_str()); - EXPECT_TRUE(command_manager_->LoadBaseCommands(*json, nullptr)); - EXPECT_TRUE(command_manager_->LoadCommands(*json, nullptr)); - } - void AddCommand(const std::string& command) { auto command_instance = CommandInstance::FromJson( test::CreateDictionaryValue(command.c_str()).get(), @@ -105,19 +137,6 @@ }; TEST_F(BaseApiHandlerTest, UpdateBaseConfiguration) { - LoadCommands(R"({ - 'base': { - 'updateBaseConfiguration': { - 'parameters': { - 'localDiscoveryEnabled': 'boolean', - 'localAnonymousAccessMaxRole': [ 'none', 'viewer', 'user' ], - 'localPairingEnabled': 'boolean' - }, - 'results': {} - } - } - })"); - const Settings& settings = dev_reg_->GetSettings(); AddCommand(R"({ @@ -182,22 +201,6 @@ } TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) { - LoadCommands(R"({ - 'base': { - 'updateDeviceInfo': { - 'parameters': { - 'description': 'string', - 'name': { - 'type': 'string', - 'minLength': 1 - }, - 'location': 'string' - }, - 'results': {} - } - } - })"); - AddCommand(R"({ 'name' : 'base.updateDeviceInfo', 'parameters': {
diff --git a/libweave/src/commands/command_manager.cc b/libweave/src/commands/command_manager.cc index 60b3d1c..9aa3385 100644 --- a/libweave/src/commands/command_manager.cc +++ b/libweave/src/commands/command_manager.cc
@@ -176,13 +176,21 @@ } void CommandManager::AddCommandAddedCallback( - const Device::CommandCallback& callback) { + const CommandQueue::CommandCallback& callback) { command_queue_.AddCommandAddedCallback(callback); } void CommandManager::AddCommandRemovedCallback( - const Device::CommandCallback& callback) { + const CommandQueue::CommandCallback& callback) { command_queue_.AddCommandRemovedCallback(callback); } +void CommandManager::AddCommandHandler( + const std::string& command_name, + const CommandQueue::CommandCallback& callback) { + CHECK(command_name.empty() || base_dictionary_.FindCommand(command_name)) + << "Command undefined: " << command_name; + command_queue_.AddCommandHandler(command_name, callback); +} + } // namespace weave
diff --git a/libweave/src/commands/command_manager.h b/libweave/src/commands/command_manager.h index 605f770..97fc73f 100644 --- a/libweave/src/commands/command_manager.h +++ b/libweave/src/commands/command_manager.h
@@ -37,8 +37,10 @@ std::string* id, ErrorPtr* error); CommandInstance* FindCommand(const std::string& id); - void AddCommandAddedCallback(const Device::CommandCallback& callback); - void AddCommandRemovedCallback(const Device::CommandCallback& callback); + void AddCommandAddedCallback(const CommandQueue::CommandCallback& callback); + void AddCommandRemovedCallback(const CommandQueue::CommandCallback& callback); + void AddCommandHandler(const std::string& command_name, + const CommandQueue::CommandCallback& callback); // Sets callback which is called when command definitions is changed. void AddCommandDefChanged(const base::Closure& callback);
diff --git a/libweave/src/commands/command_queue.cc b/libweave/src/commands/command_queue.cc index e580048..df86d77 100644 --- a/libweave/src/commands/command_queue.cc +++ b/libweave/src/commands/command_queue.cc
@@ -13,19 +13,47 @@ const int kRemoveCommandDelayMin = 5; } -void CommandQueue::AddCommandAddedCallback( - const Device::CommandCallback& callback) { +void CommandQueue::AddCommandAddedCallback(const CommandCallback& callback) { on_command_added_.push_back(callback); // Send all pre-existed commands. for (const auto& command : map_) callback.Run(command.second.get()); } -void CommandQueue::AddCommandRemovedCallback( - const Device::CommandCallback& callback) { +void CommandQueue::AddCommandRemovedCallback(const CommandCallback& callback) { on_command_removed_.push_back(callback); } +void CommandQueue::AddCommandHandler(const std::string& command_name, + const CommandCallback& callback) { + if (!command_name.empty()) { + CHECK(default_command_callback_.is_null()) + << "Commands specific handler are not allowed after default one"; + + for (const auto& command : map_) { + if (command.second->GetStatus() == CommandStatus::kQueued && + command.second->GetName() == command_name) { + callback.Run(command.second.get()); + } + } + + CHECK(command_callbacks_.emplace(command_name, callback).second) + << command_name << " already has handler"; + + } else { + for (const auto& command : map_) { + if (command.second->GetStatus() == CommandStatus::kQueued && + command_callbacks_.find(command.second->GetName()) == + command_callbacks_.end()) { + callback.Run(command.second.get()); + } + } + + CHECK(default_command_callback_.is_null()) << "Already has default handler"; + default_command_callback_ = callback; + } +} + void CommandQueue::Add(std::unique_ptr<CommandInstance> instance) { std::string id = instance->GetID(); LOG_IF(FATAL, id.empty()) << "Command has no ID"; @@ -35,6 +63,14 @@ << "' is already in the queue"; for (const auto& cb : on_command_added_) cb.Run(pair.first->second.get()); + + auto it_handler = command_callbacks_.find(pair.first->second->GetName()); + + if (it_handler != command_callbacks_.end()) + it_handler->second.Run(pair.first->second.get()); + else if (!default_command_callback_.is_null()) + default_command_callback_.Run(pair.first->second.get()); + Cleanup(); }
diff --git a/libweave/src/commands/command_queue.h b/libweave/src/commands/command_queue.h index c3e86d5..888c04f 100644 --- a/libweave/src/commands/command_queue.h +++ b/libweave/src/commands/command_queue.h
@@ -25,11 +25,16 @@ public: CommandQueue() = default; + using CommandCallback = Device::CommandHandlerCallback; + // Adds notifications callback for a new command is added to the queue. - void AddCommandAddedCallback(const Device::CommandCallback& callback); + void AddCommandAddedCallback(const CommandCallback& callback); // Adds notifications callback for a command is removed from the queue. - void AddCommandRemovedCallback(const Device::CommandCallback& callback); + void AddCommandRemovedCallback(const CommandCallback& callback); + + void AddCommandHandler(const std::string& command_name, + const CommandCallback& callback); // Checks if the command queue is empty. bool IsEmpty() const { return map_.empty(); } @@ -75,9 +80,11 @@ // Queue of commands to be removed. std::queue<std::pair<base::Time, std::string>> remove_queue_; - using CallbackList = std::vector<Device::CommandCallback>; + using CallbackList = std::vector<CommandCallback>; CallbackList on_command_added_; CallbackList on_command_removed_; + std::map<std::string, CommandCallback> command_callbacks_; + CommandCallback default_command_callback_; DISALLOW_COPY_AND_ASSIGN(CommandQueue); };
diff --git a/libweave/src/device_manager.cc b/libweave/src/device_manager.cc index 6ff3a88..009df46 100644 --- a/libweave/src/device_manager.cc +++ b/libweave/src/device_manager.cc
@@ -106,8 +106,9 @@ return command_manager_->FindCommand(id); } -void DeviceManager::AddCommandAddedCallback(const CommandCallback& callback) { - return command_manager_->AddCommandAddedCallback(callback); +void DeviceManager::AddCommandHandler(const std::string& command_name, + const CommandHandlerCallback& callback) { + return command_manager_->AddCommandHandler(command_name, callback); } bool DeviceManager::SetStateProperties(
diff --git a/libweave/src/device_manager.h b/libweave/src/device_manager.h index a72d800..72a24ab 100644 --- a/libweave/src/device_manager.h +++ b/libweave/src/device_manager.h
@@ -41,7 +41,8 @@ std::string* id, ErrorPtr* error) override; Command* FindCommand(const std::string& id) override; - void AddCommandAddedCallback(const CommandCallback& callback) override; + void AddCommandHandler(const std::string& command_name, + const CommandHandlerCallback& callback) override; void AddStateChangedCallback(const base::Closure& callback) override; bool SetStateProperties(const base::DictionaryValue& property_set, ErrorPtr* error) override;