From ca54bd15e1c4523bddcdc922f1857b3a03f01bd7 Mon Sep 17 00:00:00 2001 From: "Ira Lytvynenko (GitHub)" Date: Thu, 28 Jun 2018 16:35:09 +0300 Subject: Fix logic according to review comments In case of RC app registration OnRCStatus notification is sent only to the newly registered app --- .../rc_rpc_plugin/resource_allocation_manager.h | 8 +- .../resource_allocation_manager_impl.h | 4 +- .../rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc | 2 +- .../src/resource_allocation_manager_impl.cc | 126 +++++++++++---------- .../mock/mock_resource_allocation_manager.h | 5 +- .../test/resource_allocation_manager_impl_test.cc | 49 +++++--- 6 files changed, 111 insertions(+), 83 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h index 5140d10722..20bd438494 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager.h @@ -160,9 +160,15 @@ class ResourceAllocationManager { /** * @brief Create and send OnRCStatusNotification to mobile and HMI * @param event trigger for notification sending + * @param application - app that should receive notification + * in case of registration; in cases of RC enabling/disabling + * or module allocation - application is just empty shared ptr, + * because in these cases all registered RC apps should + * receive a notification */ virtual void SendOnRCStatusNotifications( - NotificationTrigger::eType event) = 0; + NotificationTrigger::eType event, + application_manager::ApplicationSharedPtr application) = 0; virtual bool is_rc_enabled() const = 0; diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager_impl.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager_impl.h index 368c5fa785..d6124a026b 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager_impl.h +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/resource_allocation_manager_impl.h @@ -118,7 +118,9 @@ class ResourceAllocationManagerImpl : public ResourceAllocationManager { RCAppExtensionPtr GetApplicationExtention( application_manager::ApplicationSharedPtr application) FINAL; - void SendOnRCStatusNotifications(NotificationTrigger::eType event) FINAL; + void SendOnRCStatusNotifications( + NotificationTrigger::eType event, + application_manager::ApplicationSharedPtr application) FINAL; bool is_rc_enabled() const FINAL; diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc index c086a10d3b..4378f1ea48 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/rc_rpc_plugin.cc @@ -92,7 +92,7 @@ void RCRPCPlugin::OnApplicationEvent( case plugins::kApplicationRegistered: { application->AddExtension(new RCAppExtension(kRCPluginID)); resource_allocation_manager_->SendOnRCStatusNotifications( - NotificationTrigger::APP_REGISTRATION); + NotificationTrigger::APP_REGISTRATION, application); break; } case plugins::kApplicationExit: { diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc index 2478196e86..5c2d58c4d2 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/resource_allocation_manager_impl.cc @@ -174,7 +174,9 @@ void ResourceAllocationManagerImpl::ProcessApplicationPolicyUpdate() { if (rc_extention) { rc_extention->UnsubscribeFromInteriorVehicleData(*module); } - SendOnRCStatusNotifications(NotificationTrigger::MODULE_ALLOCATION); + SendOnRCStatusNotifications( + NotificationTrigger::MODULE_ALLOCATION, + utils::SharedPtr()); } } } @@ -227,38 +229,36 @@ void ConstructOnRCStatusNotificationParams( smart_objects::SmartObject& msg_params, const std::map& allocated_resources, const std::vector& supported_resources, - const uint32_t app_id, - const bool is_rc_enabled) { + const uint32_t app_id) { namespace strings = application_manager::strings; namespace message_params = rc_rpc_plugin::message_params; using smart_objects::SmartObject; using smart_objects::SmartType_Map; using smart_objects::SmartType_Array; LOG4CXX_AUTO_TRACE(logger_); - SmartObject allocated_modules = SmartObject(SmartType_Array); - SmartObject free_modules = SmartObject(SmartType_Array); - if (is_rc_enabled) { - auto modules_inserter = [](SmartObject& result_modules) { - return [&result_modules](const std::string& module_name) { - smart_objects::SmartObject module_data = - SmartObject(smart_objects::SmartType_Map); - auto module_type = - StringToEnum(module_name); - module_data[message_params::kModuleType] = module_type; - result_modules.asArray()->push_back(module_data); - }; + auto modules_inserter = [](SmartObject& result_modules) { + return [&result_modules](const std::string& module_name) { + smart_objects::SmartObject module_data = + SmartObject(smart_objects::SmartType_Map); + auto module_type = + StringToEnum(module_name); + module_data[message_params::kModuleType] = module_type; + result_modules.asArray()->push_back(module_data); }; - for (const auto& module : allocated_resources) { - if (module.second == app_id) { - modules_inserter(allocated_modules)(module.first); - } + }; + SmartObject allocated_modules = SmartObject(SmartType_Array); + for (const auto& module : allocated_resources) { + if (module.second == app_id) { + modules_inserter(allocated_modules)(module.first); } - for (auto& module : supported_resources) { - if (allocated_resources.find(module) == allocated_resources.end()) { - modules_inserter(free_modules)(module); - } + } + SmartObject free_modules = SmartObject(SmartType_Array); + for (auto& module : supported_resources) { + if (allocated_resources.find(module) == allocated_resources.end()) { + modules_inserter(free_modules)(module); } } + msg_params[message_params::kAllocatedModules] = allocated_modules; msg_params[message_params::kFreeModules] = free_modules; } @@ -271,11 +271,17 @@ ResourceAllocationManagerImpl::CreateOnRCStatusNotificationToMobile( auto msg_to_mobile = MessageHelper::CreateNotification( mobile_apis::FunctionID::OnRCStatusID, app->app_id()); auto& msg_params = (*msg_to_mobile)[application_manager::strings::msg_params]; - ConstructOnRCStatusNotificationParams(msg_params, - allocated_resources_, - all_supported_modules(), - app->app_id(), - is_rc_enabled()); + if (is_rc_enabled()) { + ConstructOnRCStatusNotificationParams(msg_params, + allocated_resources_, + all_supported_modules(), + app->app_id()); + } else { + msg_params[message_params::kAllocatedModules] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + msg_params[message_params::kFreeModules] = + smart_objects::SmartObject(smart_objects::SmartType_Array); + } return msg_to_mobile; } @@ -287,11 +293,8 @@ ResourceAllocationManagerImpl::CreateOnRCStatusNotificationToHmi( auto msg_to_hmi = MessageHelper::CreateHMINotification(hmi_apis::FunctionID::RC_OnRCStatus); auto& msg_params = (*msg_to_hmi)[application_manager::strings::msg_params]; - ConstructOnRCStatusNotificationParams(msg_params, - allocated_resources_, - all_supported_modules(), - app->app_id(), - is_rc_enabled()); + ConstructOnRCStatusNotificationParams( + msg_params, allocated_resources_, all_supported_modules(), app->app_id()); msg_params[application_manager::strings::app_id] = app->hmi_app_id(); return msg_to_hmi; } @@ -300,39 +303,34 @@ void ResourceAllocationManagerImpl::SetResourceAquired( const std::string& module_type, const uint32_t app_id) { LOG4CXX_AUTO_TRACE(logger_); allocated_resources_[module_type] = app_id; - SendOnRCStatusNotifications(NotificationTrigger::MODULE_ALLOCATION); + SendOnRCStatusNotifications( + NotificationTrigger::MODULE_ALLOCATION, + utils::SharedPtr()); } void ResourceAllocationManagerImpl::SendOnRCStatusNotifications( - NotificationTrigger::eType event) { + NotificationTrigger::eType event, + application_manager::ApplicationSharedPtr application) { LOG4CXX_AUTO_TRACE(logger_); - auto rc_apps = RCRPCPlugin::GetRCApplications(app_mngr_); - for (const auto& rc_app : rc_apps) { - smart_objects::SmartObjectSPtr msg_to_mobile; - smart_objects::SmartObjectSPtr msg_to_hmi; - switch (event) { - case NotificationTrigger::APP_REGISTRATION: - msg_to_mobile = CreateOnRCStatusNotificationToMobile(rc_app); + smart_objects::SmartObjectSPtr msg_to_mobile; + smart_objects::SmartObjectSPtr msg_to_hmi; + if (NotificationTrigger::APP_REGISTRATION == event) { + DCHECK(application); + msg_to_mobile = CreateOnRCStatusNotificationToMobile(application); + (*msg_to_mobile)[application_manager::strings::msg_params] + [message_params::kAllowed] = is_rc_enabled(); + rpc_service_.SendMessageToMobile(msg_to_mobile); + } else { + auto rc_apps = RCRPCPlugin::GetRCApplications(app_mngr_); + for (const auto& rc_app : rc_apps) { + msg_to_mobile = CreateOnRCStatusNotificationToMobile(rc_app); + if (NotificationTrigger::RC_STATE_CHANGING == event) { (*msg_to_mobile)[application_manager::strings::msg_params] [message_params::kAllowed] = is_rc_enabled(); - rpc_service_.SendMessageToMobile(msg_to_mobile); - if (is_rc_enabled()) { - msg_to_hmi = CreateOnRCStatusNotificationToHmi(rc_app); - rpc_service_.SendMessageToHMI(msg_to_hmi); - } - break; - case NotificationTrigger::MODULE_ALLOCATION: - msg_to_mobile = CreateOnRCStatusNotificationToMobile(rc_app); - rpc_service_.SendMessageToMobile(msg_to_mobile); - msg_to_hmi = CreateOnRCStatusNotificationToHmi(rc_app); - rpc_service_.SendMessageToHMI(msg_to_hmi); - break; - case NotificationTrigger::RC_STATE_CHANGING: - msg_to_mobile = CreateOnRCStatusNotificationToMobile(rc_app); - (*msg_to_mobile)[application_manager::strings::msg_params] - [message_params::kAllowed] = is_rc_enabled(); - rpc_service_.SendMessageToMobile(msg_to_mobile); - break; + } + rpc_service_.SendMessageToMobile(msg_to_mobile); + msg_to_hmi = CreateOnRCStatusNotificationToHmi(rc_app); + rpc_service_.SendMessageToHMI(msg_to_hmi); } } } @@ -343,7 +341,9 @@ bool ResourceAllocationManagerImpl::is_rc_enabled() const { void ResourceAllocationManagerImpl::set_rc_enabled(const bool value) { is_rc_enabled_ = value; - SendOnRCStatusNotifications(NotificationTrigger::RC_STATE_CHANGING); + SendOnRCStatusNotifications( + NotificationTrigger::RC_STATE_CHANGING, + utils::SharedPtr()); } void ResourceAllocationManagerImpl::SetResourceFree( @@ -508,7 +508,9 @@ void ResourceAllocationManagerImpl::OnApplicationEvent( ReleaseResource(*module, application->app_id()); } if (!acquired_modules.empty()) { - SendOnRCStatusNotifications(NotificationTrigger::MODULE_ALLOCATION); + SendOnRCStatusNotifications( + NotificationTrigger::MODULE_ALLOCATION, + utils::SharedPtr()); } Apps app_list; app_list.push_back(application); diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/include/rc_rpc_plugin/mock/mock_resource_allocation_manager.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/include/rc_rpc_plugin/mock/mock_resource_allocation_manager.h index a443b24f3a..d74e8fcb65 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/include/rc_rpc_plugin/mock/mock_resource_allocation_manager.h +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/include/rc_rpc_plugin/mock/mock_resource_allocation_manager.h @@ -65,8 +65,9 @@ class MockResourceAllocationManager rc_rpc_plugin::RCAppExtensionPtr( application_manager::ApplicationSharedPtr application)); MOCK_METHOD0(ResetAllAllocations, void()); - MOCK_METHOD1(SendOnRCStatusNotifications, - void(rc_rpc_plugin::NotificationTrigger::eType)); + MOCK_METHOD2(SendOnRCStatusNotifications, + void(rc_rpc_plugin::NotificationTrigger::eType, + application_manager::ApplicationSharedPtr application)); MOCK_CONST_METHOD0(is_rc_enabled, bool()); MOCK_METHOD1(set_rc_enabled, void(const bool value)); }; diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/resource_allocation_manager_impl_test.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/resource_allocation_manager_impl_test.cc index 4c4752328f..9e006c56fe 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/resource_allocation_manager_impl_test.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/resource_allocation_manager_impl_test.cc @@ -517,16 +517,14 @@ TEST_F(RAManagerTest, OnRCStatus_AppRegistation_RC_allowed) { EXPECT_CALL(mock_rpc_service_, SendMessageToMobile(_, false)) .WillOnce(SaveArg<0>(&message_to_mob)); application_manager::commands::MessageSharedPtr message_to_hmi; - EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)) - .WillOnce(SaveArg<0>(&message_to_hmi)); + EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)).Times(0); // Act - ra_manager.SendOnRCStatusNotifications(NotificationTrigger::APP_REGISTRATION); + ra_manager.SendOnRCStatusNotifications(NotificationTrigger::APP_REGISTRATION, + mock_app_1_); auto msg_to_mob_params = (*message_to_mob)[application_manager::strings::msg_params]; - auto msg_to_hmi_params = - (*message_to_hmi)[application_manager::strings::msg_params]; // Assert EXPECT_EQ(msg_to_mob_params[message_params::kAllowed].asBool(), true); @@ -535,13 +533,6 @@ TEST_F(RAManagerTest, OnRCStatus_AppRegistation_RC_allowed) { 0u); EXPECT_EQ(msg_to_mob_params[message_params::kFreeModules].asArray()->size(), kSizeOfModules); - EXPECT_EQ( - msg_to_hmi_params[message_params::kAllocatedModules].asArray()->size(), - 0u); - EXPECT_EQ(msg_to_hmi_params[message_params::kFreeModules].asArray()->size(), - kSizeOfModules); - EXPECT_EQ(msg_to_hmi_params[application_manager::strings::app_id].asInt(), - kHMIAppId1); } TEST_F(RAManagerTest, OnRCStatus_AppRegistation_RC_disallowed) { @@ -558,7 +549,8 @@ TEST_F(RAManagerTest, OnRCStatus_AppRegistation_RC_disallowed) { EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)).Times(0); // Act - ra_manager.SendOnRCStatusNotifications(NotificationTrigger::APP_REGISTRATION); + ra_manager.SendOnRCStatusNotifications(NotificationTrigger::APP_REGISTRATION, + mock_app_1_); auto msg_to_mob_params = (*message_to_mob)[application_manager::strings::msg_params]; @@ -576,17 +568,22 @@ TEST_F(RAManagerTest, OnRCStatus_RCStateChanging_RC_disabling) { ResourceAllocationManagerImpl ra_manager(mock_app_mngr_, mock_rpc_service_); ON_CALL((*mock_app_1_), is_remote_control_supported()) .WillByDefault(Return(true)); + ON_CALL((*mock_app_1_), hmi_app_id()).WillByDefault(Return(kHMIAppId1)); application_manager::commands::MessageSharedPtr message_to_mob; EXPECT_CALL(mock_rpc_service_, SendMessageToMobile(_, false)) .WillOnce(SaveArg<0>(&message_to_mob)); - EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)).Times(0); + application_manager::commands::MessageSharedPtr message_to_hmi; + EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)) + .WillOnce(SaveArg<0>(&message_to_hmi)); // Act ra_manager.set_rc_enabled(false); auto msg_to_mob_params = (*message_to_mob)[application_manager::strings::msg_params]; + auto msg_to_hmi_params = + (*message_to_hmi)[application_manager::strings::msg_params]; // Assert EXPECT_EQ(msg_to_mob_params[message_params::kAllowed].asBool(), false); EXPECT_EQ( @@ -594,6 +591,13 @@ TEST_F(RAManagerTest, OnRCStatus_RCStateChanging_RC_disabling) { 0u); EXPECT_EQ(msg_to_mob_params[message_params::kFreeModules].asArray()->size(), 0u); + EXPECT_EQ( + msg_to_hmi_params[message_params::kAllocatedModules].asArray()->size(), + 0u); + EXPECT_EQ(msg_to_hmi_params[message_params::kFreeModules].asArray()->size(), + kSizeOfModules); + EXPECT_EQ(msg_to_hmi_params[application_manager::strings::app_id].asInt(), + kHMIAppId1); } TEST_F(RAManagerTest, OnRCStatus_RCStateChanging_RC_enabling) { @@ -601,17 +605,22 @@ TEST_F(RAManagerTest, OnRCStatus_RCStateChanging_RC_enabling) { ResourceAllocationManagerImpl ra_manager(mock_app_mngr_, mock_rpc_service_); ON_CALL((*mock_app_1_), is_remote_control_supported()) .WillByDefault(Return(true)); + ON_CALL((*mock_app_1_), hmi_app_id()).WillByDefault(Return(kHMIAppId1)); application_manager::commands::MessageSharedPtr message_to_mob; EXPECT_CALL(mock_rpc_service_, SendMessageToMobile(_, false)) .WillOnce(SaveArg<0>(&message_to_mob)); - EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)).Times(0); + application_manager::commands::MessageSharedPtr message_to_hmi; + EXPECT_CALL(mock_rpc_service_, SendMessageToHMI(_)) + .WillOnce(SaveArg<0>(&message_to_hmi)); // Act ra_manager.set_rc_enabled(true); auto msg_to_mob_params = (*message_to_mob)[application_manager::strings::msg_params]; + auto msg_to_hmi_params = + (*message_to_hmi)[application_manager::strings::msg_params]; // Assert EXPECT_EQ(msg_to_mob_params[message_params::kAllowed].asBool(), true); EXPECT_EQ( @@ -619,6 +628,13 @@ TEST_F(RAManagerTest, OnRCStatus_RCStateChanging_RC_enabling) { 0u); EXPECT_EQ(msg_to_mob_params[message_params::kFreeModules].asArray()->size(), kSizeOfModules); + EXPECT_EQ( + msg_to_hmi_params[message_params::kAllocatedModules].asArray()->size(), + 0u); + EXPECT_EQ(msg_to_hmi_params[message_params::kFreeModules].asArray()->size(), + kSizeOfModules); + EXPECT_EQ(msg_to_hmi_params[application_manager::strings::app_id].asInt(), + kHMIAppId1); } TEST_F(RAManagerTest, OnRCStatus_ModuleAllocation) { @@ -642,7 +658,8 @@ TEST_F(RAManagerTest, OnRCStatus_ModuleAllocation) { // Act ra_manager.SendOnRCStatusNotifications( - NotificationTrigger::MODULE_ALLOCATION); + NotificationTrigger::MODULE_ALLOCATION, + utils::SharedPtr()); auto msg_to_mob_params = (*message_to_mob)[application_manager::strings::msg_params]; -- cgit v1.2.1