Merge changes If53c6395,I972b7f0c into main

* changes:
  Added resolution check in DefaultVehicleHal
  Added resolution to SubscriptionManager in reference VHAL
This commit is contained in:
Shrikar Amirisetty
2024-02-22 21:52:32 +00:00
committed by Android (Google) Code Review
6 changed files with 210 additions and 21 deletions

View File

@@ -333,6 +333,23 @@ inline std::string propIdToString(int32_t propId) {
static_cast<aidl::android::hardware::automotive::vehicle::VehicleProperty>(propId));
}
template <typename T>
void roundToNearestResolution(std::vector<T>& arrayToSanitize, float resolution) {
if (resolution == 0) {
return;
}
for (size_t i = 0; i < arrayToSanitize.size(); i++) {
arrayToSanitize[i] = (T)((std::round(arrayToSanitize[i] / resolution)) * resolution);
}
}
inline void sanitizeByResolution(aidl::android::hardware::automotive::vehicle::RawPropValues* value,
float resolution) {
roundToNearestResolution(value->int32Values, resolution);
roundToNearestResolution(value->floatValues, resolution);
roundToNearestResolution(value->int64Values, resolution);
}
} // namespace vehicle
} // namespace automotive
} // namespace hardware

View File

@@ -25,6 +25,8 @@
#include <android-base/result.h>
#include <android-base/thread_annotations.h>
#include <cmath>
#include <limits>
#include <mutex>
#include <optional>
#include <unordered_map>
@@ -39,6 +41,7 @@ namespace vehicle {
// A structure to represent subscription config for one subscription client.
struct SubConfig {
float sampleRateHz;
float resolution;
bool enableVur;
};
@@ -47,14 +50,19 @@ class ContSubConfigs final {
public:
using ClientIdType = const AIBinder*;
void addClient(const ClientIdType& clientId, float sampleRateHz, bool enableVur);
void addClient(const ClientIdType& clientId, const SubConfig& subConfig);
void removeClient(const ClientIdType& clientId);
float getMaxSampleRateHz() const;
float getMinRequiredResolution() const;
bool isVurEnabled() const;
bool isVurEnabledForClient(const ClientIdType& clientId);
bool isVurEnabledForClient(const ClientIdType& clientId) const;
float getResolutionForClient(const ClientIdType& clientId) const;
private:
float mMaxSampleRateHz = 0.;
// Baseline for resolution is maximum possible float. We want to sanitize to the highest
// requested resolution, which is the smallest float value for resolution.
float mMinRequiredResolution = std::numeric_limits<float>::max();
bool mEnableVur;
std::unordered_map<ClientIdType, SubConfig> mConfigByClient;
@@ -117,6 +125,9 @@ class SubscriptionManager final {
// Checks whether the sample rate is valid.
static bool checkSampleRateHz(float sampleRateHz);
// Checks whether the resolution is valid.
static bool checkResolution(float resolution);
private:
// Friend class for testing.
friend class DefaultVehicleHalTest;
@@ -153,8 +164,8 @@ class SubscriptionManager final {
VhalResult<void> addContinuousSubscriberLocked(const ClientIdType& clientId,
const PropIdAreaId& propIdAreaId,
float sampleRateHz, bool enableVur)
REQUIRES(mLock);
float sampleRateHz, float resolution,
bool enableVur) REQUIRES(mLock);
VhalResult<void> addOnChangeSubscriberLocked(const PropIdAreaId& propIdAreaId) REQUIRES(mLock);
// Removes the subscription client for the continuous [propId, areaId].
VhalResult<void> removeContinuousSubscriberLocked(const ClientIdType& clientId,

View File

@@ -722,6 +722,10 @@ VhalResult<void> DefaultVehicleHal::checkSubscribeOptions(
return StatusError(StatusCode::INVALID_ARG)
<< "invalid sample rate: " << sampleRateHz << " HZ";
}
if (!SubscriptionManager::checkResolution(option.resolution)) {
return StatusError(StatusCode::INVALID_ARG)
<< "invalid resolution: " << option.resolution;
}
}
}
return {};

View File

@@ -43,11 +43,12 @@ using ::ndk::ScopedAStatus;
constexpr float ONE_SECOND_IN_NANOS = 1'000'000'000.;
SubscribeOptions newSubscribeOptions(int32_t propId, int32_t areaId, float sampleRateHz,
bool enableVur) {
float resolution, bool enableVur) {
SubscribeOptions subscribedOptions;
subscribedOptions.propId = propId;
subscribedOptions.areaIds = {areaId};
subscribedOptions.sampleRate = sampleRateHz;
subscribedOptions.resolution = resolution;
subscribedOptions.enableVariableUpdateRate = enableVur;
return subscribedOptions;
@@ -81,8 +82,18 @@ Result<int64_t> SubscriptionManager::getIntervalNanos(float sampleRateHz) {
return intervalNanos;
}
bool SubscriptionManager::checkResolution(float resolution) {
if (resolution == 0) {
return true;
}
float log = std::log10(resolution);
return log == (int)log;
}
void ContSubConfigs::refreshCombinedConfig() {
float maxSampleRateHz = 0.;
float minRequiredResolution = std::numeric_limits<float>::max();
bool enableVur = true;
// This is not called frequently so a brute-focre is okay. More efficient way exists but this
// is simpler.
@@ -90,6 +101,9 @@ void ContSubConfigs::refreshCombinedConfig() {
if (subConfig.sampleRateHz > maxSampleRateHz) {
maxSampleRateHz = subConfig.sampleRateHz;
}
if (subConfig.resolution < minRequiredResolution) {
minRequiredResolution = subConfig.resolution;
}
if (!subConfig.enableVur) {
// If one client does not enable variable update rate, we cannot enable variable update
// rate in IVehicleHardware.
@@ -97,14 +111,12 @@ void ContSubConfigs::refreshCombinedConfig() {
}
}
mMaxSampleRateHz = maxSampleRateHz;
mMinRequiredResolution = minRequiredResolution;
mEnableVur = enableVur;
}
void ContSubConfigs::addClient(const ClientIdType& clientId, float sampleRateHz, bool enableVur) {
mConfigByClient[clientId] = {
.sampleRateHz = sampleRateHz,
.enableVur = enableVur,
};
void ContSubConfigs::addClient(const ClientIdType& clientId, const SubConfig& subConfig) {
mConfigByClient[clientId] = subConfig;
refreshCombinedConfig();
}
@@ -117,12 +129,26 @@ float ContSubConfigs::getMaxSampleRateHz() const {
return mMaxSampleRateHz;
}
float ContSubConfigs::getMinRequiredResolution() const {
return mMinRequiredResolution;
}
bool ContSubConfigs::isVurEnabled() const {
return mEnableVur;
}
bool ContSubConfigs::isVurEnabledForClient(const ClientIdType& clientId) {
return mConfigByClient[clientId].enableVur;
bool ContSubConfigs::isVurEnabledForClient(const ClientIdType& clientId) const {
if (mConfigByClient.find(clientId) == mConfigByClient.end()) {
return false;
}
return mConfigByClient.at(clientId).enableVur;
}
float ContSubConfigs::getResolutionForClient(const ClientIdType& clientId) const {
if (mConfigByClient.find(clientId) == mConfigByClient.end()) {
return 0.0f;
}
return mConfigByClient.at(clientId).resolution;
}
VhalResult<void> SubscriptionManager::addOnChangeSubscriberLocked(
@@ -135,7 +161,8 @@ VhalResult<void> SubscriptionManager::addOnChangeSubscriberLocked(
int32_t propId = propIdAreaId.propId;
int32_t areaId = propIdAreaId.areaId;
if (auto status = mVehicleHardware->subscribe(
newSubscribeOptions(propId, areaId, /*updateRateHz=*/0, /*enableVur*/ false));
newSubscribeOptions(propId, areaId, /*updateRateHz=*/0, /*resolution*/ 0.0f,
/*enableVur*/ false));
status != StatusCode::OK) {
return StatusError(status)
<< StringPrintf("failed subscribe for prop: %s, areaId: %" PRId32,
@@ -146,10 +173,15 @@ VhalResult<void> SubscriptionManager::addOnChangeSubscriberLocked(
VhalResult<void> SubscriptionManager::addContinuousSubscriberLocked(
const ClientIdType& clientId, const PropIdAreaId& propIdAreaId, float sampleRateHz,
bool enableVur) {
float resolution, bool enableVur) {
// Make a copy so that we don't modify 'mContSubConfigsByPropIdArea' on failure cases.
ContSubConfigs newConfig = mContSubConfigsByPropIdArea[propIdAreaId];
newConfig.addClient(clientId, sampleRateHz, enableVur);
SubConfig subConfig = {
.sampleRateHz = sampleRateHz,
.resolution = resolution,
.enableVur = enableVur,
};
newConfig.addClient(clientId, subConfig);
return updateContSubConfigsLocked(propIdAreaId, newConfig);
}
@@ -183,7 +215,10 @@ VhalResult<void> SubscriptionManager::updateContSubConfigsLocked(const PropIdAre
const auto& oldConfig = mContSubConfigsByPropIdArea[propIdAreaId];
float newRateHz = newConfig.getMaxSampleRateHz();
float oldRateHz = oldConfig.getMaxSampleRateHz();
if (newRateHz == oldRateHz && newConfig.isVurEnabled() == oldConfig.isVurEnabled()) {
float newResolution = newConfig.getMinRequiredResolution();
float oldResolution = oldConfig.getMinRequiredResolution();
if (newRateHz == oldRateHz && newResolution == oldResolution &&
newConfig.isVurEnabled() == oldConfig.isVurEnabled()) {
mContSubConfigsByPropIdArea[propIdAreaId] = newConfig;
return {};
}
@@ -199,8 +234,8 @@ VhalResult<void> SubscriptionManager::updateContSubConfigsLocked(const PropIdAre
}
}
if (newRateHz != 0) {
if (auto status = mVehicleHardware->subscribe(
newSubscribeOptions(propId, areaId, newRateHz, newConfig.isVurEnabled()));
if (auto status = mVehicleHardware->subscribe(newSubscribeOptions(
propId, areaId, newRateHz, newResolution, newConfig.isVurEnabled()));
status != StatusCode::OK) {
return StatusError(status) << StringPrintf(
"failed subscribe for prop: %s, areaId"
@@ -231,6 +266,11 @@ VhalResult<void> SubscriptionManager::subscribe(const std::shared_ptr<IVehicleCa
if (auto result = getIntervalNanos(sampleRateHz); !result.ok()) {
return StatusError(StatusCode::INVALID_ARG) << result.error().message();
}
if (!checkResolution(option.resolution)) {
return StatusError(StatusCode::INVALID_ARG) << StringPrintf(
"SubscribeOptions.resolution %f is not an integer power of 10",
option.resolution);
}
}
if (option.areaIds.empty()) {
@@ -253,6 +293,7 @@ VhalResult<void> SubscriptionManager::subscribe(const std::shared_ptr<IVehicleCa
VhalResult<void> result;
if (isContinuousProperty) {
result = addContinuousSubscriberLocked(clientId, propIdAreaId, option.sampleRate,
option.resolution,
option.enableVariableUpdateRate);
} else {
result = addOnChangeSubscriberLocked(propIdAreaId);
@@ -393,15 +434,19 @@ SubscriptionManager::getSubscribedClients(std::vector<VehiclePropValue>&& update
for (const auto& [client, callback] : mClientsByPropIdAreaId[propIdAreaId]) {
auto& subConfigs = mContSubConfigsByPropIdArea[propIdAreaId];
// Clients must be sent different VehiclePropValues with different levels of granularity
// as requested by the client using resolution.
VehiclePropValue newValue = value;
sanitizeByResolution(&(newValue.value), subConfigs.getResolutionForClient(client));
// If client wants VUR (and VUR is supported as checked in DefaultVehicleHal), it is
// possible that VUR is not enabled in IVehicleHardware because another client does not
// enable VUR. We will implement VUR filtering here for the client that enables it.
if (subConfigs.isVurEnabledForClient(client) && !subConfigs.isVurEnabled()) {
if (isValueUpdatedLocked(callback, value)) {
clients[callback].push_back(value);
if (isValueUpdatedLocked(callback, newValue)) {
clients[callback].push_back(newValue);
}
} else {
clients[callback].push_back(value);
clients[callback].push_back(newValue);
}
}
}

View File

@@ -233,6 +233,14 @@ std::vector<SubscribeInvalidOptionsTestCase> getSubscribeInvalidOptionsTestCases
.sampleRate = 0.0,
},
},
{
.name = "invalid_resolution",
.option =
{
.propId = GLOBAL_CONTINUOUS_PROP,
.resolution = 2.0,
},
},
{
.name = "static_property",
.option =

View File

@@ -520,6 +520,14 @@ TEST_F(SubscriptionManagerTest, testCheckSampleRateHzInvalidZero) {
ASSERT_FALSE(SubscriptionManager::checkSampleRateHz(0));
}
TEST_F(SubscriptionManagerTest, testCheckResolutionValid) {
ASSERT_TRUE(SubscriptionManager::checkResolution(1.0));
}
TEST_F(SubscriptionManagerTest, testCheckResolutionInvalid) {
ASSERT_FALSE(SubscriptionManager::checkResolution(2.0));
}
TEST_F(SubscriptionManagerTest, testSubscribe_enableVur) {
std::vector<SubscribeOptions> options = {{
.propId = 0,
@@ -641,6 +649,102 @@ TEST_F(SubscriptionManagerTest, testSubscribe_enableVur_filterUnchangedEvents) {
<< "Must filter out property events if VUR is enabled";
}
TEST_F(SubscriptionManagerTest, testSubscribe_enableVur_filterUnchangedEvents_withResolution) {
SpAIBinder binder1 = ndk::SharedRefBase::make<PropertyCallback>()->asBinder();
std::shared_ptr<IVehicleCallback> client1 = IVehicleCallback::fromBinder(binder1);
SpAIBinder binder2 = ndk::SharedRefBase::make<PropertyCallback>()->asBinder();
std::shared_ptr<IVehicleCallback> client2 = IVehicleCallback::fromBinder(binder2);
SubscribeOptions client1Option = {
.propId = 0,
.areaIds = {0},
.sampleRate = 10.0,
.resolution = 0.01,
.enableVariableUpdateRate = false,
};
auto result = getManager()->subscribe(client1, {client1Option}, true);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
ASSERT_THAT(getHardware()->getSubscribeOptions(), UnorderedElementsAre(client1Option));
getHardware()->clearSubscribeOptions();
SubscribeOptions client2Option = {
.propId = 0,
.areaIds = {0, 1},
.sampleRate = 20.0,
.resolution = 0.1,
.enableVariableUpdateRate = true,
};
result = getManager()->subscribe(client2, {client2Option}, true);
ASSERT_TRUE(result.ok()) << "failed to subscribe: " << result.error().message();
ASSERT_THAT(getHardware()->getSubscribeOptions(),
UnorderedElementsAre(
SubscribeOptions{
.propId = 0,
.areaIds = {0},
.sampleRate = 20.0,
.resolution = 0.01,
// This is enabled for client2, but disabled for client1.
.enableVariableUpdateRate = false,
},
SubscribeOptions{
.propId = 0,
.areaIds = {1},
.sampleRate = 20.0,
.resolution = 0.1,
.enableVariableUpdateRate = true,
}));
std::vector<VehiclePropValue> propertyEvents = {{
.prop = 0,
.areaId = 0,
.value = {.floatValues = {1.0}},
.timestamp = 1,
},
{
.prop = 0,
.areaId = 1,
.value = {.floatValues = {1.0}},
.timestamp = 1,
}};
auto clients =
getManager()->getSubscribedClients(std::vector<VehiclePropValue>(propertyEvents));
ASSERT_THAT(clients[client1], UnorderedElementsAre(propertyEvents[0]));
ASSERT_THAT(clients[client2], UnorderedElementsAre(propertyEvents[0], propertyEvents[1]));
clients = getManager()->getSubscribedClients({{
.prop = 0,
.areaId = 0,
.value = {.floatValues = {1.01}},
.timestamp = 2,
}});
ASSERT_FALSE(clients.find(client1) == clients.end())
<< "Must not filter out property events if VUR is not enabled";
ASSERT_TRUE(clients.find(client2) == clients.end())
<< "Must filter out property events if VUR is enabled and change is too small";
ASSERT_TRUE(abs(clients[client1][0].value.floatValues[0] - 1.01) < 0.0000001)
<< "Expected property value == 1.01, instead got "
<< clients[client1][0].value.floatValues[0];
clients = getManager()->getSubscribedClients({{
.prop = 0,
.areaId = 1,
.value = {.floatValues = {1.06}},
.timestamp = 3,
}});
ASSERT_TRUE(clients.find(client1) == clients.end())
<< "Must not get property events for an areaId that the client hasn't subscribed to";
ASSERT_FALSE(clients.find(client2) == clients.end())
<< "Must get property events significant changes";
ASSERT_TRUE(abs(clients[client2][0].value.floatValues[0] - 1.1) < 0.0000001)
<< "Expected property value == 1.1, instead got "
<< clients[client2][0].value.floatValues[0];
}
TEST_F(SubscriptionManagerTest, testSubscribe_enableVur_mustNotFilterStatusChange) {
SpAIBinder binder1 = ndk::SharedRefBase::make<PropertyCallback>()->asBinder();
std::shared_ptr<IVehicleCallback> client1 = IVehicleCallback::fromBinder(binder1);