SAP: Fix missing attribute in disconnect_ind

The internal TEST flag was removing too much code.
To make it easier to run test, and avoid future issues
with removing needed code, the TEST flag is no longer used
to remove code.

This bug was noticed when running the PTS testcases without the
PTS_TEST flag set. It is assumed that this fix will make
TC_SERVER_DCN_BV_02_I work without the PTS flag, but unless PTS
have updated their test case to accept both a gracefull disconnect
and a forced disconnect, TC_SERVER_DCN_BV_03_I will still fail in
a production build. The SAP spec suggest to use a gracefull disconnect
when initiating a disconnect from a server, hence it is not a good
idea to expose a force disconnect option to the used without first
having tried a gracefull disconnect. Hence the PTS test case
implementation is in conflict with the intented use specified in
the SAP specification.

Without this fix a build with PTS_TEST=false will not work as
intented.

PTS_TEST replaced with system property bt.sap.pts. Set it to true
to enable the force disconnect button.

Bug: 23614694
Signed-off-by: Casper Bonde <c.bonde@samsung.com>
Change-Id: I776a53d6e786c21000ef62094f9a9948ee5880b8

3 files changed, 93 insertions, 97 deletions

diff --git a/src/com/android/bluetooth/sap/SapMessage.java b/src/com/android/bluetooth/sap/SapMessage.java
index c4f5afb4e..e836fba01 100644
--- a/src/com/android/bluetooth/sap/SapMessage.java
+++ b/src/com/android/bluetooth/sap/SapMessage.java
@@ -28,7 +28,7 @@ public class SapMessage {
     public static final String TAG = "SapMessage";
     public static final boolean DEBUG = SapService.DEBUG;
     public static final boolean VERBOSE = SapService.VERBOSE;
-    public static final boolean TEST = SapService.PTS_TEST;
+    public static final boolean TEST = false;
 
     /* Message IDs - SAP specification */
     public static final int ID_CONNECT_REQ        = 0x00;
@@ -404,10 +404,8 @@ public class SapMessage {
         case ID_DISCONNECT_REQ:    /* No params */
             break;
         default:
-            if(TEST == false) {
-                Log.e(TAG, "Unknown request type");
-                return null;
-            }
+            Log.e(TAG, "Unknown request type");
+            return null;
         }
         return newMessage;
     }
@@ -504,91 +502,77 @@ public class SapMessage {
                 }
                 break;
             case PARAM_CONNECTION_STATUS_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    if(paramLength != PARAM_CONNECTION_STATUS_LENGTH) {
-                        Log.e(TAG, "Received PARAM_CONNECTION_STATUS with wrong length: " +
-                                paramLength + " skipping this parameter.");
-                        skip(is, paramLength + skipLen);
-                        success = false;
-                    } else {
-                        mConnectionStatus = is.read();
-                        skip(is, 4 - PARAM_CONNECTION_STATUS_LENGTH);
-                    }
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                if(paramLength != PARAM_CONNECTION_STATUS_LENGTH) {
+                    Log.e(TAG, "Received PARAM_CONNECTION_STATUS with wrong length: " +
+                            paramLength + " skipping this parameter.");
+                    skip(is, paramLength + skipLen);
+                    success = false;
+                } else {
+                    mConnectionStatus = is.read();
+                    skip(is, 4 - PARAM_CONNECTION_STATUS_LENGTH);
+                }
+                break;
             case PARAM_CARD_READER_STATUS_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    if(paramLength != PARAM_CARD_READER_STATUS_LENGTH) {
-                        Log.e(TAG, "Received PARAM_CARD_READER_STATUS with wrong length: " +
-                                paramLength + " skipping this parameter.");
-                        skip(is, paramLength + skipLen);
-                        success = false;
-                    } else {
-                        mCardReaderStatus = is.read();
-                        skip(is, 4 - PARAM_CARD_READER_STATUS_LENGTH);
-                    }
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                if(paramLength != PARAM_CARD_READER_STATUS_LENGTH) {
+                    Log.e(TAG, "Received PARAM_CARD_READER_STATUS with wrong length: " +
+                            paramLength + " skipping this parameter.");
+                    skip(is, paramLength + skipLen);
+                    success = false;
+                } else {
+                    mCardReaderStatus = is.read();
+                    skip(is, 4 - PARAM_CARD_READER_STATUS_LENGTH);
+                }
+                break;
             case PARAM_STATUS_CHANGE_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    if(paramLength != PARAM_STATUS_CHANGE_LENGTH) {
-                        Log.e(TAG, "Received PARAM_STATUS_CHANGE with wrong length: " +
-                                paramLength + " skipping this parameter.");
-                        skip(is, paramLength + skipLen);
-                        success = false;
-                    } else {
-                        mStatusChange = is.read();
-                        skip(is, 4 - PARAM_STATUS_CHANGE_LENGTH);
-                    }
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                if(paramLength != PARAM_STATUS_CHANGE_LENGTH) {
+                    Log.e(TAG, "Received PARAM_STATUS_CHANGE with wrong length: " +
+                            paramLength + " skipping this parameter.");
+                    skip(is, paramLength + skipLen);
+                    success = false;
+                } else {
+                    mStatusChange = is.read();
+                    skip(is, 4 - PARAM_STATUS_CHANGE_LENGTH);
+                }
+                break;
             case PARAM_RESULT_CODE_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    if(paramLength != PARAM_RESULT_CODE_LENGTH) {
-                        Log.e(TAG, "Received PARAM_RESULT_CODE with wrong length: " +
-                                paramLength + " skipping this parameter.");
-                        skip(is, paramLength + skipLen);
-                        success = false;
-                    } else {
-                        mResultCode = is.read();
-                        skip(is, 4 - PARAM_RESULT_CODE_LENGTH);
-                    }
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                if(paramLength != PARAM_RESULT_CODE_LENGTH) {
+                    Log.e(TAG, "Received PARAM_RESULT_CODE with wrong length: " +
+                            paramLength + " skipping this parameter.");
+                    skip(is, paramLength + skipLen);
+                    success = false;
+                } else {
+                    mResultCode = is.read();
+                    skip(is, 4 - PARAM_RESULT_CODE_LENGTH);
+                }
+                break;
             case PARAM_DISCONNECT_TYPE_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    if(paramLength != PARAM_DISCONNECT_TYPE_LENGTH) {
-                        Log.e(TAG, "Received PARAM_DISCONNECT_TYPE_ID with wrong length: " +
-                                paramLength + " skipping this parameter.");
-                        skip(is, paramLength + skipLen);
-                        success = false;
-                    } else {
-                        mDisconnectionType = is.read();
-                        skip(is, 4 - PARAM_DISCONNECT_TYPE_LENGTH);
-                    }
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                if(paramLength != PARAM_DISCONNECT_TYPE_LENGTH) {
+                    Log.e(TAG, "Received PARAM_DISCONNECT_TYPE_ID with wrong length: " +
+                            paramLength + " skipping this parameter.");
+                    skip(is, paramLength + skipLen);
+                    success = false;
+                } else {
+                    mDisconnectionType = is.read();
+                    skip(is, 4 - PARAM_DISCONNECT_TYPE_LENGTH);
+                }
+                break;
             case PARAM_RESPONSE_APDU_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    mApduResp = new byte[paramLength];
