From d7575f7672ab6e13794cbf40911bb8a423ce9709 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Tue, 5 Oct 2021 17:52:05 -0700 Subject: [PATCH] Add vendor override property to fake VHAL hardware. Allow vendor to override default properties in fake VHAL hardware. Test: atest FakeVehicleHardwareTest Bug: 201830716 Change-Id: Ie6061d7a8123e3b22c6fa467639f1fe77dec222c --- .../aidl/impl/fake_impl/hardware/Android.bp | 8 +- .../hardware/include/FakeVehicleHardware.h | 18 +- .../hardware/src/FakeVehicleHardware.cpp | 48 ++++++ .../impl/fake_impl/hardware/test/Android.bp | 12 ++ .../hardware/test/FakeVehicleHardwareTest.cpp | 157 +++++++++++++++++- .../test/override/gear_selection.json | 9 + .../test/override/hvac_temperature_set.json | 10 ++ .../impl/utils/common/include/VehicleUtils.h | 11 +- .../utils/common/src/VehicleObjectPool.cpp | 8 +- 9 files changed, 268 insertions(+), 13 deletions(-) create mode 100644 automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/gear_selection.json create mode 100644 automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/hvac_temperature_set.json diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/Android.bp b/automotive/vehicle/aidl/impl/fake_impl/hardware/Android.bp index d614c6b448..29dec74aa0 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/Android.bp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/Android.bp @@ -30,6 +30,12 @@ cc_library { "VehicleHalDefaultConfig", ], export_header_lib_headers: ["IVehicleHardware"], - static_libs: ["VehicleHalUtils"], + static_libs: [ + "VehicleHalUtils", + "FakeVehicleHalValueGenerators", + ], + shared_libs: [ + "libjsoncpp", + ], export_static_lib_headers: ["VehicleHalUtils"], } diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h index dee36f48a4..ead8bb25fa 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h @@ -82,10 +82,8 @@ class FakeVehicleHardware final : public IVehicleHardware { void registerOnPropertySetErrorEvent(OnPropertySetErrorCallback&& callback) override; private: - void storePropInitialValue(const defaultconfig::ConfigDeclaration& config); - void init(std::shared_ptr valuePool); - void onValueChangeCallback( - const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& value); + // Expose private methods to unit test. + friend class FakeVehicleHardwareTestHelper; std::unique_ptr mServerSidePropStore; // mValuePool is also used in mServerSidePropStore. @@ -93,6 +91,18 @@ class FakeVehicleHardware final : public IVehicleHardware { std::mutex mCallbackLock; OnPropertyChangeCallback mOnPropertyChangeCallback GUARDED_BY(mCallbackLock); OnPropertySetErrorCallback mOnPropertySetErrorCallback GUARDED_BY(mCallbackLock); + + void init(std::shared_ptr valuePool); + // Stores the initial value to property store. + void storePropInitialValue(const defaultconfig::ConfigDeclaration& config); + // The callback that would be called when a vehicle property value change happens. + void onValueChangeCallback( + const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& value); + // If property "persist.vendor.vhal_init_value_override" is set to true, override the properties + // using config files in 'overrideDir'. + void maybeOverrideProperties(const char* overrideDir); + // Override the properties using config files in 'overrideDir'. + void overrideProperties(const char* overrideDir); }; } // namespace fake diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp index f8bf7dec52..684b2a7ad0 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp @@ -17,11 +17,17 @@ #include "FakeVehicleHardware.h" #include +#include #include #include +#include #include #include +#include +#include +#include +#include #include namespace android { @@ -30,6 +36,8 @@ namespace automotive { namespace vehicle { namespace fake { +namespace { + using ::aidl::android::hardware::automotive::vehicle::GetValueRequest; using ::aidl::android::hardware::automotive::vehicle::GetValueResult; using ::aidl::android::hardware::automotive::vehicle::RawPropValues; @@ -40,6 +48,11 @@ using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig; using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus; using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue; +const char* VENDOR_OVERRIDE_DIR = "/vendor/etc/automotive/vhaloverride/"; +const char* OVERRIDE_PROPERTY = "persist.vendor.vhal_init_value_override"; + +} // namespace + void FakeVehicleHardware::storePropInitialValue(const defaultconfig::ConfigDeclaration& config) { const VehiclePropConfig& vehiclePropConfig = config.config; int propId = vehiclePropConfig.prop; @@ -99,6 +112,8 @@ void FakeVehicleHardware::init(std::shared_ptr valuePool) storePropInitialValue(it); } + maybeOverrideProperties(VENDOR_OVERRIDE_DIR); + mServerSidePropStore->setOnValueChangeCallback( [this](const VehiclePropValue& value) { return onValueChangeCallback(value); }); } @@ -201,6 +216,39 @@ void FakeVehicleHardware::onValueChangeCallback(const VehiclePropValue& value) { } } +void FakeVehicleHardware::maybeOverrideProperties(const char* overrideDir) { + if (android::base::GetBoolProperty(OVERRIDE_PROPERTY, false)) { + overrideProperties(overrideDir); + } +} + +void FakeVehicleHardware::overrideProperties(const char* overrideDir) { + ALOGI("loading vendor override properties from %s", overrideDir); + if (auto dir = opendir(overrideDir); dir != NULL) { + std::regex regJson(".*[.]json", std::regex::icase); + while (auto f = readdir(dir)) { + if (!std::regex_match(f->d_name, regJson)) { + continue; + } + std::string file = overrideDir + std::string(f->d_name); + JsonFakeValueGenerator tmpGenerator(file); + + std::vector propValues = tmpGenerator.getAllEvents(); + for (const VehiclePropValue& prop : propValues) { + auto propToStore = mValuePool->obtain(prop); + propToStore->timestamp = elapsedRealtimeNano(); + if (auto result = mServerSidePropStore->writeValue(std::move(propToStore), + /*updateStatus=*/true); + !result.ok()) { + ALOGW("failed to write vendor override properties: %d, error: %s", prop.prop, + result.error().message().c_str()); + } + } + } + closedir(dir); + } +} + } // namespace fake } // namespace vehicle } // namespace automotive diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/Android.bp b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/Android.bp index 9f76d09abe..21937b9ee4 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/Android.bp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/Android.bp @@ -29,9 +29,21 @@ cc_test { static_libs: [ "VehicleHalUtils", "FakeVehicleHardware", + "FakeVehicleHalValueGenerators", "libgtest", "libgmock", ], + shared_libs: [ + "libjsoncpp", + ], + data: [ + ":FakeVehicleHardwareTestOverrideJson", + ], defaults: ["VehicleHalDefaults"], test_suites: ["device-tests"], } + +filegroup { + name: "FakeVehicleHardwareTestOverrideJson", + srcs: ["override/*"], +} diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp index 53e647db64..f915d69c9a 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp @@ -16,16 +16,21 @@ #include #include + +#include +#include #include #include +#include #include +#include + namespace android { namespace hardware { namespace automotive { namespace vehicle { namespace fake { - namespace { using ::aidl::android::hardware::automotive::vehicle::GetValueRequest; @@ -38,6 +43,8 @@ using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig; using ::aidl::android::hardware::automotive::vehicle::VehicleProperty; using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus; using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue; +using ::android::base::expected; +using ::android::base::unexpected; using ::testing::ContainerEq; using ::testing::Eq; using ::testing::WhenSortedBy; @@ -46,6 +53,17 @@ constexpr int INVALID_PROP_ID = 0; } // namespace +// A helper class to access private methods for FakeVehicleHardware. +class FakeVehicleHardwareTestHelper { + public: + FakeVehicleHardwareTestHelper(FakeVehicleHardware* hardware) { mHardware = hardware; } + + void overrideProperties(const char* overrideDir) { mHardware->overrideProperties(overrideDir); } + + private: + FakeVehicleHardware* mHardware; +}; + class FakeVehicleHardwareTest : public ::testing::Test { protected: void SetUp() override {} @@ -64,6 +82,63 @@ class FakeVehicleHardwareTest : public ::testing::Test { requests); } + StatusCode setValue(const VehiclePropValue& value) { + std::vector requests = { + SetValueRequest{ + .requestId = 0, + .value = value, + }, + }; + + if (StatusCode status = setValues(requests); status != StatusCode::OK) { + return status; + } + + const SetValueResult& result = getSetValueResults().back(); + + if (result.requestId != 0) { + ALOGE("request ID mismatch, got %" PRId64 ", expect 0", result.requestId); + return StatusCode::INTERNAL_ERROR; + } + + return result.status; + } + + expected getValue(const VehiclePropValue& value) { + std::vector requests = { + GetValueRequest{ + .requestId = 0, + .prop = value, + }, + }; + + if (StatusCode status = getValues(requests); status != StatusCode::OK) { + return unexpected(status); + } + + const GetValueResult& result = getGetValueResults().back(); + if (result.requestId != 0) { + ALOGE("request ID mismatch, got %" PRId64 ", expect 0", result.requestId); + return unexpected(StatusCode::INTERNAL_ERROR); + } + + if (result.status != StatusCode::OK) { + return unexpected(result.status); + } + + if (!result.prop.has_value()) { + ALOGE("%s", "result property is empty"); + return unexpected(StatusCode::INTERNAL_ERROR); + } + + return result.prop.value(); + } + + template + int getStatus(expected result) { + return toInt(result.error()); + } + void onSetValues(const std::vector results) { for (auto& result : results) { mSetValueResults.push_back(result); @@ -424,6 +499,86 @@ TEST_F(FakeVehicleHardwareTest, testSetStatusMustIgnore) { ASSERT_EQ(getGetValueResults()[1].prop->status, VehiclePropertyStatus::AVAILABLE); } +TEST_F(FakeVehicleHardwareTest, testVendorOverrideProperties) { + std::string overrideDir = android::base::GetExecutableDirectory() + "/override/"; + // Set vendor override directory. + FakeVehicleHardwareTestHelper helper(getHardware()); + helper.overrideProperties(overrideDir.c_str()); + + // This is the same as the prop in 'gear_selection.json'. + int gearProp = toInt(VehicleProperty::GEAR_SELECTION); + + auto result = getValue(VehiclePropValue{ + .prop = gearProp, + }); + + ASSERT_TRUE(result.ok()) << "expect to get the overridden property ok: " << getStatus(result); + ASSERT_EQ(static_cast(1), result.value().value.int32Values.size()); + ASSERT_EQ(8, result.value().value.int32Values[0]); + + // If we set the value, it should update despite the override. + ASSERT_EQ(setValue(VehiclePropValue{ + .prop = gearProp, + .value = + { + .int32Values = {5}, + }, + .timestamp = elapsedRealtimeNano(), + }), + StatusCode::OK) + << "expect to set the overridden property ok"; + + result = getValue(VehiclePropValue{ + .prop = gearProp, + }); + + ASSERT_TRUE(result.ok()) << "expect to get the overridden property after setting value ok"; + ASSERT_EQ(static_cast(1), result.value().value.int32Values.size()); + ASSERT_EQ(5, result.value().value.int32Values[0]); +} + +TEST_F(FakeVehicleHardwareTest, testVendorOverridePropertiesMultipleAreas) { + std::string overrideDir = android::base::GetExecutableDirectory() + "/override/"; + // Set vendor override directory. + FakeVehicleHardwareTestHelper helper(getHardware()); + helper.overrideProperties(overrideDir.c_str()); + + // This is the same as the prop in 'hvac_temperature_set.json'. + int hvacProp = toInt(VehicleProperty::HVAC_TEMPERATURE_SET); + + auto result = getValue(VehiclePropValue{ + .prop = hvacProp, + .areaId = HVAC_LEFT, + }); + + ASSERT_TRUE(result.ok()) << "expect to get the overridden property ok: " << getStatus(result); + ASSERT_EQ(static_cast(1), result.value().value.floatValues.size()); + ASSERT_EQ(30.0f, result.value().value.floatValues[0]); + + // HVAC_RIGHT should not be affected and return the default value. + result = getValue(VehiclePropValue{ + .prop = hvacProp, + .areaId = HVAC_RIGHT, + }); + + ASSERT_TRUE(result.ok()) << "expect to get the default property ok: " << getStatus(result); + ASSERT_EQ(static_cast(1), result.value().value.floatValues.size()); + ASSERT_EQ(20.0f, result.value().value.floatValues[0]); +} + +TEST_F(FakeVehicleHardwareTest, testVendorOverridePropertiesDirDoesNotExist) { + // Set vendor override directory to a non-existing dir + FakeVehicleHardwareTestHelper helper(getHardware()); + helper.overrideProperties("123"); + auto result = getValue(VehiclePropValue{ + .prop = toInt(VehicleProperty::GEAR_SELECTION), + }); + + ASSERT_TRUE(result.ok()) << "expect to get the default property ok: " << getStatus(result); + ASSERT_EQ(static_cast(1), result.value().value.int32Values.size()); + ASSERT_EQ(4, result.value().value.int32Values[0]); +} + } // namespace fake } // namespace vehicle } // namespace automotive diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/gear_selection.json b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/gear_selection.json new file mode 100644 index 0000000000..59666b8046 --- /dev/null +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/gear_selection.json @@ -0,0 +1,9 @@ +[ + { + "timestamp": 1000000, + "areaId": 0, + "value": 8, + // GEAR_SELECTION + "prop": 289408000 + } +] diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/hvac_temperature_set.json b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/hvac_temperature_set.json new file mode 100644 index 0000000000..93a97ed095 --- /dev/null +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/override/hvac_temperature_set.json @@ -0,0 +1,10 @@ +[ + { + "timestamp": 1000000, + // HVAC_LEFT + "areaId": 49, + "value": 30, + // HVAC_TEMPERATURE_SET + "prop": 358614275 + } +] diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h index b02aaf7dc4..95e58c6118 100644 --- a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h +++ b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h @@ -130,10 +130,9 @@ inline size_t getVehicleRawValueVectorSize( const ::aidl::android::hardware::automotive::vehicle::RawPropValues& value, ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType type) { switch (type) { - case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::INT32: // fall - // through - case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType:: - BOOLEAN: // fall through + case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::INT32: + [[fallthrough]]; + case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::BOOLEAN: return std::min(value.int32Values.size(), static_cast(1)); case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::FLOAT: return std::min(value.floatValues.size(), static_cast(1)); @@ -147,6 +146,10 @@ inline size_t getVehicleRawValueVectorSize( return value.int64Values.size(); case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::BYTES: return value.byteValues.size(); + case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::STRING: + [[fallthrough]]; + case ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType::MIXED: + return 0; default: ALOGE("getVehicleRawValueVectorSize: unknown type: %d", toInt(type)); return 0; diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehicleObjectPool.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehicleObjectPool.cpp index 1cdc461552..2480a7362e 100644 --- a/automotive/vehicle/aidl/impl/utils/common/src/VehicleObjectPool.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/src/VehicleObjectPool.cpp @@ -52,17 +52,19 @@ VehiclePropValuePool::RecyclableType VehiclePropValuePool::obtain(VehiclePropert } VehiclePropValuePool::RecyclableType VehiclePropValuePool::obtain(const VehiclePropValue& src) { - VehiclePropertyType type = getPropType(src.prop); + int propId = src.prop; + VehiclePropertyType type = getPropType(propId); size_t vectorSize = getVehicleRawValueVectorSize(src.value, type); if (vectorSize == 0 && !isComplexType(type)) { ALOGW("empty vehicle prop value, contains no content"); + ALOGW("empty vehicle prop value, contains no content, prop: %d", propId); // Return any empty VehiclePropValue. - return RecyclableType{new VehiclePropValue, mDisposableDeleter}; + return RecyclableType{new VehiclePropValue{}, mDisposableDeleter}; } auto dest = obtain(type, vectorSize); - dest->prop = src.prop; + dest->prop = propId; dest->areaId = src.areaId; dest->status = src.status; dest->timestamp = src.timestamp;