diff options
author | Igor Gapchuk (GitHub) <41586842+IGapchuk@users.noreply.github.com> | 2021-05-04 21:37:33 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-04 14:37:33 -0400 |
commit | c15ef6c045540bbfcb43dba8533b70c21616c740 (patch) | |
tree | a24b282fbfe42a6d0b5135cbfad90f8d4b0499e5 | |
parent | f9bf97c0fd19da5d8254a3ea2081dcc03aac4a6f (diff) | |
download | sdl_core-c15ef6c045540bbfcb43dba8533b70c21616c740.tar.gz |
Fix core prematurely sends success for delete interaction choice set (#2601)
* Fix core prematurely sends success for DeleteInteractionChoiceSet.
DeleteInteractionChoiceSetRequest was contain implementation, according
to that, SDL never not waits for HMI response and sends to Mobile response (success = true, resultCode = "SUCCESS").
This PR provides changes, that remove that implementation.
Also, according to requirements, when at-least on of HMI VR_DeleteCommand
response will contains any ERROR result code, except WARNINGS,
SDL has to send response with that error code and success_result as false.
And if on of HMI VR_DeleteCommand response will contains at-least one
WARNINGS resulst_code, SDL has to send to Mobile response with WARNINGS
and success_result as true.
* Address review comments
* fixup! Address review comments
* Update src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc
Co-authored-by: Collin <iCollin@users.noreply.github.com>
Co-authored-by: Andrii Kalinich <AKalinich@luxoft.com>
Co-authored-by: Collin <iCollin@users.noreply.github.com>
3 files changed, 140 insertions, 21 deletions
diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/delete_interaction_choice_set_request.h b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/delete_interaction_choice_set_request.h index c1d8759962..d5fa89c8be 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/delete_interaction_choice_set_request.h +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/mobile/delete_interaction_choice_set_request.h @@ -34,6 +34,9 @@ #ifndef SRC_COMPONENTS_APPLICATION_MANAGER_RPC_PLUGINS_SDL_RPC_PLUGIN_INCLUDE_SDL_RPC_PLUGIN_COMMANDS_MOBILE_DELETE_INTERACTION_CHOICE_SET_REQUEST_H_ #define SRC_COMPONENTS_APPLICATION_MANAGER_RPC_PLUGINS_SDL_RPC_PLUGIN_INCLUDE_SDL_RPC_PLUGIN_COMMANDS_MOBILE_DELETE_INTERACTION_CHOICE_SET_REQUEST_H_ +#include <cstdint> +#include <set> + #include "application_manager/application.h" #include "application_manager/commands/command_request_impl.h" #include "utils/macro.h" @@ -43,6 +46,8 @@ namespace app_mngr = application_manager; namespace commands { +typedef std::set<uint32_t> SentRequestsSet; + /** * @brief DeleteInteractionChoiceSetRequest command class **/ @@ -76,6 +81,20 @@ class DeleteInteractionChoiceSetRequest */ bool Init() FINAL; + /** + * @brief Interface method that is called whenever new event received + * Need to observe VR_DeleteCommand event, to send + * DeleteInteractionChoiceSetResponse when VR command was delete from HMI. + * @param event The received event. + */ + void on_event(const app_mngr::event_engine::Event& event) FINAL; + + /** + * @brief Function is called by RequestController when request execution time + * has exceed it's limit + */ + void onTimeOut() FINAL; + private: /* * @brief Check if requested choice set ID in use by perform interaction @@ -86,6 +105,24 @@ class DeleteInteractionChoiceSetRequest void SendVrDeleteCommand(app_mngr::ApplicationSharedPtr app); + void SendDeleteInteractionChoiceSetResponse(); + + /** + * @brief Final result_code for sending to Mobile. + */ + std::vector<hmi_apis::Common_Result::eType> response_result_codes_; + + sync_primitives::Lock requests_lock_; + + /** + * @brief Collection that contains sent request to HMI. + * When HMI response will start come, that collection will helps to control, + * when SDL should sends DeleteInteractionChoiceSetResponse to Mobile. + * Because, for send DeleteInteractionChoiceSetResponse SDL should will be saw + * all response results from HMI. + */ + SentRequestsSet sent_requests_; + DISALLOW_COPY_AND_ASSIGN(DeleteInteractionChoiceSetRequest); }; diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc index 3ff95a6f41..05347435d7 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/delete_interaction_choice_set_request.cc @@ -55,7 +55,8 @@ DeleteInteractionChoiceSetRequest::DeleteInteractionChoiceSetRequest( application_manager, rpc_service, hmi_capabilities, - policy_handler) {} + policy_handler) + , response_result_codes_() {} DeleteInteractionChoiceSetRequest::~DeleteInteractionChoiceSetRequest() {} @@ -89,23 +90,49 @@ void DeleteInteractionChoiceSetRequest::Run() { return; } SendVrDeleteCommand(app); +} - smart_objects::SmartObject msg_params = - smart_objects::SmartObject(smart_objects::SmartType_Map); +bool DeleteInteractionChoiceSetRequest::Init() { + hash_update_mode_ = HashUpdateMode::kDoHashUpdate; + return true; +} - msg_params[strings::interaction_choice_set_id] = choice_set_id; - msg_params[strings::app_id] = app->app_id(); +void DeleteInteractionChoiceSetRequest::on_event( + const event_engine::Event& event) { + using namespace helpers; + SDL_LOG_AUTO_TRACE(); + + if (event.id() == hmi_apis::FunctionID::VR_DeleteCommand) { + const smart_objects::SmartObject& message = event.smart_object(); + const auto result_code = static_cast<hmi_apis::Common_Result::eType>( + message[strings::params][hmi_response::code].asInt()); + response_result_codes_.push_back(result_code); + const std::uint32_t correlation_id = static_cast<uint32_t>( + message[strings::params][strings::correlation_id].asUInt()); + + bool should_send_response = false; + { + sync_primitives::AutoLock auto_lock(requests_lock_); + auto found_request = sent_requests_.find(correlation_id); + if (sent_requests_.end() == found_request) { + SDL_LOG_WARN("Request with " << correlation_id + << " correlation_id is not found."); + return; + } - app->RemoveChoiceSet(choice_set_id); + sent_requests_.erase(found_request); + should_send_response = sent_requests_.empty(); + } - // Checking of HMI responses will be implemented with APPLINK-14600 - const bool result = true; - SendResponse(result, mobile_apis::Result::SUCCESS); + if (should_send_response) { + SendDeleteInteractionChoiceSetResponse(); + } + } } -bool DeleteInteractionChoiceSetRequest::Init() { - hash_update_mode_ = HashUpdateMode::kDoHashUpdate; - return true; +void DeleteInteractionChoiceSetRequest::onTimeOut() { + SDL_LOG_AUTO_TRACE(); + SendResponse(false, mobile_apis::Result::GENERIC_ERROR); } bool DeleteInteractionChoiceSetRequest::ChoiceSetInUse( @@ -157,12 +184,54 @@ void DeleteInteractionChoiceSetRequest::SendVrDeleteCommand( msg_params[strings::type] = hmi_apis::Common_VRCommandType::Choice; msg_params[strings::grammar_id] = choice_set[strings::grammar_id]; choice_set = choice_set[strings::choice_set]; + + sync_primitives::AutoLock auto_lock(requests_lock_); for (uint32_t i = 0; i < choice_set.length(); ++i) { msg_params[strings::cmd_id] = choice_set[i][strings::choice_id]; - SendHMIRequest(hmi_apis::FunctionID::VR_DeleteCommand, &msg_params); + const uint32_t delte_cmd_hmi_corr_id = SendHMIRequest( + hmi_apis::FunctionID::VR_DeleteCommand, &msg_params, true); + sent_requests_.insert(delte_cmd_hmi_corr_id); } } +void DeleteInteractionChoiceSetRequest:: + SendDeleteInteractionChoiceSetResponse() { + hmi_apis::Common_Result::eType result_code = + hmi_apis::Common_Result::INVALID_ENUM; + for (const auto& code : response_result_codes_) { + if (result_code == hmi_apis::Common_Result::INVALID_ENUM) { + result_code = code; + continue; + } + + if (!IsHMIResultSuccess(code)) { + result_code = code; + } + } + + const bool response_result = PrepareResultForMobileResponse( + result_code, HmiInterfaces::InterfaceID::HMI_INTERFACE_VR); + + if (response_result) { + ApplicationSharedPtr app = + application_manager_.application(connection_key()); + if (!app) { + SDL_LOG_ERROR("Application with connection key " << connection_key() + << " did not find."); + return; + } + const uint32_t choice_set_id = + (*message_)[strings::msg_params][strings::interaction_choice_set_id] + .asUInt(); + app->RemoveChoiceSet(choice_set_id); + } + + SDL_LOG_DEBUG("Response sent. Result code: " << result_code + << " sussess: " << std::boolalpha + << result_code); + SendResponse(response_result, MessageHelper::HMIToMobileResult(result_code)); +} + } // namespace commands } // namespace sdl_rpc_plugin diff --git a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc index dd4c6fd526..bd4c5c1ec6 100644 --- a/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc +++ b/src/components/application_manager/rpc_plugins/sdl_rpc_plugin/test/commands/mobile/delete_interaction_choice_set_test.cc @@ -209,9 +209,7 @@ TEST_F(DeleteInteractionChoiceSetRequestTest, EXPECT_CALL(*app_, FindChoiceSet(kChoiceSetId)) .WillOnce(Return(invalid_choice_set_id)); - EXPECT_CALL(*app_, app_id()).WillOnce(Return(kConnectionKey)); - EXPECT_CALL(*app_, RemoveChoiceSet(kChoiceSetId)); - EXPECT_CALL(*app_, UpdateHash()); + EXPECT_CALL(*app_, app_id()).Times(0); } DeleteInteractionChoiceSetRequestPtr command = @@ -222,8 +220,13 @@ TEST_F(DeleteInteractionChoiceSetRequestTest, } TEST_F(DeleteInteractionChoiceSetRequestTest, Run_SendVrDeleteCommand_SUCCESS) { + using namespace application_manager; + using namespace event_engine; + (*message_)[am::strings::params][am::strings::connection_key] = kConnectionKey; + (*message_)[strings::params][hmi_response::code] = + hmi_apis::Common_Result::SUCCESS; (*message_)[am::strings::msg_params][am::strings::interaction_choice_set_id] = kChoiceSetId; (*message_)[am::strings::msg_params][am::strings::grammar_id] = kGrammarId; @@ -232,6 +235,10 @@ TEST_F(DeleteInteractionChoiceSetRequestTest, Run_SendVrDeleteCommand_SUCCESS) { smart_objects::SmartObject choice_set_id = (*message_)[am::strings::msg_params]; + application_manager::event_engine::Event event( + hmi_apis::FunctionID::VR_DeleteCommand); + event.set_smart_object(*message_); + EXPECT_CALL(app_mngr_, application(kConnectionKey)) .WillRepeatedly(Return(app_)); @@ -246,21 +253,27 @@ TEST_F(DeleteInteractionChoiceSetRequestTest, Run_SendVrDeleteCommand_SUCCESS) { EXPECT_CALL(*app_, FindChoiceSet(kChoiceSetId)) .WillOnce(Return(choice_set_id)); - EXPECT_CALL(*app_, app_id()) - .WillOnce(Return(kConnectionKey)) - .WillOnce(Return(kConnectionKey)); + EXPECT_CALL(*app_, app_id()).WillOnce(Return(kConnectionKey)); + } + + EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_, _)).WillOnce(Return(true)); + + { + InSequence seq; EXPECT_CALL(*app_, RemoveChoiceSet(kChoiceSetId)); EXPECT_CALL(*app_, UpdateHash()); } - EXPECT_CALL(mock_rpc_service_, ManageHMICommand(_, _)).WillOnce(Return(true)); - EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)); + EXPECT_CALL(mock_rpc_service_, ManageMobileCommand(_, _)) + .WillOnce(Return(true)); DeleteInteractionChoiceSetRequestPtr command = CreateCommand<DeleteInteractionChoiceSetRequest>(message_); command->Init(); command->Run(); + + command->on_event(event); } TEST_F(DeleteInteractionChoiceSetResponseTest, Run_SuccessFalse_UNSUCCESS) { |