Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b48b6e28 authored by Yu Shan's avatar Yu Shan
Browse files

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
parent a123e768

automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp

+9 −5
Original line number Diff line number Diff line
@@ -67,7 +67,8 @@ void sendGetOrSetValueResult(std::shared_ptr<IVehicleCallback> callback, const R 
    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 @@ void sendGetOrSetValueResults(std::shared_ptr<IVehicleCallback> callback, 
    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 @@ void SubscriptionClient::sendUpdatedValues(std::shared_ptr<IVehicleCallback> cal 
    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());

    }

}

automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp

+6 −0
Original line number Diff line number Diff line
@@ -218,6 +218,7 @@ void DefaultVehicleHal::onBinderDied(void* cookie) { 
}



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::onBinderUnlinked(void* cookie) { 
}



void DefaultVehicleHal::onBinderUnlinkedWithContext(const AIBinder* clientId) {

    ALOGD("binder unlinked");

    std::scoped_lock<std::mutex> lockGuard(mLock);

    mOnBinderDiedContexts.erase(clientId);

}
@@ -534,6 +536,10 @@ ScopedAStatus DefaultVehicleHal::getPropConfigs(const std::vector<int32_t>& prop 
    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);

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

+44 −0
Original line number Diff line number Diff line
@@ -514,6 +514,50 @@ TEST_F(DefaultVehicleHalTest, testGetAllPropConfigsLarge) { 
    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;

automotive/vehicle/aidl/impl/vhal/vhal-default-service.xml

+1 −5
Original line number Diff line number Diff line 
<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>