From 35616e4de74d4e96b3838eeb032c0175a8cd5cf2 Mon Sep 17 00:00:00 2001 From: Aaqib Ismail Date: Mon, 25 Sep 2023 16:11:26 -0700 Subject: [PATCH] Use HVAC_POWER_ON config array for determining power dependent props Bug: 302024222 Test: atest FakeVehicleHardwareTest Test: atest CarPropertyManagerTest Change-Id: I35dd253e804a244e28debad162e39602dcd01d5a --- .../hardware/include/FakeVehicleHardware.h | 4 + .../hardware/src/FakeVehicleHardware.cpp | 12 +-- .../hardware/test/FakeVehicleHardwareTest.cpp | 86 +++++++++++++------ .../impl/utils/common/include/PropertyUtils.h | 5 -- 4 files changed, 72 insertions(+), 35 deletions(-) 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 21ca72e8cb..844bea5a81 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h @@ -36,6 +36,7 @@ #include #include #include +#include #include namespace android { @@ -161,6 +162,9 @@ class FakeVehicleHardware : public IVehicleHardware { aidl::android::hardware::automotive::vehicle::SetValueRequest> mPendingSetValueRequests; + // Set of HVAC properties dependent on HVAC_POWER_ON + std::unordered_set hvacPowerDependentProps; + const bool mForceOverride; bool mAddExtraTestVendorConfigs; 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 9868f27e29..ee24fbda9c 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp @@ -200,6 +200,11 @@ void FakeVehicleHardware::storePropInitialValue(const ConfigDeclaration& config) bool globalProp = isGlobalProp(propId); size_t numAreas = globalProp ? 1 : vehiclePropConfig.areaConfigs.size(); + if (propId == toInt(VehicleProperty::HVAC_POWER_ON)) { + const auto& configArray = vehiclePropConfig.configArray; + hvacPowerDependentProps.insert(configArray.begin(), configArray.end()); + } + for (size_t i = 0; i < numAreas; i++) { int32_t curArea = globalProp ? 0 : vehiclePropConfig.areaConfigs[i].areaId; @@ -511,9 +516,7 @@ VhalResult FakeVehicleHardware::setHvacTemperatureValueSuggestion( } bool FakeVehicleHardware::isHvacPropAndHvacNotAvailable(int32_t propId, int32_t areaId) const { - std::unordered_set powerProps(std::begin(HVAC_POWER_PROPERTIES), - std::end(HVAC_POWER_PROPERTIES)); - if (powerProps.count(propId)) { + if (hvacPowerDependentProps.count(propId)) { auto hvacPowerOnResults = mServerSidePropStore->readValuesForProperty(toInt(VehicleProperty::HVAC_POWER_ON)); if (!hvacPowerOnResults.ok()) { @@ -732,8 +735,7 @@ FakeVehicleHardware::ValueResultType FakeVehicleHardware::getEchoReverseBytes( } void FakeVehicleHardware::sendHvacPropertiesCurrentValues(int32_t areaId, int32_t hvacPowerOnVal) { - for (size_t i = 0; i < sizeof(HVAC_POWER_PROPERTIES) / sizeof(int32_t); i++) { - int powerPropId = HVAC_POWER_PROPERTIES[i]; + for (auto& powerPropId : hvacPowerDependentProps) { auto powerPropResults = mServerSidePropStore->readValuesForProperty(powerPropId); if (!powerPropResults.ok()) { ALOGW("failed to get power prop 0x%x, error: %s", powerPropId, 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 df1f5bb261..cf9beee0fb 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp @@ -79,7 +79,9 @@ using ::aidl::android::hardware::automotive::vehicle::VehicleAreaMirror; using ::aidl::android::hardware::automotive::vehicle::VehicleHwKeyInputAction; 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::VehiclePropertyStatus; +using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyType; using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue; using ::aidl::android::hardware::automotive::vehicle::VehicleUnit; using ::android::base::expected; @@ -110,6 +112,10 @@ class FakeVehicleHardwareTestHelper { return mHardware->loadConfigDeclarations(); } + std::unordered_set getHvacPowerDependentProps() { + return mHardware->hvacPowerDependentProps; + } + private: FakeVehicleHardware* mHardware; }; @@ -1707,23 +1713,35 @@ TEST_F(FakeVehicleHardwareTest, testSetVehicleMapService) { } TEST_F(FakeVehicleHardwareTest, testGetHvacPropNotAvailable) { - int seatAreaIds[5] = {SEAT_1_LEFT, SEAT_1_RIGHT, SEAT_2_LEFT, SEAT_2_CENTER, SEAT_2_RIGHT}; - for (int areaId : seatAreaIds) { + FakeVehicleHardwareTestHelper helper(getHardware()); + auto hvacPowerOnConfig = std::move(getVehiclePropConfig(toInt(VehicleProperty::HVAC_POWER_ON))); + EXPECT_NE(hvacPowerOnConfig, nullptr); + for (auto& hvacPowerOnAreaConfig : hvacPowerOnConfig->areaConfigs) { + int hvacPowerAreaId = hvacPowerOnAreaConfig.areaId; + // Turn off HVAC_POWER_ON for only 1 area ID StatusCode status = setValue(VehiclePropValue{.prop = toInt(VehicleProperty::HVAC_POWER_ON), - .areaId = areaId, + .areaId = hvacPowerAreaId, .value.int32Values = {0}}); + EXPECT_EQ(status, StatusCode::OK); - ASSERT_EQ(status, StatusCode::OK); - - for (size_t i = 0; i < sizeof(HVAC_POWER_PROPERTIES) / sizeof(int32_t); i++) { - int powerPropId = HVAC_POWER_PROPERTIES[i]; - for (int powerDependentAreaId : seatAreaIds) { + for (auto& powerPropId : helper.getHvacPowerDependentProps()) { + auto powerPropConfig = std::move(getVehiclePropConfig(powerPropId)); + EXPECT_NE(powerPropConfig, nullptr); + if (powerPropConfig->access == VehiclePropertyAccess::WRITE) { + continue; + } + // Try getting a value at each area ID supported by the power dependent property + for (auto& powerPropAreaConfig : powerPropConfig->areaConfigs) { + int powerDependentAreaId = powerPropAreaConfig.areaId; auto getValueResult = getValue(VehiclePropValue{ .prop = powerPropId, .areaId = powerDependentAreaId, }); - if (areaId == powerDependentAreaId) { + // If the current area ID is contained within the HVAC_POWER_ON area ID + // turned off, then getValue should fail and a StatusCode error should be + // returned. Otherwise, a value should be returned. + if ((hvacPowerAreaId & powerDependentAreaId) == powerDependentAreaId) { EXPECT_FALSE(getValueResult.ok()); EXPECT_EQ(getValueResult.error(), StatusCode::NOT_AVAILABLE_DISABLED); } else { @@ -1736,28 +1754,45 @@ TEST_F(FakeVehicleHardwareTest, testGetHvacPropNotAvailable) { // on this value from any power dependent property values other than those with the same // areaId. setValue(VehiclePropValue{.prop = toInt(VehicleProperty::HVAC_POWER_ON), - .areaId = areaId, + .areaId = hvacPowerAreaId, .value.int32Values = {1}}); } } TEST_F(FakeVehicleHardwareTest, testSetHvacPropNotAvailable) { - int seatAreaIds[5] = {SEAT_1_LEFT, SEAT_1_RIGHT, SEAT_2_LEFT, SEAT_2_CENTER, SEAT_2_RIGHT}; - for (int areaId : seatAreaIds) { + FakeVehicleHardwareTestHelper helper(getHardware()); + auto hvacPowerOnConfig = std::move(getVehiclePropConfig(toInt(VehicleProperty::HVAC_POWER_ON))); + EXPECT_NE(hvacPowerOnConfig, nullptr); + for (auto& hvacPowerOnAreaConfig : hvacPowerOnConfig->areaConfigs) { + int hvacPowerAreaId = hvacPowerOnAreaConfig.areaId; + // Turn off HVAC_POWER_ON for only 1 area ID StatusCode status = setValue(VehiclePropValue{.prop = toInt(VehicleProperty::HVAC_POWER_ON), - .areaId = areaId, + .areaId = hvacPowerAreaId, .value.int32Values = {0}}); + EXPECT_EQ(status, StatusCode::OK); - ASSERT_EQ(status, StatusCode::OK); + for (auto& powerPropId : helper.getHvacPowerDependentProps()) { + auto powerPropConfig = std::move(getVehiclePropConfig(powerPropId)); + EXPECT_NE(powerPropConfig, nullptr); + if (powerPropConfig->access == VehiclePropertyAccess::READ) { + continue; + } + auto propType = getPropType(powerPropId); + // Try setting a value at each area ID supported by the power dependent property + for (auto& powerPropAreaConfig : powerPropConfig->areaConfigs) { + int powerDependentAreaId = powerPropAreaConfig.areaId; + auto val = VehiclePropValue{.prop = powerPropId, .areaId = powerDependentAreaId}; + if (propType == VehiclePropertyType::FLOAT) { + val.value.floatValues.emplace_back(20); + } else { + val.value.int32Values.emplace_back(1); + } + status = setValue(val); - for (size_t i = 0; i < sizeof(HVAC_POWER_PROPERTIES) / sizeof(int32_t); i++) { - int powerPropId = HVAC_POWER_PROPERTIES[i]; - for (int powerDependentAreaId : seatAreaIds) { - StatusCode status = setValue(VehiclePropValue{.prop = powerPropId, - .areaId = powerDependentAreaId, - .value.int32Values = {1}}); - - if (areaId == powerDependentAreaId) { + // If the current area ID is contained within the HVAC_POWER_ON area ID + // turned off, then setValue should fail and a StatusCode error should be + // returned. Otherwise, an ok StatusCode should be returned. + if ((hvacPowerAreaId & powerDependentAreaId) == powerDependentAreaId) { EXPECT_EQ(status, StatusCode::NOT_AVAILABLE_DISABLED); } else { EXPECT_EQ(status, StatusCode::OK); @@ -1769,12 +1804,13 @@ TEST_F(FakeVehicleHardwareTest, testSetHvacPropNotAvailable) { // on this value from any power dependent property values other than those with the same // areaId. setValue(VehiclePropValue{.prop = toInt(VehicleProperty::HVAC_POWER_ON), - .areaId = areaId, + .areaId = hvacPowerAreaId, .value.int32Values = {1}}); } } TEST_F(FakeVehicleHardwareTest, testHvacPowerOnSendCurrentHvacPropValues) { + FakeVehicleHardwareTestHelper helper(getHardware()); auto hvacPowerOnConfig = std::move(getVehiclePropConfig(toInt(VehicleProperty::HVAC_POWER_ON))); EXPECT_NE(hvacPowerOnConfig, nullptr); for (auto& hvacPowerOnAreaConfig : hvacPowerOnConfig->areaConfigs) { @@ -1789,7 +1825,7 @@ TEST_F(FakeVehicleHardwareTest, testHvacPowerOnSendCurrentHvacPropValues) { if (event.prop == toInt(VehicleProperty::HVAC_POWER_ON)) { continue; } - EXPECT_THAT(event.prop, AnyOfArray(HVAC_POWER_PROPERTIES)); + EXPECT_THAT(event.prop, AnyOfArray(helper.getHvacPowerDependentProps())); EXPECT_EQ((hvacPowerAreaId & event.areaId), hvacPowerAreaId); EXPECT_EQ(event.status, VehiclePropertyStatus::UNAVAILABLE); } @@ -1805,7 +1841,7 @@ TEST_F(FakeVehicleHardwareTest, testHvacPowerOnSendCurrentHvacPropValues) { if (event.prop == toInt(VehicleProperty::HVAC_POWER_ON)) { continue; } - EXPECT_THAT(event.prop, AnyOfArray(HVAC_POWER_PROPERTIES)); + EXPECT_THAT(event.prop, AnyOfArray(helper.getHvacPowerDependentProps())); EXPECT_EQ((hvacPowerAreaId & event.areaId), hvacPowerAreaId); EXPECT_EQ(event.status, VehiclePropertyStatus::AVAILABLE); } diff --git a/automotive/vehicle/aidl/impl/utils/common/include/PropertyUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/PropertyUtils.h index e41ec3074c..78b61f714e 100644 --- a/automotive/vehicle/aidl/impl/utils/common/include/PropertyUtils.h +++ b/automotive/vehicle/aidl/impl/utils/common/include/PropertyUtils.h @@ -98,11 +98,6 @@ constexpr int HVAC_LEFT = SEAT_1_LEFT | SEAT_2_LEFT | SEAT_2_CENTER; constexpr int HVAC_RIGHT = SEAT_1_RIGHT | SEAT_2_RIGHT; constexpr int HVAC_ALL = HVAC_LEFT | HVAC_RIGHT; -const int32_t HVAC_POWER_PROPERTIES[] = { - toInt(propertyutils_impl::VehicleProperty::HVAC_FAN_SPEED), - toInt(propertyutils_impl::VehicleProperty::HVAC_FAN_DIRECTION), -}; - } // namespace vehicle } // namespace automotive } // namespace hardware