Addressing comments in reviewfeature/issue_506

- Reorganize BsonTags
- Change allocation of hashmap

3 files changed, 130 insertions, 37 deletions

diff --git a/sdl_android/src/main/java/com/smartdevicelink/protocol/SdlPacket.java b/sdl_android/src/main/java/com/smartdevicelink/protocol/SdlPacket.java
index 7b73894db..cc14cdac7 100644
--- a/sdl_android/src/main/java/com/smartdevicelink/protocol/SdlPacket.java
+++ b/sdl_android/src/main/java/com/smartdevicelink/protocol/SdlPacket.java
@@ -67,7 +67,7 @@ public class SdlPacket implements Parcelable{
 	int messageId;
 	int priorityCoefficient;
 	byte[] payload = null;
-	HashMap<String, Object> bsonPayload = new HashMap<>();
+	HashMap<String, Object> bsonPayload;
 
 	public SdlPacket(int version, boolean encryption, int frameType,
 			int serviceType, int frameInfo, int sessionId,
@@ -188,8 +188,8 @@ public class SdlPacket implements Parcelable{
 		return payload;
 	}
 	
-	public byte[] constructPacket(){
-		if(!bsonPayload.isEmpty()){
+	public byte[] constructPacket() {
+		if (bsonPayload != null && !bsonPayload.isEmpty()) {
 			byte[] bsonBytes = BsonEncoder.encodeToBytes(bsonPayload);
 			payload = bsonBytes;
 			dataSize = bsonBytes.length;
@@ -355,13 +355,16 @@ public class SdlPacket implements Parcelable{
     };
 
 	public void putTag(String tag, Object data){
+		if(bsonPayload == null){
+			bsonPayload = new HashMap<>();
+		}
 		bsonPayload.put(tag, data);
 	}
 
 	public Object getTag(String tag){
 		if(payload == null){
 			return null;
-		}else if(bsonPayload.isEmpty()){
+		}else if(bsonPayload == null || bsonPayload.isEmpty()){
 			bsonPayload = BsonEncoder.decodeFromBytes(payload);
 		}
 
diff --git a/sdl_android/src/main/java/com/smartdevicelink/protocol/WiProProtocol.java b/sdl_android/src/main/java/com/smartdevicelink/protocol/WiProProtocol.java
index 6053e4ccb..453209267 100644
--- a/sdl_android/src/main/java/com/smartdevicelink/protocol/WiProProtocol.java
+++ b/sdl_android/src/main/java/com/smartdevicelink/protocol/WiProProtocol.java
@@ -1,8 +1,5 @@
 package com.smartdevicelink.protocol;

 

-

-import android.util.Log;

-

 import com.smartdevicelink.SdlConnection.SdlConnection;

 import com.smartdevicelink.SdlConnection.SdlSession;

 import com.smartdevicelink.exception.SdlException;

@@ -137,7 +134,7 @@ public class WiProProtocol extends AbstractProtocol {
 	public void StartProtocolSession(SessionType sessionType) {

 		SdlPacket header = SdlPacketFactory.createStartSession(sessionType, 0x00, getMajorVersionByte(), (byte) 0x00, false);

 		if(sessionType.equals(SessionType.RPC)){ // check for RPC session

-			header.putTag(BsonTags.PROTOCOL_VERSION, MAX_PROTOCOL_VERSION.toString());

+			header.putTag(BsonTags.RPC.StartService.PROTOCOL_VERSION, MAX_PROTOCOL_VERSION.toString());

 		}

 		handlePacketToSend(header);

 	} // end-method

@@ -150,7 +147,7 @@ public class WiProProtocol extends AbstractProtocol {
 	public void EndProtocolSession(SessionType sessionType, byte sessionID, int hashId) {

 		SdlPacket header = SdlPacketFactory.createEndSession(sessionType, sessionID, hashID, getMajorVersionByte(), BitConverter.intToByteArray(hashId));

 		if(sessionType.equals(SessionType.RPC)){ // check for RPC session

-			header.putTag(BsonTags.HASH_ID, hashID);

+			header.putTag(BsonTags.RPC.EndService.HASH_ID, hashID);

 		}

 		handlePacketToSend(header);

 

@@ -452,13 +449,21 @@ public class WiProProtocol extends AbstractProtocol {
 					_messageLocks.put((byte)packet.getSessionId(), messageLock);

 				}

 				if(packet.version >= 5){

-					Object mtu = packet.getTag(BsonTags.MTU);

+					String mtuTag = null;

+					if(serviceType.equals(SessionType.RPC)){

+						mtuTag = BsonTags.RPC.StartServiceACK.MTU;

+					}else if(serviceType.equals(SessionType.PCM)){

+						mtuTag = BsonTags.PCM.StartServiceACK.MTU;

+					}else if(serviceType.equals(SessionType.NAV)){

+						mtuTag = BsonTags.VIDEO.StartServiceACK.MTU;

+					}

+					Object mtu = packet.getTag(mtuTag);

 					if(mtu!=null){

-						mtus.put(serviceType,(Long) packet.getTag(BsonTags.MTU));

+						mtus.put(serviceType,(Long) packet.getTag(mtuTag));

 					}

 					if(serviceType.equals(SessionType.RPC)){

-						hashID = (Integer) packet.getTag(BsonTags.HASH_ID);

-						Object version = packet.getTag(BsonTags.PROTOCOL_VERSION);

+						hashID = (Integer) packet.getTag(BsonTags.RPC.StartServiceACK.HASH_ID);

+						Object version = packet.getTag(BsonTags.RPC.StartServiceACK.PROTOCOL_VERSION);

 						if(version!=null){

 							//At this point we have confirmed the negotiated version between the module and the proxy

 							protocolVersion = new Version((String)version);

@@ -474,15 +479,16 @@ public class WiProProtocol extends AbstractProtocol {
 				handleProtocolSessionStarted(serviceType,(byte) packet.getSessionId(), getMajorVersionByte(), "", hashID, packet.isEncrypted());

 			} else if (frameInfo == FrameDataControlFrameType.StartSessionNACK.getValue()) {

 				if(packet.version >= 5){

-					if(serviceType.equals(SessionType.RPC) || serviceType.equals(SessionType.PCM) ||

-							serviceType.equals(SessionType.NAV)) {

-						List<String> rejectedParams = (List<String>) packet.getTag(BsonTags.REJECTED_PARAMS);

-						if(rejectedParams != null) {

-							for (String s : rejectedParams) {

-								Log.e("Rejected BSON Parameter", s);

-							}

-						}

+					String rejectedTag = null;

+					if(serviceType.equals(SessionType.RPC)){

+						rejectedTag = BsonTags.RPC.StartServiceNAK.REJECTED_PARAMS;

+					}else if(serviceType.equals(SessionType.PCM)){

+						rejectedTag = BsonTags.PCM.StartServiceNAK.REJECTED_PARAMS;

+					}else if(serviceType.equals(SessionType.NAV)){

+						rejectedTag = BsonTags.VIDEO.StartServiceNAK.REJECTED_PARAMS;

 					}

+					List<String> rejectedParams = (List<String>) packet.getTag(rejectedTag);

+					// TODO: Pass these back

 				}

 				if (serviceType.eq(SessionType.NAV) || serviceType.eq(SessionType.PCM)) {

 					handleProtocolSessionNACKed(serviceType, (byte)packet.getSessionId(), getMajorVersionByte(), "");

@@ -499,15 +505,16 @@ public class WiProProtocol extends AbstractProtocol {
 				handleProtocolSessionEnded(serviceType, (byte)packet.getSessionId(), "");

 			} else if (frameInfo == FrameDataControlFrameType.EndSessionNACK.getValue()) {

 				if(packet.version >= 5){

-					if(serviceType.equals(SessionType.RPC) || serviceType.equals(SessionType.PCM) ||

-							serviceType.equals(SessionType.NAV)) {

-						List<String> rejectedParams = (List<String>) packet.getTag(BsonTags.REJECTED_PARAMS);

-						if(rejectedParams != null) {

-							for (String s : rejectedParams) {

-								Log.e("Rejected BSON Parameter", s);

-							}

-						}

+					String rejectedTag = null;

+					if(serviceType.equals(SessionType.RPC)){

+						rejectedTag = BsonTags.RPC.EndServiceNAK.REJECTED_PARAMS;

+					}else if(serviceType.equals(SessionType.PCM)){

+						rejectedTag = BsonTags.PCM.EndServiceNAK.REJECTED_PARAMS;

+					}else if(serviceType.equals(SessionType.NAV)){

+						rejectedTag = BsonTags.VIDEO.EndServiceNAK.REJECTED_PARAMS;

 					}

+					List<String> rejectedParams = (List<String>) packet.getTag(rejectedTag);

+					// TODO: Pass these back

 				}

 				handleProtocolSessionEndedNACK(serviceType, (byte)packet.getSessionId(), "");

 			} else if (frameInfo == FrameDataControlFrameType.ServiceDataACK.getValue()) {

diff --git a/sdl_android/src/main/java/com/smartdevicelink/protocol/enums/BsonTags.java b/sdl_android/src/main/java/com/smartdevicelink/protocol/enums/BsonTags.java
index 60d850b77..a40e6a8d5 100644
--- a/sdl_android/src/main/java/com/smartdevicelink/protocol/enums/BsonTags.java
+++ b/sdl_android/src/main/java/com/smartdevicelink/protocol/enums/BsonTags.java
@@ -1,12 +1,95 @@
 package com.smartdevicelink.protocol.enums;
 
 public class BsonTags {
-	public static final String PROTOCOL_VERSION = "protocolVersion";
-	public static final String HASH_ID = "hashId";
-	public static final String MTU = "mtu";
-	public static final String HEIGHT = "height";
-	public static final String WIDTH = "width";
-	public static final String VIDEO_PROTOCOL = "videoProtocol";
-	public static final String VIDEO_CODEC = "videoCodec";
-	public static final String REJECTED_PARAMS = "rejectedParams";
+	public static class RPC{
+		public static class StartService {
+			// The max version of the protocol supported by client requesting service to start.
+			// Must be in the format "Major.Minor.Patch"
+			public static final String PROTOCOL_VERSION = "protocolVersion";
+		}
+		public static class StartServiceACK {
+			// Hash ID to identify this service and used when sending an EndService control frame
+			public static final String HASH_ID = "hashId";
+			// Max transport unit to be used for this service
+			public static final String MTU = "mtu";
+			// The negotiated version of the protocol. Must be in the format "Major.Minor.Patch"
+			public static final String PROTOCOL_VERSION = "protocolVersion";
+		}
+		public static class StartServiceNAK {
+			// An array of rejected parameters
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+		public static class EndService {
+			// Hash ID supplied in the StartServiceACK for this service type
+			public static final String HASH_ID = "hashId";
+		}
+		public static class EndServiceACK {}
+		public static class EndServiceNAK {
+			// An array of rejected parameters such as: [hashId]
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+	}
+	public static class PCM{
+		public static class StartService {}
+		public static class StartServiceACK {
+			// Hash ID to identify this service and used when sending an EndService control frame
+			public static final String HASH_ID = "hashId";
+			// Max transport unit to be used for this service. If not included the client
+			// should use the one set via the RPC service or protocol version default.
+			public static final String MTU = "mtu";
+		}
+		public static class StartServiceNAK {
+			// An array of rejected parameters such as: [videoProtocol, videoProtocol]
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+		public static class EndService {
+			// Hash ID supplied in the StartServiceACK for this service type
+			public static final String HASH_ID = "hashId";
+		}
+		public static class EndServiceACK {}
+		public static class EndServiceNAK {
+			// An array of rejected parameters such as: [hashId]
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+	}
+	public static class VIDEO{
+		public static class StartService {
+			// Desired height in pixels from the client requesting the video service to start
+			public static final String HEIGHT = "height";
+			// Desired width in pixels from the client requesting the video service to start
+			public static final String WIDTH = "width";
+			// Desired video protocol to be used
+			public static final String VIDEO_PROTOCOL = "videoProtocol";
+			// Desired video codec to be used
+			public static final String VIDEO_CODEC = "videoCodec";
+		}
+		public static class StartServiceACK {
+			// Hash ID to identify this service and used when sending an EndService control frame
+			public static final String HASH_ID = "hashId";
+			// Max transport unit to be used for this service. If not included the client should use
+			// the one set via the RPC service or protocol version default.
+			public static final String MTU = "mtu";
+			// Accepted height in pixels from the client requesting the video service to start
+			public static final String HEIGHT = "height";
+			// Accepted width in pixels from the client requesting the video service to start
+			public static final String WIDTH = "width";
+			// Accepted video protocol to be used
+			public static final String VIDEO_PROTOCOL = "videoProtocol";
+			// Accepted video codec to be used
+			public static final String VIDEO_CODEC = "videoCodec";
+		}
+		public static class StartServiceNAK {
+			// An array of rejected parameters such as: [videoProtocol, videoProtocol]
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+		public static class EndService {
+			// Hash ID supplied in the StartServiceACK for this service type
+			public static final String HASH_ID = "hashId";
+		}
+		public static class EndServiceACK {}
+		public static class EndServiceNAK {
+			// An array of rejected parameters such as: [hashId]
+			public static final String REJECTED_PARAMS = "rejectedParams";
+		}
+	}
 }