Feature/service status update to hmi #2921

mked-luxoft · 2019-05-22T17:29:41Z

Implements #2791

This PR is [ready] for review.

Risk

This PR makes [minor] API changes.

Testing Plan

ATF tests are provided
Unit tests

Summary

ServiceStatusUpdateHandler and ServiceStatusUpdateHandlerListener entities were introduced to provide adequate level of abstraction when passing data between ProtocolHandler, SecurityManager and ApplicationManager
Added handling of negative cases in GetSystemTime, PTU timeout, and certificate decryption
Added check for maximum allowed PTU retries for PROPRIETARY flow

Other parts of delivery:

HMI integration guidelines: Feature/service status update to hmi sdl_hmi_integration_guidelines#154
SDL requirements: Feature/service status update to hmi sdl_requirements#123
ATF Scripts: Feature/service status update to hmi sdl_atf_test_scripts#2192
WebHMI: Feature/service status update to hmi sdl_hmi#184

CLA

I have signed the CLA

mked-luxoft · 2019-06-14T13:56:54Z

@yang1070 This PR was updated to comply with proposal updates. Please review.

yang1070

Ford has reviewed and approved this change

mked-luxoft · 2019-08-20T15:09:46Z

This PR was rebased on the latest develop branch and is ready for review

AStasiuk · 2019-08-20T15:12:46Z

@theresalech, this PR is already reviewed and approved by Ford (by @yang1070 ) and ready for Livio review.

...s/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/hmi/on_service_status_update_notification.h

src/appMain/sdl_preloaded_pt.json

src/components/application_manager/include/application_manager/message_helper.h

src/components/application_manager/include/application_manager/policies/policy_handler.h

...s/sdl_rpc_plugin/include/sdl_rpc_plugin/commands/hmi/on_service_status_update_notification.h

...manager/rpc_plugins/sdl_rpc_plugin/src/commands/hmi/on_service_status_update_notification.cc

src/components/application_manager/src/message_helper/message_helper.cc

src/components/protocol_handler/include/protocol_handler/service_status_update_handler.h

src/components/protocol_handler/test/service_status_update_handler_test.cc

src/components/application_manager/include/application_manager/message_helper.h

jacobkeeler · 2019-08-27T21:45:08Z

Some cases that seem to have been missed (I'll note that I'm using the EXTERNAL_PROPRIETARY flow):

If attempting to start a secured service for a type that is in ForceUnprotectedService, only OnServiceUpdate(REQUEST_RECEIVED) is sent to the HMI, whereas a OnServiceUpdate(REQUEST_REJECTED) should be sent as well.
The same issue occurs when a PTU retry sequence fails, no OnServiceUpdate(REQUEST_REJECTED) is sent

mked-luxoft · 2019-08-28T14:11:46Z

Some cases that seem to have been missed (I'll note that I'm using the EXTERNAL_PROPRIETARY flow):

1. If attempting to start a secured service for a type that is in `ForceUnprotectedService`, only `OnServiceUpdate(REQUEST_RECEIVED)` is sent to the HMI, whereas a `OnServiceUpdate(REQUEST_REJECTED)` should be sent as well.

2. The same issue occurs when a PTU retry sequence fails, no `OnServiceUpdate(REQUEST_REJECTED)` is sent

@jacobkeeler , secure services with ForceUnprotectedService flag are now rejected in 23c3e5f. Atf scripts were updated as well to cover these test cases in 18295

TinaKleczka · 2019-08-28T14:26:18Z

This is ready for re-review. @theresalech

jacobkeeler · 2019-08-28T14:29:12Z

@mked-luxoft Are you still investigating the second issue?

The same issue occurs when a PTU retry sequence fails, no OnServiceUpdate(REQUEST_REJECTED) is sent

mked-luxoft · 2019-08-28T14:37:21Z

@mked-luxoft Are you still investigating the second issue?

The same issue occurs when a PTU retry sequence fails, no OnServiceUpdate(REQUEST_REJECTED) is sent

