Coder Social home page Coder Social logo

smartdevicelink / sdl_java_suite Goto Github PK

View Code? Open in Web Editor NEW
186.0 57.0 171.0 113.52 MB

SmartDeviceLink libraries for Android, Java SE, and Java EE

License: BSD 3-Clause "New" or "Revised" License

Java 99.03% Python 0.97% AIDL 0.01%
java sdl-android smartphone sdl smartdevicelink java-se java-ee

sdl_java_suite's Introduction

Build Status codecov Slack Status

SmartDeviceLink (SDL)

SmartDeviceLink (SDL) is a standard set of protocols and messages that connect applications on a smartphone to a vehicle head unit. This messaging enables a consumer to interact with their application using common in-vehicle interfaces such as a touch screen display, embedded voice recognition, steering wheel controls and various vehicle knobs and buttons. There are three main components that make up the SDL ecosystem.

  • The Core component is the software which Vehicle Manufacturers (OEMs) implement in their vehicle head units. Integrating this component into their head unit and HMI based on a set of guidelines and templates enables access to various smartphone applications.
  • The optional SDL Server can be used by Vehicle OEMs to update application policies and gather usage information for connected applications.
  • The App Libraries - Android, iOS, JavaScript, JavaSE (Embedded), and JavaEE (Cloud) - are implemented by app developers into their applications to enable command and control of a connected head unit.

Pull Requests Welcome!

To understand if a contribution should be entered as a Java Suite Pull Request (or issue), or an SDL Evolution Proposal, please reference this document.

SmartDeviceLink

App Library

The app library component of SDL is meant to run on the end user’s smart-device from within SDL enabled apps, as an embedded app, or connected to the cloud. App libraries allow the apps to connect to SDL enabled head-units and hardware through bluetooth, USB, and TCP for Android, and cloud and embedded apps can connect through web sockets, Java Beans, and other custom transports. Once the library establishes a connection between the smart device and head-unit through the preferred method of transport, the two components are able to communicate using the SDL defined protocol. The app integrating this library project is then able to expose its functionality to the head-unit through text, media, and other interactive elements.

SmartDeviceLink Java Suite

You can find guides and API Reference Documentation specific to SDL Android, JavaSE, and JavaEE libraries on smartdevicelink.com.

Contents and timing for SDL Java Suite releases can be tracked on the GitHub Projects page.

Additional information about recent and upcoming SDL Releases can be found in the SDL Evolution README.

SmartDeviceLink Android

Maven Central

Installation

Dependency Managers

To compile with the latest release of SDL Android, include the following in your app's build.gradle file,

repositories {
    mavenCentral()
}
dependencies {
    implementation 'com.smartdevicelink:sdl_android:5.+'
}

For Maven or Ivy snippets please look at Maven Central

Manually

If you prefer not to use any of the aforementioned dependency managers, you can integrate SDL Android into your project manually.

Proguard Rules

Developers using Proguard to shrink and obfuscate their code should be sure to include the following lines in their proguard-rules.pro file:

-keep class com.smartdevicelink.** { *; }
-keep class com.livio.** { *; }
# Video streaming apps must add the following line
-keep class ** extends com.smartdevicelink.streaming.video.SdlRemoteDisplay { *; }

SmartDeviceLink Java

JavaSE

Maven Central

The JavaSE project is meant to allow SDL compatibility for embedded applications.

Dependency Managers

To compile with the latest release of SDL JavaSE, include the following in your app's build.gradle file,

repositories {
    mavenCentral()
}
dependencies {
    implementation 'com.smartdevicelink:sdl_java_se:5.+'
}

JavaEE

Maven Central

The JavaEE project is meant to allow SDL compatibility for web applications.

Dependency Managers

To compile with the latest release of SDL JavaEE, include the following in your app's build.gradle file,

repositories {
    mavenCentral()
}
dependencies {
    implementation 'com.smartdevicelink:sdl_java_ee:5.+'
}

Manually building a JAR

If you prefer making a JAR, simply call:

gradle build

from within the project (JavaSE or JavaEE) and a JAR should be generated in the build/libs folder

