Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/Remote Control - Allow Multiple Modules per Module Type #216

Merged
merged 13 commits into from
Sep 6, 2019
Merged

Feature/Remote Control - Allow Multiple Modules per Module Type #216

merged 13 commits into from
Sep 6, 2019

Conversation

LitvinenkoIra
Copy link
Contributor

@LitvinenkoIra LitvinenkoIra commented Aug 5, 2019 •
edited by IGapchuk
Loading

Implements: #198
SDL core issue : #2919

This PR is [ready] for review.

Testing Plan

Tested manually
Multiple_Modules_Manual_checks_v1.0.xlsx

Summary

Multiple modules feature implementation

CLA

@LitvinenkoIra LitvinenkoIra mentioned this pull request Aug 5, 2019
Merged
1 task
@AStasiuk AStasiuk requested a review from mrapitis August 6, 2019 06:52
@AKalinich-Luxoft AKalinich-Luxoft force-pushed the feature/multiple_modules branch from be5c9d5 to 129bd90 Compare August 19, 2019 15:49
@mrapitis
Copy link

@theresalech Ford has approved of these changes.

@theresalech theresalech requested a review from ShobhitAd August 19, 2019 18:41
@AKalinich-Luxoft AKalinich-Luxoft force-pushed the feature/multiple_modules branch from 129bd90 to f4e8b88 Compare August 26, 2019 21:04
@AKalinich-Luxoft
Copy link
Contributor

AKalinich-Luxoft commented Aug 26, 2019 •
edited
Loading

Feature was rebased on the latest develop in f4e8b88

ShobhitAd
ShobhitAd requested changes Aug 29, 2019
View reviewed changes
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/sdl/RController.js Show resolved Hide resolved
app/model/sdl/Abstract/data.js Show resolved Hide resolved
@AKalinich-Luxoft AKalinich-Luxoft force-pushed the feature/multiple_modules branch from 30ec0ee to 4ebf043 Compare August 29, 2019 19:35
@AKalinich-Luxoft
Copy link
Contributor

@ShobhitAd conflicts has been resolved. Rebased on the latest develop branch in 4ebf043

@AKalinich-Luxoft AKalinich-Luxoft force-pushed the feature/multiple_modules branch from febb809 to 7a83fa1 Compare August 30, 2019 15:27
@ShobhitAd
Copy link
Contributor

ShobhitAd commented Aug 30, 2019 •
edited
Loading

Found another issue with the appList not clearing when SDL shuts down. Here is the error in the console

Uncaught TypeError: Cannot read property 'appName' of undefined
    at d.<anonymous> (widgetPhoneView.js:66)
    at m.n.get (ember-1.0.pre.min.js:14)
    at b (ember-1.0.pre.min.js:14)
    at e (ember-1.0.pre.min.js:14)
    at f._sync (ember-1.0.pre.min.js:14)
    at c (ember-1.0.pre.min.js:14)
    at j (ember-1.0.pre.min.js:14)
    at Array.forEach (<anonymous>)
    at e.flush (ember-1.0.pre.min.js:14)
    at end (ember-1.0.pre.min.js:14)

@IGapchuk
Copy link
Contributor

@ShobhitAd We tried to reproduce this issue on SDL Core (smartdevicelink/sdl_core@1312f9b) and HMI (7a83fa1) didn't' find any issues. Please take a look at the attached SDL Core and HMI logs.
Could you please provide us with your SD LCore or HMI logs or describe reproducing steps?
SDLCore_HMI.zip

@AKalinich-Luxoft
Copy link
Contributor

AKalinich-Luxoft commented Aug 30, 2019 •
edited
Loading

Found another issue with the appList not clearing when SDL shuts down. Here is the error in the console

Uncaught TypeError: Cannot read property 'appName' of undefined
   at d.<anonymous> (widgetPhoneView.js:66)
   at m.n.get (ember-1.0.pre.min.js:14)
   at b (ember-1.0.pre.min.js:14)
   at e (ember-1.0.pre.min.js:14)
   at f._sync (ember-1.0.pre.min.js:14)
   at c (ember-1.0.pre.min.js:14)
   at j (ember-1.0.pre.min.js:14)
   at Array.forEach (<anonymous>)
   at e.flush (ember-1.0.pre.min.js:14)
   at end (ember-1.0.pre.min.js:14)