@jacobkeeler if the PTU was triggered by start of protected service with ForceUnprotectedService flag, then with current implementation SDL will reject such service without starting a PTU sequence, so this issue is also supposed to be fixed in aforementioned commit

mked-luxoft · 2019-08-28T15:17:47Z

@mked-luxoft Are you still investigating the second issue?

The same issue occurs when a PTU retry sequence fails, no OnServiceUpdate(REQUEST_REJECTED) is sent

@jacobkeeler if the PTU was triggered by start of protected service with ForceUnprotectedService flag, then with current implementation SDL will reject such service without starting a PTU sequence, so this issue is also supposed to be fixed in aforementioned commit

After communication with @jacobkeeler it became apparent that mentioned issues are two separate test cases with different preconditions.
@jacobkeeler , could you please provide answers for the following questions:

do you see this issue every time?
do you have logs from when this issue happened?
what type of service were you trying to start?
is this service ForceProtected?
were all ptu retry iteration finished?
did you receive StartServiceNAck ?

jacobkeeler · 2019-08-28T15:32:02Z

do you see this issue every time?

Yes

do you have logs from when this issue happened?

SmartDeviceLinkCore.log

what type of service were you trying to start?

Video

is this service ForceProtected?

ForceProtectedService and ForceUnprotectedService are both Non

were all ptu retry iteration finished?

It appears so

did you receive StartServiceNAck ?

Based on the Core logs, it doesn't appear so

dboltovskyi · 2019-08-28T15:52:49Z

@jacobkeeler

In case you described SDL will do the following:

Try to obtain certificate through PTU
Wait until PTU retry sequence finished (unsuccessfully)
Try to start service in unprotected mode (since ForceProtection is OFF)
Send BC.OnServiceUpdate(REQUEST_ACCEPTED, PROTECTION_DISABLED) to HMI
Send StartServiceAck(encryption=false) to mobile App

Additional script has been committed recently to cover this case: smartdevicelink/sdl_atf_test_scripts@a9caceb
It's passed on a current commit of core
However we didn't observe these messages in log you provided and continue investigation

dboltovskyi · 2019-08-28T16:20:19Z

@jacobkeeler One more question:
Have you used smartdevicelink/sdl_hmi#184 for testing and have you checked External Policies option on Web-HMI?

dboltovskyi · 2019-08-28T17:42:36Z

@jacobkeeler We were able to reproduce the issue. It looks like it's related to HMI. Going to prepare a fix.

Added new result codes for ServiceStatusUpdateReasonStructure

mked-luxoft · 2019-08-29T17:18:08Z

branch was rebased on latest develop

mked-luxoft · 2019-08-29T17:20:17Z

@jacobkeeler We were able to reproduce the issue. It looks like it's related to HMI. Going to prepare a fix.

@jacobkeeler this issue fixed in web hmi in smartdevicelink/sdl_hmi@80d2477 .

jacobkeeler · 2019-08-29T17:23:32Z

@mked-luxoft fix style

mked-luxoft · 2019-08-29T17:34:57Z

@mked-luxoft fix style

@jacobkeeler fixed in 8d85b72

jacobkeeler

Approving, waiting on Travis build before merge

mked-luxoft changed the base branch from release/5.1.1 to develop May 22, 2019 17:52

AStasiuk requested review from Jack-Byrne and jacobkeeler May 23, 2019 07:08

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from c893ef9 to 81b4515 Compare May 27, 2019 05:48

SNiukalov force-pushed the feature/service_status_update_to_hmi branch from 81b4515 to f437759 Compare May 30, 2019 07:41

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from f437759 to 39ff0b9 Compare June 14, 2019 06:59

mked-luxoft requested a review from yang1070 June 14, 2019 13:57

yang1070 approved these changes Jun 14, 2019

View reviewed changes

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 39ff0b9 to 8f06818 Compare July 24, 2019 14:28

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 8f06818 to 1bb6c99 Compare August 13, 2019 11:44

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 1bb6c99 to 0412161 Compare August 20, 2019 15:07

jacobkeeler requested changes Aug 20, 2019

