Improve getPropConfigs and error logging. am: b48b6e28cb

Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/interfaces/+/16981182

Change-Id: I2eedb36beef06fac86990174f188fc59ec6757fa
diff --git a/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp b/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
index 098bfee..830b936 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
@@ -67,7 +67,8 @@
     parcelableResults.payloads[0] = result;
     if (ScopedAStatus callbackStatus = callCallback(callback, parcelableResults);
         !callbackStatus.isOk()) {
-        ALOGE("failed to call callback, error: %s, code: %d", callbackStatus.getMessage(),
+        ALOGE("failed to call callback, error: %s, exception: %d, service specific error: %d",
+              callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
               callbackStatus.getServiceSpecificError());
     }
 }
@@ -90,8 +91,9 @@
     if (status.isOk()) {
         if (ScopedAStatus callbackStatus = callCallback(callback, parcelableResults);
             !callbackStatus.isOk()) {
-            ALOGE("failed to call callback, error: %s, code: %d", status.getMessage(),
-                  status.getServiceSpecificError());
+            ALOGE("failed to call callback, error: %s, exception: %d, service specific error: %d",
+                  callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
+                  callbackStatus.getServiceSpecificError());
         }
         return;
     }
@@ -296,8 +298,10 @@
     if (ScopedAStatus callbackStatus =
                 callback->onPropertyEvent(vehiclePropValues, sharedMemoryFileCount);
         !callbackStatus.isOk()) {
-        ALOGE("subscribe: failed to call callback, error: %s, code: %d", status.getMessage(),
-              status.getServiceSpecificError());
+        ALOGE("subscribe: failed to call callback, error: %s, exception: %d, "
+              "service specific error: %d",
+              callbackStatus.getMessage(), callbackStatus.getExceptionCode(),
+              callbackStatus.getServiceSpecificError());
     }
 }
 
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index c0a66da..52f3631 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -218,6 +218,7 @@
 }
 
 void DefaultVehicleHal::onBinderDiedWithContext(const AIBinder* clientId) {
+    ALOGD("binder died");
     std::scoped_lock<std::mutex> lockGuard(mLock);
     mSetValuesClients.erase(clientId);
     mGetValuesClients.erase(clientId);
@@ -232,6 +233,7 @@
 }
 
 void DefaultVehicleHal::onBinderUnlinkedWithContext(const AIBinder* clientId) {
+    ALOGD("binder unlinked");
     std::scoped_lock<std::mutex> lockGuard(mLock);
     mOnBinderDiedContexts.erase(clientId);
 }
@@ -534,6 +536,10 @@
     for (int32_t prop : props) {
         if (mConfigsByPropId.find(prop) != mConfigsByPropId.end()) {
             configs.push_back(mConfigsByPropId[prop]);
+        } else {
+            return ScopedAStatus::fromServiceSpecificErrorWithMessage(
+                    toInt(StatusCode::INVALID_ARG),
+                    StringPrintf("no config for property, ID: %" PRId32, prop).c_str());
         }
     }
     return vectorToStableLargeParcelable(std::move(configs), output);
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 178498b..aafa17e 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -514,6 +514,50 @@
     ASSERT_EQ(result.value().getObject()->payloads, testConfigs);
 }
 
+TEST_F(DefaultVehicleHalTest, testGetPropConfigs) {
+    auto testConfigs = std::vector<VehiclePropConfig>({
+            VehiclePropConfig{
+                    .prop = 1,
+            },
+            VehiclePropConfig{
+                    .prop = 2,
+            },
+    });
+
+    auto hardware = std::make_unique<MockVehicleHardware>();
+    hardware->setPropertyConfigs(testConfigs);
+    auto vhal = ndk::SharedRefBase::make<DefaultVehicleHal>(std::move(hardware));
+    std::shared_ptr<IVehicle> client = IVehicle::fromBinder(vhal->asBinder());
+
+    VehiclePropConfigs output;
+    auto status = client->getPropConfigs(std::vector<int32_t>({1, 2}), &output);
+
+    ASSERT_TRUE(status.isOk()) << "getPropConfigs failed: " << status.getMessage();
+    ASSERT_EQ(output.payloads, testConfigs);
+}
+
+TEST_F(DefaultVehicleHalTest, testGetPropConfigsInvalidArg) {
+    auto testConfigs = std::vector<VehiclePropConfig>({
+            VehiclePropConfig{
+                    .prop = 1,
+            },
+            VehiclePropConfig{
+                    .prop = 2,
+            },
+    });
+
+    auto hardware = std::make_unique<MockVehicleHardware>();
+    hardware->setPropertyConfigs(testConfigs);
+    auto vhal = ndk::SharedRefBase::make<DefaultVehicleHal>(std::move(hardware));
+    std::shared_ptr<IVehicle> client = IVehicle::fromBinder(vhal->asBinder());
+
+    VehiclePropConfigs output;
+    auto status = client->getPropConfigs(std::vector<int32_t>({1, 2, 3}), &output);
+
+    ASSERT_FALSE(status.isOk()) << "getPropConfigs must fail with invalid prop ID";
+    ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::INVALID_ARG));
+}
+
 TEST_F(DefaultVehicleHalTest, testGetValuesSmall) {
     GetValueRequests requests;
     std::vector<GetValueResult> expectedResults;
diff --git a/automotive/vehicle/aidl/impl/vhal/vhal-default-service.xml b/automotive/vehicle/aidl/impl/vhal/vhal-default-service.xml
index 8d237b8..4d587ee 100644
--- a/automotive/vehicle/aidl/impl/vhal/vhal-default-service.xml
+++ b/automotive/vehicle/aidl/impl/vhal/vhal-default-service.xml
@@ -1,11 +1,7 @@
 <manifest version="1.0" type="device">
     <hal format="aidl">
         <name>android.hardware.automotive.vehicle</name>
-        <transport>hwbinder</transport>
         <version>1</version>
-        <interface>
-            <name>IVehicle</name>
-            <instance>default</instance>
-        </interface>
+        <fqname>IVehicle/default</fqname>
     </hal>
 </manifest>