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
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
index 272582a..dcc7031 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
@@ -86,11 +86,15 @@
::android::base::Result<std::vector<VehiclePropValuePool::RecyclableType>>
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(
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(
int32_t prop, int32_t area = 0, int64_t token = 0) const;
diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
index 10c2468..8d12c2b 100644
--- a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
+++ b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
@@ -19,6 +19,7 @@
#include "VehiclePropertyStore.h"
+#include <VehicleHalTypes.h>
#include <VehicleUtils.h>
#include <android-base/format.h>
#include <math/HashCombine.h>
@@ -28,10 +29,12 @@
namespace automotive {
namespace vehicle {
+using ::aidl::android::hardware::automotive::vehicle::StatusCode;
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::Error;
using ::android::base::Result;
bool VehiclePropertyStore::RecordId::operator==(const VehiclePropertyStore::RecordId& other) const {
@@ -86,7 +89,8 @@
if (auto it = record.values.find(recId); it != record.values.end()) {
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,
@@ -103,15 +107,16 @@
bool updateStatus) {
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) {
- return Errorf("property: {:d} not registered", propValue->prop);
+ return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
}
- if (!isGlobalProp(propValue->prop) &&
- getAreaConfig(*propValue, record->propConfig) == nullptr) {
- return Errorf("no config for property: {:d} area: {:d}", propValue->prop,
- propValue->areaId);
+ if (!isGlobalProp(propId) && getAreaConfig(*propValue, record->propConfig) == nullptr) {
+ return Error(toInt(StatusCode::INVALID_ARG))
+ << "no config for property: " << propId << " area: " << propValue->areaId;
}
VehiclePropertyStore::RecordId recId = getRecordIdLocked(*propValue, *record);
@@ -122,7 +127,8 @@
VehiclePropertyStatus oldStatus = valueToUpdate->status;
// propValue is outdated and drops it.
if (oldTimestamp > propValue->timestamp) {
- return Errorf("outdated timestamp: {:d}", propValue->timestamp);
+ return Error(toInt(StatusCode::INVALID_ARG))
+ << "outdated timestamp: " << propValue->timestamp;
}
if (!updateStatus) {
propValue->status = oldStatus;
@@ -190,7 +196,7 @@
const VehiclePropertyStore::Record* record = getRecordLocked(propId);
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) {
@@ -203,9 +209,10 @@
const VehiclePropValue& propValue) const {
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) {
- return Errorf("property: {:d} not registered", propValue.prop);
+ return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
}
VehiclePropertyStore::RecordId recId = getRecordIdLocked(propValue, *record);
@@ -219,7 +226,7 @@
const VehiclePropertyStore::Record* record = getRecordLocked(propId);
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};
@@ -242,7 +249,7 @@
const VehiclePropertyStore::Record* record = getRecordLocked(propId);
if (record == nullptr) {
- return Errorf("property: {:d} not registered", propId);
+ return Error(toInt(StatusCode::INVALID_ARG)) << "property: " << propId << " not registered";
}
return &record->propConfig;
diff --git a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
index 4ed445b..1f230e7 100644
--- a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
+++ b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
@@ -15,6 +15,7 @@
*/
#include <PropertyUtils.h>
+#include <VehicleHalTypes.h>
#include <VehiclePropertyStore.h>
#include <VehicleUtils.h>
#include <gmock/gmock.h>
@@ -27,6 +28,7 @@
namespace {
+using ::aidl::android::hardware::automotive::vehicle::StatusCode;
using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehicleProperty;
@@ -111,7 +113,8 @@
TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) {
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() {
@@ -180,7 +183,8 @@
TEST_F(VehiclePropertyStoreTest, testReadValuesForPropertyError) {
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) {
@@ -219,15 +223,19 @@
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) {
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";
+
+ 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) {
@@ -236,8 +244,11 @@
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";
+
+ 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) {
@@ -254,8 +265,11 @@
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";
+
+ 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) {
@@ -300,8 +314,10 @@
}
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]);