View reviewed changes

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from c68887a to 9d61a18 Compare August 23, 2019 14:21

jacobkeeler reviewed Aug 26, 2019

View reviewed changes

src/components/application_manager/include/application_manager/message_helper.h Outdated Show resolved Hide resolved

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from e3bdcf6 to 0f5af0e Compare August 27, 2019 14:10

dboltovskyi mentioned this pull request Aug 28, 2019

Feature/service status update to hmi smartdevicelink/sdl_atf_test_scripts#2192

Merged

1 task

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 23c3e5f to 50a3db7 Compare August 28, 2019 20:21

SNiukalov and others added 16 commits August 29, 2019 17:50

Add UT's for service status update to HMI feature

c7d7919

Added ServiceStatusUpdateDocumentation

f52a562

Added sending error on PTU retries exceed allowed count

f3ce96a

Fix OnServiceUpdate for unsecure force protected service

387839d

Fix UTs

28edbe3

Answer PR comments

e6aa35b

Fixes after rebase

237d8f3

Updates according to proposal changes

8e00925

Added new result codes for ServiceStatusUpdateReasonStructure

Fixes after rebase

cdd3b1f

fixup! Added OnServiceUpdate notification

064cd91

fixup! Added handling of service status update

4a63327

fixup! Added sending error on PTU retries exceed allowed count

3f86f38

Remove notification builder

2d53a14

fixup! Style

dddcb6b

fixup! Remove notification builder

81f3743

Fix force unprotected service

e0f9826

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 50a3db7 to 41f18fa Compare August 29, 2019 17:16

fixup! Fixes after rebase

8d85b72

mked-luxoft force-pushed the feature/service_status_update_to_hmi branch from 41f18fa to 8d85b72 Compare August 29, 2019 17:33

jacobkeeler approved these changes Aug 29, 2019

View reviewed changes

jacobkeeler merged commit e90b4aa into develop Aug 29, 2019

jacobkeeler mentioned this pull request Aug 29, 2019

[SDL 0211] Service Status Update to HMI #2791

Closed

AByzhynar deleted the feature/service_status_update_to_hmi branch February 27, 2020 08:56

jacobkeeler mentioned this pull request Apr 15, 2020

Fix SDLCore failed to retry updating the Policy Table #3277

Closed

1 task

This was referenced Nov 24, 2020

Replaces assert macro with logic in case of failed handshake #2604

Closed

Replaces assert macro with logic in case of failed handshake #1571

Closed

Feature/service status update to hmi #2921

Feature/service status update to hmi #2921

Uh oh!

Conversation

mked-luxoft commented May 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk

Testing Plan

Summary

Other parts of delivery:

CLA

Uh oh!

mked-luxoft commented Jun 14, 2019

Uh oh!

yang1070 left a comment

Choose a reason for hiding this comment

Uh oh!

mked-luxoft commented Aug 20, 2019

Uh oh!

AStasiuk commented Aug 20, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jacobkeeler commented Aug 27, 2019

Uh oh!

mked-luxoft commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TinaKleczka commented Aug 28, 2019

Uh oh!

jacobkeeler commented Aug 28, 2019

Uh oh!

mked-luxoft commented Aug 28, 2019

Uh oh!

mked-luxoft commented Aug 28, 2019

Uh oh!

jacobkeeler commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dboltovskyi commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dboltovskyi commented Aug 28, 2019

Uh oh!

dboltovskyi commented Aug 28, 2019

Uh oh!

mked-luxoft commented Aug 29, 2019

Uh oh!

mked-luxoft commented Aug 29, 2019

Uh oh!

jacobkeeler commented Aug 29, 2019

Uh oh!

mked-luxoft commented Aug 29, 2019

Uh oh!

jacobkeeler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mked-luxoft commented May 22, 2019 •

edited

Loading

mked-luxoft commented Aug 28, 2019 •

edited

Loading

jacobkeeler commented Aug 28, 2019 •

edited

Loading

dboltovskyi commented Aug 28, 2019 •

edited

Loading