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

Commit 5bd6aefe authored by Yu Shan's avatar Yu Shan
Browse files

Add error code to VeiclePropertyStore.

Add error code to differentiate between different error cases, e.g.
when the value is not configured v.s. the value is not set.

Test: atest VehicleHalVehicleUtilsTest
Bug: 201830716
Change-Id: I1ef0716edce5bc72e07a769026769a330b4e3025
parent 8e7f7804
Loading
Loading
Loading
Loading
+6 −2
Original line number Original line Diff line number Diff line
@@ -86,11 +86,15 @@ class VehiclePropertyStore final {
    ::android::base::Result<std::vector<VehiclePropValuePool::RecyclableType>>
    ::android::base::Result<std::vector<VehiclePropValuePool::RecyclableType>>
    readValuesForProperty(int32_t propId) const;
    readValuesForProperty(int32_t propId) const;


    // Read the value for the requested property.
    // Read the value for the requested property. Returns {@code StatusCode::NOT_AVAILABLE} if the
    // value has not been set yet. Returns {@code StatusCode::INVALID_ARG} if the property is
    // not configured.
    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(
    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(
            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& request) const;
            const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& request) const;


    // Read the value for the requested property.
    // Read the value for the requested property. Returns {@code StatusCode::NOT_AVAILABLE} if the
    // value has not been set yet. Returns {@code StatusCode::INVALID_ARG} if the property is
    // not configured.
    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(
    ::android::base::Result<VehiclePropValuePool::RecyclableType> readValue(
            int32_t prop, int32_t area = 0, int64_t token = 0) const;
            int32_t prop, int32_t area = 0, int64_t token = 0) const;


+20 −13
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@


#include "VehiclePropertyStore.h"
#include "VehiclePropertyStore.h"


#include <VehicleHalTypes.h>
#include <VehicleUtils.h>
#include <VehicleUtils.h>
#include <android-base/format.h>
#include <android-base/format.h>
#include <math/HashCombine.h>
#include <math/HashCombine.h>
@@ -28,10 +29,12 @@ namespace hardware {
namespace automotive {
namespace automotive {
namespace vehicle {
namespace vehicle {


using ::aidl::android::hardware::automotive::vehicle::StatusCode;
using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
using ::android::base::Error;
using ::android::base::Result;
using ::android::base::Result;


bool VehiclePropertyStore::RecordId::operator==(const VehiclePropertyStore::RecordId& other) const {
bool VehiclePropertyStore::RecordId::operator==(const VehiclePropertyStore::RecordId& other) const {
@@ -86,7 +89,8 @@ Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValueLock
    if (auto it = record.values.find(recId); it != record.values.end()) {
    if (auto it = record.values.find(recId); it != record.values.end()) {
        return mValuePool->obtain(*(it->second));
        return mValuePool->obtain(*(it->second));
    }
    }
    return Errorf("Record ID: {} is not found", recId.toString());
    return Error(toInt(StatusCode::NOT_AVAILABLE))
           << "Record ID: " << recId.toString() << " is not found";
}
}


void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config,
void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config,
@@ -103,15 +107,16 @@ Result<void> VehiclePropertyStore::writeValue(VehiclePropValuePool::RecyclableTy
                                              bool updateStatus) {
                                              bool updateStatus) {
    std::lock_guard<std::mutex> g(mLock);
    std::lock_guard<std::mutex> g(mLock);


    VehiclePropertyStore::Record* record = getRecordLocked(propValue->prop);
    int32_t propId = propValue->prop;

    VehiclePropertyStore::Record* record = getRecordLocked(propId);
    if (record == nullptr) {
    if (record == nullptr) {
        return Errorf("property: {:d} not registered", propValue->prop);
        return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
    }
    }


    if (!isGlobalProp(propValue->prop) &&
    if (!isGlobalProp(propId) && getAreaConfig(*propValue, record->propConfig) == nullptr) {
        getAreaConfig(*propValue, record->propConfig) == nullptr) {
        return Error(toInt(StatusCode::INVALID_ARG))
        return Errorf("no config for property: {:d} area: {:d}", propValue->prop,
               << "no config for property: " << propId << " area: " << propValue->areaId;
                      propValue->areaId);
    }
    }


    VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record);
    VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record);
@@ -122,7 +127,8 @@ Result<void> VehiclePropertyStore::writeValue(VehiclePropValuePool::RecyclableTy
        VehiclePropertyStatus oldStatus = valueToUpdate->status;
        VehiclePropertyStatus oldStatus = valueToUpdate->status;
        // propValue is outdated and drops it.
        // propValue is outdated and drops it.
        if (oldTimestamp > propValue->timestamp) {
        if (oldTimestamp > propValue->timestamp) {
            return Errorf("outdated timestamp: {:d}", propValue->timestamp);
            return Error(toInt(StatusCode::INVALID_ARG))
                   << "outdated timestamp: " << propValue->timestamp;
        }
        }
        if (!updateStatus) {
        if (!updateStatus) {
            propValue->status = oldStatus;
            propValue->status = oldStatus;
@@ -190,7 +196,7 @@ VehiclePropertyStore::readValuesForProperty(int32_t propId) const {


    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    if (record == nullptr) {
    if (record == nullptr) {
        return Errorf("property: {:d} not registered", propId);
        return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
    }
    }


    for (auto const& [_, value] : record->values) {
    for (auto const& [_, value] : record->values) {
@@ -203,9 +209,10 @@ Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValue(
        const VehiclePropValue& propValue) const {
        const VehiclePropValue& propValue) const {
    std::lock_guard<std::mutex> g(mLock);
    std::lock_guard<std::mutex> g(mLock);


    const VehiclePropertyStore::Record* record = getRecordLocked(propValue.prop);
    int32_t propId = propValue.prop;
    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    if (record == nullptr) {
    if (record == nullptr) {
        return Errorf("property: {:d} not registered", propValue.prop);
        return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
    }
    }


    VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record);
    VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record);
@@ -219,7 +226,7 @@ Result<VehiclePropValuePool::RecyclableType> VehiclePropertyStore::readValue(int


    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    if (record == nullptr) {
    if (record == nullptr) {
        return Errorf("property: {:d} not registered", propId);
        return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
    }
    }


    VehiclePropertyStore::RecordId recId{.area = isGlobalProp(propId) ? 0 : areaId, .token = token};
    VehiclePropertyStore::RecordId recId{.area = isGlobalProp(propId) ? 0 : areaId, .token = token};
@@ -242,7 +249,7 @@ Result<const VehiclePropConfig*> VehiclePropertyStore::getConfig(int32_t propId)


    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    const VehiclePropertyStore::Record* record = getRecordLocked(propId);
    if (record == nullptr) {
    if (record == nullptr) {
        return Errorf("property: {:d} not registered", propId);
        return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
    }
    }


    return &record->propConfig;
    return &record->propConfig;
+26 −10
Original line number Original line Diff line number Diff line
@@ -15,6 +15,7 @@
 */
 */


#include <PropertyUtils.h>
#include <PropertyUtils.h>
#include <VehicleHalTypes.h>
#include <VehiclePropertyStore.h>
#include <VehiclePropertyStore.h>
#include <VehicleUtils.h>
#include <VehicleUtils.h>
#include <gmock/gmock.h>
#include <gmock/gmock.h>
@@ -27,6 +28,7 @@ namespace vehicle {


namespace {
namespace {


using ::aidl::android::hardware::automotive::vehicle::StatusCode;
using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehicleProperty;
using ::aidl::android::hardware::automotive::vehicle::VehicleProperty;
@@ -111,7 +113,8 @@ TEST_F(VehiclePropertyStoreTest, testGetConfig) {
TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) {
TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) {
    Result<const VehiclePropConfig*> result = mStore->getConfig(INVALID_PROP_ID);
    Result<const VehiclePropConfig*> result = mStore->getConfig(INVALID_PROP_ID);


    ASSERT_FALSE(result.ok()) << "expect error when getting a config for an invalid property ID";
    EXPECT_FALSE(result.ok()) << "expect error when getting a config for an invalid property ID";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::INVALID_ARG));
}
}


std::vector<VehiclePropValue> getTestPropValues() {
std::vector<VehiclePropValue> getTestPropValues() {
@@ -180,7 +183,8 @@ TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyMultipleValues) {
TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyError) {
TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyError) {
    auto result = mStore->readValuesForProperty(INVALID_PROP_ID);
    auto result = mStore->readValuesForProperty(INVALID_PROP_ID);


    ASSERT_FALSE(result.ok()) << "expect error when reading values for an invalid property";
    EXPECT_FALSE(result.ok()) << "expect error when reading values for an invalid property";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::INVALID_ARG));
}
}


TEST_F(VehiclePropertyStoreTest, testReadValueOk) {
TEST_F(VehiclePropertyStoreTest, testReadValueOk) {
@@ -219,15 +223,19 @@ TEST_F(VehiclePropertyStoreTest, testReadValueError) {


    auto result = mStore->readValue(toInt(VehicleProperty::TIRE_PRESSURE), WHEEL_REAR_LEFT);
    auto result = mStore->readValue(toInt(VehicleProperty::TIRE_PRESSURE), WHEEL_REAR_LEFT);


    ASSERT_FALSE(result.ok()) << "expect error when reading a value that has not been written";
    EXPECT_FALSE(result.ok()) << "expect error when reading a value that has not been written";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::NOT_AVAILABLE));
}
}


TEST_F(VehiclePropertyStoreTest, testWriteValueError) {
TEST_F(VehiclePropertyStoreTest, testWriteValueError) {
    auto v = mValuePool->obtain(VehiclePropertyType::FLOAT);
    auto v = mValuePool->obtain(VehiclePropertyType::FLOAT);
    v->prop = INVALID_PROP_ID;
    v->prop = INVALID_PROP_ID;
    v->value.floatValues = {1.0};
    v->value.floatValues = {1.0};
    ASSERT_FALSE(mStore->writeValue(std::move(v)).ok())

            << "expect error when writing value for an invalid property ID";
    auto result = mStore->writeValue(std::move(v));

    EXPECT_FALSE(result.ok()) << "expect error when writing value for an invalid property ID";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::INVALID_ARG));
}
}


TEST_F(VehiclePropertyStoreTest, testWriteValueNoAreaConfig) {
TEST_F(VehiclePropertyStoreTest, testWriteValueNoAreaConfig) {
@@ -236,8 +244,11 @@ TEST_F(VehiclePropertyStoreTest, testWriteValueNoAreaConfig) {
    v->value.floatValues = {1.0};
    v->value.floatValues = {1.0};
    // There is no config for ALL_WHEELS.
    // There is no config for ALL_WHEELS.
    v->areaId = ALL_WHEELS;
    v->areaId = ALL_WHEELS;
    ASSERT_FALSE(mStore->writeValue(std::move(v)).ok())

            << "expect error when writing value for an area without config";
    auto result = mStore->writeValue(std::move(v));

    EXPECT_FALSE(result.ok()) << "expect error when writing value for an area without config";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::INVALID_ARG));
}
}


TEST_F(VehiclePropertyStoreTest, testWriteOutdatedValue) {
TEST_F(VehiclePropertyStoreTest, testWriteOutdatedValue) {
@@ -254,8 +265,11 @@ TEST_F(VehiclePropertyStoreTest, testWriteOutdatedValue) {
    v2->prop = toInt(VehicleProperty::TIRE_PRESSURE);
    v2->prop = toInt(VehicleProperty::TIRE_PRESSURE);
    v2->value.floatValues = {180.0};
    v2->value.floatValues = {180.0};
    v2->areaId = WHEEL_FRONT_LEFT;
    v2->areaId = WHEEL_FRONT_LEFT;
    ASSERT_FALSE(mStore->writeValue(std::move(v2)).ok())

            << "expect error when writing an outdated value";
    auto result = mStore->writeValue(std::move(v2));

    EXPECT_FALSE(result.ok()) << "expect error when writing an outdated value";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::INVALID_ARG));
}
}


TEST_F(VehiclePropertyStoreTest, testToken) {
TEST_F(VehiclePropertyStoreTest, testToken) {
@@ -300,8 +314,10 @@ TEST_F(VehiclePropertyStoreTest, testRemoveValue) {
    }
    }


    mStore->removeValue(values[0]);
    mStore->removeValue(values[0]);
    auto result = mStore->readValue(values[0]);


    ASSERT_FALSE(mStore->readValue(values[0]).ok()) << "expect error when reading a removed value";
    EXPECT_FALSE(result.ok()) << "expect error when reading a removed value";
    EXPECT_EQ(result.error().code(), toInt(StatusCode::NOT_AVAILABLE));


    auto leftTirePressureResult = mStore->readValue(values[1]);
    auto leftTirePressureResult = mStore->readValue(values[1]);