Handle sample rate out of bound correctly.
This CL changes the behavior when sample rate is out of bound
for VHAL. Sample rate would be fitted to the range if out-of-bound.
Also added some comments to clearly document this behavior.
Test: atest DefaultVehicleHalTest
atest CtsCarTestCases
Bug: 225025926
Change-Id: Id853070cb0dc823855a68f86a01de2ff2a011577
diff --git a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
index a947963..dc9b876 100644
--- a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
+++ b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
@@ -165,6 +165,11 @@
* a property set failure message sent from the vehicle bus.
* @param options List of options to subscribe. SubscribeOption contains
* information such as property Id, area Id, sample rate, etc.
+ * For continuous properties, sample rate must be provided. If sample
+ * rate is less than {@link VehiclePropConfig#minSampleRate}, the sample
+ * rate would be minSampleRate. If sample rate is larger than
+ * {@link VehiclePropValue#maxSampleRate}, the sample rate would be
+ * maxSampleRate.
* @param maxSharedMemoryFileCount The maximum number of shared memory files
* allocated for in VHAL for this subscription. When a memory file is
* handled back to the client, it cannot be used by VHAL to deliver
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index 886f897..76736d7 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -78,6 +78,16 @@
return str;
}
+float getDefaultSampleRate(float sampleRate, float minSampleRate, float maxSampleRate) {
+ if (sampleRate < minSampleRate) {
+ return minSampleRate;
+ }
+ if (sampleRate > maxSampleRate) {
+ return maxSampleRate;
+ }
+ return sampleRate;
+}
+
} // namespace
std::shared_ptr<SubscriptionClient> DefaultVehicleHal::SubscriptionClients::maybeAddClient(
@@ -617,9 +627,10 @@
float minSampleRate = config.minSampleRate;
float maxSampleRate = config.maxSampleRate;
if (sampleRate < minSampleRate || sampleRate > maxSampleRate) {
- return StatusError(StatusCode::INVALID_ARG)
- << StringPrintf("sample rate: %f out of range, must be within %f and %f",
- sampleRate, minSampleRate, maxSampleRate);
+ float defaultRate = getDefaultSampleRate(sampleRate, minSampleRate, maxSampleRate);
+ ALOGW("sample rate: %f out of range, must be within %f and %f, set to %f",
+ sampleRate, minSampleRate, maxSampleRate, defaultRate);
+ sampleRate = defaultRate;
}
if (!SubscriptionManager::checkSampleRate(sampleRate)) {
return StatusError(StatusCode::INVALID_ARG)
@@ -673,6 +684,8 @@
}
if (config.changeMode == VehiclePropertyChangeMode::CONTINUOUS) {
+ optionCopy.sampleRate = getDefaultSampleRate(
+ optionCopy.sampleRate, config.minSampleRate, config.maxSampleRate);
continuousSubscriptions.push_back(std::move(optionCopy));
} else {
onChangeSubscriptions.push_back(std::move(optionCopy));
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 6f8eb1d..49f5b7e 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -197,14 +197,6 @@
},
},
{
- .name = "sample_rate_out_of_range",
- .option =
- {
- .propId = GLOBAL_CONTINUOUS_PROP,
- .sampleRate = 1000.0,
- },
- },
- {
.name = "static_property",
.option =
{
@@ -1360,6 +1352,51 @@
EXPECT_EQ(countClients(), static_cast<size_t>(1));
}
+TEST_F(DefaultVehicleHalTest, testSubscribeGlobalContinuousRateOutOfRange) {
+ VehiclePropValue testValue{
+ .prop = GLOBAL_CONTINUOUS_PROP,
+ .value.int32Values = {0},
+ };
+ // Set responses for all the hardware getValues requests.
+ getHardware()->setGetValueResponder(
+ [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
+ const std::vector<GetValueRequest>& requests) {
+ std::vector<GetValueResult> results;
+ for (auto& request : requests) {
+ VehiclePropValue prop = request.prop;
+ prop.value.int32Values = {0};
+ results.push_back({
+ .requestId = request.requestId,
+ .status = StatusCode::OK,
+ .prop = prop,
+ });
+ }
+ (*callback)(results);
+ return StatusCode::OK;
+ });
+
+ // The maxSampleRate is 100, so the sample rate should be the default max 100.
+ std::vector<SubscribeOptions> options = {
+ {
+ .propId = GLOBAL_CONTINUOUS_PROP,
+ .sampleRate = 1000.0,
+ },
+ };
+
+ auto status = getClient()->subscribe(getCallbackClient(), options, 0);
+
+ ASSERT_TRUE(status.isOk()) << "subscribe failed: " << status.getMessage();
+
+ // Sleep for 1s, which should generate ~100 events.
+ std::this_thread::sleep_for(std::chrono::seconds(1));
+
+ size_t eventCount = getCallback()->countOnPropertyEventResults();
+ ASSERT_GE(eventCount, 50u) << "expect at least 50 events to be generated";
+ ASSERT_LE(eventCount, 150u) << "expect no more than 150 events to be generated";
+
+ EXPECT_EQ(countClients(), static_cast<size_t>(1));
+}
+
TEST_F(DefaultVehicleHalTest, testSubscribeAreaContinuous) {
// Set responses for all the hardware getValues requests.
getHardware()->setGetValueResponder(
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.cpp b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.cpp
index 5e3e03c..0e46357 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.cpp
@@ -82,6 +82,11 @@
return pop(mOnPropertyEventResults);
}
+size_t MockVehicleCallback::countOnPropertyEventResults() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ return mOnPropertyEventResults.size();
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.h b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.h
index 03bfd5b..0faaa1f 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.h
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleCallback.h
@@ -62,6 +62,7 @@
nextSetValueResults();
std::optional<aidl::android::hardware::automotive::vehicle::VehiclePropValues>
nextOnPropertyEventResults();
+ size_t countOnPropertyEventResults();
private:
std::mutex mLock;