Improve getPropConfigs and error logging.
This CL improves getPropConfigs to return INVALID_ARG when one of
the prop ID is not found according to the AIDL interface definition.
This CL also improves logging to print out exception code.
This CL also updates service manifest to be consistent with
https://source.android.com/devices/architecture/aidl/aidl-hals
Test: atest DefaultVehicleHalTest
Bug: 219782023, 219612366
Change-Id: I96b091c5cf6641ab7d1df5c644bde7491cbaa5e7
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>