Remove PropertyTimeoutException

Replace PropertyTimeoutException with ServiceSpecificException.
Expose error code in *halserivce log.

Bug: 147896616
Test: carservice_test and carservice_unit_test
Change-Id: I3da90552e9c6bd6793eba1bff64d1f4a60677a83

10 files changed, 72 insertions, 97 deletions

diff --git a/service/src/com/android/car/hal/DiagnosticHalService.java b/service/src/com/android/car/hal/DiagnosticHalService.java
index 1e56d2238c..b248ac06ea 100644
--- a/service/src/com/android/car/hal/DiagnosticHalService.java
+++ b/service/src/com/android/car/hal/DiagnosticHalService.java
@@ -28,6 +28,7 @@ import android.hardware.automotive.vehicle.V2_0.VehiclePropConfig;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
 import android.hardware.automotive.vehicle.V2_0.VehicleProperty;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropertyChangeMode;
+import android.os.ServiceSpecificException;
 import android.util.Log;
 import android.util.SparseArray;
 
@@ -299,7 +300,7 @@ public class DiagnosticHalService extends  HalServiceBase{
         }
         try {
             return mVehicleHal.get(propConfig.prop);
-        } catch (PropertyTimeoutException e) {
+        } catch (ServiceSpecificException e) {
             Log.e(CarLog.TAG_DIAGNOSTIC,
                     "property not ready 0x" + toHexString(propConfig.prop), e);
             return null;
@@ -468,8 +469,8 @@ public class DiagnosticHalService extends  HalServiceBase{
         try {
             VehiclePropValue value = mVehicleHal.get(VehicleProperty.OBD2_LIVE_FRAME);
             return createCarDiagnosticEvent(value);
-        } catch (PropertyTimeoutException e) {
-            Log.e(CarLog.TAG_DIAGNOSTIC, "timeout trying to read OBD2_LIVE_FRAME");
+        } catch (ServiceSpecificException e) {
+            Log.e(CarLog.TAG_DIAGNOSTIC, "Failed to read OBD2_LIVE_FRAME.", e);
             return null;
         } catch (IllegalArgumentException e) {
             Log.e(CarLog.TAG_DIAGNOSTIC, "illegal argument trying to read OBD2_LIVE_FRAME", e);
@@ -486,8 +487,8 @@ public class DiagnosticHalService extends  HalServiceBase{
                 timestamps[i] = value.value.int64Values.get(i);
             }
             return timestamps;
-        } catch (PropertyTimeoutException e) {
-            Log.e(CarLog.TAG_DIAGNOSTIC, "timeout trying to read OBD2_FREEZE_FRAME_INFO");
+        } catch (ServiceSpecificException e) {
+            Log.e(CarLog.TAG_DIAGNOSTIC, "Failed to read OBD2_FREEZE_FRAME_INFO.", e);
             return null;
         } catch (IllegalArgumentException e) {
             Log.e(CarLog.TAG_DIAGNOSTIC,
@@ -504,8 +505,8 @@ public class DiagnosticHalService extends  HalServiceBase{
         try {
             VehiclePropValue value = mVehicleHal.get(builder.build());
             return createCarDiagnosticEvent(value);
-        } catch (PropertyTimeoutException e) {
-            Log.e(CarLog.TAG_DIAGNOSTIC, "timeout trying to read OBD2_FREEZE_FRAME");
+        } catch (ServiceSpecificException e) {
+            Log.e(CarLog.TAG_DIAGNOSTIC, "Failed to read OBD2_FREEZE_FRAME.", e);
             return null;
         } catch (IllegalArgumentException e) {
             Log.e(CarLog.TAG_DIAGNOSTIC,
@@ -520,8 +521,8 @@ public class DiagnosticHalService extends  HalServiceBase{
         builder.setInt64Value(timestamps);
         try {
             mVehicleHal.set(builder.build());
-        } catch (PropertyTimeoutException e) {
-            Log.e(CarLog.TAG_DIAGNOSTIC, "timeout trying to write OBD2_FREEZE_FRAME_CLEAR");
+        } catch (ServiceSpecificException e) {
+            Log.e(CarLog.TAG_DIAGNOSTIC, "Failed to write OBD2_FREEZE_FRAME_CLEAR.", e);
         } catch (IllegalArgumentException e) {
             Log.e(CarLog.TAG_DIAGNOSTIC,
                 "illegal argument trying to write OBD2_FREEZE_FRAME_CLEAR", e);
diff --git a/service/src/com/android/car/hal/HalClient.java b/service/src/com/android/car/hal/HalClient.java
index 06d96667f5..db85892aa7 100644
--- a/service/src/com/android/car/hal/HalClient.java
+++ b/service/src/com/android/car/hal/HalClient.java
@@ -78,7 +78,7 @@ class  HalClient {
         mVehicle.unsubscribe(mInternalCallback, prop);
     }
 
-    public void setValue(VehiclePropValue propValue) throws PropertyTimeoutException {
+    public void setValue(VehiclePropValue propValue) {
         int status = invokeRetriable(() -> {
             try {
                 return mVehicle.set(propValue);
@@ -90,22 +90,24 @@ class  HalClient {
 
         if (StatusCode.INVALID_ARG == status) {
             throw new IllegalArgumentException(
-                    String.format("Failed to set value for: 0x%x, areaId: 0x%x",
-                            propValue.prop, propValue.areaId));
-        }
-
-        if (StatusCode.TRY_AGAIN == status) {
-            throw new PropertyTimeoutException(propValue.prop);
+                    String.format("Failed to set value for: 0x%s, areaId: 0x%s",
+                            Integer.toHexString(propValue.prop),
+                            Integer.toHexString(propValue.areaId)));
         }
 
         if (StatusCode.OK != status) {
-            Log.i(CarLog.TAG_HAL, String.format("Failed to set property: 0x%x, areaId: 0x%x, "
-                    + "code: %d", propValue.prop, propValue.areaId, status));
-            throw new ServiceSpecificException(status);
+            Log.i(CarLog.TAG_HAL, String.format(
+                    "Failed to set property: 0x%s, areaId: 0x%s, code: %d",
+                    Integer.toHexString(propValue.prop),
+                    Integer.toHexString(propValue.areaId),
+                    status));
+            throw new ServiceSpecificException(status,
+                    "Failed to set property: 0x" + Integer.toHexString(propValue.prop)
+                            + " in areaId: 0x" + Integer.toHexString(propValue.areaId));
         }
     }
 
-    VehiclePropValue getValue(VehiclePropValue requestedPropValue) throws PropertyTimeoutException {
+    VehiclePropValue getValue(VehiclePropValue requestedPropValue) {
         final ObjectWrapper<VehiclePropValue> valueWrapper = new ObjectWrapper<>();
         int status = invokeRetriable(() -> {
             ValueResult res = internalGet(requestedPropValue);
@@ -117,17 +119,25 @@ class  HalClient {
         int areaId = requestedPropValue.areaId;
         if (StatusCode.INVALID_ARG == status) {
             throw new IllegalArgumentException(
-                    String.format("Failed to get value for: 0x%x, areaId: 0x%x", propId, areaId));
-        }
-
-        if (StatusCode.TRY_AGAIN == status) {
-            throw new PropertyTimeoutException(propId);
+                    String.format("Failed to get value for: 0x%s, areaId: 0x%s",
+                            Integer.toHexString(propId),
+                            Integer.toHexString(areaId)));
         }
 
         if (StatusCode.OK != status || valueWrapper.object == null) {
-            Log.i(CarLog.TAG_HAL, String.format("Failed to get property: 0x%x, areaId: 0x%x, "
-                    + "code: %d", requestedPropValue.prop, requestedPropValue.areaId, status));
-            throw new ServiceSpecificException(status);
+            // If valueWrapper.object is null and status is StatusCode.Ok, change the status to be
+            // NOT_AVAILABLE.
+            if (StatusCode.OK == status) {
+                status = StatusCode.NOT_AVAILABLE;
+            }
+            Log.i(CarLog.TAG_HAL, String.format(
+                    "Failed to get property: 0x%s, areaId: 0x%s, code: %d",
+                    Integer.toHexString(requestedPropValue.prop),
+                    Integer.toHexString(requestedPropValue.areaId),
+                    status));
+            throw new ServiceSpecificException(status,
+                    "Failed to get property: 0x" + Integer.toHexString(requestedPropValue.prop)
+                            + " in areaId: 0x" + Integer.toHexString(requestedPropValue.areaId));
         }
 
         return valueWrapper.object;
diff --git a/service/src/com/android/car/hal/PowerHalService.java b/service/src/com/android/car/hal/PowerHalService.java
index 518931ac83..8f6ac9fca2 100644
--- a/service/src/com/android/car/hal/PowerHalService.java
+++ b/service/src/com/android/car/hal/PowerHalService.java
@@ -29,6 +29,7 @@ import android.hardware.automotive.vehicle.V2_0.VehicleApPowerStateShutdownParam
 import android.hardware.automotive.vehicle.V2_0.VehiclePropConfig;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
 import android.hardware.automotive.vehicle.V2_0.VehicleProperty;
+import android.os.ServiceSpecificException;
 import android.util.Log;
 
 import com.android.car.CarLog;
@@ -282,7 +283,7 @@ public class PowerHalService extends HalServiceBase {
         try {
             mHal.set(VehicleProperty.DISPLAY_BRIGHTNESS, 0).to(brightness);
             Log.i(CarLog.TAG_POWER, "send display brightness = " + brightness);
-        } catch (PropertyTimeoutException | IllegalArgumentException e) {
+        } catch (ServiceSpecificException | IllegalArgumentException e) {
             Log.e(CarLog.TAG_POWER, "cannot set DISPLAY_BRIGHTNESS", e);
         }
     }
@@ -294,7 +295,7 @@ public class PowerHalService extends HalServiceBase {
                 mHal.set(VehicleProperty.AP_POWER_STATE_REPORT, 0).to(values);
                 Log.i(CarLog.TAG_POWER, "setPowerState=" + powerStateReportName(state)
                         + " param=" + additionalParam);
-            } catch (PropertyTimeoutException e) {
+            } catch (ServiceSpecificException e) {
                 Log.e(CarLog.TAG_POWER, "cannot set to AP_POWER_STATE_REPORT", e);
             }
         }
@@ -305,7 +306,7 @@ public class PowerHalService extends HalServiceBase {
         int[] state;
         try {
             state = mHal.get(int[].class, VehicleProperty.AP_POWER_STATE_REQ);
-        } catch (PropertyTimeoutException e) {
+        } catch (ServiceSpecificException e) {
             Log.e(CarLog.TAG_POWER, "Cannot get AP_POWER_STATE_REQ", e);
             return null;
         }
diff --git a/service/src/com/android/car/hal/PropertyHalService.java b/service/src/com/android/car/hal/PropertyHalService.java
index 047aa568de..b0f1582004 100644
--- a/service/src/com/android/car/hal/PropertyHalService.java
+++ b/service/src/com/android/car/hal/PropertyHalService.java
@@ -27,16 +27,13 @@ import android.car.hardware.CarPropertyConfig;
 import android.car.hardware.CarPropertyValue;
 import android.car.hardware.property.CarPropertyEvent;
 import android.car.hardware.property.CarPropertyManager;
-import android.car.hardware.property.VehicleHalStatusCode;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropConfig;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
 import android.hardware.automotive.vehicle.V2_0.VehicleProperty;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropertyType;
-import android.os.ServiceSpecificException;
 import android.util.Log;
 import android.util.SparseArray;
 
-import com.android.car.CarLog;
 import com.android.internal.annotations.GuardedBy;
 
 import java.io.PrintWriter;
@@ -169,13 +166,8 @@ public class PropertyHalService extends HalServiceBase {
             throw new IllegalArgumentException("Invalid property Id : 0x" + toHexString(mgrPropId));
         }
 
-        VehiclePropValue value = null;
-        try {
-            value = mVehicleHal.get(halPropId, areaId);
-        } catch (PropertyTimeoutException e) {
-            Log.e(CarLog.TAG_PROPERTY, "get, property not ready 0x" + toHexString(halPropId), e);
-        }
-
+        // CarPropertyManager catches and rethrows exception, no need to handle here.
+        VehiclePropValue value = mVehicleHal.get(halPropId, areaId);
         if (isMixedTypeProperty(halPropId)) {
             VehiclePropConfig propConfig;
             synchronized (mLock) {
@@ -245,13 +237,8 @@ public class PropertyHalService extends HalServiceBase {
         } else {
             halProp = toVehiclePropValue(prop, halPropId);
         }
-        try {
-            mVehicleHal.set(halProp);
-        } catch (PropertyTimeoutException e) {
-            // TODO(b/147896616): throw ServiceSpecificException at first place.
-            Log.e(CarLog.TAG_PROPERTY, "set, property not ready 0x" + toHexString(halPropId), e);
-            throw new ServiceSpecificException(VehicleHalStatusCode.STATUS_TRY_AGAIN);
-        }
+        // CarPropertyManager catches and rethrows exception, no need to handle here.
+        mVehicleHal.set(halProp);
     }
 
     /**
diff --git a/service/src/com/android/car/hal/PropertyTimeoutException.java b/service/src/com/android/car/hal/PropertyTimeoutException.java
deleted file mode 100644
index 2d6120dde0..0000000000
--- a/service/src/com/android/car/hal/PropertyTimeoutException.java
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * Copyright (C) 2016 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package com.android.car.hal;
-
-import static java.lang.Integer.toHexString;
-
-/**
- * This exception is raised when IVehicle#get or IVehicle#set returns StatusCode.TRY_AGAIN. This
- * usually happens during boot-up meaning that Vehicle HAL is not ready to get or set that property.
- */
-class PropertyTimeoutException extends Exception {
-    PropertyTimeoutException(int property) {
-        super("Property 0x" + toHexString(property) + " is not ready yet.");
-    }
-}
diff --git a/service/src/com/android/car/hal/UserHalService.java b/service/src/com/android/car/hal/UserHalService.java
index 7b26e82746..50d4d3f027 100644
--- a/service/src/com/android/car/hal/UserHalService.java
+++ b/service/src/com/android/car/hal/UserHalService.java
@@ -32,6 +32,7 @@ import android.hardware.automotive.vehicle.V2_0.VehiclePropConfig;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
 import android.os.Handler;
 import android.os.Looper;
+import android.os.ServiceSpecificException;
 import android.os.SystemClock;
 import android.os.UserHandle;
 import android.sysprop.CarProperties;
@@ -280,7 +281,7 @@ public final class UserHalService extends HalServiceBase {
         try {
             if (DBG) Log.d(TAG, "Calling hal.set(): " + propRequest);
             mHal.set(propRequest);
-        } catch (PropertyTimeoutException e) {
+        } catch (ServiceSpecificException e) {
             Log.w(TAG, "Failed to set INITIAL_USER_INFO", e);
             callback.onResponse(HalCallback.STATUS_HAL_SET_TIMEOUT, null);
         }
diff --git a/service/src/com/android/car/hal/VehicleHal.java b/service/src/com/android/car/hal/VehicleHal.java
index f69d23d000..318889d3e2 100644
--- a/service/src/com/android/car/hal/VehicleHal.java
+++ b/service/src/com/android/car/hal/VehicleHal.java
@@ -324,12 +324,12 @@ public class VehicleHal extends IVehicleCallback.Stub {
     public Collection<VehiclePropConfig> getAllPropConfigs() {
         return mAllProperties.values();
     }
-
-    public VehiclePropValue get(int propertyId) throws PropertyTimeoutException {
+    
+    public VehiclePropValue get(int propertyId) {
         return get(propertyId, NO_AREA);
     }
 
-    public VehiclePropValue get(int propertyId, int areaId) throws PropertyTimeoutException {
+    public VehiclePropValue get(int propertyId, int areaId) {
         if (DBG) {
             Log.i(CarLog.TAG_HAL, "get, property: 0x" + toHexString(propertyId)
                     + ", areaId: 0x" + toHexString(areaId));
@@ -340,17 +340,16 @@ public class VehicleHal extends IVehicleCallback.Stub {
         return mHalClient.getValue(propValue);
     }
 
-    public <T> T get(Class clazz, int propertyId) throws PropertyTimeoutException {
+    public <T> T get(Class clazz, int propertyId) {
         return get(clazz, createPropValue(propertyId, NO_AREA));
     }
 
-    public <T> T get(Class clazz, int propertyId, int areaId) throws PropertyTimeoutException {
+    public <T> T get(Class clazz, int propertyId, int areaId) {
         return get(clazz, createPropValue(propertyId, areaId));
     }
 
     @SuppressWarnings("unchecked")
-    public <T> T get(Class clazz, VehiclePropValue requestedPropValue)
-            throws PropertyTimeoutException {
+    public <T> T get(Class clazz, VehiclePropValue requestedPropValue) {
         VehiclePropValue propValue;
         propValue = mHalClient.getValue(requestedPropValue);
 
@@ -379,8 +378,7 @@ public class VehicleHal extends IVehicleCallback.Stub {
         }
     }
 
-    public VehiclePropValue get(VehiclePropValue requestedPropValue)
-            throws PropertyTimeoutException {
+    public VehiclePropValue get(VehiclePropValue requestedPropValue) {
         return mHalClient.getValue(requestedPropValue);
     }
 
@@ -401,7 +399,7 @@ public class VehicleHal extends IVehicleCallback.Stub {
         }
     }
 
-    protected void set(VehiclePropValue propValue) throws PropertyTimeoutException {
+    protected void set(VehiclePropValue propValue) {
         mHalClient.setValue(propValue);
     }
 
@@ -715,28 +713,28 @@ public class VehicleHal extends IVehicleCallback.Stub {
             mPropValue.areaId = areaId;
         }
 
-        void to(boolean value) throws PropertyTimeoutException {
+        void to(boolean value) {
             to(value ? 1 : 0);
         }
 
-        void to(int value) throws PropertyTimeoutException {
+        void to(int value) {
             mPropValue.value.int32Values.add(value);
             submit();
         }
 
-        void to(int[] values) throws PropertyTimeoutException {
+        void to(int[] values) {
             for (int value : values) {
                 mPropValue.value.int32Values.add(value);
             }
             submit();
         }
 
-        void to(Collection<Integer> values) throws PropertyTimeoutException {
+        void to(Collection<Integer> values) {
             mPropValue.value.int32Values.addAll(values);
             submit();
         }
 
-        void submit() throws PropertyTimeoutException {
+        void submit() {
             HalClient client =  mClient.get();
             if (client != null) {
                 if (DBG) {
diff --git a/service/src/com/android/car/hal/VmsHalService.java b/service/src/com/android/car/hal/VmsHalService.java
index c206a01d3d..7f130ba9d2 100644
--- a/service/src/com/android/car/hal/VmsHalService.java
+++ b/service/src/com/android/car/hal/VmsHalService.java
@@ -247,7 +247,7 @@ public class VmsHalService extends HalServiceBase {
         VehiclePropValue vehicleProp = null;
         try {
             vehicleProp = mVehicleHal.get(mClientMetricsProperty);
-        } catch (PropertyTimeoutException | RuntimeException e) {
+        } catch (RuntimeException e) {
             // Failures to retrieve metrics should be non-fatal
             Log.e(TAG, "While reading metrics from client", e);
         }
@@ -627,7 +627,7 @@ public class VmsHalService extends HalServiceBase {
 
         try {
             mVehicleHal.set(vehicleProp);
-        } catch (PropertyTimeoutException | RuntimeException e) {
+        } catch (RuntimeException e) {
             Log.e(TAG, "While sending " + VmsMessageType.toString(messageType), e);
             if (mPropagatePropertyException) {
                 throw new IllegalStateException(e);
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java b/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
index 1a074568be..1fc650551a 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/UserHalServiceTest.java
@@ -29,6 +29,7 @@ import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.verify;
 import static org.testng.Assert.assertThrows;
 
+import android.car.hardware.property.VehicleHalStatusCode;
 import android.hardware.automotive.vehicle.V2_0.InitialUserInfoResponse;
 import android.hardware.automotive.vehicle.V2_0.InitialUserInfoResponseAction;
 import android.hardware.automotive.vehicle.V2_0.UserFlags;
@@ -38,6 +39,7 @@ import android.hardware.automotive.vehicle.V2_0.VehiclePropConfig;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropertyAccess;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropertyChangeMode;
+import android.os.ServiceSpecificException;
 import android.os.UserHandle;
 import android.util.Log;
 
@@ -426,7 +428,8 @@ public final class UserHalServiceTest {
      * Sets the VHAL mock to emulate a property timeout exception upon a call to set a property.
      */
     private void replySetPropertyWithTimeoutException(int prop) throws Exception {
-        doThrow(new PropertyTimeoutException(prop)).when(mVehicleHal).set(isProperty(prop));
+        doThrow(new ServiceSpecificException(VehicleHalStatusCode.STATUS_TRY_AGAIN,
+                "PropId: 0x" + Integer.toHexString(prop))).when(mVehicleHal).set(isProperty(prop));
     }
 
     private void assertInitialUserInfoSetRequest(VehiclePropValue req, int requestType) {
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
index 4540b6b56b..c0657ba2da 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
@@ -25,6 +25,7 @@ import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import android.car.hardware.property.VehicleHalStatusCode;
 import android.car.vms.VmsAssociatedLayer;
 import android.car.vms.VmsAvailableLayers;
 import android.car.vms.VmsClient;
@@ -40,6 +41,7 @@ import android.hardware.automotive.vehicle.V2_0.VehicleProperty;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropertyGroup;
 import android.hardware.automotive.vehicle.V2_0.VmsMessageType;
 import android.os.Handler;
+import android.os.ServiceSpecificException;
 
 import com.android.car.R;
 import com.android.car.test.utils.TemporaryFile;
@@ -1040,7 +1042,8 @@ public class VmsHalServiceTest {
         setUp();
 
         when(mVehicleHal.get(metricsPropertyId))
-                .thenThrow(new PropertyTimeoutException(metricsPropertyId));
+                .thenThrow(new ServiceSpecificException(VehicleHalStatusCode.STATUS_TRY_AGAIN,
+                        "propertyId: 0x" + Integer.toHexString(metricsPropertyId)));
 
         mHalService.dumpMetrics(new FileDescriptor());
         verify(mVehicleHal).get(metricsPropertyId);