Feature/rc lights more names and status values · Pull Request #92 · smartdevicelink/sdl_hmi

ghost · 2018-06-12T09:07:19Z

Technical task: smartdevicelink/sdl_core#2172
Update remote control lights
Add new parameters to LightName struct:

REAR_CARGO_LIGHTS
REAR_TRUCK_BED_LIGHTS
REAR_TRAILER_LIGHTS
LEFT_SPOT_LIGHTS
RIGHT_SPOT_LIGHTS
LEFT_PUDDLE_LIGHTS
RIGHT_PUDDLE_LIGHTS
EXTERIOR_ALL_LIGHTS
Update LightStatus struct:
RAMP_UP
RAMP_DOWN
UNKNOWN
INVALID
Update light capabilities:
statusAvailable

yang1070

Can we re-base this PR so that it only include the changes for rc_lights_more_names_and_status_values, now it mixed with other stuff, it is hard to review

yang1070 · 2018-06-14T16:18:33Z

app/model/LightModel.js

                result.push({
                    name: struct[i],
                    densityAvailable: true,
+                    statusAvailable: true,


Please make some lightnames with xyz-Available as false, or not include the item at all,
since I want to test the cases that for example statusAvailable=false, or statusAvailable is not include, how core handle the request. I don't want to change the source code directly.

It is OK for me to change some configuration file (for example hmi_capabilitiy.json, but that is not used by HMI so far) and launch HMI. It is not OK for me to change the source code of HMI.

@yang1070 Can you please review this implementation at the next commit: 3527b77
I added on the main screen when the HMI starts light capabilities, which you can choose.
The checkboxes sets true or false parameter. The input sets erase or not erase parameters.

@yang1070 Sorry, that was forced updated, because was found other mistakes.See commit d0b79be

yang1070

please update

ghost · 2018-06-15T13:34:42Z

@yang1070 We cant to make single commit on the PR, because base branch for this feature which called feature/new_remote_control_modules wasn't merged into develop branch.
See please last commit on this PR which provided new feature 431974b

GetmanetsIrina · 2018-06-18T08:54:56Z

@yang1070 , for now there is check box on start HMI page with sRGBColor, statusAvailable, densityAvailable capabilities.
In case of the absence of changes ( all check boxes are checked and with value in the list Not erase ) all capabilities will have true value for all light items.
In case check box will be unchecked the corresponding capability will have false value for all items.
In case check box will be checked and value in the list will be 'erase' the corresponding capability will be omitted for all items.

Please confirm the current implementation in case it meets expectations.

Or you expect the changes of capabilities not for all items, but for each light item separately?

yang1070 · 2018-06-18T13:16:37Z

@GetmanetsIrina I see the new change. I can use it to change the default capability of all lights on HMI start, it works for my testing purposes. However, it looks really weird to put something specific at that HMI start page. The method is not scale-able.

We have the hmi_capabilites.json file in sdl_core repository, is it possible that sdl_hmi also has that file and HMI read that file and give response to get_capability request with the contents in the json file?

BSolonenko · 2018-06-18T15:56:49Z

@yang1070 We can not read from the .json file, because the script is executed by the browser and does not have access to the file system.

But we can create a simple .js file with one variable that will contains all capabilities. And you will be able to modify this file.

I already working on it.

BSolonenko · 2018-06-18T16:32:51Z

@yang1070 In 9f7262b I added file with RC_Capabilities. You can edit this file. After saving the necessary changes, you need to update the HMI page in a browser.
sdl_hmi/Capabilities/RC_Capabilities.js
https://github.com/smartdevicelink/sdl_hmi/blob/9f7262b076046a4128c11b697ef8e7c06f649b9a/Capabilities/RC_Capabilities.js

yang1070 · 2018-06-18T16:35:11Z

Capabilities/RC_Capabilities.js

+        moduleName: 'Light',
+        supportedLights: [{
+            name: 'FRONT_LEFT_HIGH_BEAM',
+            densityAvailable: false,


missing statusAvailable for all the light names?

yang1070 · 2018-06-18T16:36:07Z

Capabilities/RC_Capabilities.js

+            densityAvailable: true,
+            sRGBColorSpaceAvailable: true
+        }, {
+            name: 'EXTERIOR_RIGHT_LIGHTS',


missing EXTERIOR_ALL_LIGHTS and other new added light names.

yang1070 · 2018-06-18T16:37:00Z

Capabilities/RC_Capabilities.js

+        sourceAvailable: true,
+        volumeAvailable: true,
+        equalizerAvailable: true,
+        equalizerMaxChannelId: 100


yang1070 · 2018-06-18T16:37:30Z

Capabilities/RC_Capabilities.js

+    audioControlCapabilities: [{
+        moduleName: 'AudioControlCapabilities',
+        sourceAvailable: true,
+        volumeAvailable: true,


missing keepContextAvailable

yang1070

Looks like a few newly added stuff is missing in the capabilities, please add them

BSolonenko · 2018-06-18T16:43:30Z

@yang1070 Sorry, I mistakenly added capability not with the branch that you need. Already I fixed it

yang1070 · 2018-06-18T17:05:47Z

Capabilities/RC_Capabilities.js

-        hdChannelAvailable: true,
        rdsDataAvailable: true,
-        availableHDsAvailable: true,
-        hdRadioEnableAvailable: true,


why hdRadioEnableAvailable and siriusxmRadioAvailable is removed? Will it be added in other PR?

@yang1070 yes, this changes regarding new params for RADIO and will be in #93

yang1070 · 2018-06-18T17:08:01Z

Capabilities/RC_Capabilities.js

+            sRGBColorSpaceAvailable: false
        }]
    },
-    seatControlCapabilities: [{


will this be in other PR?

@yang1070 this branch doesn't contains implementation of SEAT module, because we have separate proposal for it.
so in RC_Capabilities.js we have the same capabilities as in https://github.com/smartdevicelink/sdl_core/blob/feature/rc_lights_more_names_and_status_values/src/appMain/hmi_capabilities.json

yang1070 · 2018-06-18T17:10:43Z

app/model/LightModel.js

        initialization(this.locationLightNameStruct, this.lightState);
    },

+    checkEraseParameter: function(data){


with the new RC_Capabilities.js change, we don't need this anymore

@yang1070 Fixed in c5cefa5

LitvinenkoIra · 2018-06-19T09:18:38Z

@yang1070 please look at the latest updates

yang1070 · 2018-06-19T13:11:39Z

app/model/LightModel.js

        initialization(this.locationLightNameStruct, this.lightState);
    },

-    checkEraseParameter: function(data){


this only include the lightModel.js, we shall also remove the UI part that show the three check-boxes when start HMI

@yang1070 removed

yang1070 · 2018-06-21T16:07:41Z

ffw/RCRPC.js

              newLightControlData = SDL.LightModel.setLightControlData(
                request.params.moduleData.lightControlData);
+
+                if (Object.keys(newLightControlData).length > 0) {


HMI shall not send a notification if a parameter is included in the request but the value is the same as the current value (i.e. no change).
I agree the issue is not limited to this PR (more lights), it applies to all modules. We shall create an issue to track that.

@yang1070
Issue created : #96

yang1070

Reviewed and tested with Iryna during the meeting.
The change looks good.

yang1070 · 2018-06-21T16:16:49Z

@theresalech Ford approves this PR.

Add new parameters to LightName struct: - `REAR_CARGO_LIGHTS` - `REAR_TRUCK_BED_LIGHTS` - `REAR_TRAILER_LIGHTS` - `LEFT_SPOT_LIGHTS` - `RIGHT_SPOT_LIGHTS` - `LEFT_PUDDLE_LIGHTS` - `RIGHT_PUDDLE_LIGHTS` - `EXTERIOR_ALL_LIGHTS` Update LightStatus struct: - `RAMP_UP` - `RAMP_DOWN` - `UNKNOWN` - `INVALID` Update light capabilities: - `statusAvailable`

ghost changed the base branch from master to develop June 12, 2018 09:07

ghost force-pushed the feature/rc_lights_more_names_and_status_values branch from 2c9e574 to c798a35 Compare June 12, 2018 09:45

AStasiuk requested review from askirk, mrapitis, robinmk and yang1070 June 13, 2018 15:05

yang1070 reviewed Jun 14, 2018

View reviewed changes

yang1070 suggested changes Jun 14, 2018

View reviewed changes

ghost force-pushed the feature/rc_lights_more_names_and_status_values branch 2 times, most recently from 3527b77 to d0b79be Compare June 15, 2018 15:47

yang1070 reviewed Jun 18, 2018

View reviewed changes

yang1070 suggested changes Jun 18, 2018

View reviewed changes

yang1070 reviewed Jun 18, 2018

View reviewed changes

BSolonenko force-pushed the feature/rc_lights_more_names_and_status_values branch from ca4bc0b to eacdd6c Compare June 19, 2018 09:07

yang1070 reviewed Jun 19, 2018

View reviewed changes

BSolonenko force-pushed the feature/rc_lights_more_names_and_status_values branch 2 times, most recently from c8082a0 to 052ac5c Compare June 21, 2018 08:50

BSolonenko force-pushed the feature/rc_lights_more_names_and_status_values branch 2 times, most recently from b702dbb to b6bd674 Compare June 21, 2018 11:05

yang1070 reviewed Jun 21, 2018

View reviewed changes

yang1070 approved these changes Jun 21, 2018

View reviewed changes

AKalinich-Luxoft force-pushed the feature/rc_lights_more_names_and_status_values branch from b6bd674 to f5fe32e Compare August 20, 2018 09:32

Valerii and others added 3 commits August 20, 2018 12:49

Added file with RC_Capabilities.

c67a4c5

Fixed onInteriorVehicleDataNotification

4948c96

AKalinich-Luxoft force-pushed the feature/rc_lights_more_names_and_status_values branch from f5fe32e to 4948c96 Compare August 20, 2018 09:49

AKalinich-Luxoft and others added 2 commits August 20, 2018 15:02

Update RC capabilities according to Revise SDL-0099

f6cb40b

Fix Brake Typo

11a7022

Jack-Byrne approved these changes Aug 20, 2018

View reviewed changes

Jack-Byrne approved these changes Aug 24, 2018

View reviewed changes

Jack-Byrne merged commit 40ad9f3 into develop Aug 24, 2018

Jack-Byrne mentioned this pull request Oct 19, 2018

Release 5.0.0 #153

Merged

jacobkeeler deleted the feature/rc_lights_more_names_and_status_values branch March 18, 2020 13:20

Conversation

ghost commented Jun 12, 2018 • edited by AStasiuk Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yang1070 left a comment

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 left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Jun 15, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GetmanetsIrina commented Jun 18, 2018

Uh oh!

yang1070 commented Jun 18, 2018

Uh oh!

BSolonenko commented Jun 18, 2018

Uh oh!

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

Uh oh!

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 left a comment

Choose a reason for hiding this comment

Uh oh!

BSolonenko commented Jun 18, 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!

LitvinenkoIra commented Jun 19, 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!

yang1070 left a comment

Choose a reason for hiding this comment

Uh oh!

yang1070 commented Jun 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ghost commented Jun 12, 2018 •

edited by AStasiuk

Loading

ghost commented Jun 15, 2018 •

edited by ghost

Loading

BSolonenko commented Jun 18, 2018 •

edited

Loading