Add on-value-change callback to propstore.
Add on-value-change callback to VehiclePropertyStore. When a new
value is written to propStore or a value is updated, the callback
would be invoked which could be used to inform VHAL client about
value updates.
Test: atest VehicleHalVehicleUtilsTest
Bug: 201830716
Change-Id: I980f5e8c34d9f872f962776859de9615ce3bf690
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
index ababf5e..272582a 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
@@ -40,11 +40,17 @@
// to get value for all areas for particular property.
//
// This class is thread-safe, however it uses blocking synchronization across all methods.
-class VehiclePropertyStore {
+class VehiclePropertyStore final {
public:
explicit VehiclePropertyStore(std::shared_ptr<VehiclePropValuePool> valuePool)
: mValuePool(valuePool) {}
+ ~VehiclePropertyStore();
+
+ // Callback when a property value has been updated or a new value added.
+ using OnValueChangeCallback = std::function<void(
+ const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue&)>;
+
// Function that used to calculate unique token for given VehiclePropValue.
using TokenFunction = ::std::function<int64_t(
const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& value)>;
@@ -97,6 +103,9 @@
const ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig*>
getConfig(int32_t propId) const;
+ // Set a callback that would be called when a property value has been updated.
+ void setOnValueChangeCallback(const OnValueChangeCallback& callback);
+
private:
struct RecordId {
int32_t area;
@@ -117,10 +126,11 @@
std::unordered_map<RecordId, VehiclePropValuePool::RecyclableType, RecordIdHash> values;
};
- mutable std::mutex mLock;
- std::unordered_map<int32_t, Record> mRecordsByPropId GUARDED_BY(mLock);
// {@code VehiclePropValuePool} is thread-safe.
std::shared_ptr<VehiclePropValuePool> mValuePool;
+ mutable std::mutex mLock;
+ std::unordered_map<int32_t, Record> mRecordsByPropId GUARDED_BY(mLock);
+ OnValueChangeCallback mOnValueChangeCallback GUARDED_BY(mLock);
const Record* getRecordLocked(int32_t propId) const;
diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
index 2869d1d..10c2468 100644
--- a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
+++ b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
@@ -49,6 +49,14 @@
return res;
}
+VehiclePropertyStore::~VehiclePropertyStore() {
+ std::lock_guard<std::mutex> lockGuard(mLock);
+
+ // Recycling record requires mValuePool, so need to recycle them before destroying mValuePool.
+ mRecordsByPropId.clear();
+ mValuePool.reset();
+}
+
const VehiclePropertyStore::Record* VehiclePropertyStore::getRecordLocked(int32_t propId) const
REQUIRES(mLock) {
auto RecordIt = mRecordsByPropId.find(propId);
@@ -75,11 +83,10 @@
Result<VehiclePropValuePool::RecyclableType> 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());
+ 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());
}
void VehiclePropertyStore::registerProperty(const VehiclePropConfig& config,
@@ -108,26 +115,31 @@
}
VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record);
- auto it = record->values.find(recId);
- if (it == record->values.end()) {
- record->values[recId] = std::move(propValue);
- if (!updateStatus) {
- record->values[recId]->status = VehiclePropertyStatus::AVAILABLE;
+ bool valueUpdated = true;
+ if (auto it = record->values.find(recId); it != record->values.end()) {
+ const VehiclePropValue* valueToUpdate = it->second.get();
+ int64_t oldTimestamp = valueToUpdate->timestamp;
+ VehiclePropertyStatus oldStatus = valueToUpdate->status;
+ // propValue is outdated and drops it.
+ if (oldTimestamp > propValue->timestamp) {
+ return Errorf("outdated timestamp: {:d}", propValue->timestamp);
}
- return {};
- }
- const VehiclePropValue* valueToUpdate = it->second.get();
- long oldTimestamp = valueToUpdate->timestamp;
- VehiclePropertyStatus oldStatus = valueToUpdate->status;
- // propValue is outdated and drops it.
- if (oldTimestamp > propValue->timestamp) {
- return Errorf("outdated timestamp: {:d}", propValue->timestamp);
- }
- record->values[recId] = std::move(propValue);
- if (!updateStatus) {
- record->values[recId]->status = oldStatus;
+ if (!updateStatus) {
+ propValue->status = oldStatus;
+ }
+
+ valueUpdated = (valueToUpdate->value != propValue->value ||
+ valueToUpdate->status != propValue->status ||
+ valueToUpdate->prop != propValue->prop ||
+ valueToUpdate->areaId != propValue->areaId);
+ } else if (!updateStatus) {
+ propValue->status = VehiclePropertyStatus::AVAILABLE;
}
+ record->values[recId] = std::move(propValue);
+ if (valueUpdated && mOnValueChangeCallback != nullptr) {
+ mOnValueChangeCallback(*(record->values[recId]));
+ }
return {};
}
@@ -236,6 +248,13 @@
return &record->propConfig;
}
+void VehiclePropertyStore::setOnValueChangeCallback(
+ const VehiclePropertyStore::OnValueChangeCallback& callback) {
+ std::lock_guard<std::mutex> g(mLock);
+
+ mOnValueChangeCallback = callback;
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
index f1d218d..4ed445b 100644
--- a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
+++ b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
@@ -382,6 +382,57 @@
ASSERT_EQ(result.value()->status, VehiclePropertyStatus::AVAILABLE);
}
+TEST_F(VehiclePropertyStoreTest, testPropertyChangeCallbackNewValue) {
+ VehiclePropValue updatedValue;
+ mStore->setOnValueChangeCallback(
+ [&updatedValue](const VehiclePropValue& value) { updatedValue = value; });
+ VehiclePropValue fuelCapacity = {
+ .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY),
+ .value = {.floatValues = {1.0}},
+ };
+ ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity)));
+
+ ASSERT_EQ(updatedValue, fuelCapacity);
+}
+
+TEST_F(VehiclePropertyStoreTest, testPropertyChangeCallbackUpdateValue) {
+ VehiclePropValue updatedValue;
+ VehiclePropValue fuelCapacity = {
+ .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY),
+ .value = {.floatValues = {1.0}},
+ };
+ ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity)));
+
+ mStore->setOnValueChangeCallback(
+ [&updatedValue](const VehiclePropValue& value) { updatedValue = value; });
+
+ fuelCapacity.value.floatValues[0] = 2.0;
+ fuelCapacity.timestamp = 1;
+
+ ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity)));
+
+ ASSERT_EQ(updatedValue, fuelCapacity);
+}
+
+TEST_F(VehiclePropertyStoreTest, testPropertyChangeCallbackNoUpdate) {
+ VehiclePropValue updatedValue{
+ .prop = INVALID_PROP_ID,
+ };
+ VehiclePropValue fuelCapacity = {
+ .prop = toInt(VehicleProperty::INFO_FUEL_CAPACITY),
+ .value = {.floatValues = {1.0}},
+ };
+ ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity)));
+
+ mStore->setOnValueChangeCallback(
+ [&updatedValue](const VehiclePropValue& value) { updatedValue = value; });
+
+ // Write the same value again should succeed but should not trigger callback.
+ ASSERT_RESULT_OK(mStore->writeValue(mValuePool->obtain(fuelCapacity)));
+
+ ASSERT_EQ(updatedValue.prop, INVALID_PROP_ID);
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware