vibrator: aidl: Apply Compose API Feedback

- Make Thud and Spin optional due to complexity.
- Make "scale" inclusive of zero, which represents minimum "feelable"
  intensity.
- Update VTS tests appropriately.
- Fix typo in VTS test names.

Bug: 151084263
Test: VTS on Flame, Walleye, and Cuttlefish
Signed-off-by: Harpreet \"Eli\" Sangha <eliptus@google.com>
Change-Id: Ib0d046be83ee79ab38e0b9c3fb87a41f23879f8b

5 files changed, 97 insertions, 14 deletions

diff --git a/vibrator/aidl/android/hardware/vibrator/CompositeEffect.aidl b/vibrator/aidl/android/hardware/vibrator/CompositeEffect.aidl
index 84556b57bb..406a899823 100644
--- a/vibrator/aidl/android/hardware/vibrator/CompositeEffect.aidl
+++ b/vibrator/aidl/android/hardware/vibrator/CompositeEffect.aidl
@@ -23,6 +23,9 @@ parcelable CompositeEffect {
     /* Period of silence preceding primitive. */
     int delayMs;
     CompositePrimitive primitive;
-    /* 0.0 (exclusive) - 1.0 (inclusive) */
+    /*
+     * 0.0 (inclusive) - 1.0 (inclusive),
+     * where 0.0 is minimum "feelable" amplitude.
+     */
     float scale;
 }
diff --git a/vibrator/aidl/android/hardware/vibrator/CompositePrimitive.aidl b/vibrator/aidl/android/hardware/vibrator/CompositePrimitive.aidl
index 0fdfa5d5c2..8e82db076b 100644
--- a/vibrator/aidl/android/hardware/vibrator/CompositePrimitive.aidl
+++ b/vibrator/aidl/android/hardware/vibrator/CompositePrimitive.aidl
@@ -21,37 +21,53 @@ package android.hardware.vibrator;
 enum CompositePrimitive {
     /**
      * No haptic effect. Used to generate extended delays between primitives.
+     *
+     * Support is required.
      */
     NOOP,
     /**
      * This effect should produce a sharp, crisp click sensation.
+     *
+     * Support is required.
      */
     CLICK,
     /**
      * A haptic effect that simulates downwards movement with gravity. Often
      * followed by extra energy of hitting and reverberation to augment
      * physicality.
+     *
+     * Support is optional.
      */
     THUD,
     /**
      * A haptic effect that simulates spinning momentum.
+     *
+     * Support is optional.
      */
     SPIN,
     /**
      * A haptic effect that simulates quick upward movement against gravity.
+     *
+     * Support is required.
      */
     QUICK_RISE,
     /**
      * A haptic effect that simulates slow upward movement against gravity.
+     *
+     * Support is required.
      */
     SLOW_RISE,
     /**
      * A haptic effect that simulates quick downwards movement with gravity.
+     *
+     * Support is required.
      */
     QUICK_FALL,
     /**
      * This very short effect should produce a light crisp sensation intended
      * to be used repetitively for dynamic feedback.
+     *
+     * Support is required.
      */
     LIGHT_TICK,
 }
