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.

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

[SDL-0293] Enable OEM exclusive apps support #374

Merged
6 changes: 2 additions & 4 deletions lib/js/src/manager/SdlManagerListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,11 @@ class SdlManagerListener {
/**
* Safely attempts to invoke the OnVehicleTypeReceived event callback function.
* @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
* @param {String} systemSoftwareVersion - software version of the system.
* @param {String} systemHardwareVersion - hardware version of the system.
* @returns {Boolean} Return true if this session should continue, false if the session should end
*/
onVehicleTypeReceived (vehicleType, systemSoftwareVersion, systemHardwareVersion) {
onVehicleTypeReceived (vehicleType) {
if (typeof this._onVehicleTypeReceived === 'function') {
return !!this._onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion);
return !!this._onVehicleTypeReceived(vehicleType);
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/js/src/manager/_SdlManagerBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class _SdlManagerBase {
* @class
* @private
* @param {AppConfig} appConfig - An instance of AppConfig describing the application's metadata and desired transport
* @param {ManagerListener} managerListener - An instance of ManagerListener to be used to listen for manager events
* @param {SdlManagerListener} managerListener - An instance of ManagerListener to be used to listen for manager events
*/
constructor (appConfig = null, managerListener = null) {
this._state = -1;
Expand Down
15 changes: 7 additions & 8 deletions lib/js/src/manager/lifecycle/_LifecycleManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,11 @@ class _LifecycleManager {
/**
* Safely attempts to invoke the onVehicleTypeReceived event.
* @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
* @param {String} systemSoftwareVersion - software version of the system.
* @param {String} systemHardwareVersion - hardware version of the system.
* @returns {Boolean} Return true if this session should continue, false if the session should end
*/
onVehicleTypeReceived (vehicleType, systemSoftwareVersion, systemHardwareVersion) {
onVehicleTypeReceived (vehicleType) {
if (typeof this._onVehicleTypeReceived === 'function') {
return !!this._onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion);
return !!this._onVehicleTypeReceived(vehicleType);
}
return true;
}
Expand Down Expand Up @@ -510,11 +508,12 @@ class _LifecycleManager {
this._cleanProxy();
}

if (this._rpcSpecVersion.isNewerThan(new Version(7, 0, 0)) === 1) {
// to avoid double firing the onVehicleTypeReceived event
// try to define vehicleType and fire onVehicleTypeReceived here
// only if that was not received from SdlSession before
if (!this._sdlSession.didReceiveVehicleType()) {
const vehicleType = registerAppInterfaceResponse.getVehicleType();
const systemSoftwareVersion = registerAppInterfaceResponse.getSystemSoftwareVersion();
const systemHardwareVersion = registerAppInterfaceResponse.getSystemHardwareVersion();
if (!this.onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion)) {
if (!this.onVehicleTypeReceived(vehicleType)) {
console.warn('(RAI) Disconnecting from head unit, the vehicle is not supported');
this.sendRpcResolve(new UnregisterAppInterface());
this._cleanProxy();
Expand Down
41 changes: 19 additions & 22 deletions lib/js/src/protocol/_SdlProtocolBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ import { _FrameType } from './enums/_FrameType.js';
import { _MessageFrameAssembler } from './_MessageFrameAssembler.js';
import { _SdlPacket } from './_SdlPacket.js';
import { _ControlFrameTags } from './enums/_ControlFrameTags.js';
import { _BitConverter } from './../util/_BitConverter.js';
import { _BitConverter } from '../util/_BitConverter.js';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ymalovanyi @vladmu Do you need unit tests for this? For example, sdl java suite has SdlProtocolTests

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santhanamk we don't need the test for this particular line as this is just a shortenest path identical to previous but aligned with the style of other imports.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladmu I meant do you need unit tests overall for the changes made to this file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@santhanamk it is hard to define expectations of such tests in the order of the library and this class as we don't have any tests of previous functionality covered here. We appreciate any help in this case.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladmu Ok no problem. It was an optional request, as these were not existing before.


import { _SdlPacketFactory } from './_SdlPacketFactory.js';
import { RpcCreator } from './../rpc/RpcCreator.js';
import { RpcCreator } from '../rpc/RpcCreator.js';
import { ImageResolution } from '../rpc/structs/ImageResolution.js';
import { VideoStreamingFormat } from '../rpc/structs/VideoStreamingFormat.js';

import { VehicleType } from './../rpc/structs/VehicleType.js';
import { VehicleType } from '../rpc/structs/VehicleType.js';

/**
* Base implementation of sdl protocol.
Expand Down Expand Up @@ -272,32 +272,29 @@ class _SdlProtocolBase {

/**
* Gets the Vehicle details received in StartService ACK protocol message
* @returns {Number} - A new numeric message ID.
* @returns {VehicleType|null} - A RPC VehicleType struct received from the packet if exists null otherwise.
*/
_getVehicleType(sdlPacket) {
const make = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_MAKE);
if (!make) {
return null;
}
const model = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_MODEL);
const modelYear = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_MODEL_YEAR);
const trim = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_TRIM);

const vehicleType = new VehicleType({
// TODO: awaiting 0293 revisions to process those fields
const systemHardwareVersion = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_SYSTEM_HARDWARE_VERSION);
const systemSoftwareVersion = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_SYSTEM_SOFTWARE_VERSION);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method seems unfinished since these values are never used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@renonick87 no, the method is finished and reflects the current state of the proposal. Without revisions, we receive those fields here but never use or process them. The TODO was added to reflect that the processing needs to be added after accepting the revisions.


// if no any VehicleType tags in the packet just return null
if (!make && !model && !modelYear && !trim) {
return null;
}

return new VehicleType({
make,
model,
modelYear,
trim,
});

const systemHardwareVersion = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_SYSTEM_HARDWARE_VERSION);
const systemSoftwareVersion = sdlPacket.getTag(_ControlFrameTags.RPC.StartServiceACK.VEHICLE_SYSTEM_SOFTWARE_VERSION);

return {
vehicleType,
systemHardwareVersion,
systemSoftwareVersion,
}
}

/**
Expand Down Expand Up @@ -438,15 +435,15 @@ class _SdlProtocolBase {
const version = sdlPacket.getVersion();
const serviceType = sdlPacket.getServiceType();
if (version >= 5) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ymalovanyi On line 440, does this need to be >= 6 instead of >= 5? In the proposal the example method onPacketRead has it as > = 6.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our initial implementation was exactly with >= 6 according to the proposal, but testing alongside the core we never met that condition, it seems the protocol version is not increased with this proposal. Therefore we rolled back to 5.

const vehicleTypeFromPacket = this._getVehicleType(sdlPacket);
// check if vehicleType data exists then fire onVehicleTypeReceived protocol listener event
const vehicleType = this._getVehicleType(sdlPacket);
if (
vehicleTypeFromPacket
vehicleType
&& this._sdlProtocolListener !== null
&& typeof this._sdlProtocolListener.onVehicleTypeReceived === 'function'
) {
const {vehicleType, systemSoftwareVersion, systemHardwareVersion } = vehicleTypeFromPacket;
if (!this._sdlProtocolListener.onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion)) {
console.warn('Disconnecting from head unit, the vehicle is not supported (ACK)');
if (!this._sdlProtocolListener.onVehicleTypeReceived(vehicleType)) {
console.warn('(ACK) Disconnecting from head unit, the vehicle is not supported');
this.endService(serviceType, sdlPacket.getSessionID());
if (serviceType === _ServiceType.RPC && this._transportManager !== null) {
this._transportManager.stop();
Expand Down
6 changes: 2 additions & 4 deletions lib/js/src/protocol/_SdlProtocolListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,11 @@ class _SdlProtocolListener {
/**
* Safely attempts to invoke the onVehicleTypeReceived event.
* @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
* @param {String} systemSoftwareVersion - software version of the system.
* @param {String} systemHardwareVersion - hardware version of the system.
* @returns {Boolean} Return true if this session should continue, false if the session should end
*/
onVehicleTypeReceived (vehicleType, systemSoftwareVersion, systemHardwareVersion) {
onVehicleTypeReceived (vehicleType) {
if (typeof this._onVehicleTypeReceived === 'function') {
return !!this._onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion);
return !!this._onVehicleTypeReceived(vehicleType);
}
return true;
}
Expand Down
23 changes: 1 addition & 22 deletions lib/js/src/rpc/messages/RegisterAppInterfaceResponse.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable camelcase */
/*
* Copyright (c) 2021, SmartDeviceLink Consortium, Inc.
* Copyright (c) 2020, SmartDeviceLink Consortium, Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
Expand Down Expand Up @@ -431,26 +431,6 @@ class RegisterAppInterfaceResponse extends RpcResponse {
return this.getParameter(RegisterAppInterfaceResponse.KEY_SYSTEM_SOFTWARE_VERSION);
}

/**
* Set the SystemHardwareVersion
* @since SmartDeviceLink 7.1.0
* @param {String} version - The hardware version of the system - The desired SystemHardwareVersion.
* {'string_min_length': 1, 'string_max_length': 500}
* @returns {RegisterAppInterfaceResponse} - The class instance for method chaining.
*/
setSystemHardwareVersion (version) {
this.setParameter(RegisterAppInterfaceResponse.KEY_SYSTEM_HARDWARE_VERSION, version);
return this;
}

/**
* Get the SystemHardwareVersion
* @returns {String} - the KEY_SYSTEM_HARDWARE_VERSION value
*/
getSystemHardwareVersion () {
return this.getParameter(RegisterAppInterfaceResponse.KEY_SYSTEM_HARDWARE_VERSION);
}

/**
* Set the IconResumed
* @since SmartDeviceLink 5.0.0
Expand Down Expand Up @@ -489,7 +469,6 @@ RegisterAppInterfaceResponse.KEY_SUPPORTED_DIAG_MODES = 'supportedDiagModes';
RegisterAppInterfaceResponse.KEY_HMI_CAPABILITIES = 'hmiCapabilities';
RegisterAppInterfaceResponse.KEY_SDL_VERSION = 'sdlVersion';
RegisterAppInterfaceResponse.KEY_SYSTEM_SOFTWARE_VERSION = 'systemSoftwareVersion';
RegisterAppInterfaceResponse.KEY_SYSTEM_HARDWARE_VERSION = 'systemHardwareVersion';
RegisterAppInterfaceResponse.KEY_ICON_RESUMED = 'iconResumed';

export { RegisterAppInterfaceResponse };
17 changes: 13 additions & 4 deletions lib/js/src/session/_SdlSession.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ class _SdlSession {
this._sdlProtocolListener = this._setupSdlProtocolListener();

this._sdlProtocol = new _SdlProtocol(baseTransportConfig, this._sdlProtocolListener);
this._didReceiveVehicleType = false;
}

/**
Expand Down Expand Up @@ -169,16 +170,24 @@ class _SdlSession {
this._sdlSessionListener.onAuthTokenReceived(authToken, this._sessionId);
}

/**
* A way to determine if VehicleType already received
* @returns {Boolean} Return true if received
*/
didReceiveVehicleType() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There needs to be a space before the function parentheses on line 177. Use the command npm run lint to confirm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

return this._didReceiveVehicleType;
}

/**
* A way to determine if this SDL session should continue to be active while
* connected to the determined vehicle type.
* @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
* @param {String} systemSoftwareVersion - software version of the system.
* @param {String} systemHardwareVersion - hardware version of the system.
* @returns {Boolean} Return true if this session should continue, false if the session should end
*/
onVehicleTypeReceived (vehicleType, systemSoftwareVersion, systemHardwareVersion) {
return this._sdlSessionListener.onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion);
onVehicleTypeReceived (vehicleType) {
// set the flag as this event fires only once if VehicleType was received
this._didReceiveVehicleType = true;
return this._sdlSessionListener.onVehicleTypeReceived(vehicleType);
}

/** **********************************************************************************************************************************************************************
Expand Down
6 changes: 2 additions & 4 deletions lib/js/src/session/_SdlSessionListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,11 @@ class _SdlSessionListener {
/**
* Safely attempts to invoke the onVehicleTypeReceived event.
* @param {VehicleType} vehicleType - the type of vehicle that this session is currently active on.
* @param {String} systemSoftwareVersion - software version of the system.
* @param {String} systemHardwareVersion - hardware version of the system.
* @returns {Boolean} Return true if this session should continue, false if the session should end
*/
onVehicleTypeReceived (vehicleType, systemSoftwareVersion, systemHardwareVersion) {
onVehicleTypeReceived (vehicleType) {
if (typeof this._onVehicleTypeReceived === 'function') {
return !!this._onVehicleTypeReceived(vehicleType, systemSoftwareVersion, systemHardwareVersion);
return !!this._onVehicleTypeReceived(vehicleType);
}
return true;
}
Expand Down
16 changes: 16 additions & 0 deletions tests/managers/lifecycle/LifecycleManagerTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,21 @@ module.exports = function (appClient) {
Validator.assertNotNullUndefined(version.getPatchVersion());
done();
});
it('testOnVehicleTypeReceived', function (done) {
const mockVehicleType = {};
const defaultResult = true;
let actualResult = sdlManager.getFileManager()._lifecycleManager.onVehicleTypeReceived(mockVehicleType);
Validator.assertEquals(actualResult, defaultResult);

const testResult = false;
const testListener = function (vehicleType) {
return testResult;
};
sdlManager.getFileManager()._lifecycleManager.setOnVehicleTypeReceived(testListener);
actualResult = sdlManager.getFileManager()._lifecycleManager.onVehicleTypeReceived(mockVehicleType);
Validator.assertEquals(actualResult, testResult);

done();
});
});
};
7 changes: 1 addition & 6 deletions tests/node/rpc/messages/RegisterAppInterfaceResponseTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ describe('RegisterAppInterfaceResponseTests', function () {
// TODO sdlVersion https://github.com/smartdevicelink/rpc_spec/blob/version/6_0_0/MOBILE_API.xml#L4663 unused?
msg.setSystemSoftwareVersion(Test.GENERAL_STRING);
msg.setIconResumed(Test.GENERAL_BOOLEAN);
msg.setSystemHardwareVersion(Test.GENERAL_STRING);

return msg;
};
Expand All @@ -58,7 +57,6 @@ describe('RegisterAppInterfaceResponseTests', function () {
expectedParameters[RegisterAppInterfaceResponse.KEY_HMI_CAPABILITIES] = Test.GENERAL_HMICAPABILITIES.getParameters();
expectedParameters[RegisterAppInterfaceResponse.KEY_SYSTEM_SOFTWARE_VERSION] = Test.GENERAL_STRING;
expectedParameters[RegisterAppInterfaceResponse.KEY_ICON_RESUMED] = Test.GENERAL_BOOLEAN;
expectedParameters[RegisterAppInterfaceResponse.KEY_SYSTEM_HARDWARE_VERSION] = Test.GENERAL_STRING;

return expectedParameters;
};
Expand Down Expand Up @@ -96,7 +94,6 @@ describe('RegisterAppInterfaceResponseTests', function () {
const testHmiCapabilities = msg.getHmiCapabilities();
const testSystemSoftwareVersion = msg.getSystemSoftwareVersion();
const testIconResumed = msg.getIconResumed();
const testSystemHardwareVersion = msg.getSystemHardwareVersion();

// Valid Tests
Validator.validateSdlMsgVersion(Test.GENERAL_SDLMSGVERSION, testMsgVersion);
Expand All @@ -117,7 +114,6 @@ describe('RegisterAppInterfaceResponseTests', function () {
Validator.validateHMICapabilities(Test.GENERAL_HMICAPABILITIES, testHmiCapabilities);
Validator.assertEquals(Test.GENERAL_STRING, testSystemSoftwareVersion);
Validator.assertEquals(Test.GENERAL_BOOLEAN, testIconResumed);
Validator.assertEquals(Test.GENERAL_STRING, testSystemHardwareVersion);

// Invalid/Null Tests
msg = new RegisterAppInterfaceResponse();
Expand Down Expand Up @@ -145,9 +141,8 @@ describe('RegisterAppInterfaceResponseTests', function () {
Validator.assertNullOrUndefined(msg.getHmiCapabilities());
Validator.assertNullOrUndefined(msg.getSystemSoftwareVersion());
Validator.assertNullOrUndefined(msg.getIconResumed());
Validator.assertNullOrUndefined(msg.getSystemHardwareVersion());


done();
});
});
});