From bb738ffc106bbc231bd0ade9dde65fff32c8b5ba Mon Sep 17 00:00:00 2001 From: "Andrii Kalinich (GitHub)" Date: Tue, 12 Oct 2021 14:33:23 -0400 Subject: Fix wrong behavior of RCOnRemoteControlSettingsNotification (#3792) * Fix wrong behavior of RCOnRemoteControlSettingsNotification Looks like `kAllowed` parameter was swithing on/off the RC functionality even if it was not present and previous value should be taken. Processing of `kAllowed` and `kAccessMode` was split on two separate and independent functions make it really isolated. Processing of both parameters was aligned, so now core does not perform extra actions if one of parameter is missing. --- .../rc_on_remote_control_settings_notification.h | 12 ++++++ .../rc_rpc_plugin/resource_allocation_manager.h | 8 ++++ .../rc_on_remote_control_settings_notification.cc | 49 ++++++++++++++-------- .../commands/on_remote_control_settings_test.cc | 49 ++++++++++++++++++++-- 4 files changed, 98 insertions(+), 20 deletions(-) diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h index 82e0a8c13b..bb5e7a9b8f 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/include/rc_rpc_plugin/commands/hmi/rc_on_remote_control_settings_notification.h @@ -76,6 +76,18 @@ class RCOnRemoteControlSettingsNotification * notifications */ void DisallowRCFunctionality(); + + /** + * @brief Performs the set of actions depending on access mode param received + * in the message + */ + void ProcessAccessModeParam(); + + /** + * @brief Performs the set of actions depending on allowed param received in + * the message + */ + void ProcessAllowedParam(); }; } // namespace commands } // namespace rc_rpc_plugin 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 623498dac7..db785e4a4f 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 @@ -209,8 +209,16 @@ class ResourceAllocationManager { NotificationTrigger::eType event, application_manager::ApplicationSharedPtr application) = 0; + /** + * @brief Returns current state of RC functionality + * @return current state of RC functionality + */ virtual bool is_rc_enabled() const = 0; + /** + * @brief Sets current state of RC functionality to a new one + * @param value new RC functionality state + */ virtual void set_rc_enabled(const bool value) = 0; /** diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc index 860e18a105..580937bc8a 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/src/commands/hmi/rc_on_remote_control_settings_notification.cc @@ -93,35 +93,50 @@ void RCOnRemoteControlSettingsNotification::Run() { return; } - hmi_apis::Common_RCAccessMode::eType access_mode = - hmi_apis::Common_RCAccessMode::INVALID_ENUM; - if ((*message_)[app_mngr::strings::msg_params].keyExists( + ProcessAccessModeParam(); + ProcessAllowedParam(); +} + +void RCOnRemoteControlSettingsNotification::ProcessAccessModeParam() { + if (!(*message_)[app_mngr::strings::msg_params].keyExists( message_params::kAccessMode)) { - access_mode = static_cast( - (*message_)[app_mngr::strings::msg_params][message_params::kAccessMode] - .asUInt()); SDL_LOG_DEBUG( - "Setting up access mode : " << AccessModeToString(access_mode)); - } else { - access_mode = resource_allocation_manager_.GetAccessMode(); - SDL_LOG_DEBUG("No access mode received. Using last known: " - << AccessModeToString(access_mode)); + "No access mode received. Using last known: " + << AccessModeToString(resource_allocation_manager_.GetAccessMode())); + return; } + + const auto access_mode = static_cast( + (*message_)[app_mngr::strings::msg_params][message_params::kAccessMode] + .asUInt()); + SDL_LOG_DEBUG("Setting up access mode : " << AccessModeToString(access_mode)); resource_allocation_manager_.SetAccessMode(access_mode); +} + +void RCOnRemoteControlSettingsNotification::ProcessAllowedParam() { + if (!(*message_)[app_mngr::strings::msg_params].keyExists( + message_params::kAllowed)) { + SDL_LOG_DEBUG("No allowed param received. Using last known: " + << std::boolalpha + << resource_allocation_manager_.is_rc_enabled()); + return; + } const bool is_allowed = (*message_)[app_mngr::strings::msg_params][message_params::kAllowed] .asBool(); + if (is_allowed) { SDL_LOG_DEBUG("Allowing RC Functionality"); resource_allocation_manager_.set_rc_enabled(true); - } else { - SDL_LOG_DEBUG("Disallowing RC Functionality"); - DisallowRCFunctionality(); - resource_allocation_manager_.ResetAllAllocations(); - resource_allocation_manager_.set_rc_enabled(false); - rc_consent_manager_.RemoveAllConsents(); + return; } + + SDL_LOG_DEBUG("Disallowing RC Functionality"); + DisallowRCFunctionality(); + resource_allocation_manager_.ResetAllAllocations(); + resource_allocation_manager_.set_rc_enabled(false); + rc_consent_manager_.RemoveAllConsents(); } } // namespace commands diff --git a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc index 7907013fa3..5d569e93d2 100644 --- a/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc +++ b/src/components/application_manager/rpc_plugins/rc_rpc_plugin/test/commands/on_remote_control_settings_test.cc @@ -117,18 +117,61 @@ class RCOnRemoteControlSettingsNotificationTest }; TEST_F(RCOnRemoteControlSettingsNotificationTest, - Run_Allowed_SetAccessMode) { // Arrange + Run_Allowed_MissedAccessMode) { // Arrange MessageSharedPtr mobile_message = CreateBasicMessage(); (*mobile_message)[application_manager::strings::msg_params] [message_params::kAllowed] = true; // Expectations + EXPECT_CALL(mock_allocation_manager_, SetAccessMode(_)).Times(0); + EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(true)); - ON_CALL(mock_allocation_manager_, GetAccessMode()) - .WillByDefault(Return(hmi_apis::Common_RCAccessMode::ASK_DRIVER)); + // Act + std::shared_ptr< + rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification> + command = CreateRCCommand< + rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>( + mobile_message); + command->Run(); +} + +TEST_F(RCOnRemoteControlSettingsNotificationTest, + Run_AccessMode_MissedAllowed) { // Arrange + MessageSharedPtr mobile_message = CreateBasicMessage(); + (*mobile_message)[application_manager::strings::msg_params] + [message_params::kAccessMode] = + hmi_apis::Common_RCAccessMode::ASK_DRIVER; + + // Expectations + EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(_)).Times(0); EXPECT_CALL(mock_allocation_manager_, SetAccessMode(hmi_apis::Common_RCAccessMode::ASK_DRIVER)); + + // Act + std::shared_ptr< + rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification> + command = CreateRCCommand< + rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification>( + mobile_message); + + command->Run(); +} + +TEST_F(RCOnRemoteControlSettingsNotificationTest, + Run_AccessModeAndAllowed_BothPresent) { // Arrange + MessageSharedPtr mobile_message = CreateBasicMessage(); + (*mobile_message)[application_manager::strings::msg_params] + [message_params::kAllowed] = true; + (*mobile_message)[application_manager::strings::msg_params] + [message_params::kAccessMode] = + hmi_apis::Common_RCAccessMode::ASK_DRIVER; + + // Expectations + EXPECT_CALL(mock_allocation_manager_, set_rc_enabled(true)); + EXPECT_CALL(mock_allocation_manager_, + SetAccessMode(hmi_apis::Common_RCAccessMode::ASK_DRIVER)); + // Act std::shared_ptr< rc_rpc_plugin::commands::RCOnRemoteControlSettingsNotification> -- cgit v1.2.1