From 25845ac908bb59263f495c28d5815772561ec59d Mon Sep 17 00:00:00 2001 From: Surender Kodam Date: Tue, 5 Apr 2016 13:56:20 -0700 Subject: add _ledflasher traits to the test_schema device. Change-Id: I79d3a9cd45461771b9129d2cea95e9c158b73dc2 Reviewed-on: https://weave-review.googlesource.com/3140 Reviewed-by: Alex Vakulenko --- tests_schema/daemon/testdevice/custom_traits.h | 46 +++++++++ tests_schema/daemon/testdevice/standard_traits.h | 4 +- tests_schema/daemon/testdevice/testdevice.cc | 116 ++++++++++++++++++++--- 3 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 tests_schema/daemon/testdevice/custom_traits.h diff --git a/tests_schema/daemon/testdevice/custom_traits.h b/tests_schema/daemon/testdevice/custom_traits.h new file mode 100644 index 0000000..b4f0e59 --- /dev/null +++ b/tests_schema/daemon/testdevice/custom_traits.h @@ -0,0 +1,46 @@ +// Copyright 2016 The Weave Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +namespace custom_traits { +const char kCustomTraits[] = R"({ + "_ledflasher": { + "commands": { + "animate": { + "minimalRole": "user", + "parameters": { + "duration": { + "type": "number", + "minimum": 0.1, + "maximum": 100.0 + }, + "type": { + "type": "string", + "enum": [ "none", "marquee_left", "marquee_right", "blink" ] + } + } + } + }, + "state": { + "status": { + "type": "string", + "enum": [ "idle", "animating" ] + } + } + } +})"; + +const char kLedflasherState[] = R"({ + "_ledflasher":{"status": "idle"} +})"; + +const char ledflasher[] = "ledflasher"; +const char led1[] = "led1"; +const char led2[] = "led2"; +const char led3[] = "led3"; +const char led4[] = "led4"; +const char led5[] = "led5"; +const size_t kLedCount = 5; +const char kLedComponentPrefix[] = "led"; +} // namespace custom_traits + diff --git a/tests_schema/daemon/testdevice/standard_traits.h b/tests_schema/daemon/testdevice/standard_traits.h index 354c783..c63c5fe 100644 --- a/tests_schema/daemon/testdevice/standard_traits.h +++ b/tests_schema/daemon/testdevice/standard_traits.h @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -namespace standardtraits { +namespace standard_traits { const char kTraits[] = R"({ "lock": { "commands": { @@ -213,4 +213,4 @@ const char kDefaultState[] = R"({ })"; const char kComponent[] = "testdevice"; -} // namespace jsontraits +} // namespace standard_traits diff --git a/tests_schema/daemon/testdevice/testdevice.cc b/tests_schema/daemon/testdevice/testdevice.cc index 7dea5ac..a69ff46 100644 --- a/tests_schema/daemon/testdevice/testdevice.cc +++ b/tests_schema/daemon/testdevice/testdevice.cc @@ -1,12 +1,14 @@ -// Copyright 2015 The Weave Authors. All rights reserved. +// Copyright 2016 The Weave Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. #include +#include #include #include #include "examples/daemon/common/daemon.h" +#include "tests_schema/daemon/testdevice/custom_traits.h" #include "tests_schema/daemon/testdevice/standard_traits.h" #include @@ -38,32 +40,55 @@ class TestDeviceHandler { void Register(weave::Device* device) { device_ = device; - device->AddTraitDefinitionsFromJson(standardtraits::kTraits); + device->AddTraitDefinitionsFromJson(standard_traits::kTraits); + device->AddTraitDefinitionsFromJson(custom_traits::kCustomTraits); + CHECK(device->AddComponent( - standardtraits::kComponent, + standard_traits::kComponent, {"lock", "onOff", "brightness", "colorTemp", "colorXy"}, nullptr)); + CHECK(device->AddComponent(custom_traits::ledflasher, {"_ledflasher"}, + nullptr)); + + CHECK(device->SetStatePropertiesFromJson( + standard_traits::kComponent, standard_traits::kDefaultState, nullptr)); CHECK(device->SetStatePropertiesFromJson( - standardtraits::kComponent, standardtraits::kDefaultState, nullptr)); + custom_traits::ledflasher, custom_traits::kLedflasherState, nullptr)); + + for (size_t led_index = 0; led_index < led_states_.size(); led_index++) { + std::string component_name = + custom_traits::kLedComponentPrefix + std::to_string(led_index + 1); + CHECK(device->AddComponent(component_name, {"onOff"}, nullptr)); + device->AddCommandHandler( + component_name, "onOff.setConfig", + base::Bind(&TestDeviceHandler::LedOnOffSetConfig, + weak_ptr_factory_.GetWeakPtr(), led_index)); + device->SetStateProperty( + component_name, "onOff.state", + base::StringValue{led_states_[led_index] ? "on" : "off"}, nullptr); + } UpdateTestDeviceState(); - device->AddCommandHandler(standardtraits::kComponent, "onOff.setConfig", + device->AddCommandHandler(standard_traits::kComponent, "onOff.setConfig", base::Bind(&TestDeviceHandler::OnOnOffSetConfig, weak_ptr_factory_.GetWeakPtr())); - device->AddCommandHandler(standardtraits::kComponent, "lock.setConfig", + device->AddCommandHandler(standard_traits::kComponent, "lock.setConfig", base::Bind(&TestDeviceHandler::OnLockSetConfig, weak_ptr_factory_.GetWeakPtr())); device->AddCommandHandler( - standardtraits::kComponent, "brightness.setConfig", + standard_traits::kComponent, "brightness.setConfig", base::Bind(&TestDeviceHandler::OnBrightnessSetConfig, weak_ptr_factory_.GetWeakPtr())); device->AddCommandHandler( - standardtraits::kComponent, "colorTemp.setConfig", + standard_traits::kComponent, "colorTemp.setConfig", base::Bind(&TestDeviceHandler::OnColorTempSetConfig, weak_ptr_factory_.GetWeakPtr())); - device->AddCommandHandler(standardtraits::kComponent, "colorXy.setConfig", + device->AddCommandHandler(standard_traits::kComponent, "colorXy.setConfig", base::Bind(&TestDeviceHandler::OnColorXySetConfig, weak_ptr_factory_.GetWeakPtr())); + device->AddCommandHandler(custom_traits::ledflasher, "_ledflasher.animate", + base::Bind(&TestDeviceHandler::OnAnimate, + weak_ptr_factory_.GetWeakPtr())); } private: @@ -222,9 +247,76 @@ class TestDeviceHandler { AbortCommand(cmd); } + void OnAnimate(const std::weak_ptr& command) { + auto cmd = command.lock(); + if (!cmd) + return; + + const auto& params = cmd->GetParameters(); + double duration = 0.0; + std::string type; + + if (params.GetDouble("duration", &duration)) { + LOG(INFO) << cmd->GetName() << " animate duration : " << duration; + + if (duration <= 0.0 || duration > 100.0) { + // Invalid animate duration value is specified. + AbortCommand(cmd); + return; + } + + if (params.GetString("type", &type)) { + LOG(INFO) << " Animation type value is : " << type; + if (type != "marquee_left" && type != "marquee_right" && + type != "blink" && type != "none") { + // Invalid animation state is specified. + AbortCommand(cmd); + return; + } + } + + cmd->Complete({}, nullptr); + return; + } + + AbortCommand(cmd); + } + + void LedOnOffSetConfig(size_t led_index, + const std::weak_ptr& command) { + auto cmd = command.lock(); + if (!cmd) + return; + LOG(INFO) << "received command: " << cmd->GetName(); + const auto& params = cmd->GetParameters(); + std::string state; + if (params.GetString("state", &state)) { + LOG(INFO) << cmd->GetName() << " led: " << led_index + << " state: " << state; + + std::string temp_state = state; + if (temp_state != "on" && temp_state != "off") { + // Invalid OnOff state is specified. + AbortCommand(cmd); + return; + } + + int current_state = led_states_[led_index]; + int new_state = (state == "on") ? 1 : 0; + led_states_[led_index] = new_state; + if (new_state != current_state) { + device_->SetStateProperty(cmd->GetComponent(), "onOff.state", + base::StringValue{state}, nullptr); + } + cmd->Complete({}, nullptr); + return; + } + AbortCommand(cmd); + } + void UpdateTestDeviceState() { std::string updated_state = weave::EnumToString(lock_state_); - device_->SetStateProperty(standardtraits::kComponent, "lock.lockedState", + device_->SetStateProperty(standard_traits::kComponent, "lock.lockedState", base::StringValue{updated_state}, nullptr); base::DictionaryValue state; state.SetString("onOff.state", light_status_ ? "on" : "off"); @@ -238,7 +330,7 @@ class TestDeviceHandler { colorXy->SetDouble("colorY", color_Y_); state.Set("colorXy.colorSetting", std::move(colorXy)); - device_->SetStateProperties(standardtraits::kComponent, state, nullptr); + device_->SetStateProperties(standard_traits::kComponent, state, nullptr); } void AbortCommand(std::shared_ptr& cmd) { @@ -259,6 +351,8 @@ class TestDeviceHandler { int32_t color_temp_max_value_{1}; double color_X_{0.0}; double color_Y_{0.0}; + double duration{0.1}; + std::bitset led_states_{0}; base::WeakPtrFactory weak_ptr_factory_{this}; }; -- cgit v1.2.3 From daeafc6e42c1acb36aae290e60462e47947fd643 Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Thu, 7 Apr 2016 11:34:25 -0700 Subject: libweave: Remove unused local constant This causes on clang. Change-Id: I8bdffc52e9000c4033927555032b8b8294b80cfd Reviewed-on: https://weave-review.googlesource.com/3172 Reviewed-by: Alex Vakulenko --- src/device_registration_info.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 195895f..b7ff589 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc @@ -39,7 +39,6 @@ const char kErrorAlreayRegistered[] = "already_registered"; namespace { const int kPollingPeriodSeconds = 7; -const int kBackupPollingPeriodMinutes = 30; namespace fetch_reason { -- cgit v1.2.3 From 8ee0c1ea94d20719c53712ef05c460e603fa9093 Mon Sep 17 00:00:00 2001 From: Mike Colagrosso Date: Thu, 7 Apr 2016 14:04:01 -0600 Subject: libweave: Reformat examples/provider/README This change indents nested lists at four spaces, which renders them properly. Change-Id: I8bc972e8c14ec973223e5115f8109aa8e79f8ba2 Reviewed-on: https://weave-review.googlesource.com/3174 Reviewed-by: Johan Euphrosine --- examples/provider/README.md | 76 ++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 29 deletions(-) diff --git a/examples/provider/README.md b/examples/provider/README.md index 607fad4..6c3f61a 100644 --- a/examples/provider/README.md +++ b/examples/provider/README.md @@ -3,32 +3,50 @@ This directory contains example implementations of `weave` system providers. ## Providers -- `avahi_client.cc` - - implements: `weave::providerDnsServiceDiscovery` - - build-depends: libavahi-client - - run-depends: `avahi-daemon` -- `bluez_client.cc` - - not-implemented -- `curl_http_client.cc` - - implements: `weave::provider::HttpClient` - - build-depends: libcurl -- `event_http_server.cc` - - implements: `weave::provider::HttpServer` - - build-depends: libevhtp -- `event_network.cc` - - implements: `weave::provider::Network` - - build-depends: libevent -- `event_task_runner.cc` - - implements: `weave::provider::TaskRunner` - - build-depends: libevent -- `file_config_store.cc` - - implements: `weave::provider::ConfigStore` -- `wifi_manager.cc` - - implements: `weave::provider::Wifi` - - build-depends: `weave::examples::EventNetworkImpl` - - run-depends: `network-manager`, `dnsmasq`, `hostapd` - -Note: -- The example providers are based on `libevent` and should be portable between most GNU/Linux distributions. -- `weave::examples::WifiImpl` currently shells out to system - command tools like `nmcli`, `dnsmasq`, `ifconfig` and `hostpad`. + +- `avahi_client.cc` + + - implements: `weave::providerDnsServiceDiscovery` + - build-depends: libavahi-client + - run-depends: `avahi-daemon` + +- `bluez_client.cc` + + - not-implemented + +- `curl_http_client.cc` + + - implements: `weave::provider::HttpClient` + - build-depends: libcurl + +- `event_http_server.cc` + + - implements: `weave::provider::HttpServer` + - build-depends: libevhtp + +- `event_network.cc` + + - implements: `weave::provider::Network` + - build-depends: libevent + +- `event_task_runner.cc` + + - implements: `weave::provider::TaskRunner` + - build-depends: libevent + +- `file_config_store.cc` + + - implements: `weave::provider::ConfigStore` + +- `wifi_manager.cc` + + - implements: `weave::provider::Wifi` + - build-depends: `weave::examples::EventNetworkImpl` + - run-depends: `network-manager`, `dnsmasq`, `hostapd` + +## Note + +- The example providers are based on `libevent` and should be portable between + most GNU/Linux distributions. +- `weave::examples::WifiImpl` currently shells out to system command tools + like `nmcli`, `dnsmasq`, `ifconfig` and `hostpad`. -- cgit v1.2.3 From bf7d6630202b63a2fbd036efb7e5894386bcbb19 Mon Sep 17 00:00:00 2001 From: Johan Euphrosine Date: Mon, 11 Apr 2016 12:39:05 -0700 Subject: README: update pre-requisites - remove outdated deps - add cmake Change-Id: I5791f215aa26f9792cf20ba8593855484a887a51 Reviewed-on: https://weave-review.googlesource.com/3182 Reviewed-by: Paul Westbrook --- README.md | 4 +--- examples/prerequisites.sh | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/README.md b/README.md index c179e61..bf04fea 100644 --- a/README.md +++ b/README.md @@ -58,14 +58,13 @@ sudo apt-get install \ autoconf \ automake \ binutils \ + cmake \ g++ \ hostapd \ libavahi-client-dev \ libcurl4-openssl-dev \ libevent-dev \ libexpat1-dev \ - libnl-3-dev \ - libnl-route-3-dev \ libssl-dev \ libtool ``` @@ -95,7 +94,6 @@ sudo apt-get install \ - libevhtp (included; see third_party/libevhtp/) - libevent-dev - # Compiling From the `libweave` directory: diff --git a/examples/prerequisites.sh b/examples/prerequisites.sh index 23d54d7..818fab6 100755 --- a/examples/prerequisites.sh +++ b/examples/prerequisites.sh @@ -20,8 +20,6 @@ sudo apt-get update && sudo apt-get install ${APT_GET_OPTS} \ libcurl4-openssl-dev \ libevent-dev \ libexpat1-dev \ - libnl-3-dev \ - libnl-route-3-dev \ libssl-dev \ libtool \ || exit 1 -- cgit v1.2.3 From e90f36f712514d9aacd4fa6a0381ad325ab68331 Mon Sep 17 00:00:00 2001 From: Mike Colagrosso Date: Mon, 11 Apr 2016 16:13:35 -0600 Subject: libweave: Update comment on handling new commands The old comment in DeviceRegistrationInfo::OnCommandCreated() was left over from when the code would use the command received via the notification channel. Now the code always fetches commands from the command queue when it receives a notification that there is a new command. The code ignores the actual command in the notification payload. Change-Id: I1b7f7eedc337280035a5a6ec45f5935f780cac7f Reviewed-on: https://weave-review.googlesource.com/3190 Reviewed-by: John Mccullough --- src/device_registration_info.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index b7ff589..6353e12 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc @@ -1330,9 +1330,7 @@ void DeviceRegistrationInfo::OnCommandCreated( VLOG(1) << "Command notification received: " << command; - // If the command was too big to be delivered over a notification channel, - // or OnCommandCreated() was initiated from the Pull notification, - // perform a manual command fetch from the server here. + // Manually fetch the command queue whenever a new command is received. FetchAndPublishCommands(fetch_reason::kNewCommand); } -- cgit v1.2.3 From 3f59975c93597ead7c6574ff780ed2acf6dc8d02 Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Mon, 18 Apr 2016 10:44:24 -0700 Subject: libweave: Fix build break on Brillo The private member |currently_online_| is no longer used and causes the following error on Android/Brillo builds: external/libweave/src/privet/wifi_bootstrap_manager.h:109:8: error: private field 'currently_online_' is not used [-Werror,-Wunused-private-field] Remove the field to fix the compile break. Change-Id: I0830a4d12f777956638a12514565bb2270680e34 Reviewed-on: https://weave-review.googlesource.com/3220 Reviewed-by: Robert Ginda --- src/privet/wifi_bootstrap_manager.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/privet/wifi_bootstrap_manager.h b/src/privet/wifi_bootstrap_manager.h index 62a77c2..468473f 100644 --- a/src/privet/wifi_bootstrap_manager.h +++ b/src/privet/wifi_bootstrap_manager.h @@ -105,8 +105,6 @@ class WifiBootstrapManager : public WifiDelegate { provider::Wifi* wifi_{nullptr}; WifiSsidGenerator ssid_generator_; base::Time monitor_until_; - - bool currently_online_{false}; std::string privet_ssid_; // Helps to reset irrelevant tasks switching state. -- cgit v1.2.3 From 22ca81ad3a68f5bc93bcce2abd4c37f3dae55bfd Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Tue, 19 Apr 2016 15:41:19 -0700 Subject: libweave: Clean up code Fixed minor issues like misspelling in comments, formatting. Removed unused function prototypes and make sure function declarations match exactly the implementations. Change-Id: Ie30342657c0c22fa19b1546321400cc65d0e028d Reviewed-on: https://weave-review.googlesource.com/3230 Reviewed-by: Robert Ginda --- include/weave/provider/http_client.h | 2 +- include/weave/provider/network.h | 2 +- include/weave/provider/test/fake_task_runner.h | 4 --- src/access_revocation_manager_impl.cc | 5 +-- src/config_unittest.cc | 42 +++++++++++++------------- src/privet/constants.h | 2 +- src/privet/privet_handler.h | 6 ---- src/privet/privet_manager.cc | 6 ++-- src/privet/security_delegate.h | 4 +-- src/privet/security_manager.cc | 2 +- src/streams.cc | 2 -- 11 files changed, 33 insertions(+), 44 deletions(-) diff --git a/include/weave/provider/http_client.h b/include/weave/provider/http_client.h index bf01022..5a53374 100644 --- a/include/weave/provider/http_client.h +++ b/include/weave/provider/http_client.h @@ -48,7 +48,7 @@ namespace provider { // request is complete), callback should be invokes on the same thread. // Callback should never be called before SendRequest(...) returns. // -// When invoking callback function, user should privide implementation +// When invoking callback function, user should provide implementation // of the Response interface. For example, the following could be used as a // simple implementation: // struct ResponseImpl : public provider::HttpClient::Response { diff --git a/include/weave/provider/network.h b/include/weave/provider/network.h index a73b75e..2be2dfb 100644 --- a/include/weave/provider/network.h +++ b/include/weave/provider/network.h @@ -20,7 +20,7 @@ namespace provider { // sockets, implementing XMPP push notifications channel. // // There are 2 main parts of this interface. One part is used to check network -// connection status and signup for notification if connection status changes. +// connection status and sign up for notification if connection status changes. // Implementation of GetConnectionState() should return current network // connection state (enum Network::State). // Implementation of AddConnectionChangedCallback() should remember callback diff --git a/include/weave/provider/test/fake_task_runner.h b/include/weave/provider/test/fake_task_runner.h index 4080072..9476d02 100644 --- a/include/weave/provider/test/fake_task_runner.h +++ b/include/weave/provider/test/fake_task_runner.h @@ -34,10 +34,6 @@ class FakeTaskRunner : public TaskRunner { size_t GetTaskQueueSize() const; private: - void SaveTask(const tracked_objects::Location& from_here, - const base::Closure& task, - base::TimeDelta delay); - using QueueItem = std::pair, base::Closure>; struct Greater { diff --git a/src/access_revocation_manager_impl.cc b/src/access_revocation_manager_impl.cc index 035e1ad..93bdf3f 100644 --- a/src/access_revocation_manager_impl.cc +++ b/src/access_revocation_manager_impl.cc @@ -159,8 +159,9 @@ bool AccessRevocationManagerImpl::IsBlocked(const std::vector& user_id, const std::vector& app_id, base::Time timestamp) const { Entry entry_to_find; - for (const auto& user : {{}, user_id}) { - for (const auto& app : {{}, app_id}) { + const std::vector no_id; + for (const auto& user : {no_id, user_id}) { + for (const auto& app : {no_id, app_id}) { entry_to_find.user_id = user; entry_to_find.app_id = app; auto match = entries_.find(entry_to_find); diff --git a/src/config_unittest.cc b/src/config_unittest.cc index 3c020f4..9e8fc42 100644 --- a/src/config_unittest.cc +++ b/src/config_unittest.cc @@ -263,27 +263,27 @@ TEST_F(ConfigTest, Setters) { .WillOnce(WithArgs<1, 2>( Invoke([](const std::string& json, const DoneCallback& callback) { auto expected = R"({ - 'version': 1, - 'api_key': 'set_api_key', - 'client_id': 'set_client_id', - 'client_secret': 'set_client_secret', - 'cloud_id': 'set_cloud_id', - 'description': 'set_description', - 'device_id': 'set_device_id', - 'last_configured_ssid': 'set_last_configured_ssid', - 'local_anonymous_access_role': 'user', - 'root_client_token_owner': 'cloud', - 'local_discovery_enabled': true, - 'local_pairing_enabled': true, - 'location': 'set_location', - 'name': 'set_name', - 'oauth_url': 'set_oauth_url', - 'refresh_token': 'set_token', - 'robot_account': 'set_account', - 'secret': 'AQIDBAU=', - 'service_url': 'set_service_url', - 'xmpp_endpoint': 'set_xmpp_endpoint' - })"; + 'version': 1, + 'api_key': 'set_api_key', + 'client_id': 'set_client_id', + 'client_secret': 'set_client_secret', + 'cloud_id': 'set_cloud_id', + 'description': 'set_description', + 'device_id': 'set_device_id', + 'last_configured_ssid': 'set_last_configured_ssid', + 'local_anonymous_access_role': 'user', + 'root_client_token_owner': 'cloud', + 'local_discovery_enabled': true, + 'local_pairing_enabled': true, + 'location': 'set_location', + 'name': 'set_name', + 'oauth_url': 'set_oauth_url', + 'refresh_token': 'set_token', + 'robot_account': 'set_account', + 'secret': 'AQIDBAU=', + 'service_url': 'set_service_url', + 'xmpp_endpoint': 'set_xmpp_endpoint' + })"; EXPECT_JSON_EQ(expected, *test::CreateValue(json)); callback.Run(nullptr); }))); diff --git a/src/privet/constants.h b/src/privet/constants.h index 67f993d..c4f3c49 100644 --- a/src/privet/constants.h +++ b/src/privet/constants.h @@ -7,7 +7,6 @@ namespace weave { namespace privet { - namespace errors { extern const char kInvalidClientCommitment[]; @@ -31,6 +30,7 @@ extern const char kInvalidPassphrase[]; extern const char kNotFound[]; extern const char kNotImplemented[]; extern const char kAlreadyClaimed[]; + } // namespace errors } // namespace privet } // namespace weave diff --git a/src/privet/privet_handler.h b/src/privet/privet_handler.h index 7b212e4..e64151b 100644 --- a/src/privet/privet_handler.h +++ b/src/privet/privet_handler.h @@ -101,12 +101,6 @@ class PrivetHandler { void HandleSetupStatus(const base::DictionaryValue&, const UserInfo& user_info, const RequestCallback& callback); - void HandleState(const base::DictionaryValue& input, - const UserInfo& user_info, - const RequestCallback& callback); - void HandleCommandDefs(const base::DictionaryValue& input, - const UserInfo& user_info, - const RequestCallback& callback); void HandleCommandsExecute(const base::DictionaryValue& input, const UserInfo& user_info, const RequestCallback& callback); diff --git a/src/privet/privet_manager.cc b/src/privet/privet_manager.cc index 1858168..54f1155 100644 --- a/src/privet/privet_manager.cc +++ b/src/privet/privet_manager.cc @@ -114,9 +114,9 @@ std::string Manager::GetCurrentlyConnectedSsid() const { } void Manager::AddOnPairingChangedCallbacks( - const SecurityManager::PairingStartListener& on_start, - const SecurityManager::PairingEndListener& on_end) { - security_->RegisterPairingListeners(on_start, on_end); + const Device::PairingBeginCallback& begin_callback, + const Device::PairingEndCallback& end_callback) { + security_->RegisterPairingListeners(begin_callback, end_callback); } void Manager::OnDeviceInfoChanged(const weave::Settings&) { diff --git a/src/privet/security_delegate.h b/src/privet/security_delegate.h index 867ff2a..0cc33b2 100644 --- a/src/privet/security_delegate.h +++ b/src/privet/security_delegate.h @@ -47,8 +47,8 @@ class SecurityDelegate { // Returns Root Client Authorization Token. virtual std::string ClaimRootClientAuthToken(ErrorPtr* error) = 0; - // Confirms pending pending token claim or checks that token is valid for the - // active secret. + // Confirms pending token claim or checks that token is valid for the active + // secret. virtual bool ConfirmClientAuthToken(const std::string& token, ErrorPtr* error) = 0; diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index 3c11935..157c8d6 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc @@ -251,7 +251,7 @@ bool SecurityManager::IsValidPairingCode( return true; } } - LOG(ERROR) << "Attempt to authenticate with invalide code."; + LOG(ERROR) << "Attempt to authenticate with invalid code."; return false; } diff --git a/src/streams.cc b/src/streams.cc index 22527c9..c5e147f 100644 --- a/src/streams.cc +++ b/src/streams.cc @@ -11,8 +11,6 @@ namespace weave { -namespace {} // namespace - MemoryStream::MemoryStream(const std::vector& data, provider::TaskRunner* task_runner) : data_{data}, task_runner_{task_runner} {} -- cgit v1.2.3 From 5e67ce2496fc5fc3655ac1c728e94626c3e8f6c9 Mon Sep 17 00:00:00 2001 From: Alex Vakulenko Date: Wed, 6 Apr 2016 12:11:09 -0700 Subject: libweave: Split "base" trait into "device" and "privet". The original "base" trait has been split into two and those traits have slightly different definitions (e.g. now device name, description and location are part of "device" trait's state, instead of the global device resource). Updated tests to reflect the new traits. BUG: None Change-Id: I9a1a1bfc3c01d67dd0ac519106c5d20c25a62f38 Reviewed-on: https://weave-review.googlesource.com/3146 Reviewed-by: Alex Vakulenko --- examples/provider/file_config_store.cc | 1 + include/weave/device.h | 2 +- include/weave/provider/test/mock_config_store.h | 3 +- include/weave/settings.h | 7 +- src/base_api_handler.cc | 156 +++++++++--------- src/base_api_handler.h | 4 +- src/base_api_handler_unittest.cc | 211 ++++++++++++++---------- src/config.cc | 40 +++-- src/config.h | 7 +- src/config_unittest.cc | 69 +++++--- src/device_manager.cc | 2 +- src/device_registration_info.cc | 18 +- src/device_registration_info.h | 5 +- src/device_registration_info_unittest.cc | 29 ++-- src/privet/privet_handler_unittest.cc | 5 +- src/privet/security_manager.cc | 2 +- 16 files changed, 308 insertions(+), 253 deletions(-) diff --git a/examples/provider/file_config_store.cc b/examples/provider/file_config_store.cc index af3f047..a9b78b5 100644 --- a/examples/provider/file_config_store.cc +++ b/examples/provider/file_config_store.cc @@ -43,6 +43,7 @@ bool FileConfigStore::LoadDefaults(Settings* settings) { uname(&uname_data); settings->firmware_version = uname_data.sysname; + settings->serial_number = ""; settings->oem_name = "Unknown"; settings->model_name = "Unknown"; settings->model_id = model_id_; diff --git a/include/weave/device.h b/include/weave/device.h index 5d455de..c0c3584 100644 --- a/include/weave/device.h +++ b/include/weave/device.h @@ -152,7 +152,7 @@ class Device { // |component| is the name of the component for which commands should be // handled. // |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. + // "device.setConfig". Each command can have no more than one handler. // Empty |component| and |command_name| sets default handler for all unhanded // commands. // No new command handlers can be set after default handler was set. diff --git a/include/weave/provider/test/mock_config_store.h b/include/weave/provider/test/mock_config_store.h index a7eb374..2a61fc7 100644 --- a/include/weave/provider/test/mock_config_store.h +++ b/include/weave/provider/test/mock_config_store.h @@ -28,6 +28,7 @@ class MockConfigStore : public ConfigStore { EXPECT_CALL(*this, LoadDefaults(_)) .WillRepeatedly(testing::Invoke([](Settings* settings) { settings->firmware_version = "TEST_FIRMWARE"; + settings->serial_number = "TEST_SERIAL_NUMBER"; settings->oem_name = "TEST_OEM"; settings->model_name = "TEST_MODEL"; settings->model_id = "ABCDE"; @@ -39,7 +40,7 @@ class MockConfigStore : public ConfigStore { })); EXPECT_CALL(*this, LoadSettings()) .WillRepeatedly(Return(R"({ - "version": 1, + "version": 2, "device_id": "TEST_DEVICE_ID" })")); EXPECT_CALL(*this, LoadSettings(_)).WillRepeatedly(Return("")); diff --git a/include/weave/settings.h b/include/weave/settings.h index 2ba619c..2dceedb 100644 --- a/include/weave/settings.h +++ b/include/weave/settings.h @@ -30,6 +30,7 @@ enum class PairingType { struct Settings { // Model specific information. Must be set by ConfigStore::LoadDefaults. std::string firmware_version; + std::string serial_number; std::string oem_name; std::string model_name; std::string model_id; @@ -47,10 +48,8 @@ struct Settings { // Options mirrored into "base" state. // Maximum role for local anonymous user. AuthScope local_anonymous_access_role{AuthScope::kViewer}; - // If true, allows local discovery using DNS-SD. - bool local_discovery_enabled{true}; - // If true, allows local pairing using Privet API. - bool local_pairing_enabled{true}; + // If true, allows local pairing using Privet API, including mDNS. + bool local_access_enabled{true}; // Set of pairing modes supported by device. std::set pairing_modes; diff --git a/src/base_api_handler.cc b/src/base_api_handler.cc index 312e20b..535fb40 100644 --- a/src/base_api_handler.cc +++ b/src/base_api_handler.cc @@ -13,115 +13,116 @@ namespace weave { namespace { -const char kBaseComponent[] = "base"; -const char kBaseTrait[] = "base"; -const char kBaseStateFirmwareVersion[] = "base.firmwareVersion"; -const char kBaseStateAnonymousAccessRole[] = "base.localAnonymousAccessMaxRole"; -const char kBaseStateDiscoveryEnabled[] = "base.localDiscoveryEnabled"; -const char kBaseStatePairingEnabled[] = "base.localPairingEnabled"; +const char kDeviceComponent[] = "device"; +const char kDeviceTrait[] = "device"; +const char kPrivetTrait[] = "privet"; } // namespace BaseApiHandler::BaseApiHandler(DeviceRegistrationInfo* device_info, Device* device) : device_info_{device_info}, device_{device} { device_->AddTraitDefinitionsFromJson(R"({ - "base": { + "device": { "commands": { - "updateBaseConfiguration": { - "minimalRole": "manager", + "setConfig": { + "minimalRole": "user", "parameters": { - "localAnonymousAccessMaxRole": { - "enum": [ "none", "viewer", "user" ], + "name": { "type": "string" }, - "localDiscoveryEnabled": { - "type": "boolean" - }, - "localPairingEnabled": { - "type": "boolean" - } - } - }, - "updateDeviceInfo": { - "minimalRole": "manager", - "parameters": { "description": { "type": "string" }, "location": { "type": "string" - }, - "name": { - "type": "string" } } - }, - "reboot": { - "minimalRole": "user", - "parameters": {}, - "errors": ["notEnoughBattery"] - }, - "identify": { - "minimalRole": "user", - "parameters": {} } }, "state": { - "firmwareVersion": { - "type": "string", - "isRequired": true + "name": { + "isRequired": true, + "type": "string" }, - "localDiscoveryEnabled": { - "type": "boolean", - "isRequired": true + "description": { + "isRequired": true, + "type": "string" }, - "localAnonymousAccessMaxRole": { - "type": "string", - "enum": [ "none", "viewer", "user" ], - "isRequired": true + "location": { + "type": "string" }, - "localPairingEnabled": { - "type": "boolean", - "isRequired": true + "hardwareId": { + "isRequired": true, + "type": "string" }, - "connectionStatus": { + "serialNumber": { + "isRequired": true, "type": "string" }, - "network": { - "type": "object", - "additionalProperties": false, - "properties": { - "name": { "type": "string" } + "firmwareVersion": { + "isRequired": true, + "type": "string" + } + } + }, + "privet": { + "commands": { + "setConfig": { + "minimalRole": "manager", + "parameters": { + "isLocalAccessEnabled": { + "type": "boolean" + }, + "maxRoleForAnonymousAccess": { + "type": "string", + "enum": [ "none", "viewer", "user", "manager" ] + } } } + }, + "state": { + "apiVersion": { + "isRequired": true, + "type": "string" + }, + "isLocalAccessEnabled": { + "isRequired": true, + "type": "boolean" + }, + "maxRoleForAnonymousAccess": { + "isRequired": true, + "type": "string", + "enum": [ "none", "viewer", "user", "manager" ] + } } } })"); - CHECK(device_->AddComponent(kBaseComponent, {kBaseTrait}, nullptr)); + CHECK(device_->AddComponent(kDeviceComponent, {kDeviceTrait, kPrivetTrait}, + nullptr)); OnConfigChanged(device_->GetSettings()); const auto& settings = device_info_->GetSettings(); base::DictionaryValue state; - state.SetString(kBaseStateFirmwareVersion, settings.firmware_version); - CHECK(device_->SetStateProperty(kBaseComponent, kBaseStateFirmwareVersion, - base::StringValue{settings.firmware_version}, - nullptr)); + state.SetString("device.firmwareVersion", settings.firmware_version); + state.SetString("device.hardwareId", settings.device_id); + state.SetString("device.serialNumber", settings.serial_number); + state.SetString("privet.apiVersion", "3"); // Presently Privet v3. + CHECK(device_->SetStateProperties(kDeviceComponent, state, nullptr)); device_->AddCommandHandler( - kBaseComponent, "base.updateBaseConfiguration", - base::Bind(&BaseApiHandler::UpdateBaseConfiguration, + kDeviceComponent, "device.setConfig", + base::Bind(&BaseApiHandler::DeviceSetConfig, weak_ptr_factory_.GetWeakPtr())); - device_->AddCommandHandler(kBaseComponent, "base.updateDeviceInfo", - base::Bind(&BaseApiHandler::UpdateDeviceInfo, + device_->AddCommandHandler(kDeviceComponent, "privet.setConfig", + base::Bind(&BaseApiHandler::PrivetSetConfig, weak_ptr_factory_.GetWeakPtr())); device_info_->GetMutableConfig()->AddOnChangedCallback(base::Bind( &BaseApiHandler::OnConfigChanged, weak_ptr_factory_.GetWeakPtr())); } -void BaseApiHandler::UpdateBaseConfiguration( - const std::weak_ptr& cmd) { +void BaseApiHandler::PrivetSetConfig(const std::weak_ptr& cmd) { auto command = cmd.lock(); if (!command) return; @@ -132,41 +133,40 @@ void BaseApiHandler::UpdateBaseConfiguration( const auto& settings = device_info_->GetSettings(); std::string anonymous_access_role{ EnumToString(settings.local_anonymous_access_role)}; - bool discovery_enabled{settings.local_discovery_enabled}; - bool pairing_enabled{settings.local_pairing_enabled}; + bool local_access_enabled{settings.local_access_enabled}; const auto& parameters = command->GetParameters(); - parameters.GetString("localAnonymousAccessMaxRole", &anonymous_access_role); - parameters.GetBoolean("localDiscoveryEnabled", &discovery_enabled); - parameters.GetBoolean("localPairingEnabled", &pairing_enabled); + parameters.GetString("maxRoleForAnonymousAccess", &anonymous_access_role); + parameters.GetBoolean("isLocalAccessEnabled", &local_access_enabled); AuthScope auth_scope{AuthScope::kNone}; if (!StringToEnum(anonymous_access_role, &auth_scope)) { ErrorPtr error; Error::AddToPrintf(&error, FROM_HERE, errors::commands::kInvalidPropValue, - "Invalid localAnonymousAccessMaxRole value '%s'", + "Invalid maxRoleForAnonymousAccess value '%s'", anonymous_access_role.c_str()); command->Abort(error.get(), nullptr); return; } - device_info_->UpdateBaseConfig(auth_scope, discovery_enabled, - pairing_enabled); + device_info_->UpdatePrivetConfig(auth_scope, local_access_enabled); command->Complete({}, nullptr); } void BaseApiHandler::OnConfigChanged(const Settings& settings) { base::DictionaryValue state; - state.SetString(kBaseStateAnonymousAccessRole, + state.SetString("privet.maxRoleForAnonymousAccess", EnumToString(settings.local_anonymous_access_role)); - state.SetBoolean(kBaseStateDiscoveryEnabled, - settings.local_discovery_enabled); - state.SetBoolean(kBaseStatePairingEnabled, settings.local_pairing_enabled); - device_->SetStateProperties(kBaseComponent, state, nullptr); + state.SetBoolean("privet.isLocalAccessEnabled", + settings.local_access_enabled); + state.SetString("device.name", settings.name); + state.SetString("device.location", settings.location); + state.SetString("device.description", settings.description); + device_->SetStateProperties(kDeviceComponent, state, nullptr); } -void BaseApiHandler::UpdateDeviceInfo(const std::weak_ptr& cmd) { +void BaseApiHandler::DeviceSetConfig(const std::weak_ptr& cmd) { auto command = cmd.lock(); if (!command) return; diff --git a/src/base_api_handler.h b/src/base_api_handler.h index 6eebfca..b22d9ae 100644 --- a/src/base_api_handler.h +++ b/src/base_api_handler.h @@ -28,8 +28,8 @@ class BaseApiHandler final { BaseApiHandler(DeviceRegistrationInfo* device_info, Device* device); private: - void UpdateBaseConfiguration(const std::weak_ptr& command); - void UpdateDeviceInfo(const std::weak_ptr& command); + void DeviceSetConfig(const std::weak_ptr& command); + void PrivetSetConfig(const std::weak_ptr& command); void OnConfigChanged(const Settings& settings); DeviceRegistrationInfo* device_info_; diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc index 12fb3b6..61262b3 100644 --- a/src/base_api_handler_unittest.cc +++ b/src/base_api_handler_unittest.cc @@ -49,8 +49,8 @@ class BaseApiHandlerTest : public ::testing::Test { })); EXPECT_CALL(device_, - AddCommandHandler(_, AnyOf("base.updateBaseConfiguration", - "base.updateDeviceInfo"), + AddCommandHandler(_, + AnyOf("device.setConfig", "privet.setConfig"), _)) .WillRepeatedly( Invoke(&component_manager_, &ComponentManager::AddCommandHandler)); @@ -76,14 +76,28 @@ class BaseApiHandlerTest : public ::testing::Test { component_manager_.FindCommand(id)->GetState()); } - std::unique_ptr GetBaseState() { + std::unique_ptr GetDeviceState() { std::unique_ptr state; - std::string path = component_manager_.FindComponentWithTrait("base"); + std::string path = component_manager_.FindComponentWithTrait("device"); EXPECT_FALSE(path.empty()); const auto* component = component_manager_.FindComponent(path, nullptr); CHECK(component); const base::DictionaryValue* base_state = nullptr; - if (component->GetDictionary("state.base", &base_state)) + if (component->GetDictionary("state.device", &base_state)) + state = base_state->CreateDeepCopy(); + else + state.reset(new base::DictionaryValue); + return state; + } + + std::unique_ptr GetPrivetState() { + std::unique_ptr state; + std::string path = component_manager_.FindComponentWithTrait("privet"); + EXPECT_FALSE(path.empty()); + const auto* component = component_manager_.FindComponent(path, nullptr); + CHECK(component); + const base::DictionaryValue* base_state = nullptr; + if (component->GetDictionary("state.privet", &base_state)) state = base_state->CreateDeepCopy(); else state.reset(new base::DictionaryValue); @@ -102,143 +116,143 @@ class BaseApiHandlerTest : public ::testing::Test { TEST_F(BaseApiHandlerTest, Initialization) { const base::DictionaryValue* trait = nullptr; - ASSERT_TRUE(component_manager_.GetTraits().GetDictionary("base", &trait)); + ASSERT_TRUE(component_manager_.GetTraits().GetDictionary("device", &trait)); - auto expected = R"({ + auto expected1 = R"({ "commands": { - "updateBaseConfiguration": { - "minimalRole": "manager", + "setConfig": { + "minimalRole": "user", "parameters": { - "localAnonymousAccessMaxRole": { - "enum": [ "none", "viewer", "user" ], + "name": { "type": "string" }, - "localDiscoveryEnabled": { - "type": "boolean" - }, - "localPairingEnabled": { - "type": "boolean" - } - } - }, - "updateDeviceInfo": { - "minimalRole": "manager", - "parameters": { "description": { "type": "string" }, "location": { "type": "string" - }, - "name": { - "type": "string" } } - }, - "reboot": { - "minimalRole": "user", - "parameters": {}, - "errors": ["notEnoughBattery"] - }, - "identify": { - "minimalRole": "user", - "parameters": {} } }, "state": { - "firmwareVersion": { - "type": "string", - "isRequired": true + "name": { + "isRequired": true, + "type": "string" }, - "localDiscoveryEnabled": { - "type": "boolean", - "isRequired": true + "description": { + "isRequired": true, + "type": "string" }, - "localAnonymousAccessMaxRole": { - "type": "string", - "enum": [ "none", "viewer", "user" ], - "isRequired": true + "location": { + "type": "string" }, - "localPairingEnabled": { - "type": "boolean", - "isRequired": true + "hardwareId": { + "isRequired": true, + "type": "string" }, - "connectionStatus": { + "serialNumber": { + "isRequired": true, "type": "string" }, - "network": { - "type": "object", - "additionalProperties": false, - "properties": { - "name": { "type": "string" } + "firmwareVersion": { + "isRequired": true, + "type": "string" + } + } + })"; + EXPECT_JSON_EQ(expected1, *trait); + + ASSERT_TRUE(component_manager_.GetTraits().GetDictionary("privet", &trait)); + + auto expected2 = R"({ + "commands": { + "setConfig": { + "minimalRole": "manager", + "parameters": { + "isLocalAccessEnabled": { + "type": "boolean" + }, + "maxRoleForAnonymousAccess": { + "type": "string", + "enum": [ "none", "viewer", "user", "manager" ] + } } } + }, + "state": { + "apiVersion": { + "isRequired": true, + "type": "string" + }, + "isLocalAccessEnabled": { + "isRequired": true, + "type": "boolean" + }, + "maxRoleForAnonymousAccess": { + "isRequired": true, + "type": "string", + "enum": [ "none", "viewer", "user", "manager" ] + } } })"; - EXPECT_JSON_EQ(expected, *trait); + EXPECT_JSON_EQ(expected2, *trait); } -TEST_F(BaseApiHandlerTest, UpdateBaseConfiguration) { +TEST_F(BaseApiHandlerTest, PrivetSetConfig) { const Settings& settings = dev_reg_->GetSettings(); AddCommand(R"({ - 'name' : 'base.updateBaseConfiguration', - 'component': 'base', + 'name' : 'privet.setConfig', + 'component': 'device', 'parameters': { - 'localDiscoveryEnabled': false, - 'localAnonymousAccessMaxRole': 'none', - 'localPairingEnabled': false + 'isLocalAccessEnabled': false, + 'maxRoleForAnonymousAccess': 'none' } })"); EXPECT_EQ(AuthScope::kNone, settings.local_anonymous_access_role); - EXPECT_FALSE(settings.local_discovery_enabled); - EXPECT_FALSE(settings.local_pairing_enabled); + EXPECT_FALSE(settings.local_access_enabled); auto expected = R"({ - 'firmwareVersion': 'TEST_FIRMWARE', - 'localAnonymousAccessMaxRole': 'none', - 'localDiscoveryEnabled': false, - 'localPairingEnabled': false + 'apiVersion': '3', + 'maxRoleForAnonymousAccess': 'none', + 'isLocalAccessEnabled': false })"; - EXPECT_JSON_EQ(expected, *GetBaseState()); + EXPECT_JSON_EQ(expected, *GetPrivetState()); AddCommand(R"({ - 'name' : 'base.updateBaseConfiguration', - 'component': 'base', + 'name' : 'privet.setConfig', + 'component': 'device', 'parameters': { - 'localDiscoveryEnabled': true, - 'localAnonymousAccessMaxRole': 'user', - 'localPairingEnabled': true + 'maxRoleForAnonymousAccess': 'user', + 'isLocalAccessEnabled': true } })"); EXPECT_EQ(AuthScope::kUser, settings.local_anonymous_access_role); - EXPECT_TRUE(settings.local_discovery_enabled); - EXPECT_TRUE(settings.local_pairing_enabled); + EXPECT_TRUE(settings.local_access_enabled); expected = R"({ - 'firmwareVersion': 'TEST_FIRMWARE', - 'localAnonymousAccessMaxRole': 'user', - 'localDiscoveryEnabled': true, - 'localPairingEnabled': true + 'apiVersion': '3', + 'maxRoleForAnonymousAccess': 'user', + 'isLocalAccessEnabled': true })"; - EXPECT_JSON_EQ(expected, *GetBaseState()); + EXPECT_JSON_EQ(expected, *GetPrivetState()); { Config::Transaction change{dev_reg_->GetMutableConfig()}; change.set_local_anonymous_access_role(AuthScope::kViewer); } expected = R"({ - 'firmwareVersion': 'TEST_FIRMWARE', - 'localAnonymousAccessMaxRole': 'viewer', - 'localDiscoveryEnabled': true, - 'localPairingEnabled': true + 'apiVersion': '3', + 'maxRoleForAnonymousAccess': 'viewer', + 'isLocalAccessEnabled': true })"; - EXPECT_JSON_EQ(expected, *GetBaseState()); + EXPECT_JSON_EQ(expected, *GetPrivetState()); } -TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) { +TEST_F(BaseApiHandlerTest, DeviceSetConfig) { AddCommand(R"({ - 'name' : 'base.updateDeviceInfo', - 'component': 'base', + 'name' : 'device.setConfig', + 'component': 'device', 'parameters': { 'name': 'testName', 'description': 'testDescription', @@ -250,10 +264,19 @@ TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) { EXPECT_EQ("testName", config.name); EXPECT_EQ("testDescription", config.description); EXPECT_EQ("testLocation", config.location); + auto expected = R"({ + 'name': 'testName', + 'description': 'testDescription', + 'location': 'testLocation', + 'hardwareId': 'TEST_DEVICE_ID', + 'serialNumber': 'TEST_SERIAL_NUMBER', + 'firmwareVersion': 'TEST_FIRMWARE' + })"; + EXPECT_JSON_EQ(expected, *GetDeviceState()); AddCommand(R"({ - 'name' : 'base.updateDeviceInfo', - 'component': 'base', + 'name' : 'device.setConfig', + 'component': 'device', 'parameters': { 'location': 'newLocation' } @@ -262,6 +285,16 @@ TEST_F(BaseApiHandlerTest, UpdateDeviceInfo) { EXPECT_EQ("testName", config.name); EXPECT_EQ("testDescription", config.description); EXPECT_EQ("newLocation", config.location); + + expected = R"({ + 'name': 'testName', + 'description': 'testDescription', + 'location': 'newLocation', + 'hardwareId': 'TEST_DEVICE_ID', + 'serialNumber': 'TEST_SERIAL_NUMBER', + 'firmwareVersion': 'TEST_FIRMWARE' + })"; + EXPECT_JSON_EQ(expected, *GetDeviceState()); } } // namespace weave diff --git a/src/config.cc b/src/config.cc index 108c6aa..fe5335f 100644 --- a/src/config.cc +++ b/src/config.cc @@ -40,8 +40,7 @@ const char kName[] = "name"; const char kDescription[] = "description"; const char kLocation[] = "location"; const char kLocalAnonymousAccessRole[] = "local_anonymous_access_role"; -const char kLocalDiscoveryEnabled[] = "local_discovery_enabled"; -const char kLocalPairingEnabled[] = "local_pairing_enabled"; +const char kLocalAccessEnabled[] = "local_access_enabled"; const char kRefreshToken[] = "refresh_token"; const char kCloudId[] = "cloud_id"; const char kDeviceId[] = "device_id"; @@ -58,17 +57,28 @@ const char kXmppEndpoint[] = "talk.google.com:5223"; namespace { -const int kCurrentConfigVersion = 1; +const int kCurrentConfigVersion = 2; void MigrateFromV0(base::DictionaryValue* dict) { std::string cloud_id; if (dict->GetString(config_keys::kCloudId, &cloud_id) && !cloud_id.empty()) return; - scoped_ptr tmp; + std::unique_ptr tmp; if (dict->Remove(config_keys::kDeviceId, &tmp)) dict->Set(config_keys::kCloudId, std::move(tmp)); } +void MigrateFromV1(base::DictionaryValue* dict) { + // "local_discovery_enabled" and "local_pairing_enabled" are merged into one + // setting: "local_access_enabled". + std::unique_ptr bool_val; + // Use the value of "local_discovery_enabled" for "local_access_enabled" and + // remove "local_pairing_enabled". + if (dict->Remove("local_discovery_enabled", &bool_val)) + dict->Set(config_keys::kLocalAccessEnabled, std::move(bool_val)); + dict->Remove("local_pairing_enabled", nullptr); +} + Config::Settings CreateDefaultSettings(provider::ConfigStore* config_store) { Config::Settings result; result.oauth_url = "https://accounts.google.com/o/oauth2/"; @@ -171,8 +181,13 @@ void Config::Transaction::LoadState() { save_ = true; } - if (loaded_version == 0) { - MigrateFromV0(dict); + switch (loaded_version) { + case 0: + MigrateFromV0(dict); + break; + case 1: + MigrateFromV1(dict); + break; } std::string tmp; @@ -215,11 +230,8 @@ void Config::Transaction::LoadState() { set_local_anonymous_access_role(scope); } - if (dict->GetBoolean(config_keys::kLocalDiscoveryEnabled, &tmp_bool)) - set_local_discovery_enabled(tmp_bool); - - if (dict->GetBoolean(config_keys::kLocalPairingEnabled, &tmp_bool)) - set_local_pairing_enabled(tmp_bool); + if (dict->GetBoolean(config_keys::kLocalAccessEnabled, &tmp_bool)) + set_local_access_enabled(tmp_bool); if (dict->GetString(config_keys::kCloudId, &tmp)) set_cloud_id(tmp); @@ -274,10 +286,8 @@ void Config::Save() { dict.SetString(config_keys::kLocation, settings_.location); dict.SetString(config_keys::kLocalAnonymousAccessRole, EnumToString(settings_.local_anonymous_access_role)); - dict.SetBoolean(config_keys::kLocalDiscoveryEnabled, - settings_.local_discovery_enabled); - dict.SetBoolean(config_keys::kLocalPairingEnabled, - settings_.local_pairing_enabled); + dict.SetBoolean(config_keys::kLocalAccessEnabled, + settings_.local_access_enabled); std::string json_string; base::JSONWriter::WriteWithOptions( diff --git a/src/config.h b/src/config.h index 692d9c5..04ba78a 100644 --- a/src/config.h +++ b/src/config.h @@ -82,11 +82,8 @@ class Config final { void set_local_anonymous_access_role(AuthScope role) { settings_->local_anonymous_access_role = role; } - void set_local_discovery_enabled(bool enabled) { - settings_->local_discovery_enabled = enabled; - } - void set_local_pairing_enabled(bool enabled) { - settings_->local_pairing_enabled = enabled; + void set_local_access_enabled(bool enabled) { + settings_->local_access_enabled = enabled; } void set_cloud_id(const std::string& id) { settings_->cloud_id = id; } void set_refresh_token(const std::string& token) { diff --git a/src/config_unittest.cc b/src/config_unittest.cc index 9e8fc42..18d2aac 100644 --- a/src/config_unittest.cc +++ b/src/config_unittest.cc @@ -66,6 +66,7 @@ TEST_F(ConfigTest, Defaults) { EXPECT_EQ("", GetSettings().model_id); EXPECT_FALSE(GetSettings().device_id.empty()); EXPECT_EQ("", GetSettings().firmware_version); + EXPECT_EQ("", GetSettings().serial_number); EXPECT_TRUE(GetSettings().wifi_auto_setup_enabled); EXPECT_EQ("", GetSettings().test_privet_ssid); EXPECT_EQ(std::set{PairingType::kPinCode}, @@ -75,8 +76,7 @@ TEST_F(ConfigTest, Defaults) { EXPECT_EQ("", GetSettings().description); EXPECT_EQ("", GetSettings().location); EXPECT_EQ(AuthScope::kViewer, GetSettings().local_anonymous_access_role); - EXPECT_TRUE(GetSettings().local_pairing_enabled); - EXPECT_TRUE(GetSettings().local_discovery_enabled); + EXPECT_TRUE(GetSettings().local_access_enabled); EXPECT_EQ("", GetSettings().cloud_id); EXPECT_EQ("", GetSettings().refresh_token); EXPECT_EQ("", GetSettings().robot_account); @@ -127,7 +127,7 @@ TEST_F(ConfigTest, LoadStateNamed) { TEST_F(ConfigTest, LoadState) { auto state = R"({ - "version": 1, + "version": 2, "api_key": "state_api_key", "client_id": "state_client_id", "client_secret": "state_client_secret", @@ -137,8 +137,7 @@ TEST_F(ConfigTest, LoadState) { "last_configured_ssid": "state_last_configured_ssid", "local_anonymous_access_role": "user", "root_client_token_owner": "client", - "local_discovery_enabled": false, - "local_pairing_enabled": false, + "local_access_enabled": false, "location": "state_location", "name": "state_name", "oauth_url": "state_oauth_url", @@ -172,8 +171,7 @@ TEST_F(ConfigTest, LoadState) { EXPECT_EQ("state_description", GetSettings().description); EXPECT_EQ("state_location", GetSettings().location); EXPECT_EQ(AuthScope::kUser, GetSettings().local_anonymous_access_role); - EXPECT_FALSE(GetSettings().local_pairing_enabled); - EXPECT_FALSE(GetSettings().local_discovery_enabled); + EXPECT_FALSE(GetSettings().local_access_enabled); EXPECT_EQ("state_cloud_id", GetSettings().cloud_id); EXPECT_EQ("state_refresh_token", GetSettings().refresh_token); EXPECT_EQ("state_robot_account", GetSettings().robot_account); @@ -183,6 +181,44 @@ TEST_F(ConfigTest, LoadState) { GetSettings().root_client_token_owner); } +TEST_F(ConfigTest, LoadStateV1) { + auto state = R"({ + "version": 1, + "local_discovery_enabled": false, + "local_pairing_enabled": false + })"; + EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(state)); + Reload(); + EXPECT_FALSE(GetSettings().local_access_enabled); + + state = R"({ + "version": 1, + "local_discovery_enabled": true, + "local_pairing_enabled": false + })"; + EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(state)); + Reload(); + EXPECT_TRUE(GetSettings().local_access_enabled); + + state = R"({ + "version": 1, + "local_discovery_enabled": false, + "local_pairing_enabled": true + })"; + EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(state)); + Reload(); + EXPECT_FALSE(GetSettings().local_access_enabled); + + state = R"({ + "version": 1, + "local_discovery_enabled": true, + "local_pairing_enabled": true + })"; + EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(state)); + Reload(); + EXPECT_TRUE(GetSettings().local_access_enabled); +} + TEST_F(ConfigTest, Setters) { Config::Transaction change{config_.get()}; @@ -222,17 +258,11 @@ TEST_F(ConfigTest, Setters) { change.set_local_anonymous_access_role(AuthScope::kUser); EXPECT_EQ(AuthScope::kUser, GetSettings().local_anonymous_access_role); - change.set_local_discovery_enabled(false); - EXPECT_FALSE(GetSettings().local_discovery_enabled); - - change.set_local_pairing_enabled(false); - EXPECT_FALSE(GetSettings().local_pairing_enabled); - - change.set_local_discovery_enabled(true); - EXPECT_TRUE(GetSettings().local_discovery_enabled); + change.set_local_access_enabled(false); + EXPECT_FALSE(GetSettings().local_access_enabled); - change.set_local_pairing_enabled(true); - EXPECT_TRUE(GetSettings().local_pairing_enabled); + change.set_local_access_enabled(true); + EXPECT_TRUE(GetSettings().local_access_enabled); change.set_cloud_id("set_cloud_id"); EXPECT_EQ("set_cloud_id", GetSettings().cloud_id); @@ -263,7 +293,7 @@ TEST_F(ConfigTest, Setters) { .WillOnce(WithArgs<1, 2>( Invoke([](const std::string& json, const DoneCallback& callback) { auto expected = R"({ - 'version': 1, + 'version': 2, 'api_key': 'set_api_key', 'client_id': 'set_client_id', 'client_secret': 'set_client_secret', @@ -273,8 +303,7 @@ TEST_F(ConfigTest, Setters) { 'last_configured_ssid': 'set_last_configured_ssid', 'local_anonymous_access_role': 'user', 'root_client_token_owner': 'cloud', - 'local_discovery_enabled': true, - 'local_pairing_enabled': true, + 'local_access_enabled': true, 'location': 'set_location', 'name': 'set_name', 'oauth_url': 'set_oauth_url', diff --git a/src/device_manager.cc b/src/device_manager.cc index 3f8e4d7..53a8d18 100644 --- a/src/device_manager.cc +++ b/src/device_manager.cc @@ -197,7 +197,7 @@ void DeviceManager::AddPairingChangedCallbacks( } void DeviceManager::OnSettingsChanged(const Settings& settings) { - if (settings.local_discovery_enabled && http_server_) { + if (settings.local_access_enabled && http_server_) { StartPrivet(); } else { StopPrivet(); diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 6353e12..20fd9c0 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc @@ -489,11 +489,6 @@ DeviceRegistrationInfo::BuildDeviceResource() const { std::unique_ptr resource{new base::DictionaryValue}; if (!GetSettings().cloud_id.empty()) resource->SetString("id", GetSettings().cloud_id); - resource->SetString("name", GetSettings().name); - if (!GetSettings().description.empty()) - resource->SetString("description", GetSettings().description); - if (!GetSettings().location.empty()) - resource->SetString("location", GetSettings().location); resource->SetString("modelManifestId", GetSettings().model_id); std::unique_ptr channel{new base::DictionaryValue}; if (current_notification_channel_) { @@ -877,19 +872,14 @@ void DeviceRegistrationInfo::UpdateDeviceInfo(const std::string& name, change.set_description(description); change.set_location(location); change.Commit(); - - if (HaveRegistrationCredentials()) { - UpdateDeviceResource(base::Bind(&IgnoreCloudError)); - } } -void DeviceRegistrationInfo::UpdateBaseConfig(AuthScope anonymous_access_role, - bool local_discovery_enabled, - bool local_pairing_enabled) { +void DeviceRegistrationInfo::UpdatePrivetConfig(AuthScope anonymous_access_role, + bool local_access_enabled) { Config::Transaction change(config_); change.set_local_anonymous_access_role(anonymous_access_role); - change.set_local_discovery_enabled(local_discovery_enabled); - change.set_local_pairing_enabled(local_pairing_enabled); + change.set_local_access_enabled(local_access_enabled); + change.Commit(); } void DeviceRegistrationInfo::UpdateCommand( diff --git a/src/device_registration_info.h b/src/device_registration_info.h index 5ecdcac..c7a26ee 100644 --- a/src/device_registration_info.h +++ b/src/device_registration_info.h @@ -71,9 +71,8 @@ class DeviceRegistrationInfo : public NotificationDelegate, void UpdateDeviceInfo(const std::string& name, const std::string& description, const std::string& location); - void UpdateBaseConfig(AuthScope anonymous_access_role, - bool local_discovery_enabled, - bool local_pairing_enabled); + void UpdatePrivetConfig(AuthScope anonymous_access_role, + bool local_access_enabled); void GetDeviceInfo(const CloudRequestDoneCallback& callback); diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc index 757b1c9..ff32c3d 100644 --- a/src/device_registration_info_unittest.cc +++ b/src/device_registration_info_unittest.cc @@ -408,7 +408,7 @@ void DeviceRegistrationInfoTest::RegisterDevice( const RegistrationData registration_data, const RegistrationData& expected_data) { auto json_traits = CreateDictionaryValue(R"({ - 'base': { + '_foo': { 'commands': { 'reboot': { 'parameters': {'delay': {'minimum': 10, 'type': 'integer'}}, @@ -419,9 +419,9 @@ void DeviceRegistrationInfoTest::RegisterDevice( 'firmwareVersion': {'type': 'string'} } }, - 'robot': { + '_robot': { 'commands': { - '_jump': { + 'jump': { 'parameters': {'_height': {'type': 'integer'}}, 'minimalRole': 'user' } @@ -430,10 +430,10 @@ void DeviceRegistrationInfoTest::RegisterDevice( })"); EXPECT_TRUE(component_manager_.LoadTraits(*json_traits, nullptr)); EXPECT_TRUE( - component_manager_.AddComponent("", "comp", {"base", "robot"}, nullptr)); + component_manager_.AddComponent("", "comp", {"_foo", "_robot"}, nullptr)); base::StringValue ver{"1.0"}; EXPECT_TRUE(component_manager_.SetStateProperty( - "comp", "base.firmwareVersion", ver, nullptr)); + "comp", "_foo.firmwareVersion", ver, nullptr)); std::string ticket_url = expected_data.service_url + "registrationTickets/" + expected_data.ticket_id; @@ -454,20 +454,17 @@ void DeviceRegistrationInfoTest::RegisterDevice( EXPECT_EQ("pull", value); EXPECT_TRUE(json->GetString("oauthClientId", &value)); EXPECT_EQ(expected_data.client_id, value); - EXPECT_TRUE(json->GetString("deviceDraft.description", &value)); - EXPECT_EQ("Easy to clean", value); - EXPECT_TRUE(json->GetString("deviceDraft.location", &value)); - EXPECT_EQ("Kitchen", value); + EXPECT_FALSE(json->GetString("deviceDraft.description", &value)); + EXPECT_FALSE(json->GetString("deviceDraft.location", &value)); EXPECT_TRUE(json->GetString("deviceDraft.modelManifestId", &value)); EXPECT_EQ("AAAAA", value); - EXPECT_TRUE(json->GetString("deviceDraft.name", &value)); - EXPECT_EQ("Coffee Pot", value); + EXPECT_FALSE(json->GetString("deviceDraft.name", &value)); base::DictionaryValue* dict = nullptr; EXPECT_FALSE(json->GetDictionary("deviceDraft.commandDefs", &dict)); EXPECT_FALSE(json->GetDictionary("deviceDraft.state", &dict)); EXPECT_TRUE(json->GetDictionary("deviceDraft.traits", &dict)); auto expectedTraits = R"({ - 'base': { + '_foo': { 'commands': { 'reboot': { 'parameters': {'delay': {'minimum': 10, 'type': 'integer'}}, @@ -478,9 +475,9 @@ void DeviceRegistrationInfoTest::RegisterDevice( 'firmwareVersion': {'type': 'string'} } }, - 'robot': { + '_robot': { 'commands': { - '_jump': { + 'jump': { 'parameters': {'_height': {'type': 'integer'}}, 'minimalRole': 'user' } @@ -492,9 +489,9 @@ void DeviceRegistrationInfoTest::RegisterDevice( EXPECT_TRUE(json->GetDictionary("deviceDraft.components", &dict)); auto expectedComponents = R"({ 'comp': { - 'traits': ['base', 'robot'], + 'traits': ['_foo', '_robot'], 'state': { - 'base': { 'firmwareVersion': '1.0' } + '_foo': { 'firmwareVersion': '1.0' } } } })"; diff --git a/src/privet/privet_handler_unittest.cc b/src/privet/privet_handler_unittest.cc index 0c9f158..f0846f9 100644 --- a/src/privet/privet_handler_unittest.cc +++ b/src/privet/privet_handler_unittest.cc @@ -77,9 +77,8 @@ std::unique_ptr StripDebugErrorDetails( auto result = value.CreateDeepCopy(); base::DictionaryValue* error_dict = nullptr; EXPECT_TRUE(result->GetDictionary(path_to_error_object, &error_dict)); - scoped_ptr dummy; - error_dict->RemovePath("error.debugInfo", &dummy); - error_dict->RemovePath("error.message", &dummy); + error_dict->RemovePath("error.debugInfo", nullptr); + error_dict->RemovePath("error.message", nullptr); return result; } diff --git a/src/privet/security_manager.cc b/src/privet/security_manager.cc index 157c8d6..fe2bc4b 100644 --- a/src/privet/security_manager.cc +++ b/src/privet/security_manager.cc @@ -438,7 +438,7 @@ bool SecurityManager::IsAnonymousAuthSupported() const { } bool SecurityManager::IsPairingAuthSupported() const { - return GetSettings().local_pairing_enabled; + return GetSettings().local_access_enabled; } bool SecurityManager::IsLocalAuthSupported() const { -- cgit v1.2.3