diff --git a/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl b/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl
index 6489c1d600..0b21248220 100644
--- a/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl
+++ b/vibrator/aidl/android/hardware/vibrator/IVibrator.aidl
@@ -161,8 +161,8 @@ interface IVibrator {
      * List of supported effect primitive.
      *
      * Return the effect primitives which are supported by the compose API.
-     * Implementations are expected to support all primitives of the interface
-     * version that they implement.
+     * Implementations are expected to support all required primitives of the
+     * interface version that they implement (see primitive definitions).
      */
     CompositePrimitive[] getSupportedPrimitives();
 
diff --git a/vibrator/aidl/default/Vibrator.cpp b/vibrator/aidl/default/Vibrator.cpp
index 9236b95ddc..b359100d1a 100644
--- a/vibrator/aidl/default/Vibrator.cpp
+++ b/vibrator/aidl/default/Vibrator.cpp
@@ -146,7 +146,7 @@ ndk::ScopedAStatus Vibrator::compose(const std::vector<CompositeEffect>& composi
         if (e.delayMs > kComposeDelayMaxMs) {
             return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
         }
-        if (e.scale <= 0.0f || e.scale > 1.0f) {
+        if (e.scale < 0.0f || e.scale > 1.0f) {
             return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
         }
         if (std::find(supported.begin(), supported.end(), e.primitive) == supported.end()) {
diff --git a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp
index 8b16025b5d..8340517de6 100644
--- a/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp
+++ b/vibrator/aidl/vts/VtsHalVibratorTargetTest.cpp
@@ -21,6 +21,7 @@
 #include <binder/IServiceManager.h>
 #include <binder/ProcessState.h>
 
+#include <cmath>
 #include <future>
 
 using android::ProcessState;
@@ -53,6 +54,11 @@ const std::vector<CompositePrimitive> kCompositePrimitives{
         android::enum_range<CompositePrimitive>().begin(),
         android::enum_range<CompositePrimitive>().end()};
 
+const std::vector<CompositePrimitive> kOptionalPrimitives = {
+        CompositePrimitive::THUD,
+        CompositePrimitive::SPIN,
+};
+
 const std::vector<CompositePrimitive> kInvalidPrimitives = {
         static_cast<CompositePrimitive>(static_cast<int32_t>(kCompositePrimitives.front()) - 1),
         static_cast<CompositePrimitive>(static_cast<int32_t>(kCompositePrimitives.back()) + 1),
@@ -264,38 +270,56 @@ TEST_P(VibratorAidl, GetSupportedPrimitives) {
 
         EXPECT_EQ(Status::EX_NONE, vibrator->getSupportedPrimitives(&supported).exceptionCode());
 
-        std::sort(supported.begin(), supported.end());
+        for (auto primitive : kCompositePrimitives) {
+            bool isPrimitiveSupported =
+                    std::find(supported.begin(), supported.end(), primitive) != supported.end();
+            bool isPrimitiveOptional =
+                    std::find(kOptionalPrimitives.begin(), kOptionalPrimitives.end(), primitive) !=
+                    kOptionalPrimitives.end();
 
-        EXPECT_EQ(kCompositePrimitives, supported);
+            EXPECT_TRUE(isPrimitiveSupported || isPrimitiveOptional) << toString(primitive);
+        }
     }
 }
 
 TEST_P(VibratorAidl, GetPrimitiveDuration) {
     if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
-        int32_t duration;
+        std::vector<CompositePrimitive> supported;
+        ASSERT_TRUE(vibrator->getSupportedPrimitives(&supported).isOk());
 
         for (auto primitive : kCompositePrimitives) {
-            EXPECT_EQ(Status::EX_NONE,
-                      vibrator->getPrimitiveDuration(primitive, &duration).exceptionCode());
+            bool isPrimitiveSupported =
+                    std::find(supported.begin(), supported.end(), primitive) != supported.end();
+            int32_t duration;
+
+            Status status = vibrator->getPrimitiveDuration(primitive, &duration);
+
+            if (isPrimitiveSupported) {
+                EXPECT_EQ(Status::EX_NONE, status.exceptionCode());
+            } else {
+                EXPECT_EQ(Status::EX_UNSUPPORTED_OPERATION, status.exceptionCode());
+            }
         }
     }
 }
 
 TEST_P(VibratorAidl, ComposeValidPrimitives) {
     if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
+        std::vector<CompositePrimitive> supported;
         int32_t maxDelay, maxSize;
 
+        ASSERT_TRUE(vibrator->getSupportedPrimitives(&supported).isOk());
         EXPECT_EQ(Status::EX_NONE, vibrator->getCompositionDelayMax(&maxDelay).exceptionCode());
         EXPECT_EQ(Status::EX_NONE, vibrator->getCompositionSizeMax(&maxSize).exceptionCode());
 
         std::vector<CompositeEffect> composite;
 
-        for (auto primitive : kCompositePrimitives) {
+        for (auto primitive : supported) {
             CompositeEffect effect;
 
             effect.delayMs = std::rand() % (maxDelay + 1);
             effect.primitive = primitive;
-            effect.scale = static_cast<float>(std::rand()) / RAND_MAX ?: 1.0f;
+            effect.scale = static_cast<float>(std::rand()) / RAND_MAX;
             composite.emplace_back(effect);
 
             if (composite.size() == maxSize) {
@@ -314,7 +338,21 @@ TEST_P(VibratorAidl, ComposeValidPrimitives) {
 
 TEST_P(VibratorAidl, ComposeUnsupportedPrimitives) {
     if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
-        for (auto primitive : kInvalidPrimitives) {
+        auto unsupported = kInvalidPrimitives;
+        std::vector<CompositePrimitive> supported;
+
+        ASSERT_TRUE(vibrator->getSupportedPrimitives(&supported).isOk());
+
+        for (auto primitive : kCompositePrimitives) {
+            bool isPrimitiveSupported =
+                    std::find(supported.begin(), supported.end(), primitive) != supported.end();
+
+            if (!isPrimitiveSupported) {
+                unsupported.push_back(primitive);
+            }
+        }
+
+        for (auto primitive : unsupported) {
             std::vector<CompositeEffect> composite(1);
 
             for (auto& effect : composite) {
@@ -329,7 +367,33 @@ TEST_P(VibratorAidl, ComposeUnsupportedPrimitives) {
     }
 }
 
-TEST_P(VibratorAidl, CompseDelayBoundary) {
+TEST_P(VibratorAidl, ComposeScaleBoundary) {
+    if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
+        std::vector<CompositeEffect> composite(1);
+        CompositeEffect& effect = composite[0];
+
+        effect.delayMs = 0;
+        effect.primitive = CompositePrimitive::CLICK;
+
+        effect.scale = std::nextafter(0.0f, -1.0f);
+        EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+                  vibrator->compose(composite, nullptr).exceptionCode());
+
+        effect.scale = 0.0f;
+        EXPECT_EQ(Status::EX_NONE, vibrator->compose(composite, nullptr).exceptionCode());
+
+        effect.scale = 1.0f;
+        EXPECT_EQ(Status::EX_NONE, vibrator->compose(composite, nullptr).exceptionCode());
+
+        effect.scale = std::nextafter(1.0f, 2.0f);
+        EXPECT_EQ(Status::EX_ILLEGAL_ARGUMENT,
+                  vibrator->compose(composite, nullptr).exceptionCode());
+
+        vibrator->off();
+    }
+}
+
+TEST_P(VibratorAidl, ComposeDelayBoundary) {
     if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
         int32_t maxDelay;
 
@@ -354,7 +418,7 @@ TEST_P(VibratorAidl, CompseDelayBoundary) {
     }
 }
 
-TEST_P(VibratorAidl, CompseSizeBoundary) {
+TEST_P(VibratorAidl, ComposeSizeBoundary) {
     if (capabilities & IVibrator::CAP_COMPOSE_EFFECTS) {
         int32_t maxSize;