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 acee9b327b..23f446bebb 100644 --- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp +++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp @@ -304,13 +304,13 @@ void FakeVehicleHardware::init() { } // OBD2_LIVE_FRAME and OBD2_FREEZE_FRAME must be configured in default configs. - auto maybeObd2LiveFrame = mServerSidePropStore->getConfig(OBD2_LIVE_FRAME); + auto maybeObd2LiveFrame = mServerSidePropStore->getPropConfig(OBD2_LIVE_FRAME); if (maybeObd2LiveFrame.has_value()) { - mFakeObd2Frame->initObd2LiveFrame(*maybeObd2LiveFrame.value()); + mFakeObd2Frame->initObd2LiveFrame(maybeObd2LiveFrame.value()); } - auto maybeObd2FreezeFrame = mServerSidePropStore->getConfig(OBD2_FREEZE_FRAME); + auto maybeObd2FreezeFrame = mServerSidePropStore->getPropConfig(OBD2_FREEZE_FRAME); if (maybeObd2FreezeFrame.has_value()) { - mFakeObd2Frame->initObd2FreezeFrame(*maybeObd2FreezeFrame.value()); + mFakeObd2Frame->initObd2FreezeFrame(maybeObd2FreezeFrame.value()); } mServerSidePropStore->setOnValueChangeCallback( @@ -472,7 +472,7 @@ void FakeVehicleHardware::updateHvacTemperatureValueSuggestionInput( VhalResult FakeVehicleHardware::setHvacTemperatureValueSuggestion( const VehiclePropValue& hvacTemperatureValueSuggestion) { auto hvacTemperatureSetConfigResult = - mServerSidePropStore->getConfig(toInt(VehicleProperty::HVAC_TEMPERATURE_SET)); + mServerSidePropStore->getPropConfig(toInt(VehicleProperty::HVAC_TEMPERATURE_SET)); if (!hvacTemperatureSetConfigResult.ok()) { return StatusError(getErrorCode(hvacTemperatureSetConfigResult)) << StringPrintf( @@ -499,7 +499,7 @@ VhalResult FakeVehicleHardware::setHvacTemperatureValueSuggestion( } auto updatedValue = mValuePool->obtain(hvacTemperatureValueSuggestion); - const auto& hvacTemperatureSetConfigArray = hvacTemperatureSetConfigResult.value()->configArray; + const auto& hvacTemperatureSetConfigArray = hvacTemperatureSetConfigResult.value().configArray; auto& hvacTemperatureValueSuggestionInput = updatedValue->value.floatValues; updateHvacTemperatureValueSuggestionInput(hvacTemperatureSetConfigArray, @@ -822,14 +822,14 @@ void FakeVehicleHardware::sendHvacPropertiesCurrentValues(int32_t areaId, int32_ void FakeVehicleHardware::sendAdasPropertiesState(int32_t propertyId, int32_t state) { auto& adasDependentPropIds = mAdasEnabledPropToAdasPropWithErrorState.find(propertyId)->second; for (auto dependentPropId : adasDependentPropIds) { - auto dependentPropConfigResult = mServerSidePropStore->getConfig(dependentPropId); + auto dependentPropConfigResult = mServerSidePropStore->getPropConfig(dependentPropId); if (!dependentPropConfigResult.ok()) { ALOGW("Failed to get config for ADAS property 0x%x, error: %s", dependentPropId, getErrorMsg(dependentPropConfigResult).c_str()); continue; } auto& dependentPropConfig = dependentPropConfigResult.value(); - for (auto& areaConfig : dependentPropConfig->areaConfigs) { + for (auto& areaConfig : dependentPropConfig.areaConfigs) { int32_t hardcoded_state = state; // TODO: restore old/initial values here instead of hardcoded value (b/295542701) if (state == 1 && dependentPropId == toInt(VehicleProperty::CRUISE_CONTROL_TYPE)) { @@ -1702,12 +1702,12 @@ std::string FakeVehicleHardware::dumpSpecificProperty(const std::vectorgetConfig(prop); + auto result = mServerSidePropStore->getPropConfig(prop); if (!result.ok()) { msg += StringPrintf("No property %d\n", prop); continue; } - msg += dumpOnePropertyByConfig(rowNumber++, *result.value()); + msg += dumpOnePropertyByConfig(rowNumber++, result.value()); } return msg; } @@ -2014,7 +2014,7 @@ void FakeVehicleHardware::registerOnPropertySetErrorEvent( StatusCode FakeVehicleHardware::subscribe(SubscribeOptions options) { int32_t propId = options.propId; - auto configResult = mServerSidePropStore->getConfig(propId); + auto configResult = mServerSidePropStore->getPropConfig(propId); if (!configResult.ok()) { ALOGE("subscribe: property: %" PRId32 " is not supported", propId); return StatusCode::INVALID_ARG; @@ -2024,7 +2024,7 @@ StatusCode FakeVehicleHardware::subscribe(SubscribeOptions options) { for (int areaId : options.areaIds) { if (StatusCode status = subscribePropIdAreaIdLocked(propId, areaId, options.sampleRate, options.enableVariableUpdateRate, - *configResult.value()); + configResult.value()); status != StatusCode::OK) { return status; } diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h index ef36532801..7b328f2e9e 100644 --- a/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h +++ b/automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h @@ -143,11 +143,17 @@ class VehiclePropertyStore final { std::vector getAllConfigs() const EXCLUDES(mLock); - // Get the property config for the requested property. + // Deprecated, use getPropConfig instead. This is unsafe to use if registerProperty overwrites + // an existing config. android::base::Result getConfig(int32_t propId) const EXCLUDES(mLock); + // Get the property config for the requested property. + android::base::Result + getPropConfig(int32_t propId) const EXCLUDES(mLock); + // Set a callback that would be called when a property value has been updated. void setOnValueChangeCallback(const OnValueChangeCallback& callback) EXCLUDES(mLock); diff --git a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp index 7d9d8b70ba..4171cf710a 100644 --- a/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp @@ -318,6 +318,17 @@ VhalResult VehiclePropertyStore::getConfig(int32_t pro return &record->propConfig; } +VhalResult VehiclePropertyStore::getPropConfig(int32_t propId) const { + std::scoped_lock g(mLock); + + const VehiclePropertyStore::Record* record = getRecordLocked(propId); + if (record == nullptr) { + return StatusError(StatusCode::INVALID_ARG) << "property: " << propId << " not registered"; + } + + return record->propConfig; +} + void VehiclePropertyStore::setOnValueChangeCallback( const VehiclePropertyStore::OnValueChangeCallback& callback) { std::scoped_lock g(mLock); diff --git a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp index 625652e41e..328d2440e3 100644 --- a/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp +++ b/automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp @@ -101,16 +101,16 @@ TEST_F(VehiclePropertyStoreTest, testGetAllConfigs) { ASSERT_EQ(configs.size(), static_cast(2)); } -TEST_F(VehiclePropertyStoreTest, testGetConfig) { - VhalResult result = - mStore->getConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); +TEST_F(VehiclePropertyStoreTest, testGetPropConfig) { + VhalResult result = + mStore->getPropConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); ASSERT_RESULT_OK(result); - ASSERT_EQ(*(result.value()), mConfigFuelCapacity); + ASSERT_EQ(result.value(), mConfigFuelCapacity); } -TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) { - VhalResult result = mStore->getConfig(INVALID_PROP_ID); +TEST_F(VehiclePropertyStoreTest, testGetPropConfigWithInvalidPropId) { + VhalResult result = mStore->getPropConfig(INVALID_PROP_ID); EXPECT_FALSE(result.ok()) << "expect error when getting a config for an invalid property ID"; EXPECT_EQ(result.error().code(), StatusCode::INVALID_ARG);