Suppress DTMF tone start when a call is already held.
It is possible that the Dialer UX can send a DTMF tone start when a call
is already held. Detecting this and skipping the DTMF tone play in that
circumstance.
Test: Manually verified issue; added new unit tests.
Bug: 33771011
Change-Id: Ib063237d768fc2f6c512d81d356ad0874410ce7b
diff --git a/src/com/android/server/telecom/Call.java b/src/com/android/server/telecom/Call.java
index c17c447..35ba93a 100644
--- a/src/com/android/server/telecom/Call.java
+++ b/src/com/android/server/telecom/Call.java
@@ -92,6 +92,9 @@
private static final int RTT_PIPE_WRITE_SIDE_INDEX = 1;
private static final int INVALID_RTT_REQUEST_ID = -1;
+
+ private static final char NO_DTMF_TONE = '\0';
+
/**
* Listener for events on the call.
*/
@@ -403,6 +406,7 @@
private final String mId;
private String mConnectionId;
private Analytics.CallInfo mAnalytics;
+ private char mPlayingDtmfTone;
private boolean mWasConferencePreviouslyMerged = false;
private boolean mWasHighDefAudio = false;
@@ -1571,7 +1575,8 @@
/**
* Plays the specified DTMF tone.
*/
- void playDtmfTone(char digit) {
+ @VisibleForTesting
+ public void playDtmfTone(char digit) {
if (mConnectionService == null) {
Log.w(this, "playDtmfTone() request on a call without a connection service.");
} else {
@@ -1579,12 +1584,14 @@
mConnectionService.playDtmfTone(this, digit);
Log.addEvent(this, LogUtils.Events.START_DTMF, Log.pii(digit));
}
+ mPlayingDtmfTone = digit;
}
/**
* Stops playing any currently playing DTMF tone.
*/
- void stopDtmfTone() {
+ @VisibleForTesting
+ public void stopDtmfTone() {
if (mConnectionService == null) {
Log.w(this, "stopDtmfTone() request on a call without a connection service.");
} else {
@@ -1592,6 +1599,15 @@
Log.addEvent(this, LogUtils.Events.STOP_DTMF);
mConnectionService.stopDtmfTone(this);
}
+ mPlayingDtmfTone = NO_DTMF_TONE;
+ }
+
+ /**
+ * @return {@code true} if a DTMF tone has been started via {@link #playDtmfTone(char)} but has
+ * not been stopped via {@link #stopDtmfTone()}, {@code false} otherwise.
+ */
+ boolean isDtmfTonePlaying() {
+ return mPlayingDtmfTone != NO_DTMF_TONE;
}
/**
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index 81d6251..34f299d 100644
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -1492,8 +1492,12 @@
if (!mCalls.contains(call)) {
Log.i(this, "Request to play DTMF in a non-existent call %s", call);
} else {
- call.playDtmfTone(digit);
- mDtmfLocalTonePlayer.playTone(call, digit);
+ if (call.getState() != CallState.ON_HOLD) {
+ call.playDtmfTone(digit);
+ mDtmfLocalTonePlayer.playTone(call, digit);
+ } else {
+ Log.i(this, "Request to play DTMF tone for held call %s", call.getId());
+ }
}
}
@@ -1791,7 +1795,8 @@
maybeMoveToSpeakerPhone(call);
}
- void markCallAsOnHold(Call call) {
+ @VisibleForTesting
+ public void markCallAsOnHold(Call call) {
setCallState(call, CallState.ON_HOLD, "on-hold set explicitly");
}
@@ -2262,6 +2267,12 @@
Log.i(this, "setCallState %s -> %s, call: %s", CallState.toString(oldState),
CallState.toString(newState), call);
if (newState != oldState) {
+ // If the call switches to held state while a DTMF tone is playing, stop the tone to
+ // ensure that the tone generator stops playing the tone.
+ if (newState == CallState.ON_HOLD && call.isDtmfTonePlaying()) {
+ stopDtmfTone(call);
+ }
+
// Unfortunately, in the telephony world the radio is king. So if the call notifies
// us that the call is in a particular state, we allow it even if it doesn't make
// sense (e.g., STATE_ACTIVE -> STATE_RINGING).
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index 02d8222..714261a 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -18,8 +18,11 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.anyChar;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import android.content.ComponentName;
@@ -37,6 +40,7 @@
import com.android.server.telecom.CallerInfoAsyncQueryFactory;
import com.android.server.telecom.CallsManager;
import com.android.server.telecom.ClockProxy;
+import com.android.server.telecom.ConnectionServiceWrapper;
import com.android.server.telecom.ContactsAsyncHelper;
import com.android.server.telecom.DefaultDialerCache;
import com.android.server.telecom.EmergencyCallHelper;
@@ -59,6 +63,7 @@
import com.android.server.telecom.bluetooth.BluetoothRouteManager;
import org.mockito.Mock;
+import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import java.util.ArrayList;
@@ -334,6 +339,65 @@
assertTrue(accounts.contains(SIM_2_HANDLE));
}
+ /**
+ * Verifies that an active call will result in playing a DTMF tone when requested.
+ * @throws Exception
+ */
+ @MediumTest
+ public void testPlayDtmfWhenActive() throws Exception {
+ Call callSpy = addSpyCall();
+ mCallsManager.playDtmfTone(callSpy, '1');
+ verify(callSpy).playDtmfTone(anyChar());
+ }
+
+ /**
+ * Verifies that DTMF requests are suppressed when a call is held.
+ * @throws Exception
+ */
+ @MediumTest
+ public void testSuppessDtmfWhenHeld() throws Exception {
+ Call callSpy = addSpyCall();
+ callSpy.setState(CallState.ON_HOLD, "test");
+
+ mCallsManager.playDtmfTone(callSpy, '1');
+ verify(callSpy, never()).playDtmfTone(anyChar());
+ }
+
+ /**
+ * Verifies that DTMF requests are suppressed when a call is held.
+ * @throws Exception
+ */
+ @MediumTest
+ public void testCancelDtmfWhenHeld() throws Exception {
+ Call callSpy = addSpyCall();
+ mCallsManager.playDtmfTone(callSpy, '1');
+ mCallsManager.markCallAsOnHold(callSpy);
+ verify(callSpy).stopDtmfTone();
+ }
+
+ private Call addSpyCall() {
+ Call ongoingCall = new Call("1", /* callId */
+ mComponentContextFixture.getTestDouble(),
+ mCallsManager,
+ mLock, /* ConnectionServiceRepository */
+ null,
+ mContactsAsyncHelper,
+ mCallerInfoAsyncQueryFactory,
+ mPhoneNumberUtilsAdapter,
+ TEST_ADDRESS,
+ null /* GatewayInfo */,
+ null /* connectionManagerPhoneAccountHandle */,
+ SIM_2_HANDLE,
+ Call.CALL_DIRECTION_INCOMING,
+ false /* shouldAttachToExistingConnection*/,
+ false /* isConference */,
+ mClockProxy);
+ ongoingCall.setState(CallState.ACTIVE, "just cuz");
+ Call callSpy = Mockito.spy(ongoingCall);
+ mCallsManager.addCall(callSpy);
+ return callSpy;
+ }
+
private void setupMsimAccounts() {
TelephonyManager mockTelephonyManager = mComponentContextFixture.getTelephonyManager();
when(mockTelephonyManager.getMultiSimConfiguration()).thenReturn(