From ae963f1cd514d9d7c1e91fe8b74502cacd09997b Mon Sep 17 00:00:00 2001
From: Etienne Ruffieux <eruffieux@google.com>
Date: Thu, 11 May 2023 18:38:19 -0700
Subject: [PATCH] Add direct Telecom check for call state in HFP service

Telecom is using BluetoothHeadset#connectAudio to start
SCO, but HeadsetService checks if a call is ongoing (or
ringing) before sending the command to the native stack.

It can happen that BluetoothInCallService isn't yet
connected to Telecom service when that connectAudio
is called, resulting in SCO not started at that time
but when BluetoothInCallService is connected instead, which
can be an issue as we will return false at Telecom's
connect audio request.

Bug: 274081739
Tag: #feature
Test: manual
Change-Id: I74b8f56912cbd03e68321db1c5389e1374bca9bd
---
 .../bluetooth/hfp/HeadsetObjectsFactory.java  |  8 ++++
 .../android/bluetooth/hfp/HeadsetService.java | 44 ++++++++++++++++---
 .../bluetooth/hfp/HeadsetServiceTest.java     | 24 ++++++++--
 3 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/android/app/src/com/android/bluetooth/hfp/HeadsetObjectsFactory.java b/android/app/src/com/android/bluetooth/hfp/HeadsetObjectsFactory.java
index 3f5867fbd77..93f5242d7e6 100644
--- a/android/app/src/com/android/bluetooth/hfp/HeadsetObjectsFactory.java
+++ b/android/app/src/com/android/bluetooth/hfp/HeadsetObjectsFactory.java
@@ -17,6 +17,7 @@ package com.android.bluetooth.hfp;
 
 import android.bluetooth.BluetoothDevice;
 import android.os.Looper;
+import android.telecom.TelecomManager;
 import android.util.Log;
 
 import com.android.bluetooth.Utils;
@@ -104,4 +105,11 @@ public class HeadsetObjectsFactory {
     public HeadsetNativeInterface getNativeInterface() {
         return HeadsetNativeInterface.getInstance();
     }
+
+    /**
+     * @return an instance of Telecom manager
+     */
+    public TelecomManager getTelecomManager(HeadsetService service) {
+        return service.getSystemService(TelecomManager.class);
+    }
 }
diff --git a/android/app/src/com/android/bluetooth/hfp/HeadsetService.java b/android/app/src/com/android/bluetooth/hfp/HeadsetService.java
index eccc79b3cd1..b2369ec27e2 100644
--- a/android/app/src/com/android/bluetooth/hfp/HeadsetService.java
+++ b/android/app/src/com/android/bluetooth/hfp/HeadsetService.java
@@ -47,6 +47,8 @@ import android.os.SystemProperties;
 import android.os.UserHandle;
 import android.sysprop.BluetoothProperties;
 import android.telecom.PhoneAccount;
+import android.telecom.TelecomManager;
+import android.telephony.TelephonyManager;
 import android.util.Log;
 
 import com.android.bluetooth.BluetoothMetricsProto;