-                    read(is, mApduResp);
-                    skip(is, skipLen);
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                mApduResp = new byte[paramLength];
+                read(is, mApduResp);
+                skip(is, skipLen);
+                break;
             case PARAM_ATR_ID:
-                // not needed - server -> client
-                if(TEST) {
-                    mAtr = new byte[paramLength];
-                    read(is, mAtr);
-                    skip(is, skipLen);
-                    break;
-                } // Fall through if TEST == false
+                // not needed for server role, but used for module test
+                mAtr = new byte[paramLength];
+                read(is, mAtr);
+                skip(is, skipLen);
+                break;
             default:
                 Log.e(TAG, "Received unknown parameter ID: " + paramId + " length: " +
                         paramLength + " skipping this parameter.");
@@ -677,7 +661,7 @@ public class SapMessage {
             writeParameter(os, PARAM_RESULT_CODE_ID, mResultCode,
                             PARAM_RESULT_CODE_LENGTH);
         }
-        if(mDisconnectionType != INVALID_VALUE && TEST) {
+        if(mDisconnectionType != INVALID_VALUE) {
             writeParameter(os, PARAM_DISCONNECT_TYPE_ID, mDisconnectionType,
                             PARAM_DISCONNECT_TYPE_LENGTH);
         }
@@ -689,14 +673,14 @@ public class SapMessage {
             writeParameter(os, PARAM_STATUS_CHANGE_ID, mStatusChange,
                             PARAM_STATUS_CHANGE_LENGTH);
         }
-        if(mTransportProtocol != INVALID_VALUE && TEST) {
+        if(mTransportProtocol != INVALID_VALUE) {
             writeParameter(os, PARAM_TRANSPORT_PROTOCOL_ID, mTransportProtocol,
                             PARAM_TRANSPORT_PROTOCOL_LENGTH);
         }
-        if(mApdu != null && TEST) {
+        if(mApdu != null) {
             writeParameter(os, PARAM_COMMAND_APDU_ID, mApdu);
         }
-        if(mApdu7816 != null  && TEST) {
+        if(mApdu7816 != null) {
             writeParameter(os, PARAM_COMMAND_APDU7816_ID, mApdu7816);
         }
         if(mApduResp != null) {
@@ -847,10 +831,8 @@ public class SapMessage {
             break;
         }
         default:
-            if(TEST == false) {
-                Log.e(TAG, "Unknown request type");
-                throw new IllegalArgumentException();
-            }
+            Log.e(TAG, "Unknown request type");
+            throw new IllegalArgumentException();
         }
         /* Update the ongoing requests queue */
         if(mClearRilQueue == true) {
@@ -1223,7 +1205,7 @@ public class SapMessage {
 
 
     public static String getMsgTypeName(int msgType) {
-        if(TEST || VERBOSE) {
+        if(DEBUG || VERBOSE) {
             switch (msgType)
             {
                 case ID_CONNECT_REQ: return "ID_CONNECT_REQ";
diff --git a/src/com/android/bluetooth/sap/SapServer.java b/src/com/android/bluetooth/sap/SapServer.java
index e38024b26..87856b3d8 100644
--- a/src/com/android/bluetooth/sap/SapServer.java
+++ b/src/com/android/bluetooth/sap/SapServer.java
@@ -8,6 +8,7 @@ import java.io.OutputStream;
 import java.util.concurrent.CountDownLatch;
 
 import com.android.bluetooth.R;
+
 import android.app.AlarmManager;
 import android.app.Notification;
 import android.app.NotificationManager;
@@ -26,8 +27,10 @@ import android.os.Looper;
 import android.os.Message;
 import android.os.Parcel;
 import android.os.SystemClock;
+import android.os.SystemProperties;
 import android.telephony.TelephonyManager;
 import android.util.Log;
+
 //import com.android.internal.telephony.RIL;
 import com.google.protobuf.micro.CodedOutputStreamMicro;
 
@@ -48,7 +51,6 @@ public class SapServer extends Thread implements Callback {
     private static final String TAG_HANDLER = "SapServerHandler";
     public static final boolean DEBUG = SapService.DEBUG;
     public static final boolean VERBOSE = SapService.VERBOSE;
-    public static final boolean PTS_TEST = SapService.PTS_TEST;
 
     private enum SAP_STATE    {
         DISCONNECTED, CONNECTING, CONNECTING_CALL_ONGOING, CONNECTED,
@@ -212,6 +214,13 @@ public class SapServer extends Thread implements Callback {
         String title, text, button, ticker;
         Notification notification;
         if(VERBOSE) Log.i(TAG, "setNotification type: " + type);
+        /* For PTS TC_SERVER_DCN_BV_03_I we need to expose the option to send immediate disconnect
+         * without first sending a graceful disconnect.
+         * To enable this option set
+         * bt.sap.pts="true" */
+        String pts_enabled = SystemProperties.get("bt.sap.pts");
+        Boolean pts_test = Boolean.parseBoolean(pts_enabled);
+
         /* put notification up for the user to be able to disconnect from the client*/
         Intent sapDisconnectIntent = new Intent(SapServer.SAP_DISCONNECT_ACTION);
         if(type == SapMessage.DISC_GRACEFULL){
@@ -225,7 +234,7 @@ public class SapServer extends Thread implements Callback {
             text = mContext.getString(R.string.bluetooth_sap_notif_disconnecting);
             ticker = mContext.getString(R.string.bluetooth_sap_notif_ticker);
         }
-        if(!PTS_TEST)
+        if(!pts_test)
         {
             sapDisconnectIntent.putExtra(SapServer.SAP_DISCONNECT_TYPE_EXTRA, type);
             PendingIntent pIntentDisconnect = PendingIntent.getBroadcast(mContext, type,
@@ -822,12 +831,18 @@ public class SapServer extends Thread implements Callback {
         if(VERBOSE) Log.i(TAG_HANDLER, "sendRilMessage() - "
                 + SapMessage.getMsgTypeName(sapMsg.getMsgType()));
         try {
-            if (mRilBtOutStream != null)
+            if(mRilBtOutStream != null) {
                 sapMsg.writeReqToStream(mRilBtOutStream);
+            } /* Else SAP was enabled on a build that did not support SAP, which we will not
+               * handle. */
         } catch (IOException e) {
             Log.e(TAG_HANDLER, "Unable to send message to RIL", e);
             SapMessage errorReply = new SapMessage(SapMessage.ID_ERROR_RESP);
             sendClientMessage(errorReply);
+        } catch (IllegalArgumentException e) {
+            Log.e(TAG_HANDLER, "Unable encode message", e);
+            SapMessage errorReply = new SapMessage(SapMessage.ID_ERROR_RESP);
+            sendClientMessage(errorReply);
         }
     }
 
diff --git a/src/com/android/bluetooth/sap/SapService.java b/src/com/android/bluetooth/sap/SapService.java
index d5caf8065..23d69ff03 100644
--- a/src/com/android/bluetooth/sap/SapService.java
+++ b/src/com/android/bluetooth/sap/SapService.java
@@ -47,7 +47,6 @@ public class SapService extends ProfileService {
     private static final String TAG = "SapService";
     public static final boolean DEBUG = false;
     public static final boolean VERBOSE = false;
-    public static final boolean PTS_TEST = false;
 
     /* Message ID's */
     private static final int START_LISTENER = 1;