From 3d8595e3f91734da263f395403d2de6d65c11803 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Wed, 29 Sep 2021 15:48:33 -0700 Subject: [PATCH] Use ObjectPool objects in property store. Use object pool recyclable objects in property store API to improve code efficiency. This would reduce the number of unnecssary copy of property values. This CL also adds a upateStatus option which by default is set to false, so property from client would not override the property status. Test: atest VehicleHalVehicleUtilsTest Bug: 200737967 Change-Id: Ie96a41ee441e085e10ad1059b67c708ff7173ae0 --- automotive/vehicle/aidl/impl/Android.bp | 1 + .../common/include/VehiclePropertyStore.h | 48 +++-- .../utils/common/src/VehiclePropertyStore.cpp | 79 ++++--- .../common/test/VehiclePropertyStoreTest.cpp | 196 ++++++++++++------ 4 files changed, 208 insertions(+), 116 deletions(-) diff --git a/automotive/vehicle/aidl/impl/Android.bp b/automotive/vehicle/aidl/impl/Android.bp index fc3d40bdd8..94f590dd3e 100644 --- a/automotive/vehicle/aidl/impl/Android.bp +++ b/automotive/vehicle/aidl/impl/Android.bp @@ -22,6 +22,7 @@ cc_defaults { name: "VehicleHalDefaults", static_libs: [ "android.hardware.automotive.vehicle-V1-ndk", + "libmath", ], shared_libs: [ "libbase", diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h index b19ab841a8..ababf5e8fd 100644 --- a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h +++ b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h @@ -24,6 +24,7 @@ #include #include +#include #include #include @@ -41,6 +42,9 @@ namespace vehicle { // This class is thread-safe, however it uses blocking synchronization across all methods. class VehiclePropertyStore { public: + explicit VehiclePropertyStore(std::shared_ptr valuePool) + : mValuePool(valuePool) {} + // Function that used to calculate unique token for given VehiclePropValue. using TokenFunction = ::std::function; @@ -53,10 +57,13 @@ class VehiclePropertyStore { const ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig& config, TokenFunction tokenFunc = nullptr); - // Stores provided value. Returns true if value was written returns false if config wasn't - // registered. - ::android::base::Result writeValue( - const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue); + // Stores provided value. Returns error if config wasn't registered. If 'updateStatus' is + // true, the 'status' in 'propValue' would be stored. Otherwise, if this is a new value, + // 'status' would be initialized to {@code VehiclePropertyStatus::AVAILABLE}, if this is to + // override an existing value, the status for the existing value would be used for the + // overridden value. + ::android::base::Result writeValue(VehiclePropValuePool::RecyclableType propValue, + bool updateStatus = false); // Remove a given property value from the property store. The 'propValue' would be used to // generate the key for the value to remove. @@ -67,24 +74,19 @@ class VehiclePropertyStore { void removeValuesForProperty(int32_t propId); // Read all the stored values. - std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue> readAllValues() - const; + std::vector readAllValues() const; // Read all the values for the property. - ::android::base::Result< - std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>> + ::android::base::Result> readValuesForProperty(int32_t propId) const; // Read the value for the requested property. - ::android::base::Result< - std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>> - readValue( + ::android::base::Result readValue( const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& request) const; // Read the value for the requested property. - ::android::base::Result< - std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>> - readValue(int32_t prop, int32_t area = 0, int64_t token = 0) const; + ::android::base::Result readValue( + int32_t prop, int32_t area = 0, int64_t token = 0) const; // Get all property configs. std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropConfig> getAllConfigs() @@ -100,20 +102,25 @@ class VehiclePropertyStore { int32_t area; int64_t token; - bool operator==(const RecordId& other) const; - bool operator<(const RecordId& other) const; - std::string toString() const; + + bool operator==(const RecordId& other) const; + }; + + struct RecordIdHash { + size_t operator()(RecordId const& recordId) const; }; struct Record { ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig propConfig; TokenFunction tokenFunction; - std::map values; + std::unordered_map values; }; mutable std::mutex mLock; std::unordered_map mRecordsByPropId GUARDED_BY(mLock); + // {@code VehiclePropValuePool} is thread-safe. + std::shared_ptr mValuePool; const Record* getRecordLocked(int32_t propId) const; @@ -123,9 +130,8 @@ class VehiclePropertyStore { const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue, const Record& record) const; - ::android::base::Result< - std::unique_ptr<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>> - readValueLocked(const RecordId& recId, const Record& record) const; + ::android::base::Result readValueLocked( + const RecordId& recId, const Record& record) const; }; } // namespace vehicle diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp index b660f36d65..2869d1d621 100644 --- a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp @@ -21,6 +21,7 @@ #include #include +#include namespace android { namespace hardware { @@ -29,6 +30,7 @@ namespace vehicle { using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig; using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig; +using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus; using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue; using ::android::base::Result; @@ -36,14 +38,17 @@ bool VehiclePropertyStore::RecordId::operator==(const VehiclePropertyStore::Reco return area == other.area && token == other.token; } -bool VehiclePropertyStore::RecordId::operator<(const VehiclePropertyStore::RecordId& other) const { - return area < other.area || (area == other.area && token < other.token); -} - std::string VehiclePropertyStore::RecordId::toString() const { return ::fmt::format("RecordID{{.areaId={:d}, .token={:d}}}", area, token); } +size_t VehiclePropertyStore::RecordIdHash::operator()(RecordId const& recordId) const { + size_t res = 0; + hashCombine(res, recordId.area); + hashCombine(res, recordId.token); + return res; +} + const VehiclePropertyStore::Record* VehiclePropertyStore::getRecordLocked(int32_t propId) const REQUIRES(mLock) { auto RecordIt = mRecordsByPropId.find(propId); @@ -68,13 +73,13 @@ VehiclePropertyStore::RecordId VehiclePropertyStore::getRecordIdLocked( return recId; } -Result> VehiclePropertyStore::readValueLocked( +Result VehiclePropertyStore::readValueLocked( const RecordId& recId, const Record& record) const REQUIRES(mLock) { auto it = record.values.find(recId); if (it == record.values.end()) { return Errorf("Record ID: {} is not found", recId.toString()); } - return std::make_unique(it->second); + return mValuePool->obtain(*(it->second)); } void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config, @@ -87,36 +92,42 @@ void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config, }; } -Result VehiclePropertyStore::writeValue(const VehiclePropValue& propValue) { +Result VehiclePropertyStore::writeValue(VehiclePropValuePool::RecyclableType propValue, + bool updateStatus) { std::lock_guard g(mLock); - VehiclePropertyStore::Record* record = getRecordLocked(propValue.prop); + VehiclePropertyStore::Record* record = getRecordLocked(propValue->prop); if (record == nullptr) { - return Errorf("property: {:d} not registered", propValue.prop); + return Errorf("property: {:d} not registered", propValue->prop); } - if (!isGlobalProp(propValue.prop) && getAreaConfig(propValue, record->propConfig) == nullptr) { - return Errorf("no config for property: {:d} area: {:d}", propValue.prop, propValue.areaId); + if (!isGlobalProp(propValue->prop) && + getAreaConfig(*propValue, record->propConfig) == nullptr) { + return Errorf("no config for property: {:d} area: {:d}", propValue->prop, + propValue->areaId); } - VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record); + VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record); auto it = record->values.find(recId); if (it == record->values.end()) { - record->values[recId] = propValue; + record->values[recId] = std::move(propValue); + if (!updateStatus) { + record->values[recId]->status = VehiclePropertyStatus::AVAILABLE; + } return {}; } - VehiclePropValue* valueToUpdate = &(it->second); - + const VehiclePropValue* valueToUpdate = it->second.get(); + long oldTimestamp = valueToUpdate->timestamp; + VehiclePropertyStatus oldStatus = valueToUpdate->status; // propValue is outdated and drops it. - if (valueToUpdate->timestamp > propValue.timestamp) { - return Errorf("outdated timestamp: {:d}", propValue.timestamp); + if (oldTimestamp > propValue->timestamp) { + return Errorf("outdated timestamp: {:d}", propValue->timestamp); } - // Update the propertyValue. - // The timestamp in propertyStore should only be updated by the server side. It indicates - // the time when the event is generated by the server. - valueToUpdate->timestamp = propValue.timestamp; - valueToUpdate->value = propValue.value; - valueToUpdate->status = propValue.status; + record->values[recId] = std::move(propValue); + if (!updateStatus) { + record->values[recId]->status = oldStatus; + } + return {}; } @@ -145,25 +156,25 @@ void VehiclePropertyStore::removeValuesForProperty(int32_t propId) { record->values.clear(); } -std::vector VehiclePropertyStore::readAllValues() const { +std::vector VehiclePropertyStore::readAllValues() const { std::lock_guard g(mLock); - std::vector allValues; + std::vector allValues; for (auto const& [_, record] : mRecordsByPropId) { for (auto const& [_, value] : record.values) { - allValues.push_back(value); + allValues.push_back(std::move(mValuePool->obtain(*value))); } } return allValues; } -Result> VehiclePropertyStore::readValuesForProperty( - int32_t propId) const { +Result> +VehiclePropertyStore::readValuesForProperty(int32_t propId) const { std::lock_guard g(mLock); - std::vector values; + std::vector values; const VehiclePropertyStore::Record* record = getRecordLocked(propId); if (record == nullptr) { @@ -171,12 +182,12 @@ Result> VehiclePropertyStore::readValuesForPropert } for (auto const& [_, value] : record->values) { - values.push_back(value); + values.push_back(std::move(mValuePool->obtain(*value))); } return values; } -Result> VehiclePropertyStore::readValue( +Result VehiclePropertyStore::readValue( const VehiclePropValue& propValue) const { std::lock_guard g(mLock); @@ -189,9 +200,9 @@ Result> VehiclePropertyStore::readValue( return readValueLocked(recId, *record); } -Result> VehiclePropertyStore::readValue(int32_t propId, - int32_t areaId, - int64_t token) const { +Result VehiclePropertyStore::readValue(int32_t propId, + int32_t areaId, + int64_t token) const { std::lock_guard g(mLock); const VehiclePropertyStore::Record* record = getRecordLocked(propId); diff --git a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp index 8c70fea924..f1d218d2da 100644 --- a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp @@ -32,6 +32,8 @@ using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig; using ::aidl::android::hardware::automotive::vehicle::VehicleProperty; using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyAccess; using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyChangeMode; +using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus; +using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType; using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue; using ::android::base::Result; using ::testing::ElementsAre; @@ -51,6 +53,17 @@ int64_t timestampToken(const VehiclePropValue& value) { return value.timestamp; } +// A helper function to turn value pointer to value structure for easier comparison. +std::vector convertValuePtrsToValues( + const std::vector& values) { + std::vector returnValues; + returnValues.reserve(values.size()); + for (auto& value : values) { + returnValues.push_back(*value); + } + return returnValues; +} + } // namespace class VehiclePropertyStoreTest : public ::testing::Test { @@ -70,30 +83,33 @@ class VehiclePropertyStoreTest : public ::testing::Test { VehicleAreaConfig{.areaId = WHEEL_REAR_LEFT}, VehicleAreaConfig{.areaId = WHEEL_REAR_RIGHT}}, }; - mStore.registerProperty(mConfigFuelCapacity); - mStore.registerProperty(configTirePressure); + mValuePool = std::make_shared(); + mStore.reset(new VehiclePropertyStore(mValuePool)); + mStore->registerProperty(mConfigFuelCapacity); + mStore->registerProperty(configTirePressure); } - VehiclePropertyStore mStore; VehiclePropConfig mConfigFuelCapacity; + std::shared_ptr mValuePool; + std::unique_ptr mStore; }; TEST_F(VehiclePropertyStoreTest, testGetAllConfigs) { - std::vector configs = mStore.getAllConfigs(); + std::vector configs = mStore->getAllConfigs(); ASSERT_EQ(configs.size(), static_cast(2)); } TEST_F(VehiclePropertyStoreTest, testGetConfig) { Result result = - mStore.getConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); + mStore->getConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); ASSERT_RESULT_OK(result); ASSERT_EQ(*(result.value()), mConfigFuelCapacity); } TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) { - Result result = mStore.getConfig(INVALID_PROP_ID); + Result result = mStore->getConfig(INVALID_PROP_ID); ASSERT_FALSE(result.ok()) << "expect error when getting a config for an invalid property ID"; } @@ -122,46 +138,47 @@ std::vector getTestPropValues() { TEST_F(VehiclePropertyStoreTest, testWriteValueOk) { auto values = getTestPropValues(); - ASSERT_RESULT_OK(mStore.writeValue(values[0])); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(values[0]))); } TEST_F(VehiclePropertyStoreTest, testReadAllValues) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - auto gotValues = mStore.readAllValues(); + auto gotValues = mStore->readAllValues(); - ASSERT_THAT(gotValues, WhenSortedBy(propValueCmp, Eq(values))); + ASSERT_THAT(convertValuePtrsToValues(gotValues), WhenSortedBy(propValueCmp, Eq(values))); } TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyOneValue) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - auto result = mStore.readValuesForProperty(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); + auto result = mStore->readValuesForProperty(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); ASSERT_RESULT_OK(result); - ASSERT_THAT(result.value(), ElementsAre(values[0])); + ASSERT_THAT(convertValuePtrsToValues(result.value()), ElementsAre(values[0])); } TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyMultipleValues) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - auto result = mStore.readValuesForProperty(toInt(VehicleProperty::TIRE_PRESSURE)); + auto result = mStore->readValuesForProperty(toInt(VehicleProperty::TIRE_PRESSURE)); ASSERT_RESULT_OK(result); - ASSERT_THAT(result.value(), WhenSortedBy(propValueCmp, ElementsAre(values[1], values[2]))); + ASSERT_THAT(convertValuePtrsToValues(result.value()), + WhenSortedBy(propValueCmp, ElementsAre(values[1], values[2]))); } 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"; } @@ -169,7 +186,7 @@ TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyError) { TEST_F(VehiclePropertyStoreTest, testReadValueOk) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } VehiclePropValue requestValue = { @@ -177,7 +194,7 @@ TEST_F(VehiclePropertyStoreTest, testReadValueOk) { .areaId = WHEEL_FRONT_LEFT, }; - auto result = mStore.readValue(requestValue); + auto result = mStore->readValue(requestValue); ASSERT_RESULT_OK(result); ASSERT_EQ(*(result.value()), values[1]); @@ -186,10 +203,10 @@ TEST_F(VehiclePropertyStoreTest, testReadValueOk) { TEST_F(VehiclePropertyStoreTest, testReadValueByPropIdOk) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - auto result = mStore.readValue(toInt(VehicleProperty::TIRE_PRESSURE), WHEEL_FRONT_RIGHT); + auto result = mStore->readValue(toInt(VehicleProperty::TIRE_PRESSURE), WHEEL_FRONT_RIGHT); ASSERT_EQ(*(result.value()), values[2]); } @@ -197,50 +214,47 @@ TEST_F(VehiclePropertyStoreTest, testReadValueByPropIdOk) { TEST_F(VehiclePropertyStoreTest, testReadValueError) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - 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"; } TEST_F(VehiclePropertyStoreTest, testWriteValueError) { - ASSERT_FALSE(mStore.writeValue({ - .prop = INVALID_PROP_ID, - .value = {.floatValues = {1.0}}, - }) - .ok()) + auto v = mValuePool->obtain(VehiclePropertyType::FLOAT); + v->prop = INVALID_PROP_ID; + v->value.floatValues = {1.0}; + ASSERT_FALSE(mStore->writeValue(std::move(v)).ok()) << "expect error when writing value for an invalid property ID"; } TEST_F(VehiclePropertyStoreTest, testWriteValueNoAreaConfig) { - ASSERT_FALSE(mStore.writeValue({ - .prop = toInt(VehicleProperty::TIRE_PRESSURE), - .value = {.floatValues = {180.0}}, - // There is no config for ALL_WHEELS. - .areaId = ALL_WHEELS, - }) - .ok()) + auto v = mValuePool->obtain(VehiclePropertyType::FLOAT); + v->prop = toInt(VehicleProperty::TIRE_PRESSURE); + v->value.floatValues = {1.0}; + // There is no config for ALL_WHEELS. + v->areaId = ALL_WHEELS; + ASSERT_FALSE(mStore->writeValue(std::move(v)).ok()) << "expect error when writing value for an area without config"; } TEST_F(VehiclePropertyStoreTest, testWriteOutdatedValue) { - ASSERT_RESULT_OK(mStore.writeValue({ - .timestamp = 1, - .prop = toInt(VehicleProperty::TIRE_PRESSURE), - .value = {.floatValues = {180.0}}, - .areaId = WHEEL_FRONT_LEFT, - })); + auto v = mValuePool->obtain(VehiclePropertyType::FLOAT); + v->timestamp = 1; + v->prop = toInt(VehicleProperty::TIRE_PRESSURE); + v->value.floatValues = {180.0}; + v->areaId = WHEEL_FRONT_LEFT; + ASSERT_RESULT_OK(mStore->writeValue(std::move(v))); // Write an older value. - ASSERT_FALSE(mStore.writeValue({ - .timestamp = 0, - .prop = toInt(VehicleProperty::TIRE_PRESSURE), - .value = {.floatValues = {180.0}}, - .areaId = WHEEL_FRONT_LEFT, - }) - .ok()) + auto v2 = mValuePool->obtain(VehiclePropertyType::FLOAT); + v2->timestamp = 0; + v2->prop = toInt(VehicleProperty::TIRE_PRESSURE); + v2->value.floatValues = {180.0}; + v2->areaId = WHEEL_FRONT_LEFT; + ASSERT_FALSE(mStore->writeValue(std::move(v2)).ok()) << "expect error when writing an outdated value"; } @@ -251,7 +265,7 @@ TEST_F(VehiclePropertyStoreTest, testToken) { }; // Replace existing config. - mStore.registerProperty(config, timestampToken); + mStore->registerProperty(config, timestampToken); VehiclePropValue fuelCapacityValueToken1 = { .timestamp = 1, @@ -265,15 +279,15 @@ TEST_F(VehiclePropertyStoreTest, testToken) { .value = {.floatValues = {2.0}}, }; - ASSERT_RESULT_OK(mStore.writeValue(fuelCapacityValueToken1)); - ASSERT_RESULT_OK(mStore.writeValue(fuelCapacityValueToken2)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacityValueToken1))); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacityValueToken2))); - auto result = mStore.readValuesForProperty(propId); + auto result = mStore->readValuesForProperty(propId); ASSERT_RESULT_OK(result); ASSERT_EQ(result.value().size(), static_cast(2)); - auto tokenResult = mStore.readValue(propId, /*areaId=*/0, /*token=*/2); + auto tokenResult = mStore->readValue(propId, /*areaId=*/0, /*token=*/2); ASSERT_RESULT_OK(tokenResult); ASSERT_EQ(*(tokenResult.value()), fuelCapacityValueToken2); @@ -282,14 +296,14 @@ TEST_F(VehiclePropertyStoreTest, testToken) { TEST_F(VehiclePropertyStoreTest, testRemoveValue) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(value))); } - mStore.removeValue(values[0]); + mStore->removeValue(values[0]); - ASSERT_FALSE(mStore.readValue(values[0]).ok()) << "expect error when reading a removed value"; + ASSERT_FALSE(mStore->readValue(values[0]).ok()) << "expect error when reading a removed value"; - auto leftTirePressureResult = mStore.readValue(values[1]); + auto leftTirePressureResult = mStore->readValue(values[1]); ASSERT_RESULT_OK(leftTirePressureResult); ASSERT_EQ(*(leftTirePressureResult.value()), values[1]); @@ -298,16 +312,76 @@ TEST_F(VehiclePropertyStoreTest, testRemoveValue) { TEST_F(VehiclePropertyStoreTest, testRemoveValuesForProperty) { auto values = getTestPropValues(); for (const auto& value : values) { - ASSERT_RESULT_OK(mStore.writeValue(value)); + ASSERT_RESULT_OK(mStore->writeValue(std::move(mValuePool->obtain(value)))); } - mStore.removeValuesForProperty(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); - mStore.removeValuesForProperty(toInt(VehicleProperty::TIRE_PRESSURE)); + mStore->removeValuesForProperty(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); + mStore->removeValuesForProperty(toInt(VehicleProperty::TIRE_PRESSURE)); - auto gotValues = mStore.readAllValues(); + auto gotValues = mStore->readAllValues(); ASSERT_TRUE(gotValues.empty()); } +TEST_F(VehiclePropertyStoreTest, testWriteValueUpdateStatus) { + VehiclePropValue fuelCapacity = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + .value = {.floatValues = {1.0}}, + }; + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity), true)); + + fuelCapacity.status = VehiclePropertyStatus::UNAVAILABLE; + + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity), true)); + + VehiclePropValue requestValue = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + }; + + auto result = mStore->readValue(requestValue); + + ASSERT_RESULT_OK(result); + ASSERT_EQ(result.value()->status, VehiclePropertyStatus::UNAVAILABLE); +} + +TEST_F(VehiclePropertyStoreTest, testWriteValueNoUpdateStatus) { + VehiclePropValue fuelCapacity = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + .value = {.floatValues = {1.0}}, + }; + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity), true)); + + fuelCapacity.status = VehiclePropertyStatus::UNAVAILABLE; + + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity), false)); + + VehiclePropValue requestValue = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + }; + + auto result = mStore->readValue(requestValue); + + ASSERT_RESULT_OK(result); + ASSERT_EQ(result.value()->status, VehiclePropertyStatus::AVAILABLE); +} + +TEST_F(VehiclePropertyStoreTest, testWriteValueNoUpdateStatusForNewValue) { + VehiclePropValue fuelCapacity = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + .value = {.floatValues = {1.0}}, + .status = VehiclePropertyStatus::UNAVAILABLE, + }; + ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity), false)); + + VehiclePropValue requestValue = { + .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY), + }; + + auto result = mStore->readValue(requestValue); + + ASSERT_RESULT_OK(result); + ASSERT_EQ(result.value()->status, VehiclePropertyStatus::AVAILABLE); +} + } // namespace vehicle } // namespace automotive } // namespace hardware