From f81abdaa9ea2817a03f5ea10f2fd082d84c34617 Mon Sep 17 00:00:00 2001 From: Atneya Nair Date: Mon, 27 Mar 2023 16:22:54 -0700 Subject: [PATCH] Migrate ST to new error APIs onError is overloaded, so we will migrate ST to dispatch individual callbacks depending on error case. Unify these callbacks with VIService new SoundTriggerFailure class. Remove onError up to the SDK interface, since it is now redundant. Fixes: 272123006 Test: atest AlwaysOnHotwordDetectorTest Change-Id: Ifca877649b5ef361229bc9015b31c77e2e36d758 --- .../IRecognitionStatusCallback.aidl | 30 +++++++-- .../voice/AlwaysOnHotwordDetector.java | 16 ++--- .../voice/SoftwareHotwordDetector.java | 15 +++-- .../service/voice/SoundTriggerFailure.java | 21 +++++-- .../service/voice/VisualQueryDetector.java | 11 ++-- .../IHotwordRecognitionStatusCallback.aidl | 17 ++--- .../soundtrigger/SoundTriggerDetector.java | 45 +++++++++++--- .../soundtrigger/SoundTriggerHelper.java | 62 +++++++------------ .../soundtrigger/SoundTriggerService.java | 28 ++++++++- .../HotwordDetectionConnection.java | 28 ++++++++- 10 files changed, 178 insertions(+), 95 deletions(-) diff --git a/core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl b/core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl index dcc3369668100..466373030c780 100644 --- a/core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl +++ b/core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl @@ -41,12 +41,6 @@ oneway interface IRecognitionStatusCallback { void onGenericSoundTriggerDetected(in SoundTrigger.GenericRecognitionEvent recognitionEvent); - /** - * Called when the detection fails due to an error. - * - * @param status The error code that was seen. - */ - void onError(int status); /** * Called when the recognition is paused temporarily for some reason. */ @@ -55,4 +49,28 @@ oneway interface IRecognitionStatusCallback { * Called when the recognition is resumed after it was temporarily paused. */ void onRecognitionResumed(); + + // Error callbacks to follow + /** + * Called when this recognition has been preempted by another. + */ + void onPreempted(); + + /** + * Called when the underlying ST module service has died. + */ + void onModuleDied(); + + /** + * Called when the service failed to gracefully resume recognition following a pause. + * @param status - The received error code. + */ + void onResumeFailed(int status); + + /** + * Called when the service failed to pause recognition when required. + * TODO(b/276507281) Remove. This should never happen, so we should abort instead. + * @param status - The received error code. + */ + void onPauseFailed(int status); } diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java index 8688a18880b7c..24c96eae03cb5 100644 --- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java +++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java @@ -1572,16 +1572,6 @@ public class AlwaysOnHotwordDetector extends AbstractDetector { Message.obtain(mHandler, MSG_HOTWORD_REJECTED, result).sendToTarget(); } - @Override - public void onError(int status) { - Slog.i(TAG, "onError: " + status); - // TODO(b/271534248): This is a workaround before the sound trigger uses the new error - // method. - Message.obtain(mHandler, MSG_DETECTION_SOUND_TRIGGER_FAILURE, - new SoundTriggerFailure(SoundTriggerFailure.ERROR_CODE_UNKNOWN, - "Sound trigger error")).sendToTarget(); - } - @Override public void onHotwordDetectionServiceFailure( HotwordDetectionServiceFailure hotwordDetectionServiceFailure) { @@ -1604,6 +1594,12 @@ public class AlwaysOnHotwordDetector extends AbstractDetector { "onVisualQueryDetectionServiceFailure: " + visualQueryDetectionServiceFailure); } + @Override + public void onSoundTriggerFailure(SoundTriggerFailure soundTriggerFailure) { + Message.obtain(mHandler, MSG_DETECTION_SOUND_TRIGGER_FAILURE, + Objects.requireNonNull(soundTriggerFailure)).sendToTarget(); + } + @Override public void onUnknownFailure(String errorMessage) throws RemoteException { Slog.v(TAG, "onUnknownFailure: " + errorMessage); diff --git a/core/java/android/service/voice/SoftwareHotwordDetector.java b/core/java/android/service/voice/SoftwareHotwordDetector.java index eac7aee43859e..7ab4fafcf3121 100644 --- a/core/java/android/service/voice/SoftwareHotwordDetector.java +++ b/core/java/android/service/voice/SoftwareHotwordDetector.java @@ -233,14 +233,6 @@ class SoftwareHotwordDetector extends AbstractDetector { } } - @Override - public void onError(int status) throws RemoteException { - if (DEBUG) { - Slog.i(TAG, "Ignored #onError (" + status + ") event"); - } - // TODO: Check if we still need to implement this method with DetectorFailure mechanism. - } - @Override public void onHotwordDetectionServiceFailure( HotwordDetectionServiceFailure hotwordDetectionServiceFailure) @@ -264,6 +256,13 @@ class SoftwareHotwordDetector extends AbstractDetector { + visualQueryDetectionServiceFailure); } + @Override + public void onSoundTriggerFailure(SoundTriggerFailure onSoundTriggerFailure) + throws RemoteException { + // It should never be called here. + Slog.wtf(TAG, "Unexpected STFailure in software detector: " + onSoundTriggerFailure); + } + @Override public void onUnknownFailure(String errorMessage) throws RemoteException { Slog.v(TAG, "onUnknownFailure: " + errorMessage); diff --git a/core/java/android/service/voice/SoundTriggerFailure.java b/core/java/android/service/voice/SoundTriggerFailure.java index 5560800a373f3..2ce5e5da47242 100644 --- a/core/java/android/service/voice/SoundTriggerFailure.java +++ b/core/java/android/service/voice/SoundTriggerFailure.java @@ -73,18 +73,28 @@ public final class SoundTriggerFailure implements Parcelable { @Retention(RetentionPolicy.SOURCE) public @interface SoundTriggerErrorCode {} - private int mErrorCode = ERROR_CODE_UNKNOWN; - private String mErrorMessage = "Unknown"; + private final int mErrorCode; + private final String mErrorMessage; /** * @hide */ @TestApi - public SoundTriggerFailure(int errorCode, @NonNull String errorMessage) { + public SoundTriggerFailure(@SoundTriggerErrorCode int errorCode, + @NonNull String errorMessage) { if (TextUtils.isEmpty(errorMessage)) { throw new IllegalArgumentException("errorMessage is empty or null."); } - mErrorCode = errorCode; + switch (errorCode) { + case ERROR_CODE_UNKNOWN: + case ERROR_CODE_MODULE_DIED: + case ERROR_CODE_RECOGNITION_RESUME_FAILED: + case ERROR_CODE_UNEXPECTED_PREEMPTION: + mErrorCode = errorCode; + break; + default: + throw new IllegalArgumentException("Invalid ErrorCode: " + errorCode); + } mErrorMessage = errorMessage; } @@ -110,13 +120,14 @@ public final class SoundTriggerFailure implements Parcelable { @FailureSuggestedAction.FailureSuggestedActionDef public int getSuggestedAction() { switch (mErrorCode) { + case ERROR_CODE_UNKNOWN: case ERROR_CODE_MODULE_DIED: case ERROR_CODE_UNEXPECTED_PREEMPTION: return FailureSuggestedAction.RECREATE_DETECTOR; case ERROR_CODE_RECOGNITION_RESUME_FAILED: return FailureSuggestedAction.RESTART_RECOGNITION; default: - return FailureSuggestedAction.NONE; + throw new AssertionError("Unexpected error code"); } } diff --git a/core/java/android/service/voice/VisualQueryDetector.java b/core/java/android/service/voice/VisualQueryDetector.java index b4f5ff1046ae6..93b7964705bae 100644 --- a/core/java/android/service/voice/VisualQueryDetector.java +++ b/core/java/android/service/voice/VisualQueryDetector.java @@ -390,12 +390,6 @@ public class VisualQueryDetector { () -> mCallback.onVisualQueryDetectionServiceRestarted())); } - @Override - public void onError(int status) throws RemoteException { - Slog.v(TAG, "Initialization Error: (" + status + ")"); - // Do nothing - } - @Override public void onHotwordDetectionServiceFailure( HotwordDetectionServiceFailure hotwordDetectionServiceFailure) @@ -419,6 +413,11 @@ public class VisualQueryDetector { })); } + @Override + public void onSoundTriggerFailure(SoundTriggerFailure soundTriggerFailure) { + Slog.wtf(TAG, "Unexpected STFailure in VisualQueryDetector" + soundTriggerFailure); + } + @Override public void onUnknownFailure(String errorMessage) throws RemoteException { Slog.v(TAG, "onUnknownFailure: " + errorMessage); diff --git a/core/java/com/android/internal/app/IHotwordRecognitionStatusCallback.aidl b/core/java/com/android/internal/app/IHotwordRecognitionStatusCallback.aidl index ad0d1a4019916..380118846dc7c 100644 --- a/core/java/com/android/internal/app/IHotwordRecognitionStatusCallback.aidl +++ b/core/java/com/android/internal/app/IHotwordRecognitionStatusCallback.aidl @@ -20,6 +20,7 @@ import android.hardware.soundtrigger.SoundTrigger; import android.service.voice.HotwordDetectedResult; import android.service.voice.HotwordDetectionServiceFailure; import android.service.voice.HotwordRejectedResult; +import android.service.voice.SoundTriggerFailure; import android.service.voice.VisualQueryDetectionServiceFailure; /** @@ -56,13 +57,6 @@ oneway interface IHotwordRecognitionStatusCallback { */ void onRejected(in HotwordRejectedResult result); - /** - * Called when the detection fails due to an error. - * - * @param status The error code that was seen. - */ - void onError(int status); - /** * Called when the detection fails due to an error occurs in the * {@link HotwordDetectionService}. @@ -83,6 +77,15 @@ oneway interface IHotwordRecognitionStatusCallback { void onVisualQueryDetectionServiceFailure( in VisualQueryDetectionServiceFailure visualQueryDetectionServiceFailure); + /** + * Called when the detection fails due to an error occurs in the + * {@link com.android.server.soundtrigger.SoundTriggerService}. + * + * @param soundTriggerFailure It provides the error code, error message and + * suggested action. + */ + void onSoundTriggerFailure(in SoundTriggerFailure soundTriggerFailure); + /** * Called when the detection fails due to an unknown error occurs. * diff --git a/media/java/android/media/soundtrigger/SoundTriggerDetector.java b/media/java/android/media/soundtrigger/SoundTriggerDetector.java index 1a3e54d54ee7a..afa0a32719069 100644 --- a/media/java/android/media/soundtrigger/SoundTriggerDetector.java +++ b/media/java/android/media/soundtrigger/SoundTriggerDetector.java @@ -387,15 +387,6 @@ public final class SoundTriggerDetector { Slog.e(TAG, "Ignoring onKeyphraseDetected() called for " + event); } - /** - * @hide - */ - @Override - public void onError(int status) { - Slog.d(TAG, "onError()" + status); - mHandler.sendEmptyMessage(MSG_DETECTION_ERROR); - } - /** * @hide */ @@ -413,6 +404,42 @@ public final class SoundTriggerDetector { Slog.d(TAG, "onRecognitionResumed()"); mHandler.sendEmptyMessage(MSG_DETECTION_RESUME); } + + /** + * @hide + */ + @Override + public void onPreempted() { + Slog.d(TAG, "onPreempted()"); + mHandler.sendEmptyMessage(MSG_DETECTION_ERROR); + } + + /** + * @hide + */ + @Override + public void onModuleDied() { + Slog.d(TAG, "onModuleDied()"); + mHandler.sendEmptyMessage(MSG_DETECTION_ERROR); + } + + /** + * @hide + */ + @Override + public void onResumeFailed(int status) { + Slog.d(TAG, "onResumeFailed()" + status); + mHandler.sendEmptyMessage(MSG_DETECTION_ERROR); + } + + /** + * @hide + */ + @Override + public void onPauseFailed(int status) { + Slog.d(TAG, "onPauseFailed()" + status); + mHandler.sendEmptyMessage(MSG_DETECTION_ERROR); + } } private class MyHandler extends Handler { diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java index 5efd158133edf..07dc1c66bc4df 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerHelper.java @@ -315,12 +315,13 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { IRecognitionStatusCallback callback, RecognitionConfig recognitionConfig, int keyphraseId, boolean runInBatterySaverMode) { synchronized (mLock) { + // TODO Remove previous callback handling IRecognitionStatusCallback oldCallback = modelData.getCallback(); if (oldCallback != null && oldCallback.asBinder() != callback.asBinder()) { Slog.w(TAG, "Canceling previous recognition for model id: " + modelData.getModelId()); try { - oldCallback.onError(STATUS_ERROR); + oldCallback.onPreempted(); } catch (RemoteException e) { Slog.w(TAG, "RemoteException in onDetectionStopped", e); } @@ -759,15 +760,12 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { onRecognitionAbortLocked(event); break; case SoundTrigger.RECOGNITION_STATUS_FAILURE: - // Fire failures to all listeners since it's not tied to a keyphrase. - onRecognitionFailureLocked(); - break; case SoundTrigger.RECOGNITION_STATUS_SUCCESS: case SoundTrigger.RECOGNITION_STATUS_GET_STATE_RESPONSE: if (isKeyphraseRecognitionEvent(event)) { - onKeyphraseRecognitionSuccessLocked((KeyphraseRecognitionEvent) event); + onKeyphraseRecognitionLocked((KeyphraseRecognitionEvent) event); } else { - onGenericRecognitionSuccessLocked((GenericRecognitionEvent) event); + onGenericRecognitionLocked((GenericRecognitionEvent) event); } break; } @@ -778,7 +776,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return event instanceof KeyphraseRecognitionEvent; } - private void onGenericRecognitionSuccessLocked(GenericRecognitionEvent event) { + private void onGenericRecognitionLocked(GenericRecognitionEvent event) { MetricsLogger.count(mContext, "sth_generic_recognition_event", 1); if (event.status != SoundTrigger.RECOGNITION_STATUS_SUCCESS && event.status != SoundTrigger.RECOGNITION_STATUS_GET_STATE_RESPONSE) { @@ -901,17 +899,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } } - private void onRecognitionFailureLocked() { - Slog.w(TAG, "Recognition failure"); - MetricsLogger.count(mContext, "sth_recognition_failure_event", 1); - try { - sendErrorCallbacksToAllLocked(STATUS_ERROR); - } finally { - internalClearModelStateLocked(); - internalClearGlobalStateLocked(); - } - } - private int getKeyphraseIdFromEvent(KeyphraseRecognitionEvent event) { if (event == null) { Slog.w(TAG, "Null RecognitionEvent received."); @@ -927,7 +914,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { return keyphraseExtras[0].id; } - private void onKeyphraseRecognitionSuccessLocked(KeyphraseRecognitionEvent event) { + private void onKeyphraseRecognitionLocked(KeyphraseRecognitionEvent event) { Slog.i(TAG, "Recognition success"); MetricsLogger.count(mContext, "sth_keyphrase_recognition_event", 1); int keyphraseId = getKeyphraseIdFromEvent(event); @@ -1001,7 +988,17 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { private void onServiceDiedLocked() { try { MetricsLogger.count(mContext, "sth_service_died", 1); - sendErrorCallbacksToAllLocked(SoundTrigger.STATUS_DEAD_OBJECT); + for (ModelData modelData : mModelDataMap.values()) { + IRecognitionStatusCallback callback = modelData.getCallback(); + if (callback != null) { + try { + callback.onModuleDied(); + } catch (RemoteException e) { + Slog.w(TAG, "RemoteException send moduleDied for model handle " + + modelData.getHandle(), e); + } + } + } } finally { internalClearModelStateLocked(); internalClearGlobalStateLocked(); @@ -1111,21 +1108,6 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { } } - // Sends an error callback to all models with a valid registered callback. - private void sendErrorCallbacksToAllLocked(int errorCode) { - for (ModelData modelData : mModelDataMap.values()) { - IRecognitionStatusCallback callback = modelData.getCallback(); - if (callback != null) { - try { - callback.onError(errorCode); - } catch (RemoteException e) { - Slog.w(TAG, "RemoteException sendErrorCallbacksToAllLocked for model handle " + - modelData.getHandle(), e); - } - } - } - } - /** * Stops and unloads all models. This is intended as a clean-up call with the expectation that * this instance is not used after. @@ -1342,11 +1324,11 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { // Notify of error if needed. if (notifyClientOnError) { try { - callback.onError(status); + callback.onResumeFailed(status); } catch (DeadObjectException e) { forceStopAndUnloadModelLocked(modelData, e); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in onError", e); + Slog.w(TAG, "RemoteException in onResumeFailed", e); } } } else { @@ -1382,15 +1364,15 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener { status = mModule.stopRecognition(modelData.getHandle()); if (status != SoundTrigger.STATUS_OK) { - Slog.w(TAG, "stopRecognition call failed with " + status); + Slog.e(TAG, "stopRecognition call failed with " + status); MetricsLogger.count(mContext, "sth_stop_recognition_error", 1); if (notify) { try { - callback.onError(status); + callback.onPauseFailed(status); } catch (DeadObjectException e) { forceStopAndUnloadModelLocked(modelData, e); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in onError", e); + Slog.w(TAG, "RemoteException in onPauseFailed", e); } } } else { diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java index a54e3560e5a31..00974ac8f1f7e 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java @@ -25,6 +25,7 @@ import static android.content.pm.PackageManager.GET_META_DATA; import static android.content.pm.PackageManager.GET_SERVICES; import static android.content.pm.PackageManager.MATCH_DEBUG_TRIAGED_MISSING; import static android.hardware.soundtrigger.SoundTrigger.STATUS_BAD_VALUE; +import static android.hardware.soundtrigger.SoundTrigger.STATUS_DEAD_OBJECT; import static android.hardware.soundtrigger.SoundTrigger.STATUS_ERROR; import static android.hardware.soundtrigger.SoundTrigger.STATUS_OK; import static android.provider.Settings.Global.MAX_SOUND_TRIGGER_DETECTION_SERVICE_OPS_PER_DAY; @@ -1387,8 +1388,7 @@ public class SoundTriggerService extends SystemService { })); } - @Override - public void onError(int status) { + private void onError(int status) { if (DEBUG) Slog.v(TAG, mPuuid + ": onError: " + status); sEventLogger.enqueue(new EventLogger.StringEvent(mPuuid @@ -1410,6 +1410,30 @@ public class SoundTriggerService extends SystemService { null)); } + @Override + public void onPreempted() { + if (DEBUG) Slog.v(TAG, mPuuid + ": onPreempted"); + onError(STATUS_ERROR); + } + + @Override + public void onModuleDied() { + if (DEBUG) Slog.v(TAG, mPuuid + ": onModuleDied"); + onError(STATUS_DEAD_OBJECT); + } + + @Override + public void onResumeFailed(int status) { + if (DEBUG) Slog.v(TAG, mPuuid + ": onResumeFailed: " + status); + onError(status); + } + + @Override + public void onPauseFailed(int status) { + if (DEBUG) Slog.v(TAG, mPuuid + ": onPauseFailed: " + status); + onError(status); + } + @Override public void onRecognitionPaused() { Slog.i(TAG, mPuuid + "->" + mServiceName + ": IGNORED onRecognitionPaused"); diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java index f3cb9baedd4ba..a1adee732701e 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java @@ -56,6 +56,7 @@ import android.service.voice.HotwordDetector; import android.service.voice.IMicrophoneHotwordDetectionVoiceInteractionCallback; import android.service.voice.ISandboxedDetectionService; import android.service.voice.IVisualQueryDetectionVoiceInteractionCallback; +import android.service.voice.SoundTriggerFailure; import android.service.voice.VisualQueryDetectionService; import android.service.voice.VisualQueryDetectionServiceFailure; import android.service.voice.VoiceInteractionManagerInternal.HotwordDetectionServiceIdentity; @@ -576,8 +577,31 @@ final class HotwordDetectionConnection { } @Override - public void onError(int status) throws RemoteException { - mExternalCallback.onError(status); + public void onPreempted() throws RemoteException { + mExternalCallback.onSoundTriggerFailure(new SoundTriggerFailure( + SoundTriggerFailure.ERROR_CODE_UNEXPECTED_PREEMPTION, + "Unexpected startRecognition on already started ST session")); + } + + @Override + public void onModuleDied() throws RemoteException { + mExternalCallback.onSoundTriggerFailure(new SoundTriggerFailure( + SoundTriggerFailure.ERROR_CODE_MODULE_DIED, + "STHAL died")); + } + + @Override + public void onResumeFailed(int status) throws RemoteException { + mExternalCallback.onSoundTriggerFailure(new SoundTriggerFailure( + SoundTriggerFailure.ERROR_CODE_RECOGNITION_RESUME_FAILED, + "STService recognition resume failed with: " + status)); + } + + @Override + public void onPauseFailed(int status) throws RemoteException { + mExternalCallback.onSoundTriggerFailure(new SoundTriggerFailure( + SoundTriggerFailure.ERROR_CODE_RECOGNITION_RESUME_FAILED, + "STService recognition pause failed with: " + status)); } @Override