Merge pull request #206 from LuxoftSDL/impl/sdl_0192_fix_resumption_o…

…f_button_subscriptions

Separate subscription processing logic for mobile requests and resumption

src/components/application_manager/include/application_manager/resumption/resumption_data_processor_impl.h

@@ -367,6 +367,17 @@ class ResumptionDataProcessorImpl
       const smart_objects::SmartObject& request,
       const smart_objects::SmartObject& response) const;
 
+  /**
+   * @brief Checks whether SubscribeButton response successful or not and
+   * subscribes application if successful
+   * @param app_id application id
+   * @param request reference to request SO
+   * @param response reference to response SO
+   */
+  void ProcessSubscribeButtonResponse(
+      const uint32_t app_id,
+      const smart_objects::SmartObject& request,
+      const smart_objects::SmartObject& response);
   app_mngr::ApplicationManager& application_manager_;
 
   /**

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/hmi/subscribe_button_request.h

@@ -75,17 +75,6 @@ class SubscribeButtonRequest : public app_mngr::commands::RequestToHMI,
  private:
   DISALLOW_COPY_AND_ASSIGN(SubscribeButtonRequest);
 
-  /**
-   * @brief Determines whether internal unsubscription must be performed
-   * and HMI UnsubscribeButton request should be sent
-   * @param hmi_result - result code received from HMI
-   * @param app - reference to application instance
-   * @return bool - true if app should unsubscribe internally
-   **/
-  bool ShouldUnsubscribeIntertally(
-      const hmi_apis::Common_Result::eType hmi_result,
-      const app_mngr::Application& app) const;
-
   hmi_apis::Common_ButtonName::eType button_name_;
 };
 

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/subscribe_button_request.cc

