From 22754c5fa75bdd210d7e597bad78176d6b9390a0 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Tue, 14 Sep 2021 12:21:59 +0000 Subject: [PATCH] Update ActivePwle description for amplitude values Update description to clarify that the value 1 should map to the maximum output acceleration supported by the device at any frequency, not exclusively to the one at resonant frequency. Also add information linking the ActivePwle input parameters of composePwle to the output values returned by getBandwidthAmplitudeMap, and update the bandwidth description as well. Change-Id: Id6e02c5d323aec57db7e7127e219a9705d0289a3 Fix: 199753151 Test: VtsHalVibratorTargetTest --- .../android/hardware/vibrator/ActivePwle.aidl | 28 ++++++- .../android/hardware/vibrator/IVibrator.aidl | 4 + .../aidl/vts/VtsHalVibratorTargetTest.cpp | 78 ++++++++----------- 3 files changed, 62 insertions(+), 48 deletions(-) diff --git a/vibrator/aidl/android/hardware/vibrator/ActivePwle.aidl b/vibrator/aidl/android/hardware/vibrator/ActivePwle.aidl index fd5f8d1973..6757476e01 100644 --- a/vibrator/aidl/android/hardware/vibrator/ActivePwle.aidl +++ b/vibrator/aidl/android/hardware/vibrator/ActivePwle.aidl @@ -22,26 +22,50 @@ parcelable ActivePwle { * Amplitude ranging from 0.0 (inclusive) to 1.0 (inclusive) * in units of output acceleration amplitude, not voltage amplitude. * + * Values should fall within the range of 0.0 (inclusive) to the maximum defined + * by the corresponding entries in IVibrator#getBandwidthAmplitudeMap (inclusive). + * If startFrequency falls between two entries, the value will not exceed the + * largest amplitude of the two bounding frequencies. + * * 0.0 represents no output acceleration amplitude - * 1.0 represents maximum output acceleration amplitude at resonant frequency + * 1.0 represents maximum output acceleration amplitude + * across all supported frequencies */ float startAmplitude; + /** * Absolute frequency point in the units of hertz + * + * Values are within the continuous inclusive frequency range defined by + * IVibrator#getBandwidthAmplitudeMap, and not limited by the + * IVibrator#getFrequencyResolution. */ float startFrequency; + /** * Amplitude ranging from 0.0 (inclusive) to 1.0 (inclusive) * in units of output acceleration amplitude, not voltage amplitude. * + * Values should fall within the range of 0.0 (inclusive) to the maximum defined + * by the corresponding entries in IVibrator#getBandwidthAmplitudeMap (inclusive). + * If endFrequency falls between two entries, the value will not exceed the + * largest amplitude of the two bounding frequencies. + * * 0.0 represents no output acceleration amplitude - * 1.0 represents maximum output acceleration amplitude at resonant frequency + * 1.0 represents maximum output acceleration amplitude + * across all supported frequencies */ float endAmplitude; + /** * Absolute frequency point in the units of hertz + * + * Values are within the continuous inclusive frequency range defined by + * IVibrator#getBandwidthAmplitudeMap, and not limited by the + * IVibrator#getFrequencyResolution. */ float endFrequency; + /** * Total duration from start point to end point in the units of milliseconds */ diff --git a/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl b/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl index 592d151d32..b4e7e44fe0 100644 --- a/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl +++ b/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl @@ -305,6 +305,10 @@ interface IVibrator { * of getFrequencyResolution(). The value returned by getResonantFrequency() must be * represented in the returned list. * + * The amplitude values represent the maximum output acceleration amplitude supported for each + * given frequency. Equal amplitude values for different frequencies represent equal output + * accelerations. + * * @return The maximum output acceleration amplitude for each supported frequency, * starting at getMinimumFrequency() */ diff --git a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp index 553d7f0a4a..53f8c0ef24 100644 --- a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp +++ b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp @@ -717,27 +717,33 @@ TEST_P(VibratorAidl, GetSupportedBraking) { TEST_P(VibratorAidl, ComposeValidPwle) { if (capabilities & IVibrator::CAP_COMPOSE_PWLE_EFFECTS) { - ActivePwle active = composeValidActivePwle(vibrator, capabilities); + ActivePwle firstActive = composeValidActivePwle(vibrator, capabilities); std::vector supported; ASSERT_TRUE(vibrator->getSupportedBraking(&supported).isOk()); bool isClabSupported = std::find(supported.begin(), supported.end(), Braking::CLAB) != supported.end(); - BrakingPwle braking; - braking.braking = isClabSupported ? Braking::CLAB : Braking::NONE; - braking.duration = 100; + BrakingPwle firstBraking; + firstBraking.braking = isClabSupported ? Braking::CLAB : Braking::NONE; + firstBraking.duration = 100; - std::vector pwleQueue; - PrimitivePwle pwle; - pwle = active; - pwleQueue.emplace_back(std::move(pwle)); - pwle = braking; - pwleQueue.emplace_back(std::move(pwle)); - pwle = active; - pwleQueue.emplace_back(std::move(pwle)); + ActivePwle secondActive = composeValidActivePwle(vibrator, capabilities); + if (capabilities & IVibrator::CAP_FREQUENCY_CONTROL) { + float minFrequencyHz = getFrequencyMinimumHz(vibrator, capabilities); + float maxFrequencyHz = getFrequencyMaximumHz(vibrator, capabilities); + float freqResolutionHz = getFrequencyResolutionHz(vibrator, capabilities); + secondActive.startFrequency = minFrequencyHz + (freqResolutionHz / 2.0f); + secondActive.endFrequency = maxFrequencyHz - (freqResolutionHz / 3.0f); + } + BrakingPwle secondBraking; + secondBraking.braking = Braking::NONE; + secondBraking.duration = 10; + + auto pwleQueue = + std::vector{firstActive, firstBraking, secondActive, secondBraking}; EXPECT_EQ(Status::EX_NONE, vibrator->composePwle(pwleQueue, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); } } @@ -765,14 +771,7 @@ TEST_P(VibratorAidl, ComposeValidPwleWithCallback) { braking.braking = isClabSupported ? Braking::CLAB : Braking::NONE; braking.duration = 100; - std::vector pwleQueue; - PrimitivePwle pwle; - pwle = active; - pwleQueue.emplace_back(std::move(pwle)); - pwle = braking; - pwleQueue.emplace_back(std::move(pwle)); - pwle = active; - pwleQueue.emplace_back(std::move(pwle)); + auto pwleQueue = std::vector{active, braking, active}; EXPECT_TRUE(vibrator->composePwle(pwleQueue, callback).isOk()); EXPECT_EQ(completionFuture.wait_for(timeout), std::future_status::ready); @@ -785,7 +784,7 @@ TEST_P(VibratorAidl, ComposePwleSegmentBoundary) { // test empty queue EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueue, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); ActivePwle active = composeValidActivePwle(vibrator, capabilities); @@ -801,7 +800,7 @@ TEST_P(VibratorAidl, ComposePwleSegmentBoundary) { EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueue, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); } } @@ -811,25 +810,20 @@ TEST_P(VibratorAidl, ComposePwleAmplitudeParameterBoundary) { active.startAmplitude = getAmplitudeMax() + 1.0; // Amplitude greater than allowed active.endAmplitude = getAmplitudeMax() + 1.0; // Amplitude greater than allowed - std::vector pwleQueueGreater; - PrimitivePwle pwle; - pwle = active; - pwleQueueGreater.emplace_back(std::move(pwle)); + auto pwleQueueGreater = std::vector{active}; EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueueGreater, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); active.startAmplitude = getAmplitudeMin() - 1.0; // Amplitude less than allowed active.endAmplitude = getAmplitudeMin() - 1.0; // Amplitude less than allowed - std::vector pwleQueueLess; - pwle = active; - pwleQueueLess.emplace_back(std::move(pwle)); + auto pwleQueueLess = std::vector{active}; EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueueLess, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); } } @@ -845,25 +839,20 @@ TEST_P(VibratorAidl, ComposePwleFrequencyParameterBoundary) { freqMaximumHz + freqResolutionHz; // Frequency greater than allowed active.endFrequency = freqMaximumHz + freqResolutionHz; // Frequency greater than allowed - std::vector pwleQueueGreater; - PrimitivePwle pwle; - pwle = active; - pwleQueueGreater.emplace_back(std::move(pwle)); + auto pwleQueueGreater = std::vector{active}; EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueueGreater, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); active.startFrequency = freqMinimumHz - freqResolutionHz; // Frequency less than allowed active.endFrequency = freqMinimumHz - freqResolutionHz; // Frequency less than allowed - std::vector pwleQueueLess; - pwle = active; - pwleQueueLess.emplace_back(std::move(pwle)); + auto pwleQueueLess = std::vector{active}; EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueueLess, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); } } @@ -875,14 +864,11 @@ TEST_P(VibratorAidl, ComposePwleSegmentDurationBoundary) { vibrator->getPwlePrimitiveDurationMax(&segmentDurationMaxMs); active.duration = segmentDurationMaxMs + 10; // Segment duration greater than allowed - std::vector pwleQueue; - PrimitivePwle pwle; - pwle = active; - pwleQueue.emplace_back(std::move(pwle)); + auto pwleQueue = std::vector{active}; EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT, vibrator->composePwle(pwleQueue, nullptr).exceptionCode()); - vibrator->off(); + EXPECT_TRUE(vibrator->off().isOk()); } }