@@ -141,6 +143,8 @@ public class HeadsetService extends ProfileService {
     private VoiceRecognitionTimeoutEvent mVoiceRecognitionTimeoutEvent;
     // Timeout when voice recognition is started by remote device
     @VisibleForTesting static int sStartVrTimeoutMs = 5000;
+    // Used to retrieve call state before BluetoothInCallService is connected
+    public TelecomManager mTelecomManager;
     private ArrayList<StateMachineTask> mPendingClccResponses = new ArrayList<>();
     private boolean mStarted;
     private boolean mCreated;
@@ -191,13 +195,15 @@ public class HeadsetService extends ProfileService {
         mNativeInterface = HeadsetObjectsFactory.getInstance().getNativeInterface();
         // Add 1 to allow a pending device to be connecting or disconnecting
         mNativeInterface.init(mMaxHeadsetConnections + 1, isInbandRingingEnabled());
-        // Step 5: Check if state machine table is empty, crash if not
+        // Step 5: Get TelecomManager
+        mTelecomManager = HeadsetObjectsFactory.getInstance().getTelecomManager(this);
+        // Step 6: Check if state machine table is empty, crash if not
         if (mStateMachines.size() > 0) {
             throw new IllegalStateException(
                     "start(): mStateMachines is not empty, " + mStateMachines.size()
                             + " is already created. Was stop() called properly?");
         }
-        // Step 6: Setup broadcast receivers
+        // Step 7: Setup broadcast receivers
         IntentFilter filter = new IntentFilter();
         filter.setPriority(IntentFilter.SYSTEM_HIGH_PRIORITY);
         filter.addAction(Intent.ACTION_BATTERY_CHANGED);
@@ -205,7 +211,7 @@ public class HeadsetService extends ProfileService {
         filter.addAction(BluetoothDevice.ACTION_CONNECTION_ACCESS_REPLY);
         filter.addAction(BluetoothDevice.ACTION_BOND_STATE_CHANGED);
         registerReceiver(mHeadsetReceiver, filter);
-        // Step 7: Mark service as started
+        // Step 8: Mark service as started
         mStarted = true;
         BluetoothDevice activeDevice = getActiveDevice();
         String deviceAddress = activeDevice != null ?
@@ -2038,9 +2044,35 @@ public class HeadsetService extends ProfileService {
         }
     }
 
-    private boolean shouldCallAudioBeActive() {
-        return mSystemInterface.isInCall() || (mSystemInterface.isRinging()
-                && isInbandRingingEnabled());
+    /**
+     * Check if there is an active or ringing call
+     *
+     * <p>This will check for both {@link HeadsetSystemInterface} state and {@link TelecomManager}
+     * state as {@link HeadsetSystemInterface} values are provided by {@link BluetoothInCallService}
+     * which can be bound to Telecom after the call to {@link #connectAudio} is made.
+     *
+     * @return whether there is an active or ringing call
+     */
+    @VisibleForTesting
+    boolean shouldCallAudioBeActive() {
+        int telecomCallState = TelephonyManager.CALL_STATE_IDLE;
+        if (mTelecomManager != null) {
+            telecomCallState = mTelecomManager.getCallState();
+        }
+        Log.i(
+                TAG,
+                "shouldCallAudioBeActive: "
+                        + "Telecom state: "
+                        + telecomCallState
+                        + ", HeadsetSystemInterface inCall: "
+                        + mSystemInterface.isInCall()
+                        + ", isRinging: "
+                        + mSystemInterface.isRinging());
+        return ((mSystemInterface.isInCall()
+                        || telecomCallState == TelephonyManager.CALL_STATE_OFFHOOK)
+                || ((mSystemInterface.isRinging()
+                                || telecomCallState == TelephonyManager.CALL_STATE_RINGING)
+                        && isInbandRingingEnabled()));
     }
 
     /**
diff --git a/android/app/tests/unit/src/com/android/bluetooth/hfp/HeadsetServiceTest.java b/android/app/tests/unit/src/com/android/bluetooth/hfp/HeadsetServiceTest.java
index 11177b6cd45..a72703a7ff8 100644
--- a/android/app/tests/unit/src/com/android/bluetooth/hfp/HeadsetServiceTest.java
+++ b/android/app/tests/unit/src/com/android/bluetooth/hfp/HeadsetServiceTest.java
@@ -25,19 +25,19 @@ import android.bluetooth.BluetoothHeadset;
 import android.bluetooth.BluetoothProfile;
 import android.bluetooth.BluetoothStatusCodes;
 import android.bluetooth.BluetoothUuid;
-import android.bluetooth.IBluetoothHeadset;
 import android.content.Context;
 import android.media.AudioManager;
 import android.os.ParcelUuid;
 import android.os.RemoteException;
 import android.os.SystemClock;
+import android.telecom.TelecomManager;
+import android.telephony.TelephonyManager;
 
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.MediumTest;
 import androidx.test.rule.ServiceTestRule;
 import androidx.test.runner.AndroidJUnit4;
 
-import com.android.bluetooth.R;
 import com.android.bluetooth.TestUtils;
 import com.android.bluetooth.btservice.AdapterService;
 import com.android.bluetooth.btservice.storage.DatabaseManager;
@@ -45,7 +45,6 @@ import com.android.bluetooth.btservice.storage.DatabaseManager;
 import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Assert;
-import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -86,6 +85,7 @@ public class HeadsetServiceTest {
     @Mock private HeadsetSystemInterface mSystemInterface;
     @Mock private AudioManager mAudioManager;
     @Mock private HeadsetPhoneState mPhoneState;
+    @Mock private TelecomManager mTelecomManager;
 
     @Before
     public void setUp() throws Exception {
@@ -143,12 +143,15 @@ public class HeadsetServiceTest {
         }).when(mObjectsFactory).makeStateMachine(any(), any(), any(), any(), any(), any());
         doReturn(mSystemInterface).when(mObjectsFactory).makeSystemInterface(any());
         doReturn(mNativeInterface).when(mObjectsFactory).getNativeInterface();
+        doReturn(mTelecomManager).when(mObjectsFactory).getTelecomManager(any());
+
         TestUtils.startService(mServiceRule, HeadsetService.class);
         mHeadsetService = HeadsetService.getHeadsetService();
         Assert.assertNotNull(mHeadsetService);
         verify(mAdapterService).notifyActivityAttributionInfo(any(), any());
         verify(mObjectsFactory).makeSystemInterface(mHeadsetService);
         verify(mObjectsFactory).getNativeInterface();
+        verify(mObjectsFactory).getTelecomManager(any());
         mHeadsetService.setForceScoAudio(true);
     }
 
@@ -702,6 +705,21 @@ public class HeadsetServiceTest {
                 eq(HeadsetStateMachine.CONNECT_AUDIO), any());
     }
 
+    /**
+     * Test to verify that {@link HeadsetService#ShouldCallAudioBeActive()} succeeds even when
+     * {@link HeadsetSystemInterface} has not all call infos.
+     */
+    @Test
+    public void testShouldCallAudioBeActive_missingInfo() {
+        when(mTelecomManager.getCallState()).thenReturn(TelephonyManager.CALL_STATE_IDLE);
+        Assert.assertFalse(mHeadsetService.shouldCallAudioBeActive());
+        when(mTelecomManager.getCallState()).thenReturn(TelephonyManager.CALL_STATE_OFFHOOK);
+        Assert.assertTrue(mHeadsetService.shouldCallAudioBeActive());
+        when(mTelecomManager.getCallState()).thenReturn(TelephonyManager.CALL_STATE_RINGING);
+        Assert.assertTrue(mHeadsetService.shouldCallAudioBeActive());
+        when(mTelecomManager.getCallState()).thenReturn(TelephonyManager.CALL_STATE_IDLE);
+    }
+
     /**
      * Verifies that phone state change will trigger a system-wide saving of call state even when
      * no device is connected
-- 
GitLab