Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2897aff9 authored by Yu Shan's avatar Yu Shan
Browse files

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
parent a5e8da7b
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -165,6 +165,11 @@ interface IVehicle {
     *    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
+16 −3
Original line number Diff line number Diff line
@@ -78,6 +78,16 @@ std::string toString(const std::unordered_set<int64_t>& values) {
    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 @@ Result<void, VhalError> DefaultVehicleHal::checkSubscribeOptions(
            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 @@ ScopedAStatus DefaultVehicleHal::subscribe(const CallbackType& callback,
        }

        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));
+45 −8
Original line number Diff line number Diff line
@@ -196,14 +196,6 @@ std::vector<SubscribeInvalidOptionsTestCase> getSubscribeInvalidOptionsTestCases
                                    .sampleRate = 0.0,
                            },
            },
            {
                    .name = "sample_rate_out_of_range",
                    .option =
                            {
                                    .propId = GLOBAL_CONTINUOUS_PROP,
                                    .sampleRate = 1000.0,
                            },
            },
            {
                    .name = "static_property",
                    .option =
@@ -1360,6 +1352,51 @@ TEST_F(DefaultVehicleHalTest, testSubscribeGlobalContinuous) {
    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(
+5 −0
Original line number Diff line number Diff line
@@ -82,6 +82,11 @@ std::optional<VehiclePropValues> MockVehicleCallback::nextOnPropertyEventResults
    return pop(mOnPropertyEventResults);
}

size_t MockVehicleCallback::countOnPropertyEventResults() {
    std::scoped_lock<std::mutex> lockGuard(mLock);
    return mOnPropertyEventResults.size();
}

}  // namespace vehicle
}  // namespace automotive
}  // namespace hardware
+1 −0
Original line number Diff line number Diff line
@@ -62,6 +62,7 @@ class MockVehicleCallback final
    nextSetValueResults();
    std::optional<aidl::android::hardware::automotive::vehicle::VehiclePropValues>
    nextOnPropertyEventResults();
    size_t countOnPropertyEventResults();

  private:
    std::mutex mLock;