Java Suite Repo Structure

Java Suite Folder Structure

base Folder

The base folder contains the source set that is shared between all of the compilable projects. This folder does not contain a compilable project.

android Folder

The android folder contains the SDL Android library as well as the sample project for Android. Both of those are compilable projects.

javaSE

The javaSE folder contains the SDL JavaSE Library. The base folder source set is added as a dependency. This project can be used for embedded or remote SDL applications. It uses a web socket transport by default but can be made to work with other transports via the CustomTransport.

JavaSE Sample App

The JavaSE sample app is in the hello_sdl_java folder. It demonstrates an efficient way to structure a Java app using the JavaSE library.

javaEE

The javaEE folder contains the SDL JavaEE library. The JavaSE folder is used as a source set and added as a dependency. This library is based off the JavaSE library and will contain specifics for the JavaEe platform.

JavaEE Sample App

The JavaEE sample app is in the hello_sdl_java_ee folder. Most of the code is commented out since the library and sample app do not include the dependencies of JavaEE due to licensing issues. However, the commented out code demonstrates how to build a Java based app into the JavaEE bean architecture.

sdl_java_suite's People

Contributors

akalinich-luxoft avatar anildahiya avatar askirk avatar bilal-alsharifi avatar boristm avatar brettywhite avatar chloemjm avatar enricogueli-tomtom avatar joeljfischer avatar joeygrover avatar jordynmackool avatar juliankast avatar justinjdickow avatar kboskin avatar kshala-ford avatar mikaylaray44 avatar mjuarez-ford avatar mongothegeek avatar mrapitis avatar nicoleyarroch avatar noah-livio avatar o-mishch avatar rhenigan avatar shiniwat avatar shoamano83 avatar theresalech avatar thomasheike avatar tnguy238-ford avatar tnnnguyen avatar vladmu avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

sdl_java_suite's Issues

RPC classes should have constructors

RPC classes have been designed to have empty constructors:

public AddCommand() {
    super("AddCommand");
}

The only way for developers to set information within an RPC class is to create an empty object and add information using setter methods. For example, to initialize an AddCommand, a developer currently has to do this:

AddCommand item = new AddCommand();
item.setCmdID(100);

Vector<String> vrCommands = new Vector<String>();
vrCommands.add("item");
item.setVrCommands(vrCommands);

Image icon = new Image();
icon.setValue("icon.png");
icon.setImageType(ImageType.DYNAMIC);
item.setCmdIcon(icon);

MenuParams params = new MenuParams();
params.setParentID(1000);
params.setPosition(0);
params.setMenuName("item");
item.setMenuParams(params);

If all RPCs define proper constructors, the above code could be reduced to something like this:

AddCommand item = new AddCommand(100, new MenuParams(1000, 0, "item"),
  new Image("icon.png", ImageType.DYNAMIC), Arrays.asList(new String[]{"item"}));

This saves developers 12 lines of code for a single AddCommand object.

In my feature/refactor_rpcs branch on GENIVI, I have defined constructors in the newly designed RPC classes, which can be used for reference.

DisplayCapabilities ImageFields parameter is of type TextField

Appears to be a copy/paste error.

getImageFields() should return a list of ImageField objects, not TextField objects. Similarly, setImageFields() should take a list of ImageField objects, not TextField objects.

public Vector<TextField> getImageFields() {
    if (store.get(Names.imageFields) instanceof Vector<?>) {
        Vector<?> list = (Vector<?>)store.get(Names.imageFields);
        if (list != null && list.size() > 0) {
            Object obj = list.get(0);
            if (obj instanceof TextField) {
                return (Vector<TextField>) list;
            } else if (obj instanceof Hashtable) {
                Vector<TextField> newList = new Vector<TextField>();
                for (Object hashObj : list) {
                    newList.add(new TextField((Hashtable)hashObj));
                }
                return newList;
            }
        }
    }
    return null;
}

public void setImageFields( Vector<TextField> imageFields ) {
    if (imageFields != null) {
        store.put(Names.imageFields, imageFields );
    }
    else
    {
        store.remove(Names.imageFields);
    }
}

