Audio: Add VTS tests for invalid enum-strings, Part 3

Add checks for rejection of invalid port configs.
Fix the default implementation to pass the tests.

Clarified that patch-related operations in IDevice
are optional.

Bug: 142480271
Test: atest VtsHalAudioV6_0TargetTest
Test: atest VtsHalAudioV7_0TargetTest
      with side-loaded V7 default wrapper
Change-Id: I08e85d4883938b4d8f3c411be9fb1bd597eea0c2

3 files changed, 169 insertions, 16 deletions

diff --git a/audio/7.0/IDevice.hal b/audio/7.0/IDevice.hal
index d9e0ad205f..e423f2967a 100644
--- a/audio/7.0/IDevice.hal
+++ b/audio/7.0/IDevice.hal
@@ -174,6 +174,9 @@ interface IDevice {
      * Creates an audio patch between several source and sink ports.  The handle
      * is allocated by the HAL and must be unique for this audio HAL module.
      *
+     * Optional method. HAL must support it if 'supportsAudioPatches' returns
+     * 'true'.
+     *
      * @param sources patch sources.
      * @param sinks patch sinks.
      * @return retval operation completion status.
@@ -189,6 +192,9 @@ interface IDevice {
      * as the HAL module can figure out a way of switching the route without
      * causing audio disruption.
      *
+     * Optional method. HAL must support it if 'supportsAudioPatches' returns
+     * 'true'.
+     *
      * @param previousPatch handle of the previous patch to update.
      * @param sources new patch sources.
      * @param sinks new patch sinks.
@@ -204,6 +210,9 @@ interface IDevice {
     /**
      * Release an audio patch.
      *
+     * Optional method. HAL must support it if 'supportsAudioPatches' returns
+     * 'true'.
+     *
      * @param patch patch handle.
      * @return retval operation completion status.
      */
diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp
index 05c1066ba9..7caed44bc9 100644
--- a/audio/core/all-versions/default/Device.cpp
+++ b/audio/core/all-versions/default/Device.cpp
@@ -325,11 +325,17 @@ std::tuple<Result, AudioPatchHandle> Device::createOrUpdateAudioPatch(
         const hidl_vec<AudioPortConfig>& sinks) {
     Result retval(Result::NOT_SUPPORTED);
     if (version() >= AUDIO_DEVICE_API_VERSION_3_0) {
+        audio_patch_handle_t halPatch = static_cast<audio_patch_handle_t>(patch);
         std::unique_ptr<audio_port_config[]> halSources;
-        HidlUtils::audioPortConfigsToHal(sources, &halSources);
+        if (status_t status = HidlUtils::audioPortConfigsToHal(sources, &halSources);
+            status != NO_ERROR) {
+            return {analyzeStatus("audioPortConfigsToHal;sources", status), patch};
+        }
         std::unique_ptr<audio_port_config[]> halSinks;
-        HidlUtils::audioPortConfigsToHal(sinks, &halSinks);
-        audio_patch_handle_t halPatch = static_cast<audio_patch_handle_t>(patch);
+        if (status_t status = HidlUtils::audioPortConfigsToHal(sinks, &halSinks);
+            status != NO_ERROR) {
+            return {analyzeStatus("audioPortConfigsToHal;sinks", status), patch};
+        }
         retval = analyzeStatus("create_audio_patch",
                                mDevice->create_audio_patch(mDevice, sources.size(), &halSources[0],
                                                            sinks.size(), &halSinks[0], &halPatch));
@@ -364,7 +370,10 @@ Return<void> Device::getAudioPort(const AudioPort& port, getAudioPort_cb _hidl_c
 Return<Result> Device::setAudioPortConfig(const AudioPortConfig& config) {
     if (version() >= AUDIO_DEVICE_API_VERSION_3_0) {
         struct audio_port_config halPortConfig;
-        HidlUtils::audioPortConfigToHal(config, &halPortConfig);
+        if (status_t status = HidlUtils::audioPortConfigToHal(config, &halPortConfig);
+            status != NO_ERROR) {
+            return analyzeStatus("audioPortConfigToHal", status);
+        }
         return analyzeStatus("set_audio_port_config",
                              mDevice->set_audio_port_config(mDevice, &halPortConfig));
     }
diff --git a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
index 0f52a900c2..ef4dabae60 100644
--- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
+++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp
@@ -298,6 +298,18 @@ INSTANTIATE_TEST_CASE_P(
         &DeviceConfigParameterToString);
 GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(InputBufferSizeInvalidConfig);
 
+static const DeviceAddress& getValidInputDeviceAddress() {
+    static const DeviceAddress valid = {
+            .deviceType = toString(xsd::AudioDevice::AUDIO_DEVICE_IN_DEFAULT)};
+    return valid;
+}
+
+static const DeviceAddress& getValidOutputDeviceAddress() {
+    static const DeviceAddress valid = {
+            .deviceType = toString(xsd::AudioDevice::AUDIO_DEVICE_OUT_DEFAULT)};
+    return valid;
+}
+
 static const DeviceAddress& getInvalidDeviceAddress() {
     static const DeviceAddress valid = {.deviceType = "random_string"};
     return valid;
@@ -311,6 +323,141 @@ TEST_P(AudioHidlDeviceTest, SetConnectedStateInvalidDeviceAddress) {
                   getDevice()->setConnectedState(getInvalidDeviceAddress(), false));
 }
 
+static std::vector<AudioPortConfig>& generatePortConfigs(bool valid) {
+    enum {  // Note: This is for convenience when deriving "invalid" configs from "valid".
+        PORT_CONF_MINIMAL,
+        PORT_CONF_WITH_GAIN,
+        PORT_CONF_EXT_DEVICE,
+        PORT_CONF_EXT_MIX_SOURCE,
+        PORT_CONF_EXT_MIX_SINK,
+        PORT_CONF_EXT_SESSION
+    };
+    static std::vector<AudioPortConfig> valids = [] {
+        std::vector<AudioPortConfig> result;
+        result.reserve(PORT_CONF_EXT_SESSION + 1);
+        result.push_back(AudioPortConfig{});
+        AudioPortConfig configWithGain{};
+        configWithGain.gain.config(AudioGainConfig{
+                .index = 0,
+                .mode = {toString(xsd::AudioGainMode::AUDIO_GAIN_MODE_JOINT)},
+                .channelMask = toString(xsd::AudioChannelMask::AUDIO_CHANNEL_OUT_MONO),
+                .rampDurationMs = 1});
+        configWithGain.gain.config().values.resize(1);
+        configWithGain.gain.config().values[0] = 1000;
+        result.push_back(std::move(configWithGain));
+        AudioPortConfig configWithPortExtDevice{};
+        configWithPortExtDevice.ext.device(getValidOutputDeviceAddress());
+        result.push_back(std::move(configWithPortExtDevice));
+        AudioPortConfig configWithPortExtMixSource{};
+        configWithPortExtMixSource.ext.mix({});
+        configWithPortExtMixSource.ext.mix().useCase.stream(
+                toString(xsd::AudioStreamType::AUDIO_STREAM_VOICE_CALL));
+        result.push_back(std::move(configWithPortExtMixSource));
+        AudioPortConfig configWithPortExtMixSink{};
+        configWithPortExtMixSink.ext.mix({});
+        configWithPortExtMixSink.ext.mix().useCase.source(
+                toString(xsd::AudioSource::AUDIO_SOURCE_DEFAULT));
+        result.push_back(std::move(configWithPortExtMixSink));
+        AudioPortConfig configWithPortExtSession{};
+        configWithPortExtSession.ext.session(
+                static_cast<AudioSession>(AudioSessionConsts::OUTPUT_MIX));
+        result.push_back(std::move(configWithPortExtSession));
+        return result;
+    }();
+    static std::vector<AudioPortConfig> invalids = [&] {
+        std::vector<AudioPortConfig> result;
+        AudioPortConfig invalidBaseChannelMask = valids[PORT_CONF_MINIMAL];
+        invalidBaseChannelMask.base.channelMask = "random_string";
+        result.push_back(std::move(invalidBaseChannelMask));
+        AudioPortConfig invalidBaseFormat = valids[PORT_CONF_MINIMAL];
+        invalidBaseFormat.base.format = "random_string";
+        result.push_back(std::move(invalidBaseFormat));
+        AudioPortConfig invalidGainMode = valids[PORT_CONF_WITH_GAIN];
+        invalidGainMode.gain.config().mode = {{"random_string"}};
+        result.push_back(std::move(invalidGainMode));
+        AudioPortConfig invalidGainChannelMask = valids[PORT_CONF_WITH_GAIN];
+        invalidGainChannelMask.gain.config().channelMask = "random_string";
+        result.push_back(std::move(invalidGainChannelMask));
+        AudioPortConfig invalidDeviceType = valids[PORT_CONF_EXT_DEVICE];
+        invalidDeviceType.ext.device().deviceType = "random_string";
+        result.push_back(std::move(invalidDeviceType));
+        AudioPortConfig invalidStreamType = valids[PORT_CONF_EXT_MIX_SOURCE];
+        invalidStreamType.ext.mix().useCase.stream() = "random_string";
+        result.push_back(std::move(invalidStreamType));
+        AudioPortConfig invalidSource = valids[PORT_CONF_EXT_MIX_SINK];
+        invalidSource.ext.mix().useCase.source() = "random_string";
+        result.push_back(std::move(invalidSource));
+        return result;
+    }();
+    return valid ? valids : invalids;
+}
+
+TEST_P(AudioHidlDeviceTest, SetAudioPortConfigInvalidArguments) {
+    doc::test("Check that invalid port configs are rejected by IDevice::setAudioPortConfig");
+    for (const auto& invalidConfig : generatePortConfigs(false /*valid*/)) {
+        EXPECT_RESULT(invalidArgsOrNotSupported, getDevice()->setAudioPortConfig(invalidConfig))
+                << ::testing::PrintToString(invalidConfig);
+    }
+}
+
+TEST_P(AudioPatchHidlTest, CreatePatchInvalidArguments) {
+    doc::test("Check that invalid port configs are rejected by IDevice::createAudioPatch");
+    // Note that HAL actually might reject the proposed source / sink combo
+    // due to other reasons than presence of invalid enum-strings.
+    // TODO: Come up with a way to guarantee validity of a source / sink combo.
+    for (const auto& validSource : generatePortConfigs(true /*valid*/)) {
+        for (const auto& invalidSink : generatePortConfigs(false /*valid*/)) {
+            AudioPatchHandle handle;
+            EXPECT_OK(getDevice()->createAudioPatch(hidl_vec<AudioPortConfig>{validSource},
+                                                    hidl_vec<AudioPortConfig>{invalidSink},
+                                                    returnIn(res, handle)));
+            EXPECT_EQ(Result::INVALID_ARGUMENTS, res)
+                    << "Source: " << ::testing::PrintToString(validSource)
+                    << "; Sink: " << ::testing::PrintToString(invalidSink);
+        }
+    }
+    for (const auto& validSink : generatePortConfigs(true /*valid*/)) {
+        for (const auto& invalidSource : generatePortConfigs(false /*valid*/)) {
+            AudioPatchHandle handle;
+            EXPECT_OK(getDevice()->createAudioPatch(hidl_vec<AudioPortConfig>{invalidSource},
+                                                    hidl_vec<AudioPortConfig>{validSink},
+                                                    returnIn(res, handle)));
+            EXPECT_EQ(Result::INVALID_ARGUMENTS, res)
+                    << "Source: " << ::testing::PrintToString(invalidSource)
+                    << "; Sink: " << ::testing::PrintToString(validSink);
+        }
+    }
+}
+
+TEST_P(AudioPatchHidlTest, UpdatePatchInvalidArguments) {
+    doc::test("Check that invalid port configs are rejected by IDevice::updateAudioPatch");
+    // Note that HAL actually might reject the proposed source / sink combo
+    // due to other reasons than presence of invalid enum-strings.
+    // TODO: Come up with a way to guarantee validity of a source / sink combo.
+    for (const auto& validSource : generatePortConfigs(true /*valid*/)) {
+        for (const auto& invalidSink : generatePortConfigs(false /*valid*/)) {
+            AudioPatchHandle handle{};
+            EXPECT_OK(getDevice()->updateAudioPatch(handle, hidl_vec<AudioPortConfig>{validSource},
+                                                    hidl_vec<AudioPortConfig>{invalidSink},
+                                                    returnIn(res, handle)));
+            EXPECT_EQ(Result::INVALID_ARGUMENTS, res)
+                    << "Source: " << ::testing::PrintToString(validSource)
+                    << "; Sink: " << ::testing::PrintToString(invalidSink);
+        }
+    }
+    for (const auto& validSink : generatePortConfigs(true /*valid*/)) {
+        for (const auto& invalidSource : generatePortConfigs(false /*valid*/)) {
+            AudioPatchHandle handle{};
+            EXPECT_OK(getDevice()->updateAudioPatch(
+                    handle, hidl_vec<AudioPortConfig>{invalidSource},
+                    hidl_vec<AudioPortConfig>{validSink}, returnIn(res, handle)));
+            EXPECT_EQ(Result::INVALID_ARGUMENTS, res)
+                    << "Source: " << ::testing::PrintToString(invalidSource)
+                    << "; Sink: " << ::testing::PrintToString(validSink);
+        }
+    }
+}
+
 enum { PARAM_DEVICE_CONFIG, PARAM_ADDRESS, PARAM_METADATA };
 enum { INDEX_SINK, INDEX_SOURCE };
 using SinkOrSourceMetadata = std::variant<SinkMetadata, SourceMetadata>;
@@ -362,18 +509,6 @@ class StreamOpenTest : public HidlTest, public ::testing::WithParamInterface<Str
     }
 };
 
-static const DeviceAddress& getValidInputDeviceAddress() {
-    static const DeviceAddress valid = {
-            .deviceType = toString(xsd::AudioDevice::AUDIO_DEVICE_IN_DEFAULT)};
-    return valid;
-}
-
-static const DeviceAddress& getValidOutputDeviceAddress() {
-    static const DeviceAddress valid = {
-            .deviceType = toString(xsd::AudioDevice::AUDIO_DEVICE_OUT_DEFAULT)};
-    return valid;
-}
-
 static const RecordTrackMetadata& getValidRecordTrackMetadata() {
     static const RecordTrackMetadata valid = {
             .source = toString(xsd::AudioSource::AUDIO_SOURCE_DEFAULT), .gain = 1};