Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #712

mrapitis · 2018-03-15T15:16:13Z

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

Added new unit tests and expanded existing ones

Summary

Added RPC's, structs, enums as defined in the RC Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data), RC Lights – More Names and Status Values, Audio Source AM/FM/XM/DAB, Updating DOP value range for GPS notification proposals

CLA

I have signed the CLA

codecov-io · 2018-03-15T15:31:05Z

Codecov Report

Merging #712 into develop will increase coverage by 0.84%.
The diff coverage is 91.22%.

@@              Coverage Diff              @@
##             develop     #712      +/-   ##
=============================================
+ Coverage      43.41%   44.25%   +0.84%     
- Complexity      3029     3165     +136     
=============================================
  Files            382      397      +15     
  Lines          17461    17776     +315     
  Branches        1736     1736              
=============================================
+ Hits            7580     7867     +287     
- Misses          9563     9591      +28     
  Partials         318      318

Impacted Files	Coverage Δ	Complexity Δ
...in/java/com/smartdevicelink/proxy/rpc/GPSData.java	`95.5% <0%> (-4.5%)`	`39 <0> (ø)`
...devicelink/proxy/rpc/RadioControlCapabilities.java	`100% <100%> (ø)`	`25 <2> (+2)`	⬆️
...com/smartdevicelink/proxy/rpc/enums/LightName.java	`100% <100%> (ø)`	`2 <2> (?)`
...com/smartdevicelink/proxy/rpc/StationIDNumber.java	`100% <100%> (ø)`	`6 <6> (?)`
...java/com/smartdevicelink/proxy/rpc/ModuleData.java	`100% <100%> (ø)`	`17 <6> (+6)`	⬆️
.../smartdevicelink/proxy/rpc/enums/DistanceUnit.java	`100% <100%> (ø)`	`2 <2> (?)`
...devicelink/proxy/rpc/enums/PrimaryAudioSource.java	`100% <100%> (ø)`	`2 <1> (ø)`	⬇️
...in/java/com/smartdevicelink/proxy/rpc/SisData.java	`100% <100%> (ø)`	`12 <12> (?)`
...rtdevicelink/proxy/rpc/HMISettingsControlData.java	`100% <100%> (ø)`	`8 <8> (?)`
...m/smartdevicelink/proxy/rpc/enums/DisplayMode.java	`100% <100%> (ø)`	`2 <2> (?)`
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 89bcf24...9031e80. Read the comment docs.

mrapitis · 2018-08-01T13:22:58Z

@joeygrover we have updated this PR to include the latest develop, please let us know if anything else is required for review. If desired, we can combine PR's #712, #821, #822 into a single PR to simplify the review process (as the proposals depend on each other).

BrettyWhite · 2018-08-09T17:15:32Z

...androidTest/java/com/smartdevicelink/test/rpc/datatypes/ClimateControlCapabilitiesTests.java

 * {@link com.smartdevicelink.rpc.ClimateControlCapabilities}
 */
