diff options
author | Andrey Oleynik (GitHub) <dev-gh@users.noreply.github.com> | 2017-11-02 17:52:16 +0200 |
---|---|---|
committer | JackLivio <jack@livio.io> | 2017-11-02 11:52:16 -0400 |
commit | 5f9a93c34aac15c04181ecb28180dc6762f2daac (patch) | |
tree | 39b12d832338fd72bfb0c77402b1b3b7ce0c9cd4 | |
parent | a63a225f1bf20ccb78ba17e79613bdc095a85c14 (diff) | |
download | sdl_core-5f9a93c34aac15c04181ecb28180dc6762f2daac.tar.gz |
Fixes responses 'success' value in case of UNSUPPORTED_RESOURCE (#1576)
* Fixes 'success' value in case of UNSUPPORTED_RESOURCE
There is only one case for 'success' to have 'false' with UNSUPPORTED_RESOURCE
- when there is single-interface RPC comes and that specific interface is
not available i.e. <interface>.IsReady returned 'false'
Changes done for base class and unit tests with wrong expectations.
* Fix response to mobile result
Fixed result to mobile in case of GENERIC_ERROR from hmi and iface does
not response
* Fix ut tests according to code changes
* Fix regression
7 files changed, 77 insertions, 41 deletions
diff --git a/src/components/application_manager/include/application_manager/commands/command_request_impl.h b/src/components/application_manager/include/application_manager/commands/command_request_impl.h index aa3c216d4d..614f86a6f1 100644 --- a/src/components/application_manager/include/application_manager/commands/command_request_impl.h +++ b/src/components/application_manager/include/application_manager/commands/command_request_impl.h @@ -49,7 +49,7 @@ struct ResponseInfo { , interface_state(HmiInterfaces::STATE_NOT_RESPONSE) , is_ok(false) , is_unsupported_resource(false) - , is_invalid_enum(false) {} + , is_not_used(false) {} ResponseInfo(hmi_apis::Common_Result::eType result, HmiInterfaces::InterfaceID interface) : result_code(result) @@ -57,13 +57,13 @@ struct ResponseInfo { , interface_state(HmiInterfaces::STATE_NOT_RESPONSE) , is_ok(false) , is_unsupported_resource(false) - , is_invalid_enum(false) {} + , is_not_used(false) {} hmi_apis::Common_Result::eType result_code; HmiInterfaces::InterfaceID interface; HmiInterfaces::InterfaceState interface_state; bool is_ok; bool is_unsupported_resource; - bool is_invalid_enum; + bool is_not_used; }; namespace NsSmart = NsSmartDeviceLink::NsSmartObjects; diff --git a/src/components/application_manager/src/commands/command_request_impl.cc b/src/components/application_manager/src/commands/command_request_impl.cc index 3f9a1d13b3..efd8c8f042 100644 --- a/src/components/application_manager/src/commands/command_request_impl.cc +++ b/src/components/application_manager/src/commands/command_request_impl.cc @@ -105,18 +105,31 @@ const std::string CreateInfoForUnsupportedResult( } bool CheckResultCode(const ResponseInfo& first, const ResponseInfo& second) { - if (first.is_ok && second.is_unsupported_resource && - second.interface_state == HmiInterfaces::STATE_NOT_AVAILABLE) { - return true; + if (!first.is_ok && !second.is_ok) { + return false; } - return false; + + if (!first.is_ok && second.is_not_used) { + return false; + } + + if (!second.is_ok && first.is_not_used) { + return false; + } + + if (first.is_unsupported_resource && second.is_not_used && + HmiInterfaces::STATE_NOT_AVAILABLE == first.interface_state) { + return false; + } + + return true; } bool IsResultCodeUnsupported(const ResponseInfo& first, const ResponseInfo& second) { - return ((first.is_ok || first.is_invalid_enum) && + return ((first.is_ok || first.is_not_used) && second.is_unsupported_resource) || - ((second.is_ok || second.is_invalid_enum) && + ((second.is_ok || second.is_not_used) && first.is_unsupported_resource) || (first.is_unsupported_resource && second.is_unsupported_resource); } @@ -204,6 +217,7 @@ void CommandRequestImpl::SendResponse( const mobile_apis::Result::eType& result_code, const char* info, const smart_objects::SmartObject* response_params) { + LOG4CXX_AUTO_TRACE(logger_); { sync_primitives::AutoLock auto_lock(state_lock_); if (kTimedOut == current_state_) { @@ -777,6 +791,7 @@ bool CommandRequestImpl::PrepareResultForMobileResponse( bool CommandRequestImpl::PrepareResultForMobileResponse( ResponseInfo& out_first, ResponseInfo& out_second) const { + LOG4CXX_AUTO_TRACE(logger_); using namespace helpers; out_first.is_ok = Compare<hmi_apis::Common_Result::eType, EQ, ONE>( @@ -795,10 +810,10 @@ bool CommandRequestImpl::PrepareResultForMobileResponse( hmi_apis::Common_Result::RETRY, hmi_apis::Common_Result::SAVED); - out_first.is_invalid_enum = + out_first.is_not_used = hmi_apis::Common_Result::INVALID_ENUM == out_first.result_code; - out_second.is_invalid_enum = + out_second.is_not_used = hmi_apis::Common_Result::INVALID_ENUM == out_second.result_code; out_first.is_unsupported_resource = @@ -810,13 +825,15 @@ bool CommandRequestImpl::PrepareResultForMobileResponse( out_first.interface_state = application_manager_.hmi_interfaces().GetInterfaceState( out_first.interface); + out_second.interface_state = application_manager_.hmi_interfaces().GetInterfaceState( out_second.interface); bool result = (out_first.is_ok && out_second.is_ok) || - (out_second.is_invalid_enum && out_first.is_ok) || - (out_first.is_invalid_enum && out_second.is_ok); + (out_second.is_not_used && out_first.is_ok) || + (out_first.is_not_used && out_second.is_ok); + result = result || CheckResultCode(out_first, out_second); result = result || CheckResultCode(out_second, out_first); return result; @@ -835,6 +852,7 @@ void CommandRequestImpl::GetInfo( mobile_apis::Result::eType CommandRequestImpl::PrepareResultCodeForResponse( const ResponseInfo& first, const ResponseInfo& second) { + LOG4CXX_AUTO_TRACE(logger_); mobile_apis::Result::eType result_code = mobile_apis::Result::INVALID_ENUM; if (IsResultCodeUnsupported(first, second)) { result_code = mobile_apis::Result::UNSUPPORTED_RESOURCE; diff --git a/src/components/application_manager/src/commands/mobile/alert_request.cc b/src/components/application_manager/src/commands/mobile/alert_request.cc index 3288870b92..53515a810a 100644 --- a/src/components/application_manager/src/commands/mobile/alert_request.cc +++ b/src/components/application_manager/src/commands/mobile/alert_request.cc @@ -219,7 +219,7 @@ bool AlertRequest::PrepareResponseParameters( result = false; } result_code = mobile_apis::Result::WARNINGS; - if ((ui_alert_info.is_ok || ui_alert_info.is_invalid_enum) && + if ((ui_alert_info.is_ok || ui_alert_info.is_not_used) && tts_alert_info.is_unsupported_resource && HmiInterfaces::STATE_AVAILABLE == tts_alert_info.interface_state) { tts_response_info_ = "Unsupported phoneme type sent in a prompt"; @@ -230,6 +230,10 @@ bool AlertRequest::PrepareResponseParameters( result_code = PrepareResultCodeForResponse(ui_alert_info, tts_alert_info); info = MergeInfos( ui_alert_info, ui_response_info_, tts_alert_info, tts_response_info_); + // Mobile Alert request is successful when UI_Alert is successful + if (is_ui_alert_sent_ && !ui_alert_info.is_ok) { + return false; + } return result; } diff --git a/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc b/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc index 007440e8e6..b199e652a6 100644 --- a/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc +++ b/src/components/application_manager/src/commands/mobile/perform_interaction_request.cc @@ -143,11 +143,10 @@ void PerformInteractionRequest::Run() { } } - if (choice_set_id_list_length && - (!CheckChoiceIDFromRequest( - app, - choice_set_id_list_length, - msg_params[strings::interaction_choice_set_id_list]))) { + if (!CheckChoiceIDFromRequest( + app, + choice_set_id_list_length, + msg_params[strings::interaction_choice_set_id_list])) { LOG4CXX_ERROR(logger_, "PerformInteraction has choice sets with " "duplicated IDs or application does not have choice sets"); diff --git a/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc b/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc index ee0662e5c6..9a101e6352 100644 --- a/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc +++ b/src/components/application_manager/test/commands/mobile/alert_maneuver_request_test.cc @@ -59,6 +59,7 @@ namespace alert_maneuver_request { using ::testing::_; using ::testing::Return; using ::testing::ReturnRef; +using ::testing::Mock; namespace am = ::application_manager; using am::commands::AlertManeuverRequest; using am::commands::MessageSharedPtr; @@ -70,6 +71,9 @@ typedef SharedPtr<AlertManeuverRequest> CommandPtr; class AlertManeuverRequestTest : public CommandRequestTest<CommandsTestMocks::kIsNice> { public: + AlertManeuverRequestTest() + : mock_message_helper_(*MockMessageHelper::message_helper_mock()) {} + void CheckExpectations(const hmi_apis::Common_Result::eType hmi_response, const mobile_apis::Result::eType mobile_response, const am::HmiInterfaces::InterfaceState state, @@ -87,10 +91,10 @@ class AlertManeuverRequestTest MockAppPtr mock_app(CreateMockApp()); ON_CALL(app_mngr_, application(_)).WillByDefault(Return(mock_app)); - MockMessageHelper* mock_message_helper = - MockMessageHelper::message_helper_mock(); - EXPECT_CALL(*mock_message_helper, HMIToMobileResult(_)) - .WillOnce(Return(mobile_apis::Result::UNSUPPORTED_RESOURCE)); + if (hmi_apis::Common_Result::UNSUPPORTED_RESOURCE != hmi_response) { + EXPECT_CALL(mock_message_helper_, HMIToMobileResult(hmi_response)) + .WillOnce(Return(mobile_response)); + } EXPECT_CALL(mock_hmi_interfaces_, GetInterfaceState(_)) .WillRepeatedly(Return(state)); @@ -112,7 +116,16 @@ class AlertManeuverRequestTest static_cast<int32_t>(mobile_response)); } + void SetUp() OVERRIDE { + Mock::VerifyAndClearExpectations(&mock_message_helper_); + } + + void TearDown() OVERRIDE { + Mock::VerifyAndClearExpectations(&mock_message_helper_); + } + protected: + MockMessageHelper& mock_message_helper_; NiceMock<policy_test::MockPolicyHandlerInterface> policy_interface_; }; @@ -237,30 +250,30 @@ TEST_F(AlertManeuverRequestTest, OnEvent_ReceivedUnknownEvent_UNSUCCESS) { .asInt())); } -TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case1) { +TEST_F(AlertManeuverRequestTest, OnEvent_SUCCESS) { CheckExpectations(hmi_apis::Common_Result::SUCCESS, - mobile_apis::Result::UNSUPPORTED_RESOURCE, + mobile_apis::Result::SUCCESS, am::HmiInterfaces::STATE_AVAILABLE, true); } -TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case2) { - CheckExpectations(hmi_apis::Common_Result::SUCCESS, +TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE) { + CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE, mobile_apis::Result::UNSUPPORTED_RESOURCE, - am::HmiInterfaces::STATE_NOT_AVAILABLE, - true); + am::HmiInterfaces::STATE_AVAILABLE, + false); } -TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case3) { - CheckExpectations(hmi_apis::Common_Result::SUCCESS, - mobile_apis::Result::UNSUPPORTED_RESOURCE, +TEST_F(AlertManeuverRequestTest, OnEvent_WARNINGS) { + CheckExpectations(hmi_apis::Common_Result::WARNINGS, + mobile_apis::Result::WARNINGS, am::HmiInterfaces::STATE_NOT_RESPONSE, true); } -TEST_F(AlertManeuverRequestTest, OnEvent_UNSUPPORTED_RESOURCE_Case4) { +TEST_F(AlertManeuverRequestTest, OnEvent_GENERIC_ERROR) { CheckExpectations(hmi_apis::Common_Result::GENERIC_ERROR, - mobile_apis::Result::UNSUPPORTED_RESOURCE, + mobile_apis::Result::GENERIC_ERROR, am::HmiInterfaces::STATE_NOT_RESPONSE, false); } diff --git a/src/components/application_manager/test/commands/mobile/change_registration_test.cc b/src/components/application_manager/test/commands/mobile/change_registration_test.cc index 2fd43a6353..cbf8ce1abe 100644 --- a/src/components/application_manager/test/commands/mobile/change_registration_test.cc +++ b/src/components/application_manager/test/commands/mobile/change_registration_test.cc @@ -80,7 +80,6 @@ namespace strings = ::application_manager::strings; namespace hmi_response = ::application_manager::hmi_response; namespace { -const int32_t kCommandId = 1; const uint32_t kAppId = 1u; const uint32_t kCmdId = 1u; const uint32_t kConnectionKey = 2u; @@ -391,7 +390,7 @@ TEST_F(ChangeRegistrationRequestTest, CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE, mobile_apis::Result::UNSUPPORTED_RESOURCE, am::HmiInterfaces::STATE_NOT_RESPONSE, - false); + true); } TEST_F(ChangeRegistrationRequestTest, @@ -399,7 +398,7 @@ TEST_F(ChangeRegistrationRequestTest, CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE, mobile_apis::Result::UNSUPPORTED_RESOURCE, am::HmiInterfaces::STATE_AVAILABLE, - false); + true); } TEST_F(ChangeRegistrationRequestTest, @@ -407,7 +406,7 @@ TEST_F(ChangeRegistrationRequestTest, CheckExpectations(hmi_apis::Common_Result::UNSUPPORTED_RESOURCE, mobile_apis::Result::UNSUPPORTED_RESOURCE, am::HmiInterfaces::STATE_AVAILABLE, - false, + true, hmi_apis::Common_Result::SUCCESS, hmi_apis::Common_Result::SUCCESS); } diff --git a/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc b/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc index 3056d0e9fc..d91087acae 100644 --- a/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc +++ b/src/components/application_manager/test/commands/mobile/set_media_clock_timer_test.cc @@ -329,10 +329,11 @@ TEST_F(SetMediaClockRequestTest, OnEvent_Success) { MessageSharedPtr msg = CreateMessage(); (*msg)[am::strings::params][am::hmi_response::code] = mobile_apis::Result::SUCCESS; - (*msg)[am::strings::msg_params] = SmartObject(smart_objects::SmartType_Null); + (*msg)[am::strings::msg_params] = SmartObject(smart_objects::SmartType_Map); - SharedPtr<SetMediaClockRequest> command( - CreateCommand<SetMediaClockRequest>(msg)); + EXPECT_CALL(mock_message_helper_, + HMIToMobileResult(hmi_apis::Common_Result::SUCCESS)) + .WillOnce(Return(mobile_apis::Result::SUCCESS)); EXPECT_CALL(app_mngr_, ManageMobileCommand(_, _)); @@ -342,6 +343,8 @@ TEST_F(SetMediaClockRequestTest, OnEvent_Success) { Event event(hmi_apis::FunctionID::UI_SetMediaClockTimer); event.set_smart_object(*msg); + SharedPtr<SetMediaClockRequest> command( + CreateCommand<SetMediaClockRequest>(msg)); command->on_event(event); } |