Feature/mobile projection 2 by LuxoftAKutsan · Pull Request #2155 · smartdevicelink/sdl_core

LuxoftAKutsan · 2018-04-24T18:36:13Z

This PR is ready for review.

Risk

This PR makes major API changes.

Testing Plan

Tested with ATF : https://github.com/smartdevicelink/sdl_atf/tree/develop
ATF Scripts are "PASSED" from smartdevicelink/sdl_atf_test_scripts#1879.

Manual testing:
Used HMI from: smartdevicelink/sdl_hmi#53

Summary

Implementation of proposal https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0150-video-streaming-state.md

Changelog

Breaking Changes

Enhancements

Bug Fixes

Tasks Remaining:

CLA

I have signed the CLA

yang1070 · 2018-05-04T18:07:04Z

src/components/application_manager/include/application_manager/hmi_state.h

 * @brief The HmiState class
-*  Handle Hmi state of application (hmi level,
+*  Handle HMI state of application (HMI level,
 *  audio streaming state, system context)


video streaming state

yang1070 · 2018-05-04T18:38:37Z

src/components/application_manager/src/state_controller_impl.cc

-  return false;
+namespace {
+bool IsStateChanged(const HmiState& old_state, const HmiState& new_state) {
+  return std::make_tuple(old_state.hmi_level(),


what's the benefit of using std::make_tuple() != std::make_tuple() comparing to the (condition 1 || condition 2 || condition 3 || condition 4) here?
I thought it can only make final binary code bigger.

@yang1070 , this is common technik(since C++11) to compare complex objects, to make it easier to write a correct operators(==,!=,<,<=,>,>=) than rolling it yourself, to provide more readable code.
http://en.cppreference.com/w/cpp/utility/tuple/operator_cmp

…ed method

Add projection app to audio app list as it can stream audio This is done to enable HMI leve switching to LIMITED level.

… Phase 2'

There was added PROJECTION HMI type to the notifications_per_minute_by_priority structure. Also this structure was updated in the policy types to make everything work correctly.

Fix HMI state resolver according to requirments of 'Mobile Projection Phase 2'

yang1070 · 2018-05-18T16:29:15Z

src/components/application_manager/include/application_manager/hmi_state.h


-  DEPRECATED NaviStreamingHmiState(uint32_t app_id,
-                                   const ApplicationManager& app_mngr);
+  DEPRECATED VideoStreamingHmiState(uint32_t app_id,


DEPRECATED a new VideoStreamingHmiState?

@yang1070 , agree. done
see cf144fc

yang1070 · 2018-05-18T16:35:03Z

src/components/application_manager/src/hmi_state.cc

+    : HmiState(app, app_mngr, STATE_ID_VIDEO_STREAMING) {}

-DEPRECATED NaviStreamingHmiState::NaviStreamingHmiState(
+DEPRECATED VideoStreamingHmiState::VideoStreamingHmiState(


does not make sense to DEPRECATED a new function name. remove the function completely.

@yang1070 , agree. done
see cf144fc

yang1070 · 2018-05-21T16:03:07Z

src/components/application_manager/include/application_manager/state_controller_impl.h

      ApplicationSharedPtr app,
      const mobile_apis::HMILevel::eType hmi_level) const;

+  mobile_apis::VideoStreamingState::eType CalcVideoState(


missing description of the function?

yang1070

The code change looks good,

vkushnirenko-luxoft · 2018-05-22T12:45:42Z

Rebuild required

yang1070 · 2018-05-22T18:02:44Z

@theresalech Ford reviewed and approve this change

AStasiuk · 2018-05-24T15:28:22Z

@EKuliiev and @LuxoftAKutsan are working on fixing issue from Zhimin:

when 2nd app is in FULL, the 1st app shall be in LIMITED hmi level if either it is AUDIBLE or STREAMABLE or both.
If an app is in BACKGROUND, it shall be neither AUDIBLE, nor STREAMABLE.

But I have seen BACKGROUND app still is STREAMABLE or AUDIBLE during test.

yang1070 · 2018-05-29T14:09:52Z

@theresalech Ford has tested and reviewed this work. Ford approve this change and it is ready for Livio to review. Thanks,

Jack-Byrne · 2018-06-05T19:39:32Z

src/components/application_manager/doc/SDL.SDD.ApplicationManager.dox

@@ -0,0 +1,76 @@
+/**
+\page application_manager Application Manager Detailed Design
+## Table of contents


I think this information should be added to a different repository: https://github.com/smartdevicelink/sdl_core_documentation

@JackLivio sdl_documentation contains the high-level Software Architecture design.
This is rather a low-level detailed design, that actually generated by Doxygen from the code.
Such implementation details should not be in SAD (software architecture design), the should be in SDD.

Jack-Byrne · 2018-06-06T14:31:51Z

src/components/application_manager/src/hmi_state.cc

    , app_mngr_(app_mngr)
    , hmi_level_(mobile_apis::HMILevel::INVALID_ENUM)
    , audio_streaming_state_(mobile_apis::AudioStreamingState::INVALID_ENUM)
+    , video_streaming_state_(mobile_apis::VideoStreamingState::INVALID_ENUM)


This modifies a deprecated method. The next version of SDL Core is 5.0 a major version change. The DEPRECATED tag can be removed

@JackLivio , agree.
see 3cec396

Jack-Byrne · 2018-06-06T14:32:03Z

src/components/application_manager/src/hmi_state.cc

    , app_mngr_(app_mngr)
    , hmi_level_(mobile_apis::HMILevel::INVALID_ENUM)
    , audio_streaming_state_(mobile_apis::AudioStreamingState::INVALID_ENUM)
+    , video_streaming_state_(mobile_apis::VideoStreamingState::INVALID_ENUM)


This modifies a deprecated method. The next version of SDL Core is 5.0 a major version change. The DEPRECATED tag can be removed

@JackLivio , agree.
see 3cec396

EKuliiev force-pushed the feature/mobile_projection_2 branch 2 times, most recently from 4f7b4af to ea47b07 Compare April 25, 2018 10:59

AStasiuk requested review from mrapitis, robinmk and yang1070 April 27, 2018 08:07

yang1070 reviewed May 4, 2018

View reviewed changes

EKuliiev force-pushed the feature/mobile_projection_2 branch 4 times, most recently from 8276621 to 0a2489f Compare May 17, 2018 14:58

EKuliiev and others added 16 commits May 17, 2018 18:00

Remove unused method StateControllerImpl::SetupRegularHmiState

3f0dff2

Guarantee mutex release in StateControllerImpl::OnApplicationRegister…

859f140

…ed method

Add missed include in state_controller_impl.h

dcc8a72

Fix A/V streaming starting for PROJECTION apps

cc681f4

Fix video state for Deactivate/SafetyMode/AudioSource HMI states

af7276b

Treat projection app as audio app

9e76a1f

Add projection app to audio app list as it can stream audio This is done to enable HMI leve switching to LIMITED level.

Fix HMI state resolver according to requirments of 'Mobile Projection…

594be81

… Phase 2'

Update SDL preloaded PT with PROJECTION HMI type

186385f

There was added PROJECTION HMI type to the notifications_per_minute_by_priority structure. Also this structure was updated in the policy types to make everything work correctly.

Add State Controller documentation

9639167

Fix affected unit tests

859fc6c

Minor changes: extend logs, fix methods descriptions

1ce9e8a

Initial implementation

6b43ffc

Fix UTs according to updated logic of mobile projection 2

07ea6e5

Remove unused code

b9f4fc2

Disable failing state controller UTs

3e59189

Fix UTs according to new implementation

43bdbd3

EKuliiev force-pushed the feature/mobile_projection_2 branch from 0a2489f to 43bdbd3 Compare May 17, 2018 15:03

EKuliiev added 2 commits May 18, 2018 17:23

Change behavior for EmbeddedNavi event

aa68e6b

Merge pull request #2167 from EKuliiev/feature/mobile_projection_2

18ff422

Fix HMI state resolver according to requirments of 'Mobile Projection Phase 2'

yang1070 reviewed May 18, 2018

View reviewed changes

Review issues: remove deprecated VideoSourceHmiState constructor

cf144fc

yang1070 reviewed May 21, 2018

View reviewed changes

Review issues: add missing function descriptions

cc926af

yang1070 approved these changes May 22, 2018

View reviewed changes

yang1070 mentioned this pull request May 22, 2018

[SDL 0150] Enhancing onHMIStatus with a New Parameter for Video Streaming State #2129

Closed

theresalech requested a review from jacobkeeler May 22, 2018 19:35

Fix HMI conflict resolving for applied app in LIMITED state

e76fd11

theresalech removed the request for review from jacobkeeler May 24, 2018 15:32

EKuliiev added 2 commits May 25, 2018 19:15

Provide specific behavior for navigation streaming HMI state

6e9ce7f

Disable failed state_controller unit tests

6cc4375

EKuliiev force-pushed the feature/mobile_projection_2 branch from 37ed8a5 to 6cc4375 Compare May 25, 2018 16:16

theresalech requested a review from jacobkeeler May 29, 2018 15:38

AStasiuk requested review from askirk and removed request for askirk May 30, 2018 13:29

GetmanetsIrina mentioned this pull request Jun 1, 2018

Media Projection app is set to BACKGROUND in case AUDIO_SOURCE is active #2227

Closed

Jack-Byrne reviewed Jun 5, 2018

View reviewed changes

Jack-Byrne reviewed Jun 6, 2018

View reviewed changes

Jack-Byrne requested changes Jun 6, 2018

View reviewed changes

Review issues: Remove changes from DEPRECATED HmiState methods

3cec396

EKuliiev force-pushed the feature/mobile_projection_2 branch from bc04fef to 3cec396 Compare June 6, 2018 16:46

Jack-Byrne approved these changes Jun 13, 2018

View reviewed changes

Jack-Byrne merged commit aaadc90 into develop Jun 13, 2018

Jack-Byrne deleted the feature/mobile_projection_2 branch June 13, 2018 14:30

jacobkeeler mentioned this pull request Sep 11, 2018

Feature/mobile projection #2080

Closed

1 task

Conversation

LuxoftAKutsan commented Apr 24, 2018 • edited by AStasiuk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Risk

Testing Plan

Summary

Changelog

Breaking Changes

Enhancements

Bug Fixes

Tasks Remaining:

CLA

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yang1070 May 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yang1070 left a comment

Choose a reason for hiding this comment

Uh oh!

vkushnirenko-luxoft commented May 22, 2018

Uh oh!

yang1070 commented May 22, 2018

Uh oh!

AStasiuk commented May 24, 2018

Uh oh!

yang1070 commented May 29, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

LuxoftAKutsan commented Apr 24, 2018 •

edited by AStasiuk

Loading

yang1070 May 18, 2018 •

edited

Loading