-public class ClimateControlCapabilitiesTests extends TestCase{


Is there a reason this class is diffing existing code to 2 tabs in instead of 1?

I am actually seeing that on a lot of the tests

We can update to a single tab. The existing tabbing in many of the test files has been a bit off.

Understood, Just trying to keep things as consistent as possible. Thanks

BrettyWhite · 2018-08-09T17:53:30Z

sdl_android/src/androidTest/java/com/smartdevicelink/test/rpc/enums/DistanceUnitTests.java

+		String example = "mILES";
+		try {
+			DistanceUnit temp = DistanceUnit.valueForString(example);
+            assertNull("Result of valueForString should be null.", temp);


Spacing fixes, please. There are a few in this class.

BrettyWhite · 2018-08-09T17:55:15Z

sdl_android/src/androidTest/java/com/smartdevicelink/test/rpc/enums/LightNameTests.java

+		String example = "fRONT_LEFT_HIGH_BEAM";
+		try {
+			LightName temp = LightName.valueForString(example);
+            assertNull("Result of valueForString should be null.", temp);


spacing in this and the next method are off

BrettyWhite · 2018-08-09T17:55:37Z

sdl_android/src/androidTest/java/com/smartdevicelink/test/rpc/enums/LightStatusTests.java

+		String example = "oN";
+		try {
+			LightStatus temp = LightStatus.valueForString(example);
+            assertNull("Result of valueForString should be null.", temp);


spacing in this and next method

BrettyWhite · 2018-08-09T17:58:53Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/ClimateControlCapabilities.java

 import java.util.Hashtable;
 import java.util.List;

 /**
 * Contains information about a climate control module's capabilities.
 */

-public class ClimateControlCapabilities extends RPCStruct{


For history purposes, can this entire class not be changed?

BrettyWhite · 2018-08-09T18:00:42Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/ClimateControlData.java

 import java.util.Hashtable;

-public class ClimateControlData extends RPCStruct{


For history, is it possible to only change the additions and not the whole class? It also makes it easier to review.

BrettyWhite · 2018-08-09T18:09:37Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/RadioControlCapabilities.java

@@ -19,6 +19,7 @@
    public static final String KEY_STATE_AVAILABLE= "stateAvailable";
    public static final String KEY_SIGNAL_STRENGTH_AVAILABLE= "signalStrengthAvailable";
    public static final String KEY_SIGNAL_CHANGE_THRESHOLD_AVAILABLE= "signalChangeThresholdAvailable";
+	public static final String KEY_SIS_DATA_AVAILABLE= "sisDataAvailable";


BrettyWhite · 2018-08-09T18:09:46Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/RadioControlCapabilities.java

+	 * True: Available, False: Not Available, Not present: Not Available.
+	 */
+	public Boolean getSisDataAvailable() {
+        return getBoolean(KEY_SIS_DATA_AVAILABLE);


BrettyWhite · 2018-08-09T18:10:01Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/RadioControlData.java

@@ -20,6 +21,7 @@
    public static final String KEY_SIGNAL_CHANGE_THRESHOLD= "signalChangeThreshold";
    public static final String KEY_RADIO_ENABLE= "radioEnable";
    public static final String KEY_STATE= "state";
+	public static final String KEY_SIS_DATA= "sisData";


BrettyWhite · 2018-08-09T18:11:16Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/StationIDNumber.java

+	public StationIDNumber() {}
+
+	public StationIDNumber(Hashtable<String, Object> hash) {
+        super(hash);


there are a few areas in this class where spacing is off

…parameters (SIS Data) along with updated test cases and documentation.

…emoving sRGBColor struct and reuse RGBColor.

…long with that implementing/updating test cases for same.

…aced with GPSData / RGBColor.

mrapitis · 2018-08-13T15:04:04Z

@therealbrett PR has been updated, please re-review when time permits. Thanks!

BrettyWhite · 2018-08-13T15:09:26Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/GPSData.java

@@ -181,30 +181,10 @@ public GPSData(Hashtable<String, Object> hash) {
    /**
     * Constructs a newly allocated GPSData object
     */


Removing this old constructor is a major change. Please keep it and deprecate it.

BrettyWhite · 2018-08-13T15:32:36Z

sdl_android/src/androidTest/assets/xml/MOBILE_API_4.5.0.xml

@@ -1688,55 +1688,55 @@
        </param>
        <param name="latitudeDegrees" type="Float" minvalue="-90" maxvalue="90" mandatory="true">
        </param>
-        <param name="utcYear" type="Integer" minvalue="2010" maxvalue="2100" mandatory="true">


This file should not have any changes to it, as this is not part of the 4.5 spec

Would it be preferred for us to create a MOBILE_API_4.7.0.xml file with the API changes, add it to the project and reference it from: https://github.com/smartdevicelink/sdl_android/blob/develop/sdl_android/src/androidTest/java/com/smartdevicelink/test/rpc/RPCConstructorsTests.java#L28
(leaving MOBILE_API_4.5.0.xml unchanged)

I need to look into this and will get back to you soon

BrettyWhite · 2018-08-20T20:48:37Z

sdl_android/src/androidTest/java/com/smartdevicelink/test/rpc/enums/LightNameTests.java

+		LightName enumRearLeftTailLight = LightName.valueForString(example);
+		example = "REAR_RIGHT_TAIL_LIGHT";
+		LightName enumRearRightTailLight = LightName.valueForString(example);
+		example = "REAR_LEFT_BREAK_LIGHT";


These now need to be updated per a proposal update to REAR_LEFT_BRAKE_LIGHT and REAR_RIGHT_BRAKE_LIGHT

BrettyWhite · 2018-08-20T20:49:15Z

sdl_android/src/main/java/com/smartdevicelink/proxy/rpc/enums/LightName.java

+	REAR_RIGHT_FOG_LIGHT,
+	REAR_LEFT_TAIL_LIGHT,
+	REAR_RIGHT_TAIL_LIGHT,
+	REAR_LEFT_BREAK_LIGHT,


These now need to be updated per a proposal update to REAR_LEFT_BRAKE_LIGHT and REAR_RIGHT_BRAKE_LIGHT

…RAKE_LIGHT as per suggested changes in proposal.

mrapitis · 2018-08-21T12:40:48Z

@therealbrett enums have been updated to reflect the updated proposal. Please re-review when time permits. Thanks!

mrapitis force-pushed the feature/issue_624 branch 2 times, most recently from 8044831 to 10386b1 Compare July 19, 2018 20:18

mrapitis force-pushed the feature/issue_624 branch from 10386b1 to bf01f18 Compare August 1, 2018 13:18

BrettyWhite reviewed Aug 9, 2018

View reviewed changes

anildahiya and others added 7 commits August 11, 2018 15:18

Updates for new remote control modules (LIGHT, AUDIO, HMI_SETTINGS), …

eed337b

…parameters (SIS Data) along with updated test cases and documentation.

Adding Changes for : removing GPSLocation struct and reuse GPSData, r…

71f57e8

…emoving sRGBColor struct and reuse RGBColor.

Changes for Remote Control – Lights – More Names and Status Values. A…

0fb2ead

…long with that implementing/updating test cases for same.

Changes for Audio Source AM/FM/XM/DAB instead of RADIO_TUNER.

01287c8

Changes for Updating DOP value range for GPS notification.

94922d4

More cleanup for spacing.

7fc8ccf

Removing the files for GPSLocation / SRGBColor as their usage is repl…

567a8a6

…aced with GPSData / RGBColor.

BrettyWhite mentioned this pull request Aug 13, 2018

[SDL 0175] Updating DOP value range for GPS notification #803

Closed

mrapitis force-pushed the feature/issue_624 branch from bf01f18 to 567a8a6 Compare August 13, 2018 14:56

mrapitis changed the title ~~Feature/issue 624 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data)~~ Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) Aug 13, 2018

This was referenced Aug 13, 2018

Feature/issue 751 Remote Control – Lights – More Names and Status Values #821

Closed

Feature/issue 809 Audio Source AM/FM/XM/DAB #822

Closed

BrettyWhite reviewed Aug 13, 2018

View reviewed changes

mrapitis and others added 3 commits August 13, 2018 13:41

Added and deprecated GPSData per review comments.

523ca99

Reverted changes to mobile api xml file.

e7d40ed

Update LightCapabilities.java

1cf727d

BrettyWhite reviewed Aug 20, 2018

View reviewed changes

anildahiya added 2 commits August 21, 2018 14:31

Updates to LightName fields i.e. REAR_LEFT_BRAKE_LIGHT / REAR_RIGHT_B…

1282386

…RAKE_LIGHT as per suggested changes in proposal.

Merge branch 'develop' into feature/issue_624

9031e80

BrettyWhite approved these changes Aug 27, 2018

View reviewed changes

joeygrover approved these changes Aug 30, 2018

View reviewed changes

joeygrover merged commit 6aa0acb into smartdevicelink:develop Aug 30, 2018

This was referenced Aug 30, 2018

[SDL 0099] New remote control modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #624

Closed

[SDL 0165] Remote Control – Lights – More Names and Status Values #751

Closed

[SDL 0182] Audio Source AM/FM/XM/DAB #809

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #712

Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #712

mrapitis commented Mar 15, 2018 •

edited

Loading

codecov-io commented Mar 15, 2018 •

edited

Loading

mrapitis commented Aug 1, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

mrapitis Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

BrettyWhite Aug 9, 2018

mrapitis commented Aug 13, 2018

BrettyWhite Aug 13, 2018

BrettyWhite Aug 13, 2018

mrapitis Aug 13, 2018

BrettyWhite Aug 13, 2018

BrettyWhite Aug 20, 2018

BrettyWhite Aug 20, 2018

mrapitis commented Aug 21, 2018

		import java.util.Hashtable;

		public class ClimateControlData extends RPCStruct{

Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #712

Feature/issue 624,751,809,803 -Remote Control Modules (LIGHT, AUDIO, HMI_SETTINGS) and parameters (SIS Data) #712

Conversation

mrapitis commented Mar 15, 2018 • edited Loading

Risk

Testing Plan

Summary

CLA

codecov-io commented Mar 15, 2018 • edited Loading

Codecov Report

mrapitis commented Aug 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrapitis commented Aug 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrapitis commented Aug 21, 2018

mrapitis commented Mar 15, 2018 •

edited

Loading

codecov-io commented Mar 15, 2018 •

edited

Loading