@@ -69,21 +69,18 @@ SubscribeButtonRequest::~SubscribeButtonRequest() {}
 void SubscribeButtonRequest::Run() {
   SDL_LOG_AUTO_TRACE();
 
-  subscribe_on_event(hmi_apis::FunctionID::Buttons_SubscribeButton,
-                     correlation_id());
-  SendRequest();
-}
-
-void SubscribeButtonRequest::onTimeOut() {
-  SDL_LOG_AUTO_TRACE();
-
-  application_manager_.AddExpiredButtonRequest(
-      application_id(), correlation_id(), button_name_);
+  const auto btn_id = static_cast<mobile_apis::ButtonName::eType>(
+      (*message_)[strings::msg_params][strings::button_name].asInt());
+
+  // Specific case when app subscribes to CUSTOM_BUTTON upon registration and no
+  // explicit mobile request exist when response arrives. In this case event
+  // should be catched by HMI request itself.
+  if (mobile_apis::ButtonName::CUSTOM_BUTTON == btn_id) {
+    subscribe_on_event(hmi_apis::FunctionID::Buttons_SubscribeButton,
+                       correlation_id());
+  }
 
-  auto& resume_ctrl = application_manager_.resume_controller();
-  resume_ctrl.HandleOnTimeOut(
-      correlation_id(),
-      static_cast<hmi_apis::FunctionID::eType>(function_id()));
+  SendRequest();
 }
 
 void SubscribeButtonRequest::on_event(const event_engine::Event& event) {
@@ -112,25 +109,26 @@ void SubscribeButtonRequest::on_event(const event_engine::Event& event) {
       static_cast<hmi_apis::Common_Result::eType>(
           message[strings::params][hmi_response::code].asInt());
 
-  const mobile_apis::ButtonName::eType btn_id =
-      static_cast<mobile_apis::ButtonName::eType>(
-          (*message_)[strings::msg_params][strings::button_name].asInt());
-
   if (CommandImpl::IsHMIResultSuccess(hmi_result,
                                       HmiInterfaces::HMI_INTERFACE_Buttons)) {
+    const mobile_apis::ButtonName::eType btn_id =
+        static_cast<mobile_apis::ButtonName::eType>(
+            (*message_)[strings::msg_params][strings::button_name].asInt());
+
     app->SubscribeToButton(static_cast<mobile_apis::ButtonName::eType>(btn_id));
-  } else if (ShouldUnsubscribeIntertally(hmi_result, *app)) {
-    app->UnsubscribeFromButton(
-        static_cast<mobile_apis::ButtonName::eType>(btn_id));
   }
 }
 
-bool SubscribeButtonRequest::ShouldUnsubscribeIntertally(
-    const hmi_apis::Common_Result::eType hmi_result,
-    const app_mngr::Application& app) const {
-  return (!CommandImpl::IsHMIResultSuccess(
-              hmi_result, HmiInterfaces::HMI_INTERFACE_Buttons) &&
-          app.is_resuming());
+void SubscribeButtonRequest::onTimeOut() {
+  SDL_LOG_AUTO_TRACE();
+
+  application_manager_.AddExpiredButtonRequest(
+      application_id(), correlation_id(), button_name_);
+
+  auto& resume_ctrl = application_manager_.resume_controller();
+  resume_ctrl.HandleOnTimeOut(
+      correlation_id(),
+      static_cast<hmi_apis::FunctionID::eType>(function_id()));
 }
 
 }  // namespace hmi

src/components/application_manager/rpc_plugins/sdl_rpc_plugin/src/commands/mobile/subscribe_button_request.cc

@@ -143,6 +143,11 @@ void SubscribeButtonRequest::on_event(const event_engine::Event& event) {
   const bool result = PrepareResultForMobileResponse(
       hmi_result, HmiInterfaces::HMI_INTERFACE_Buttons);
 
+  if (result) {
+    const auto btn_id = static_cast<mobile_apis::ButtonName::eType>(
+        (*message_)[str::msg_params][str::button_name].asInt());
+    app->SubscribeToButton(btn_id);
+  }
   mobile_apis::Result::eType result_code =
       MessageHelper::HMIToMobileResult(hmi_result);
 

src/components/application_manager/src/resumption/resumption_data_processor_impl.cc

@@ -162,6 +162,11 @@ void ResumptionDataProcessorImpl::ProcessResumptionStatus(
     CheckVehicleDataResponse(found_request.message, response, status);
   }
 
+  if (hmi_apis::FunctionID::Buttons_SubscribeButton ==
+      found_request.request_id.function_id) {
+    ProcessSubscribeButtonResponse(app_id, found_request.message, response);
+  }
+
   if (hmi_apis::FunctionID::UI_CreateWindow ==
       found_request.request_id.function_id) {
     CheckCreateWindowResponse(found_request.message, response);
@@ -857,19 +862,19 @@ void ResumptionDataProcessorImpl::AddButtonsSubscriptions(
   const smart_objects::SmartObject& subscriptions =
       saved_app[strings::application_subscriptions];
 
+  ButtonSubscriptions button_subscriptions;
   if (subscriptions.keyExists(strings::application_buttons)) {
     const smart_objects::SmartObject& subscriptions_buttons =
         subscriptions[strings::application_buttons];
     mobile_apis::ButtonName::eType btn;
     for (size_t i = 0; i < subscriptions_buttons.length(); ++i) {
       btn = static_cast<mobile_apis::ButtonName::eType>(
           (subscriptions_buttons[i]).asInt());
-      application->SubscribeToButton(btn);
+      if (mobile_apis::ButtonName::CUSTOM_BUTTON != btn) {
+        button_subscriptions.insert(btn);
+      }
     }
 
-    ButtonSubscriptions button_subscriptions =
-        GetButtonSubscriptionsToResume(application);
-
     ProcessMessagesToHMI(
         MessageHelper::CreateButtonSubscriptionsHandlingRequestsList(
             application,
@@ -879,19 +884,6 @@ void ResumptionDataProcessorImpl::AddButtonsSubscriptions(
   }
 }
 
-ButtonSubscriptions ResumptionDataProcessorImpl::GetButtonSubscriptionsToResume(
-    ApplicationSharedPtr application) const {
-  ButtonSubscriptions button_subscriptions =
-      application->SubscribedButtons().GetData();
-  auto it = button_subscriptions.find(mobile_apis::ButtonName::CUSTOM_BUTTON);
-
-  if (it != button_subscriptions.end()) {
-    button_subscriptions.erase(it);
-  }
-
-  return button_subscriptions;
-}
-
 void ResumptionDataProcessorImpl::AddPluginsSubscriptions(
     ApplicationSharedPtr application,
     const smart_objects::SmartObject& saved_app) {
@@ -1051,6 +1043,26 @@ void ResumptionDataProcessorImpl::CheckVehicleDataResponse(
   }
 }
 
+void ResumptionDataProcessorImpl::ProcessSubscribeButtonResponse(
+    const uint32_t app_id,
+    const smart_objects::SmartObject& request,
+    const smart_objects::SmartObject& response) {
+  SDL_LOG_AUTO_TRACE();
+  if (!IsResponseSuccessful(response)) {
+    return;
+  }
+
+  ApplicationSharedPtr app = application_manager_.application(app_id);
+  if (!app) {
+    SDL_LOG_ERROR("NULL pointer.");
+    return;
+  }
+  const mobile_apis::ButtonName::eType btn_id =
+      static_cast<mobile_apis::ButtonName::eType>(
+          request[strings::msg_params][strings::button_name].asInt());
+  app->SubscribeToButton(btn_id);
+}
+
 void ResumptionDataProcessorImpl::CheckModuleDataSubscription(
     const ns_smart_device_link::ns_smart_objects::SmartObject& request,
     const ns_smart_device_link::ns_smart_objects::SmartObject& response,

src/components/application_manager/test/resumption/resume_ctrl_test.cc

@@ -611,12 +611,6 @@ TEST_F(ResumeCtrlTest, StartResumption_AppWithSubscribeOnButtons) {
 
   EXPECT_CALL(*mock_app_, set_grammar_id(kTestGrammarId_));
 
-  for (uint32_t i = 0; i < count_of_buttons; ++i) {
-    EXPECT_CALL(
-        *mock_app_,
-        SubscribeToButton(static_cast<mobile_apis::ButtonName::eType>(i)));
-  }
-
   std::list<application_manager::AppExtensionPtr> extensions;
   extensions.insert(extensions.begin(), mock_app_extension_);