getLockScreenStatus Method Needs to Check for Null Pointer

Currently the getLockScreenStatus method inside of LockScreenManager.java does not check for null. If a driver distraction notification comes in from the head unit prior to the first HMI notification an NPE will be encountered. This should be a one line change.

Before

        if ( hmiLevel.equals(HMILevel.HMI_NONE) )
        {
            return LockScreenStatus.OFF;
        }
        else if ( hmiLevel.equals(HMILevel.HMI_BACKGROUND) )
        {

After

        if ( (hmiLevel == null) || (hmiLevel.equals(HMILevel.HMI_NONE)) )
        {
            return LockScreenStatus.OFF;
        }
        else if ( hmiLevel.equals(HMILevel.HMI_BACKGROUND) )
        {

Support for Lock Screen Icon

See issue on SDL core here.

Head-unit wants to add support for showing a different image on the app's lock screen based on what type of radio is connected. This requires adding a new request type to OnSystemRequest called LOCK_SCREEN_ICON_URL. The head-unit will send OnSystemRequest with request type LOCK_SCREEN_ICON_URL and URL contained in the URL field after it receives the RegisterAppInterface command from the app. The app is responsible for handling this command in onOnSystemRequest callback, downloading the image from the URL and displaying the image on the lock screen.

Only thing that needs to change in the Android library is to add the new request type.

ClusterModeStatus setPowerModeQualificationStatus takes Names class

Appears to be a copy/paste error.

setPowerModeQualificationStatus() should take a PowerModeQualificationStatus enum value, but instead takes a Names value.

public void setPowerModeQualificationStatus(Names powerModeQualificationStatus) {
    if (powerModeQualificationStatus != null) {
        store.put(Names.powerModeQualificationStatus, powerModeQualificationStatus);
    } else {
        store.remove(Names.powerModeQualificationStatus);
    }
}

Some RPC classes don't handle null inputs correctly

Most RPC classes have setter methods defined such that if a non-null value is passed in, that value is stored in the underlying hashtable and if a null value is passed in, that value is removed from the underlying hashtable. However, there are some RPC classes that simply ignore the null input.

This is how a "typical" RPC class defines a setter method (from AddCommand):

public void setCmdID(Integer cmdID) {
    if (cmdID != null) {
        parameters.put(Names.cmdID, cmdID);
    } else {
        parameters.remove(Names.cmdID);
    }
}

Some classes, however, incorrectly implement setter methods (from AddSubMenu):

public void setMenuID( Integer menuID ) {
    if (menuID != null) {
        parameters.put(Names.menuID, menuID );
    }
}

As you can see, a null input is simply ignored instead of removing any previous value from the underlying hashtable.

So far, these are the classes that have defined incorrect setter methods:

This is not an exhaustive list by any means, so I will continue to add other RPC classes to the list as I find them.

Clickable items should have OnClick callbacks

Items that can be touched or clicked on the head-unit should have associated OnClick callbacks so that the click event can be triggered by the library instead of handled by each individual developer.

For example, the Livio SDL Tester application on GENIVI has defined a MenuManager class to handle click events from onPerformInteractionResponse, onOnButtonPress & onOnCommand. Within these callbacks, a "disptachClick" method is called, which, in turn, calls the onClickListener that was defined for each clickable item:

@Override
public void onOnCommand(final OnCommand notification){
        int buttonId = notification.getCmdID();
        menuManager.dispatchClick(buttonId);
}

RPC messages should sanitize user inputs

RPC messages should sanitize user inputs before storing them in the underlying hashtable.

For example, in the AddCommand RPC message, the documentation claims that CmdID can only be between 0 and 2,000,000,000. However, the code doesn't check that the user input is within this range.

The RPC code should check the user input and throw an InvalidParameterException if it does not meet the specified criteria.

Reformat java files

Java files are currently not formatted consistently (in terms of spaces vs. tabs, line character width, etc). For example, you can easily see the formatting inconsistency when looking at the raw text - AddCommand.java

I propose we define a standard format that will make the code look consistent in IDEs and text editors other than eclipse and implement that format on the current code base. Future pull requests should implement this standard format to be merged into the project.

ChangeRegistrationResponse function name

While merging the pull requests today, I noticed that ChangeRegistrationResponse names the function name "ChangeLanguageRegistration" instead of "ChangeRegistration".

Is this correct, or a typo? Does SDL core respond to ChangeRegistration request with a function name of "ChangeLanguageRegistration" in the response?

I'm going to assume it's correct and define this as a function name in FunctionID.java, but please let me know if this is not correct.

How to handle USB support?

It looks like the current manifest has minSdkVersion set to 12 (Android 3.1), due to the USB support.

<uses-sdk android:minSdkVersion="12"/>

If an app doesn't need USB support, the minSdkVersion can probably go back to 8 (Android 2.2), but we need to figure out how to make that work. Maybe a different branch for USB support so master can support as many Android phones as possible over bluetooth? Another possibility is adding gradle support to enable/disable USB within the gradle config files, but that option, of course, requires integrating gradle into the project, which is probably a different discussion.

Here's the Android documentation for USB Accessory, which says USB support is back-ported to sdk 10 (Android 2.3.4) using add-on libraries.

Any other ideas for how to handle USB support while still supporting as many phones as possible over bluetooth?

Version tags on master branch

We're going to be following the Atlassian GitFlow workflow for this repository, which means the master branch will only contain stable releases of the code and all development work is done on feature branches and merged into the develop branch prior to being merged into master.

My question is: what do we want to label the current master version as? v1.0? v2.0? v3.0?

How are JSON-level changes going to be supported in future versions?

In the current code base, the JSON structure of an RPC message is defined during run-time by storing variables in an underlying hashtable using hard-coded strings as JSON keys. This obviously only works if there is a single JSON protocol that is never changed.

Let's say, for example, that a JSON parameter key where to change in the future. How does the current code base handle the two different JSON parameters within an RPC? How does the RPC know what string to use in the underlying hashtable?

VehicleData fuelLevel_State

VehicleData fuelLevel_State doesn't follow Java naming conventions or the naming conventions set forth in the project. This variable should be re-named fuelLevelState, its setter and getter methods should be deprecated and renamed to setFuelLevelState() and getFuelLevelState(), respectively.

PutFile doesn't handle file data correctly

PutFile has a setter/getter method to set/get FileData, which takes in a byte array and stores it in the hashtable.

    public void setFileData(byte[] fileData) {
        if (fileData != null) {
            parameters.put(RPCStruct.KEY_BULK_DATA, fileData);
        } else {
            parameters.remove(RPCStruct.KEY_BULK_DATA);
        }
    }
    public byte[] getFileData() {
        return (byte[]) parameters.get(RPCStruct.KEY_BULK_DATA);
    }

This is incorrect behavior, as binary data is sent through the binary protocol, not through the JSON protocol. When I send a PutFile using this method to set the file data, the request is rejected saying "Binary data empty". When I send a PutFile using setBulkData() instead of setFileData(), it works correctly.

Recommend removing these two functions entirely or calling the bulk data methods directly from these methods.

Vehicle Data classes are not scalable

There are currently 25 supported vehicle data types with demand to add even more in the future.

However, current vehicle data RPC classes contain a distinct setter/getter method for each individual vehicle data type - which means each RPC related to vehicle data needs to define 50 separate methods. As we add more vehicle data types, 2 additional methods (a setter & getter) will need to be added to each RPC.

A better way would be to define all vehicle data types in an enum and provide a generic hash map that maps the vehicle data enum to it's associated object (depends on the specific RPC in question). In this design, adding a new type of vehicle data only requires that it be added to the enum.

getImageTypeSupported should return a List

The imageTypeSupported parameter from ImageField should return an array. Currently the method only returns a single file type.

https://github.com/smartdevicelink/sdl_android/blob/master/sdl_android_lib/src/com/smartdevicelink/proxy/rpc/ImageField.java

From the xml spec:

  <struct name="ImageField">
    <param name="name" type="ImageFieldName">
      <description>The name that identifies the field. See ImageFieldName.</description>
    </param>
    <param name="imageTypeSupported" type="FileType" maxlength="100" array="true">
      <description>The image types that are supported in this field. See FileType.</description>
    </param>
    <param name="imageResolution" type="ImageResolution">
      <description>The image resolution of this field.</description>
    </param>
  </struct>

Make RPC classes immutable

RPC classes are currently mutable objects - there are setter methods that allow the user to change the data within the RPC after creation. However, within the library, RPC classes are treated as immutable (the library never changes them).

I propose we make all RPC classes immutable objects. Doing so would greatly reduce complexity and remove the need for defensive copying of RPC data types since they would be immutable.

Also, the developer would create the object with all data being set in a constructor instead of using setter methods. This would also make developer code more readable since they can create and initialize an object on a single line instead of creating an empty object and calling setter methods on it.

WiFi freezes UI thread

Multiple WiFi connection attempts when the phone has WiFi disabled causes UI thread to freeze. I've confirmed on 3 different Livio SDL-connected apps on my Android 4.2.2 Galaxy Nexus. This could be related to this issue, as both involve freezing UI thread over invalid WiFi connection.

Steps to Reproduce:

  • Ensure WiFi is disabled on the Android device
  • Open app under test
  • Attempt to connect to SDL core via WiFi
  • Close app under test
  • Re-open app under test
  • Attempt to connect to SDL core via WiFi

UI thread will be frozen at this point.

Work-around could be to ensure WiFi is enabled prior to creating SdlProxy object.

Marking as low priority since WiFi functionality is for debugging only and not currently used in production.

Disposing of the proxy on a Bluetooth disconnect can result in an exception

Our recommended service management practice is to stop the AppLink service on a Bluetooth disconnect (ACL_DISCONNECTED) and dispose of the proxy (if it isn't null). However, the SDK also internally tries to dispose/recycle the proxy when it observes the same disconnect.

Some comments from a thread on the Ford Developer forums:

When I turn off bluetooth in my phone, I get this exception from the SyncProxyALM. It appears as though it is attempting to read after the BT transport is closed. Is this a bug or am I doing something wrong? I am stopping my service (and disposing off the proxy object) when the BT state changes to "turning_off" or if "acl_disconnected" is received.

11-07 22:05:15.941: W/System.err(32154): java.io.IOException: read failed, socket might closed or timeout, read ret: -1 
11-07 22:05:15.941: W/System.err(32154): at android.bluetooth.BluetoothSocket.readAll(BluetoothSocket.java:492) 
11-07 22:05:15.951: W/System.err(32154): at android.bluetooth.BluetoothSocket.waitSocketSignal(BluetoothSocket.java:469) 
11-07 22:05:15.991: W/System.err(32154): at android.bluetooth.BluetoothSocket.accept(BluetoothSocket.java:393) 
11-07 22:05:15.991: W/System.err(32154): at android.bluetooth.BluetoothServerSocket.accept(BluetoothServerSocket.java:131) 
11-07 22:05:15.991: W/System.err(32154): at android.bluetooth.BluetoothServerSocket.accept(BluetoothServerSocket.java:117) 
11-07 22:05:15.991: W/System.err(32154): at com.ford.syncV4.transport.BTTransport$TransportReaderThread.acceptConnection(BTTransport.java:219) 
11-07 22:05:16.011: W/System.err(32154): at com.ford.syncV4.transport.BTTransport$TransportReaderThread.run(BTTransport.java:302) 
11-07 22:05:16.031: W/System.err(32154): java.io.IOException: read failed, socket might closed or timeout, read ret: -1 
11-07 22:05:16.031: W/System.err(32154): at android.bluetooth.BluetoothSocket.readAll(BluetoothSocket.java:492) 
11-07 22:05:16.031: W/System.err(32154): at android.bluetooth.BluetoothSocket.waitSocketSignal(BluetoothSocket.java:469) 
11-07 22:05:16.041: W/System.err(32154): at android.bluetooth.BluetoothSocket.accept(BluetoothSocket.java:393) 
11-07 22:05:16.041: W/System.err(32154): at android.bluetooth.BluetoothServerSocket.accept(BluetoothServerSocket.java:131) 
11-07 22:05:16.041: W/System.err(32154): at android.bluetooth.BluetoothServerSocket.accept(BluetoothServerSocket.java:117) 
11-07 22:05:16.041: W/System.err(32154): at com.ford.syncV4.transport.BTTransport$TransportReaderThread.acceptConnection(BTTransport.java:219) 
11-07 22:05:16.041: W/System.err(32154): at com.ford.syncV4.transport.BTTransport$TransportReaderThread.run(BTTransport.java:302) 
  • praneetloke

I am having the same issue. When I get ACL_DISCONNECTED I clean up by calling disposeSyncProxy (base off the sample code provided). Quite often this results in the following exception:

2.2: Transport failure: Failed to read from Bluetooth transport. java.io.IOException: bt socket closed, read return: -1 
android.bluetooth.BluetoothSocket.read(BluetoothSocket.java:429) 
android.bluetooth.BluetoothInputStream.read(BluetoothInputStream.java:96) 
java.io.InputStream.read(InputStream.java:162) com.ford.syncV4.transport.BTTransport$TransportReaderThread.readFromTransport(BTTransport.java:364) 
com.ford.syncV4.transport.BTTransport$TransportReaderThread.run(BTTransport.java:405) 

This hangs my call to dispose so the rest of my application specific cleanup code doesn't get called. It seems like my only other option at this point is call dispose on a new thread and let it crash while my calling thread will continue to clean up other code.

  • rebutl

Broadcast logging can cause an exception if the apps service becomes unavailable

If the apps service becomes unexpectedly unavailable. In the code below, if the service becomes unavailable the getApplicationContext call will fail causing an exception.

private void sendBroadcastIntent(Intent sendIntent)
    {
        Service myService = null;       
        if (_proxyListener != null && _proxyListener instanceof Service)
        {
            myService = (Service) _proxyListener;               
        }
        else if (_appService != null)
        {
            myService = _appService;
        }
        else
        {
            return;
        }
        Context myContext = myService.getApplicationContext();
        if (myContext != null) myContext.sendBroadcast(sendIntent);     
    }

JSON parameter key strings should be moved to associated classes

Currently, all JSON parameter key strings are defined in Names.java. To comply with an object-oriented design, these strings should be defined within the class in which they are relevant.

For example, the AddCommand RPC has 4 parameters (vrCommands, cmdID, menuParams and cmdIcon). These parameters should be moved from Names.java into the AddCommand class:

public class AddCommand extends RPCRequest{
    public static final class ParameterKeys{

        private ParameterKeys(){} // prevent instantiation

        public static final String VR_COMMANDS  = "vrCommands";
        public static final String COMMAND_ID   = "cmdID";
        public static final String MENU_PARAMS  = "menuParams";
        public static final String COMMAND_ICON = "cmdIcon";
    }
    ...
}

JSON parameter key strings can be accessed via

AddCommand.ParameterKeys.VR_COMMANDS

instead of

Names.vrCommands

OnSystemRequest getBody() method has side-effects

OnSystemRequest getBody() method has side effects:

    public String getBody(){                

        JSONObject jLayer1 = null;
        String sReturn = null;        
        try
        {
            if (httpreqparams != null)
            {
                jLayer1 = myJSONObj.getJSONObject("HTTPRequest");           
                sReturn = jLayer1.getString("body");
                return sReturn;
            }
            else if (myJSONObj != null)
            {
                //jLayer1 =  new JSONObject();
                //jLayer1.put("data", myJSONObj);
                return myJSONObj.toString();
            }
            else
            {
                return null;
            }

        }
        catch (Exception e) 
        {
            return null;            
        }
    }

As you can see, this "getter" method is modifying class variables, and reading from JSON objects with hard-coded key values. A getter method should just return the variable the outside world wants to get and nothing else.

Here's a good article about misusing getter and setter methods.

SdlProxyBase doesn't update DisplayCapabilities on SetDisplayLayoutResponse

Mike: SdlProxyBase keeps a global variable containing the display capabilities for the head-unit. This global variable is only set on register app interface - it isn't updated when the layout changes. When the layout changes, size of images, number of text fields, etc may change, so the global variable should be updated to reflect these changes.

getKeyboardProperties inside of SetGlobalProperties has wrong instanceof check

Looks like a copy / paste error should be checking instanceof KeyboardProperties

https://github.com/smartdevicelink/sdl_android/blob/master/sdl_android_lib/src/com/smartdevicelink/proxy/rpc/SetGlobalProperties.java

    public KeyboardProperties getKeyboardProperties() {
        Object obj = parameters.get(Names.keyboardProperties);
        if (obj instanceof Image) {
            return (KeyboardProperties) obj;
        } else if (obj instanceof Hashtable) {
            try {
                return new KeyboardProperties((Hashtable) obj);
            } catch (Exception e) {
                DebugTool.logError("Failed to parse " + getClass().getSimpleName() + "." + Names.keyboardProperties, e);
            }
        }
        return null;
    }   

Libs should prefer specific exceptions to generic exceptions

The current libraries don't follow best practices for handling exceptions. Here is a good guide for Java Exception best practices.

For example, SDL only has defined a single custom exception - SdlException and the exception has a SdlExceptionCause field explaining why the exception was thrown. This is really against Java best practices for Exceptions, as throwing a generic exception is hiding the real reason the exception was thrown and doesn't give developers much information when they get the exception.

The current implementation requires developers to check the cause of the exception and add a lot of code into a single catch block instead of giving them the ability to catch different exceptions in different catch blocks based on different exceptions that could be thrown. This also makes it more difficult to throw specific exceptions because whenever an exception is thrown, the method has to do additional work to set the cause field. If the cause field isn't set correctly, catch blocks won't handle it correctly, which will cause bugs that are extremely difficult to track down.

In another example, the following code (from SubscribeVehicleDataResponse) contains a try/catch block around the code that creates a new object using a hashtable and catches a generic exception. If I trace this call up, I can see that the object just passes the hash table up the hierarchy (RPCStruct) and the superclass just saves a reference to that hashtable.

None of these steps could possibly throw an exception, so I believe these try/catch blocks can be removed since it's unreachable code. This issue is contained in a lot of other RPCs classes.

@SuppressWarnings("unchecked")
public VehicleDataResult getGps() {
  Object obj = parameters.get(KEY_GPS);
  if (obj instanceof VehicleDataResult) {
    return (VehicleDataResult) obj;
  } else if (obj instanceof Hashtable) {
    try {
      return new VehicleDataResult((Hashtable<String, Object>) obj);
    } catch (Exception e) {
      DebugTool.logError("Failed to parse " + getClass().getSimpleName() + "." + KEY_GPS, e);
    }
  }
  return null;
}

I think this issue requires a lot of thought and discussion as to how the generic SdlException could be split up into more specific exceptions. The code also needs to be thoroughly investigated to remove unreachable catch blocks.

Clean up warning messages

The project currently has 844 warning messages. Most of them are related to type safety in relation to generic data types, so they can be easily cleaned up by explicitly declaring generic data types and suppressing warnings for unchecked casts.

DisplayType valueForString throws a runtime exception

DisplayType enum throws an exception when valueForString is called with a value that isn't defined in the enum. Other similar enums simply return null, which I think is the appropriate action.

public static DisplayType valueForString(String value) {
    for (DisplayType type : DisplayType.values()) {
        if (type.toString().equals(value)) {
            return type;
        }
    }

    throw new IllegalArgumentException("Unknown value " + value);
}

getAudioPassThruCapabilities getter missing from proxyALM object

The alm proxy object contains getter methods for the data structures that are returned from a register app interface response, however the proxy object is missing a getter method for AudioPassThruCapabilities. A getter method needs to be added to return this information to the app developer.

Improve JavaDoc Documentation

Some classes have full and complete JavaDoc documentation; however, not all of them do. To allow generation of JavaDocs as part of the build process, all classes should be completely documented with JavaDoc-style comments.

Mobile libraries should provide detailed unit tests

Unit tests should be provided for all parts of the mobile libraries. Unit testing should be a big focus moving forward so automated testing can become part of the build process and ensure new code and pull requests don't break any existing code.

Unit tests for RPC classes are currently being written by the Livio team.

Feedback on #23

In the meeting last week, some people wanted to review #23, prior to it being merged. I accidentally merged it along with the other pull requests, but want to ensure I get any feedback in regards to what was changed in case I need to revert any changes. Just comment on this thread if all is good, or create issues for anything that needs to be reverted and reference the new issues on this thread for tracking purposes.

Sorry for the mixup and thanks for reviewing this for me!

Refactor Vectors into Lists

Vector objects should be refactored into List objects where synchronization is not explicitly needed. For example, RPC classes don't modify underlying Vector objects, which means that synchronization is not an issue in RPC classes.

List is a generic interface, which allows app developers to use any type of List - including Vector, ArrayList, LinkedList or a custom List implementation.

RPC getter/setter methods should support defensive copying

Mutable data that is accessed or mutated within an RPC message should be defensive copied to prevent developers from accessing underlying class variables directly.

For example, the following code will not work as expected due to the lack of defensive copying in RPC classes:

Vector<TTSChunk> ttsChunks = new Vector<TTSChunk>();

ttsChunks.add(TTSChunkFactory.createChunk(SpeechCapabilities.TEXT, "Test Message 1"));
Speak speak1 = new Speak();
speak1.setTtsChunks(ttsChunks);

ttsChunks.clear();
ttsChunks.add(TTSChunkFactory.createChunk(SpeechCapabilities.TEXT, "Test Message 2"));
Speak speak2 = new Speak();
speak2.setTtsChunks(ttsChunks);

Since ttsChunks is the same data object and it is not defensive copied within the setTtsChunks() method, this code does not work as expected. To fix this issue, both the setter & getter methods should make a copy of any mutable data before exposing it to the outside world.

Disable logs for production

SDL library needs to introduce a mechanism to enable logs messages for debugging, while disabling log messages for production. In Livio Connect, we implemented a static Logger class, which basically wraps the Android log methods, but allows a single point to enable/disable logs for the entire library. This would create an unneeded dependency, however, because classes wouldn't be able to compile without the Logger class present and any changes to the Logger API would break pretty much every class.

Another option would be the manifest declaration for application attribute called android:debuggable that can be set to false to disable logs. I haven't tried that before, but Android recommends setting this attribute to false prior to production release.

I'm sure there are other options to achieve this goal, so feel free to comment with any ideas.

This is also a good task for gradle to complete during the build process, if we decide to integrate gradle into the project. We could do gradle build debug, which would enable debugging; or, we could do gradle build production, which would disable debugging.

Here's the Google "Preparing for Release" guide.

OnSdlChoiceChosen contains duplicate class definitions

OnSdlChoiceChosen contains class definitions that are already defined elsewhere.

For example, the class has the following definition:

public class SdlSubMenu {
    private Integer _menuID = null;
    private Integer _position = null;
    private String _menuName = null;

    // Constructor
    SdlSubMenu(Integer menuID, Integer position, String menuName) {
        _menuID = menuID;
        _position = position;
        _menuName = menuName;
    }

    // Restrict no-arg constructor
    private SdlSubMenu() {}

        // Public Getters
    public Integer getMenuID() {
        return _menuID;
    }

    public String getMenuName() {
        return _menuName;
    }

    public String toString() {
        return _menuName;
    }
}

This class contains nearly the exact same methods and variables that are already defined as part of the AddSubMenu class.

These duplicate classes should be removed in favor of the existing classes.

Refactor RPC Classes

Earlier this year, I worked to refactor RPC classes to be faster, more readable and more scalable.

The refactored design is on the GENIVI git repo under the feature/refactor_rpcs branch. I have also put together this presentation explaining what changed, why I felt it needed to be changed and metrics as to how much faster the new design is.

Livio has also been working on full unit test coverage for this new design. The unit test suite should be available in the next few weeks (see GENIVI feature/unit_tests for more details on unit testing progress).

I would love to get some feedback from the open-source community in regards to my new design. If there is enough interest, I would be happy to hold a conference call to go through the presentation and answer any questions.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.