Merge pull request #198 from LuxoftSDL/DeleteCommand_WARNING

Fix WARNING result interpretation in Mobile.DeleteCommandRequest

5 files changed, 38 insertions, 37 deletions

diff --git a/src/components/application_manager/src/commands/mobile/delete_command_request.cc b/src/components/application_manager/src/commands/mobile/delete_command_request.cc
index a0efb89c05..58fd06532a 100644
--- a/src/components/application_manager/src/commands/mobile/delete_command_request.cc
+++ b/src/components/application_manager/src/commands/mobile/delete_command_request.cc
@@ -62,8 +62,8 @@ void DeleteCommandRequest::Run() {
       application(connection_key());
 
   if (!application) {
-    SendResponse(false, mobile_apis::Result::APPLICATION_NOT_REGISTERED);
     LOG4CXX_ERROR(logger_, "Application is not registered");
+    SendResponse(false, mobile_apis::Result::APPLICATION_NOT_REGISTERED);
     return;
   }
 
@@ -73,8 +73,8 @@ void DeleteCommandRequest::Run() {
   smart_objects::SmartObject* command = application->FindCommand(cmd_id);
 
   if (!command) {
-    SendResponse(false, mobile_apis::Result::INVALID_ID);
     LOG4CXX_ERROR(logger_, "Command with id " << cmd_id << " is not found.");
+    SendResponse(false, mobile_apis::Result::INVALID_ID);
     return;
   }
 
@@ -119,19 +119,19 @@ void DeleteCommandRequest::on_event(const event_engine::Event& event) {
 
   switch (event.id()) {
     case hmi_apis::FunctionID::UI_DeleteCommand: {
-      LOG4CXX_INFO(logger_, "Received UI_DeleteCommand event");
       is_ui_received_ = true;
-      ui_result_ = static_cast<hmi_apis::Common_Result::eType>(
-          message[strings::params][hmi_response::code].asInt());
-
+      const int result = message[strings::params][hmi_response::code].asInt();
+      ui_result_ = static_cast<hmi_apis::Common_Result::eType>(result);
+      LOG4CXX_DEBUG(logger_, "Received UI_DeleteCommand event with result "
+                    << MessageHelper::HMIResultToString(ui_result_));
       break;
     }
     case hmi_apis::FunctionID::VR_DeleteCommand: {
-      LOG4CXX_INFO(logger_, "Received VR_DeleteCommand event");
       is_vr_received_ = true;
-      vr_result_ = static_cast<hmi_apis::Common_Result::eType>(
-          message[strings::params][hmi_response::code].asInt());
-
+      const int result = message[strings::params][hmi_response::code].asInt();
+      vr_result_ = static_cast<hmi_apis::Common_Result::eType>(result);
+      LOG4CXX_DEBUG(logger_, "Received VR_DeleteCommand event with result "
+                    << MessageHelper::HMIResultToString(vr_result_));
       break;
     }
     default: {
@@ -149,12 +149,12 @@ void DeleteCommandRequest::on_event(const event_engine::Event& event) {
       ApplicationManagerImpl::instance()->application(connection_key());
 
   if (!application) {
-    LOG4CXX_ERROR(logger_, "NULL pointer");
+    LOG4CXX_ERROR(logger_, "Application is not registered");
     return;
   }
+  smart_objects::SmartObject& msg_params = (*message_)[strings::msg_params];
 
-  const int32_t cmd_id =
-      (*message_)[strings::msg_params][strings::cmd_id].asInt();
+  const int32_t cmd_id = msg_params[strings::cmd_id].asInt();
 
   smart_objects::SmartObject* command = application->FindCommand(cmd_id);
 
@@ -164,9 +164,6 @@ void DeleteCommandRequest::on_event(const event_engine::Event& event) {
     return;
   }
 
-  mobile_apis::Result::eType result_code =
-      mobile_apis::Result::INVALID_ENUM;
-
   const bool is_vr_success_invalid =
       Compare<hmi_apis::Common_Result::eType, EQ, ONE>(
         vr_result_,
@@ -185,33 +182,37 @@ void DeleteCommandRequest::on_event(const event_engine::Event& event) {
         vr_result_,
         ui_result_);
 
-  bool result =
-      is_ui_success_invalid &&
-      is_vr_success_invalid &&
-      !is_vr_ui_invalid;
-
-  if (result) {
-    application->RemoveCommand(
-      (*message_)[strings::msg_params][strings::cmd_id].asInt());
-  }
-
   const bool is_vr_or_ui_warning =
       Compare<hmi_apis::Common_Result::eType, EQ, ONE>(
         hmi_apis::Common_Result::WARNINGS,
         ui_result_,
         vr_result_);
 
+  const bool result =
+      // In case of UI/VR is SUCCESS and other is SUCCESS/INVALID_ENUM
+      (is_vr_success_invalid && is_ui_success_invalid && !is_vr_ui_invalid) ||
+      // or one of them is WARNINGS
+      is_vr_or_ui_warning;
+
+  LOG4CXX_DEBUG(logger_, "Result code is " << (result ? "true" : "false"));
+
+  if (result) {
+    application->RemoveCommand(msg_params[strings::cmd_id].asInt());
+  }
+
+  mobile_apis::Result::eType result_code = mobile_apis::Result::INVALID_ENUM;
   if (!result &&
       hmi_apis::Common_Result::REJECTED == ui_result_) {
     result_code = MessageHelper::HMIToMobileResult(vr_result_);
   } else if (is_vr_or_ui_warning) {
+    LOG4CXX_DEBUG(logger_, "VR or UI result is warning");
     result_code = mobile_apis::Result::WARNINGS;
   } else {
     result_code = MessageHelper::HMIToMobileResult(
           std::max(ui_result_, vr_result_));
   }
 
-  SendResponse(result, result_code, NULL, &(message[strings::msg_params]));
+  SendResponse(result, result_code, NULL, &msg_params);
   if (result) {
     application->UpdateHash();
   }
diff --git a/src/components/include/protocol/common.h b/src/components/include/protocol/common.h
index 45d3a9e569..73c3935537 100644
--- a/src/components/include/protocol/common.h
+++ b/src/components/include/protocol/common.h
@@ -198,7 +198,7 @@ enum {
  *\brief If FRAME_TYPE_CONTROL: Constant: Maximum size of one frame excluding
  *\brief frame header (used Ethernet MTU as default target transport)
  */
-const uint32_t MAXIMUM_FRAME_DATA_SIZE = 1500;
+const uint32_t MAXIMUM_FRAME_DATA_V2_SIZE = 1500;
 
 /**
  *\brief If FRAME_TYPE_CONSECUTIVE: Constant: Size of first frame in
diff --git a/src/components/protocol_handler/src/protocol_handler_impl.cc b/src/components/protocol_handler/src/protocol_handler_impl.cc
index 3e31e928b2..9ae8128f66 100644
--- a/src/components/protocol_handler/src/protocol_handler_impl.cc
+++ b/src/components/protocol_handler/src/protocol_handler_impl.cc
@@ -361,7 +361,7 @@ void ProtocolHandlerImpl::SendMessageToMobileApp(const RawMessagePtr message,
 
   const uint32_t header_size = (PROTOCOL_VERSION_1 == message->protocol_version())
       ? PROTOCOL_HEADER_V1_SIZE : PROTOCOL_HEADER_V2_SIZE;
-  uint32_t max_frame_size = MAXIMUM_FRAME_DATA_SIZE - header_size;
+  uint32_t max_frame_size = MAXIMUM_FRAME_DATA_V2_SIZE - header_size;
 #ifdef ENABLE_SECURITY
   const security_manager::SSLContext *ssl_context = session_observer_->
       GetSSLContext(message->connection_key(), message->service_type());
@@ -377,7 +377,7 @@ void ProtocolHandlerImpl::SendMessageToMobileApp(const RawMessagePtr message,
   }
   LOG4CXX_DEBUG(logger_, "Optimal packet size is " << max_frame_size);
 #endif  // ENABLE_SECURITY
-  DCHECK(MAXIMUM_FRAME_DATA_SIZE > max_frame_size);
+  DCHECK(MAXIMUM_FRAME_DATA_V2_SIZE > max_frame_size);
 
 
   if (message->data_size() <= max_frame_size) {
diff --git a/src/components/protocol_handler/test/incoming_data_handler_test.cc b/src/components/protocol_handler/test/incoming_data_handler_test.cc
index feb81300f8..d000ea68e6 100644
--- a/src/components/protocol_handler/test/incoming_data_handler_test.cc
+++ b/src/components/protocol_handler/test/incoming_data_handler_test.cc
@@ -59,7 +59,7 @@ class IncomingDataHandlerTest : public ::testing::Test {
     protov1_message_id = 0x0;
     some_message_id = 0xABCDEF0;
     some_session_id = 0xFEDCBA0;
-    payload_bigger_mtu.resize(MAXIMUM_FRAME_DATA_SIZE + 1);
+    payload_bigger_mtu.resize(MAXIMUM_FRAME_DATA_V2_SIZE + 1);
   }
   void TearDown() OVERRIDE {
     delete[] some_data;
diff --git a/src/components/protocol_handler/test/protocol_header_validator_test.cc b/src/components/protocol_handler/test/protocol_header_validator_test.cc
index 0ae7910145..45c65e5b0f 100644
--- a/src/components/protocol_handler/test/protocol_header_validator_test.cc
+++ b/src/components/protocol_handler/test/protocol_header_validator_test.cc
@@ -57,7 +57,7 @@ class ProtocolHeaderValidatorTest : public ::testing::Test {
 TEST_F(ProtocolHeaderValidatorTest, MaxPayloadSizeSetGet) {
   EXPECT_EQ(std::numeric_limits<size_t>::max(),
             header_validator.max_payload_size());
-  for (size_t value = 0; value < MAXIMUM_FRAME_DATA_SIZE * 2; ++value) {
+  for (size_t value = 0; value < MAXIMUM_FRAME_DATA_V2_SIZE * 2; ++value) {
     header_validator.set_max_payload_size(value);
     EXPECT_EQ(value, header_validator.max_payload_size());
   }
@@ -180,9 +180,9 @@ TEST_F(ProtocolHeaderValidatorTest, Malformed_ControlFrame_EmptyPayload) {
         PROTOCOL_VERSION_3, PROTECTION_ON, FRAME_TYPE_CONSECUTIVE, kControl,
         FRAME_DATA_LAST_CONSECUTIVE, some_session_id, payload_size, some_message_id);
 
-  for (uint32_t max_payload_size = 0; max_payload_size < MAXIMUM_FRAME_DATA_SIZE * 2;
+  for (uint32_t max_payload_size = 0; max_payload_size < MAXIMUM_FRAME_DATA_V2_SIZE * 2;
        ++max_payload_size) {
-    header_validator.set_max_payload_size(MAXIMUM_FRAME_DATA_SIZE + max_payload_size);
+    header_validator.set_max_payload_size(MAXIMUM_FRAME_DATA_V2_SIZE + max_payload_size);
 
     // For Control frames Data Size value could be zero
     EXPECT_EQ(RESULT_OK, header_validator.validate(control_message_header));
@@ -194,7 +194,7 @@ TEST_F(ProtocolHeaderValidatorTest, Malformed_ControlFrame_EmptyPayload) {
 
 // For Control frames Data Size value shall be less than MTU header
 TEST_F(ProtocolHeaderValidatorTest, Malformed_Payload) {
-  const size_t payload_size = MAXIMUM_FRAME_DATA_SIZE;
+  const size_t payload_size = MAXIMUM_FRAME_DATA_V2_SIZE;
   const ProtocolPacket::ProtocolHeader control_message_header(
         PROTOCOL_VERSION_3, PROTECTION_ON, FRAME_TYPE_CONTROL, kControl,
         FRAME_DATA_HEART_BEAT, some_session_id, payload_size, some_message_id);
@@ -205,7 +205,7 @@ TEST_F(ProtocolHeaderValidatorTest, Malformed_Payload) {
         PROTOCOL_VERSION_3, PROTECTION_ON, FRAME_TYPE_CONSECUTIVE, kControl,
         FRAME_DATA_LAST_CONSECUTIVE, some_session_id, payload_size, some_message_id);
 
-  for (uint32_t max_payload_size = 0; max_payload_size < MAXIMUM_FRAME_DATA_SIZE;
+  for (uint32_t max_payload_size = 0; max_payload_size < MAXIMUM_FRAME_DATA_V2_SIZE;
        ++max_payload_size) {
     header_validator.set_max_payload_size(max_payload_size);
     EXPECT_EQ(RESULT_FAIL, header_validator.validate(control_message_header));
@@ -213,7 +213,7 @@ TEST_F(ProtocolHeaderValidatorTest, Malformed_Payload) {
     EXPECT_EQ(RESULT_FAIL, header_validator.validate(consecutive_message_header));
   }
 
-  for (uint32_t max_payload_size = MAXIMUM_FRAME_DATA_SIZE + 1; max_payload_size < MAXIMUM_FRAME_DATA_SIZE * 2;
+  for (uint32_t max_payload_size = MAXIMUM_FRAME_DATA_V2_SIZE + 1; max_payload_size < MAXIMUM_FRAME_DATA_V2_SIZE * 2;
        ++max_payload_size) {
     header_validator.set_max_payload_size(max_payload_size);
     EXPECT_EQ(RESULT_OK, header_validator.validate(control_message_header));