tequilaOS/platform_hardware_interfaces
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Return config using a copy not a pointer.

Browse source 
Introduce VehiclePropertyStore.getPropConfig that returns a copy of
the config instead of a constant pointer. If the internal map is updated,
the pointer might become invalid, so it is safer to just return a copy.

We do not modify the existing API for backward compatibility.

Test: atest VehiclePropertyStoreTest
Bug: 308202443
Change-Id: I769866a09577cc69d25276349b7688cabcbc0c20
This commit is contained in:
Yu Shan 2023-12-04 18:35:49 -08:00
parent ffb0704a43
commit b2d54bf984
4 changed files with 36 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
View file

@ -304,13 +304,13 @@ void FakeVehicleHardware::init() {
} }
// OBD2_LIVE_FRAME and OBD2_FREEZE_FRAME must be configured in default configs. // 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()) { 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()) { if (maybeObd2FreezeFrame.has_value()) {
mFakeObd2Frame->initObd2FreezeFrame(*maybeObd2FreezeFrame.value()); mFakeObd2Frame->initObd2FreezeFrame(maybeObd2FreezeFrame.value());
} }
mServerSidePropStore->setOnValueChangeCallback( mServerSidePropStore->setOnValueChangeCallback(
@ -472,7 +472,7 @@ void FakeVehicleHardware::updateHvacTemperatureValueSuggestionInput(
VhalResult<void> FakeVehicleHardware::setHvacTemperatureValueSuggestion( VhalResult<void> FakeVehicleHardware::setHvacTemperatureValueSuggestion(
const VehiclePropValue& hvacTemperatureValueSuggestion) { const VehiclePropValue& hvacTemperatureValueSuggestion) {
auto hvacTemperatureSetConfigResult = auto hvacTemperatureSetConfigResult =
mServerSidePropStore->getConfig(toInt(VehicleProperty::HVAC_TEMPERATURE_SET)); mServerSidePropStore->getPropConfig(toInt(VehicleProperty::HVAC_TEMPERATURE_SET));
if (!hvacTemperatureSetConfigResult.ok()) { if (!hvacTemperatureSetConfigResult.ok()) {
return StatusError(getErrorCode(hvacTemperatureSetConfigResult)) << StringPrintf( return StatusError(getErrorCode(hvacTemperatureSetConfigResult)) << StringPrintf(
@ -499,7 +499,7 @@ VhalResult<void> FakeVehicleHardware::setHvacTemperatureValueSuggestion(
} }
auto updatedValue = mValuePool->obtain(hvacTemperatureValueSuggestion); auto updatedValue = mValuePool->obtain(hvacTemperatureValueSuggestion);
const auto& hvacTemperatureSetConfigArray = hvacTemperatureSetConfigResult.value()->configArray; const auto& hvacTemperatureSetConfigArray = hvacTemperatureSetConfigResult.value().configArray;
auto& hvacTemperatureValueSuggestionInput = updatedValue->value.floatValues; auto& hvacTemperatureValueSuggestionInput = updatedValue->value.floatValues;
updateHvacTemperatureValueSuggestionInput(hvacTemperatureSetConfigArray, updateHvacTemperatureValueSuggestionInput(hvacTemperatureSetConfigArray,
@ -822,14 +822,14 @@ void FakeVehicleHardware::sendHvacPropertiesCurrentValues(int32_t areaId, int32_
void FakeVehicleHardware::sendAdasPropertiesState(int32_t propertyId, int32_t state) { void FakeVehicleHardware::sendAdasPropertiesState(int32_t propertyId, int32_t state) {
auto& adasDependentPropIds = mAdasEnabledPropToAdasPropWithErrorState.find(propertyId)->second; auto& adasDependentPropIds = mAdasEnabledPropToAdasPropWithErrorState.find(propertyId)->second;
for (auto dependentPropId : adasDependentPropIds) { for (auto dependentPropId : adasDependentPropIds) {
auto dependentPropConfigResult = mServerSidePropStore->getConfig(dependentPropId); auto dependentPropConfigResult = mServerSidePropStore->getPropConfig(dependentPropId);
if (!dependentPropConfigResult.ok()) { if (!dependentPropConfigResult.ok()) {
ALOGW("Failed to get config for ADAS property 0x%x, error: %s", dependentPropId, ALOGW("Failed to get config for ADAS property 0x%x, error: %s", dependentPropId,
getErrorMsg(dependentPropConfigResult).c_str()); getErrorMsg(dependentPropConfigResult).c_str());
continue; continue;
} }
auto& dependentPropConfig = dependentPropConfigResult.value(); auto& dependentPropConfig = dependentPropConfigResult.value();
for (auto& areaConfig : dependentPropConfig->areaConfigs) { for (auto& areaConfig : dependentPropConfig.areaConfigs) {
int32_t hardcoded_state = state; int32_t hardcoded_state = state;
// TODO: restore old/initial values here instead of hardcoded value (b/295542701) // TODO: restore old/initial values here instead of hardcoded value (b/295542701)
if (state == 1 && dependentPropId == toInt(VehicleProperty::CRUISE_CONTROL_TYPE)) { if (state == 1 && dependentPropId == toInt(VehicleProperty::CRUISE_CONTROL_TYPE)) {
@ -1702,12 +1702,12 @@ std::string FakeVehicleHardware::dumpSpecificProperty(const std::vector<std::str
continue; continue;
} }
int32_t prop = propResult.value(); int32_t prop = propResult.value();
auto result = mServerSidePropStore->getConfig(prop); auto result = mServerSidePropStore->getPropConfig(prop);
if (!result.ok()) { if (!result.ok()) {
msg += StringPrintf("No property %d\n", prop); msg += StringPrintf("No property %d\n", prop);
continue; continue;
} }
msg += dumpOnePropertyByConfig(rowNumber++, *result.value()); msg += dumpOnePropertyByConfig(rowNumber++, result.value());
} }
return msg; return msg;
} }
@ -2014,7 +2014,7 @@ void FakeVehicleHardware::registerOnPropertySetErrorEvent(
StatusCode FakeVehicleHardware::subscribe(SubscribeOptions options) { StatusCode FakeVehicleHardware::subscribe(SubscribeOptions options) {
int32_t propId = options.propId; int32_t propId = options.propId;
auto configResult = mServerSidePropStore->getConfig(propId); auto configResult = mServerSidePropStore->getPropConfig(propId);
if (!configResult.ok()) { if (!configResult.ok()) {
ALOGE("subscribe: property: %" PRId32 " is not supported", propId); ALOGE("subscribe: property: %" PRId32 " is not supported", propId);
return StatusCode::INVALID_ARG; return StatusCode::INVALID_ARG;
@ -2024,7 +2024,7 @@ StatusCode FakeVehicleHardware::subscribe(SubscribeOptions options) {
for (int areaId : options.areaIds) { for (int areaId : options.areaIds) {
if (StatusCode status = subscribePropIdAreaIdLocked(propId, areaId, options.sampleRate, if (StatusCode status = subscribePropIdAreaIdLocked(propId, areaId, options.sampleRate,
options.enableVariableUpdateRate, options.enableVariableUpdateRate,
*configResult.value()); configResult.value());
status != StatusCode::OK) { status != StatusCode::OK) {
return status; return status;
} }

8
automotive/vehicle/aidl/impl/utils/common/include/VehiclePropertyStore.h
View file

@ -143,11 +143,17 @@ class VehiclePropertyStore final {
std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropConfig> getAllConfigs() std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropConfig> getAllConfigs()
const EXCLUDES(mLock); 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<const aidl::android::hardware::automotive::vehicle::VehiclePropConfig*, android::base::Result<const aidl::android::hardware::automotive::vehicle::VehiclePropConfig*,
VhalError> VhalError>
getConfig(int32_t propId) const EXCLUDES(mLock); getConfig(int32_t propId) const EXCLUDES(mLock);
// Get the property config for the requested property.
android::base::Result<aidl::android::hardware::automotive::vehicle::VehiclePropConfig,
VhalError>
getPropConfig(int32_t propId) const EXCLUDES(mLock);
// Set a callback that would be called when a property value has been updated. // Set a callback that would be called when a property value has been updated.
void setOnValueChangeCallback(const OnValueChangeCallback& callback) EXCLUDES(mLock); void setOnValueChangeCallback(const OnValueChangeCallback& callback) EXCLUDES(mLock);

11
automotive/vehicle/aidl/impl/utils/common/src/VehiclePropertyStore.cpp
View file

@ -318,6 +318,17 @@ VhalResult<const VehiclePropConfig*> VehiclePropertyStore::getConfig(int32_t pro
return &record->propConfig; return &record->propConfig;
} }
VhalResult<VehiclePropConfig> VehiclePropertyStore::getPropConfig(int32_t propId) const {
std::scoped_lock<std::mutex> 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( void VehiclePropertyStore::setOnValueChangeCallback(
const VehiclePropertyStore::OnValueChangeCallback& callback) { const VehiclePropertyStore::OnValueChangeCallback& callback) {
std::scoped_lock<std::mutex> g(mLock); std::scoped_lock<std::mutex> g(mLock);

12
automotive/vehicle/aidl/impl/utils/common/test/VehiclePropertyStoreTest.cpp
View file

@ -101,16 +101,16 @@ TEST_F(VehiclePropertyStoreTest, testGetAllConfigs) {
ASSERT_EQ(configs.size(), static_cast<size_t>(2)); ASSERT_EQ(configs.size(), static_cast<size_t>(2));
} }
TEST_F(VehiclePropertyStoreTest, testGetConfig) { TEST_F(VehiclePropertyStoreTest, testGetPropConfig) {
VhalResult<const VehiclePropConfig*> result = VhalResult<VehiclePropConfig> result =
mStore->getConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY)); mStore->getPropConfig(toInt(VehicleProperty::INFO_FUEL_CAPACITY));
ASSERT_RESULT_OK(result); ASSERT_RESULT_OK(result);
ASSERT_EQ(*(result.value()), mConfigFuelCapacity); ASSERT_EQ(result.value(), mConfigFuelCapacity);
} }
TEST_F(VehiclePropertyStoreTest, testGetConfigWithInvalidPropId) { TEST_F(VehiclePropertyStoreTest, testGetPropConfigWithInvalidPropId) {
VhalResult<const VehiclePropConfig*> result = mStore->getConfig(INVALID_PROP_ID); VhalResult<VehiclePropConfig> result = mStore->getPropConfig(INVALID_PROP_ID);
EXPECT_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(), StatusCode::INVALID_ARG); EXPECT_EQ(result.error().code(), StatusCode::INVALID_ARG);