Merge changes If53c6395,I972b7f0c into main
* changes:
Added resolution check in DefaultVehicleHal
Added resolution to SubscriptionManager in reference VHAL
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
index 523cac5..aca725d 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
@@ -333,6 +333,23 @@
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
diff --git a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
index 5053c96..2f16fca 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
@@ -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 @@
// A structure to represent subscription config for one subscription client.
struct SubConfig {
float sampleRateHz;
+ float resolution;
bool enableVur;
};
@@ -47,14 +50,19 @@
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 @@
// 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 @@
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,
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index cc5edcc..a29861f 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -722,6 +722,10 @@
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 {};
diff --git a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
index 29d81a7..f1106ee 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
@@ -43,11 +43,12 @@
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 @@
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 @@
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 @@
}
}
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 @@
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 @@
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::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 @@
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 @@
}
}
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 @@
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> result;
if (isContinuousProperty) {
result = addContinuousSubscriberLocked(clientId, propIdAreaId, option.sampleRate,
+ option.resolution,
option.enableVariableUpdateRate);
} else {
result = addOnChangeSubscriberLocked(propIdAreaId);
@@ -393,15 +434,19 @@
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);
}
}
}
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index bb82108..11a8fc7 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -234,6 +234,14 @@
},
},
{
+ .name = "invalid_resolution",
+ .option =
+ {
+ .propId = GLOBAL_CONTINUOUS_PROP,
+ .resolution = 2.0,
+ },
+ },
+ {
.name = "static_property",
.option =
{
diff --git a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
index aa5f003..f377202 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
@@ -520,6 +520,14 @@
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 @@
<< "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);