[SDL-0190] Handle response from HMI during resumption data #3475

ychernysheva · 2020-08-14T14:25:09Z

Fixes #2486

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

ATF Tests
Manual testing
Unit tests

Fixes:

Summary

Implemented logic of HMI erroneous responses handling during resumption process

CLA

I have signed the CLA

…ppExtension

Fix RAI default timeout As RAI request does not depend on any HMI response there is no need to track any timeout for it. RAI request will be removed from/ RequestController queue upon RAI response which will be sent anyway Update UT

Update SDL logic to avoid sending of unsubscribeVD to HMI during vehicle data resumption for case when another application is also have pending subscription for the same vehicle data.

Changed SDL logic to update subscription results for each pending subscription request before notifying resumption processor about current subscription is processed. This should be done beforehand because after raising event to processor it can finish resumption process and if it has failed then it can trigger sending of the next subscription request. So at that point, each subscription request should contain updated subscriptions status, otherwise some redundant requests might be sent.

LuxoftAKutsan · 2020-08-18T14:03:36Z

...nents/application_manager/include/application_manager/resumption/resumption_data_processor.h

+ * @brief The ResumptionRequestIDs struct contains fields, needed during
+ * processing events, related to responses from HMI to each resumption request
+ */
+struct ResumptionRequestIDs {


ResumptionRequestIDs -> ResumptionRequestID

Done in eb41075

LuxoftAKutsan · 2020-08-18T14:16:40Z

src/components/application_manager/src/resumption/resumption_data_processor.cc

+  return has_subscriptions_to_restore;
+}
+
+bool ResumptionRequestIDs::operator<(const ResumptionRequestIDs& other) const {


move it upper as util function

Done in eb41075

LuxoftAKutsan · 2020-08-18T14:22:07Z

src/components/application_manager/src/resumption/resumption_data_processor.cc

+                    << function_id << " correlation id: " << corr_id);
+
+  auto found_app_id = GetAppIdWaitingForResponse(function_id, corr_id);
+  if (!found_app_id) {


Add logging that response was not expected

Done in eb41075

SNiukalov · 2020-09-02T12:20:18Z

Please be informed, we have added two more additional commits (5360321 & cf5b42d), according to
comments: #3480 (comment) #3480 (comment) #3480 (comment)

UPD: cherry-picked after 'develop' merge commit one more time:
5360321 -> f35e573
cf5b42d - > 47832c9

…esumption_data_error_handling

...ts/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/sdl_app_extension.h

...ation_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_pending_resumption_handler.cc

iCollin · 2020-09-03T14:45:40Z

@AKalinich-Luxoft We would appreciate if developers did not force push on sdl_core repository to a pr that is in review. It makes it difficult to review a PR because we cannot tell what was changed since the last review. In general, please do not rebase, do not force push against sdl_core. Instead, you can merge the current develop branch into your PR branch. Thank you

iCollin · 2020-09-03T18:54:40Z

If we are going to include f35e573 in this PR, lets fix the structure of extension_subscriptions in this PR.

Should be subscriptions[vehicle_info] = Array<bool> where they are not at the root level. We should not be just differentiating the subscriptions by type. I am fine with all this code going in 0188 as you said previously, but there is no reason to include f35e573 in this PR if we go that route.

This reverts commit f35e573.

AKalinich-Luxoft · 2020-09-03T20:09:25Z

@AKalinich-Luxoft We would appreciate if developers did not force push on sdl_core repository to a pr that is in review. It makes it difficult to review a PR because we cannot tell what was changed since the last review. In general, please do not rebase, do not force push against sdl_core. Instead, you can merge the current develop branch into your PR branch. Thank you

@iCollin sorry for that. We will avoid force-pushes in a future during PR review

AKalinich-Luxoft · 2020-09-03T20:11:17Z

If we are going to include f35e573 in this PR, lets fix the structure of extension_subscriptions in this PR.

Should be subscriptions[vehicle_info] = Array<bool> where they are not at the root level. We should not be just differentiating the subscriptions by type. I am fine with all this code going in 0188 as you said previously, but there is no reason to include f35e573 in this PR if we go that route.

I think it is better to update entire structure and align it with the rest of plugins in 0188. In terms of this PR, we can keep this structure simple as it has the only place of usage.
Reverted in 4bb87a5
Should be addressed in #3480

src/components/application_manager/include/application_manager/display_capabilities_builder.h

src/components/application_manager/src/resumption/resumption_data_processor.cc

...rpc_plugins/app_service_rpc_plugin/test/app_service_plugin/app_service_app_extension_test.cc

...n_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/sdl_pending_resumption_handler.h

...ponents/application_manager/rpc_plugins/sdl_rpc_plugin/src/sdl_pending_resumption_handler.cc

…fication

…mCapabilitiesAppExtension

…nction

ychernysheva added 23 commits August 14, 2020 12:47

Add changes to RAI request

b43e92a

Add new class ResumptionDataProcessor

3ea77e8

Add new class SDLAppExtension

3f86f12

Add new class ExtensionPendingResumptionHandler

fa6605e

Add new class SDLPendingResumptionHandler

1739dca

New class VehicleInfoPendingResumptionHandler

eb91325

Add changes related to ApplicationManager class

976aa03

Add changes related to MessageHelper class

f259f86

Add changes related to StateController class

da854cf

Add changes related to ResumeController class

7b92692

Add changes related to PolicyHandler class

7edcf47

Add changes related to SDLRPCPlugin

0637a6f

Add changes related to VehicleInfoPlugin

022e50d

Add changes related to AppServiceAppExtension and SystemCapabilitiesA…

5e210b5

…ppExtension

Add changes related to ResetGlobalProperties request

e9096e5

Add changes related to VehicleInfoAppExtension

149b36e

Add changes related to AppExtension and RCAppExtension

ae12891

Change OnTimeOut() method in affected requests

d80637c

Add changes related to method IsAppSubscribedForWayPoints

4ccecdf

Fix problem with multiple definitions during building of unit tests

bb7f379

Add changes related to ApplicationHelper class

ed2ad53

Add changes related to EventDispatcher

3d81e0d

Add changes related to OnSystemCapabilitiesUpdated notification

7a433a3

ychernysheva mentioned this pull request Aug 14, 2020

Feature/resumption data error handling #2619

Closed

2 tasks

AKalinich-Luxoft added 3 commits August 14, 2020 15:15

Fix/fix rai default timeout (#108)

746f855

Fix RAI default timeout As RAI request does not depend on any HMI response there is no need to track any timeout for it. RAI request will be removed from/ RequestController queue upon RAI response which will be sent anyway Update UT

Do not unsubscribe from pending VD

c9418c8

Update SDL logic to avoid sending of unsubscribeVD to HMI during vehicle data resumption for case when another application is also have pending subscription for the same vehicle data.

LuxoftAKutsan reviewed Aug 18, 2020

View reviewed changes

AKalinich-Luxoft and others added 3 commits September 2, 2020 12:32

Merge remote-tracking branch 'origin/develop' into feature/sdl_0190_r…

a4215f5

…esumption_data_error_handling

Filter RC subscriptions in vehicle data plugin

f35e573

Timeout process for reset global properties

47832c9

AKalinich-Luxoft force-pushed the feature/sdl_0190_resumption_data_error_handling branch from cf5b42d to 47832c9 Compare September 2, 2020 22:07

iCollin reviewed Sep 3, 2020

View reviewed changes

...ts/application_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/sdl_app_extension.h Outdated Show resolved Hide resolved

...ation_manager/rpc_plugins/vehicle_info_plugin/src/vehicle_info_pending_resumption_handler.cc Outdated Show resolved Hide resolved

LuxoftAKutsan and others added 4 commits September 3, 2020 15:50

Use ResumptionDataProcessor interface instead of subscriber function

e601b42

Rename ExtensionPendingResumptionHandler

28da143

Rename SDLAppExtension

7fa98ac

Revert "Filter RC subscriptions in vehicle data plugin"

4bb87a5

This reverts commit f35e573.

iCollin reviewed Sep 3, 2020

View reviewed changes

...n_manager/rpc_plugins/sdl_rpc_plugin/include/sdl_rpc_plugin/sdl_pending_resumption_handler.h Outdated Show resolved Hide resolved

iCollin reviewed Sep 3, 2020

View reviewed changes

AKalinich-Luxoft added 5 commits September 3, 2020 18:57

Rename resumption_data_processor.cc

96d0396

Rename SDLPendingResumptionHandler

8913095

fixup! fixup! Add changes related to OnSystemCapabilitiesUpdated noti…

dcf3dc5

…fication

fixup! fixup! Add changes related to AppServiceAppExtension and Syste…

8db46ca

…mCapabilitiesAppExtension

fixup! Use ResumptionDataProcessor interface instead of subscriber fu…

1bda351

…nction

iCollin approved these changes Sep 4, 2020

View reviewed changes

iCollin merged commit b683e7c into smartdevicelink:develop Sep 4, 2020

This was referenced Sep 4, 2020

SDL does not send SubscribeWayPoints request to HMI in case of resumption #2460

Closed

Fix problem with subscription on way points during resumption #3450

Closed

[SDL 0190] Handle response from HMI during resumption data #2486

Closed

LuxoftAKutsan deleted the feature/sdl_0190_resumption_data_error_handling branch October 5, 2020 08:06

This was referenced Nov 24, 2020

Fix/Core dump during usb reconnection #3348

Closed

Core dump during the USB device reconnection #3345

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDL-0190] Handle response from HMI during resumption data #3475

[SDL-0190] Handle response from HMI during resumption data #3475

ychernysheva commented Aug 14, 2020 •

edited by LuxoftAKutsan

Loading

LuxoftAKutsan Aug 18, 2020

ychernysheva Aug 19, 2020

LuxoftAKutsan Aug 18, 2020

ychernysheva Aug 19, 2020

LuxoftAKutsan Aug 18, 2020

ychernysheva Aug 19, 2020

SNiukalov commented Sep 2, 2020 •

edited by AKalinich-Luxoft

Loading

iCollin commented Sep 3, 2020

iCollin commented Sep 3, 2020 •

edited

Loading

AKalinich-Luxoft commented Sep 3, 2020

AKalinich-Luxoft commented Sep 3, 2020

[SDL-0190] Handle response from HMI during resumption data #3475

[SDL-0190] Handle response from HMI during resumption data #3475

Conversation

ychernysheva commented Aug 14, 2020 • edited by LuxoftAKutsan Loading

Risk

Testing Plan

Fixes:

Summary

CLA

LuxoftAKutsan Aug 18, 2020

Choose a reason for hiding this comment

ychernysheva Aug 19, 2020

Choose a reason for hiding this comment

LuxoftAKutsan Aug 18, 2020

Choose a reason for hiding this comment

ychernysheva Aug 19, 2020

Choose a reason for hiding this comment

LuxoftAKutsan Aug 18, 2020

Choose a reason for hiding this comment

ychernysheva Aug 19, 2020

Choose a reason for hiding this comment

SNiukalov commented Sep 2, 2020 • edited by AKalinich-Luxoft Loading

iCollin commented Sep 3, 2020

iCollin commented Sep 3, 2020 • edited Loading

AKalinich-Luxoft commented Sep 3, 2020

AKalinich-Luxoft commented Sep 3, 2020

ychernysheva commented Aug 14, 2020 •

edited by LuxoftAKutsan

Loading

SNiukalov commented Sep 2, 2020 •

edited by AKalinich-Luxoft

Loading

iCollin commented Sep 3, 2020 •

edited

Loading