@ShobhitAd seems like this is an issue from develop branch and related to rcStatusLabel which is displaying allocated and free resources for each RC application. Sometimes due to timings, HMI tries to print out the application which was unregistered/unexpectedly disconected.
I added a check to prevent this exception in b296a66
Note, that it's not related to the feature itself

@AKalinich-Luxoft AKalinich-Luxoft force-pushed the feature/multiple_modules branch from b296a66 to 91253f4 Compare August 30, 2019 18:30
@AKalinich-Luxoft
Copy link
Contributor

@ShobhitAd HMI has been rebased on the latest develop and conflicts has been resolved.

@AKalinich-Luxoft AKalinich-Luxoft requested review from AKalinich-Luxoft and removed request for AKalinich-Luxoft August 30, 2019 19:02
valerii and others added 7 commits September 3, 2019 19:00
@AKalinich-Luxoft
Implementation of feature allow multiple modules per module type
d154766 
The Changes are:

- Were added two emulation type of vehicle:
	- 2x2
	- 2x3
- Were added views to choose emulation type and configure coverage:
	- VehicleEmulationView.js
	- VehicleModuleCoverageView.js

- AudioModel object was created to agregate all media models:
	- CDModel
	- USBModel
	- BlueToothModel
	- LineInModel
	- IPodModel
	- RadioModel
- RCModulesController object was created to aggregate multiple instances of
	all RC components:
	- AudioModel
	- ClimateModel
	- SeatModel
	- RadioModel
	- HMISettingsModel
	- LightModel
- All data and RPC related to RC processed by RCModulesController:
	- Set/Get OnInteriorVehicleData
	- GetCapabilities
	- OnInteriorVehicleData
@AKalinich-Luxoft
Extend ability to change moduleInfo for every RC Modules
d1765f9
@AKalinich-Luxoft
Answer for review comments
cfdbb11 
- Renamed methods createDriverPassengerCoverage
	and getModuleInfoForType correctly.
- Added comment for driver coverage generating.
- Added missed seatLocationCapability in RC.GetCapabilities and
OnSystemCapabilityUpdated notification.
@AKalinich-Luxoft
Fix exception on media app unregistration
54ca9a6
@AKalinich-Luxoft
Populate models on HMI start
8f1cd5e 
It's quite late to populate models after clicking OK as user is able to
change the coverage settings till that moment.
@AKalinich-Luxoft
Prevent UUID update before validation
524e884 
Module info and UUID value from edited coverage
settings should be applied only after validation.
There is no sense to update modules with invalid
data as behavior will be undefined.
@AKalinich-Luxoft
Fix exception in RC status label
cab15b1
@AKalinich-Luxoft @ShobhitAd
Make consents in the reverse order
10bedb1 
Co-Authored-By: Shobhit Adlakha <ShobhitAd@users.noreply.github.com>
@AKalinich-Luxoft
Copy link
Contributor

Rebased on the latest develop branch

ShobhitAd
ShobhitAd requested changes Sep 4, 2019
View reviewed changes
app/controller/VehicleModuleCoverageController.js Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Show resolved Hide resolved
@ghost ghost force-pushed the feature/multiple_modules branch from aff34f1 to d48bcb9 Compare September 5, 2019 13:15
ShobhitAd
ShobhitAd reviewed Sep 5, 2019
View reviewed changes
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/VehicleModuleCoverageController.js Outdated Show resolved Hide resolved
ShobhitAd
ShobhitAd requested changes Sep 6, 2019
View reviewed changes
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
app/controller/sdl/RCModulesController.js Outdated Show resolved Hide resolved
@ShobhitAd ShobhitAd merged commit 4a7e6eb into develop Sep 6, 2019
@ShobhitAd ShobhitAd deleted the feature/multiple_modules branch September 6, 2019 19:36
@ShobhitAd ShobhitAd mentioned this pull request Sep 6, 2019
Closed
@mked-luxoft mked-luxoft mentioned this pull request Sep 12, 2019
Closed
1 task
jacobkeeler added a commit that referenced this pull request Sep 17, 2019
@jacobkeeler
Re-add removed function
a44e39c 
The `OnSystemRequestHandler` function was introduced in #210, but removed when #216 was improperly rebased. This reintroduces that change
This was referenced Sep 17, 2019
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AKalinich-Luxoft AKalinich-Luxoft AKalinich-Luxoft left review comments

@ShobhitAd ShobhitAd ShobhitAd approved these changes

@mrapitis mrapitis mrapitis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@LitvinenkoIra @mrapitis @AKalinich-Luxoft @ShobhitAd @IGapchuk