From 79903fea44549cc4f81af4f54124c96a7b6d3f27 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Mon, 14 Dec 2020 17:28:29 -0800 Subject: [PATCH] Audio VTS: Improve stability, update config validation test Major challenge: the configuration of HAL tests depends on the APM XML configuration file which may be invalid. The code that reads the configuration has been updated to avoid crashes when the config is invalid. In CheckConfig_audioPolicyConfigurationValidation the dependency on the config parser was removed. Previously, a failure to parse the config by the config parser would lead to the test being skipped, which isn't correct as it must fail in this case. Minor fixes to V7 tests to pass on a real life legacy HAL. Bug: 36733185 Test: run VtsHalAudioV7_0TargetTest on a device with V6 Test: run VtsHalAudioV7_0TargetTest on a device with a side-loaded V7 and invalid APM config file Test: run VtsHalAudioV7_0TargetTest on a device with a side-loaded V7 Change-Id: I746339ff69ab455dc64eef9a17827d047b357329 --- .../4.0/AudioPrimaryHidlHalTest.cpp | 13 ++++--- .../7.0/AudioPrimaryHidlHalTest.cpp | 34 ++++++++++++------- .../vts/functional/7.0/PolicyConfig.h | 20 +++++++---- .../vts/functional/AudioPrimaryHidlHalTest.h | 30 ++++++++-------- 4 files changed, 57 insertions(+), 40 deletions(-) diff --git a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp index 1612d3c30c..9a4a8b23a8 100644 --- a/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/4.0/AudioPrimaryHidlHalTest.cpp @@ -140,20 +140,23 @@ TEST_P(AudioHidlDeviceTest, SetConnectedState) { #if MAJOR_VERSION <= 6 using AD = AudioDevice; for (auto deviceType : {AD::OUT_HDMI, AD::OUT_WIRED_HEADPHONE, AD::IN_USB_HEADSET}) { + SCOPED_TRACE("device=" + ::testing::PrintToString(deviceType)); #elif MAJOR_VERSION >= 7 using AD = xsd::AudioDevice; - for (auto deviceType : - {toString(AD::AUDIO_DEVICE_OUT_HDMI), toString(AD::AUDIO_DEVICE_OUT_WIRED_HEADPHONE), - toString(AD::AUDIO_DEVICE_IN_USB_HEADSET)}) { + for (auto deviceType : {AD::AUDIO_DEVICE_OUT_HDMI, AD::AUDIO_DEVICE_OUT_WIRED_HEADPHONE, + AD::AUDIO_DEVICE_IN_USB_HEADSET}) { + SCOPED_TRACE("device=" + toString(deviceType)); #endif - SCOPED_TRACE("device=" + ::testing::PrintToString(deviceType)); for (bool state : {true, false}) { SCOPED_TRACE("state=" + ::testing::PrintToString(state)); DeviceAddress address = {}; #if MAJOR_VERSION <= 6 address.device = deviceType; #elif MAJOR_VERSION >= 7 - address.deviceType = deviceType; + address.deviceType = toString(deviceType); + if (deviceType == AD::AUDIO_DEVICE_IN_USB_HEADSET) { + address.address.alsa({0, 0}); + } #endif auto ret = getDevice()->setConnectedState(address, state); ASSERT_TRUE(ret.isOk()); 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 941c4bdbc9..6bb7995353 100644 --- a/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp +++ b/audio/core/all-versions/vts/functional/7.0/AudioPrimaryHidlHalTest.cpp @@ -44,22 +44,26 @@ const std::vector& getOutputDeviceConfigParameters() { for (const auto& device : getDeviceParameters()) { auto module = getCachedPolicyConfig().getModuleFromName(std::get(device)); + if (!module || !module->getFirstMixPorts()) break; for (const auto& mixPort : module->getFirstMixPorts()->getMixPort()) { if (mixPort.getRole() != xsd::Role::source) continue; // not an output profile - auto xsdFlags = mixPort.getFlags(); - const bool isOffload = - std::find(xsdFlags.begin(), xsdFlags.end(), - xsd::AudioInOutFlag::AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) != - xsdFlags.end(); std::vector flags; - if (!isOffload) { - for (auto flag : xsdFlags) { - if (flag != xsd::AudioInOutFlag::AUDIO_OUTPUT_FLAG_PRIMARY) { - flags.push_back(toString(flag)); + bool isOffload = false; + if (mixPort.hasFlags()) { + auto xsdFlags = mixPort.getFlags(); + isOffload = + std::find(xsdFlags.begin(), xsdFlags.end(), + xsd::AudioInOutFlag::AUDIO_OUTPUT_FLAG_COMPRESS_OFFLOAD) != + xsdFlags.end(); + if (!isOffload) { + for (auto flag : xsdFlags) { + if (flag != xsd::AudioInOutFlag::AUDIO_OUTPUT_FLAG_PRIMARY) { + flags.push_back(toString(flag)); + } } + } else { + flags = offloadFlags; } - } else { - flags = offloadFlags; } for (const auto& profile : mixPort.getProfile()) { auto configs = @@ -94,11 +98,15 @@ const std::vector& getInputDeviceConfigParameters() { for (const auto& device : getDeviceParameters()) { auto module = getCachedPolicyConfig().getModuleFromName(std::get(device)); + if (!module || !module->getFirstMixPorts()) break; for (const auto& mixPort : module->getFirstMixPorts()->getMixPort()) { if (mixPort.getRole() != xsd::Role::sink) continue; // not an input profile std::vector flags; - std::transform(mixPort.getFlags().begin(), mixPort.getFlags().end(), flags.begin(), - [](auto flag) { return toString(flag); }); + if (mixPort.hasFlags()) { + std::transform(mixPort.getFlags().begin(), mixPort.getFlags().end(), + std::back_inserter(flags), + [](auto flag) { return toString(flag); }); + } for (const auto& profile : mixPort.getProfile()) { auto configs = combineAudioConfig(profile.getChannelMasks(), diff --git a/audio/core/all-versions/vts/functional/7.0/PolicyConfig.h b/audio/core/all-versions/vts/functional/7.0/PolicyConfig.h index d790b34c46..7d8864284c 100644 --- a/audio/core/all-versions/vts/functional/7.0/PolicyConfig.h +++ b/audio/core/all-versions/vts/functional/7.0/PolicyConfig.h @@ -33,10 +33,14 @@ class PolicyConfig { if (mConfig) { mStatus = OK; mPrimaryModule = getModuleFromName(DeviceManager::kPrimaryDevice); - for (const auto& module : mConfig->getFirstModules()->get_module()) { - auto attachedDevices = module.getFirstAttachedDevices()->getItem(); - if (!attachedDevices.empty()) { - mModulesWithDevicesNames.insert(module.getName()); + if (mConfig->getFirstModules()) { + for (const auto& module : mConfig->getFirstModules()->get_module()) { + if (module.getFirstAttachedDevices()) { + auto attachedDevices = module.getFirstAttachedDevices()->getItem(); + if (!attachedDevices.empty()) { + mModulesWithDevicesNames.insert(module.getName()); + } + } } } } @@ -52,7 +56,7 @@ class PolicyConfig { } const std::string& getFilePath() const { return mFilePath; } const xsd::Module* getModuleFromName(const std::string& name) const { - if (mConfig) { + if (mConfig && mConfig->getFirstModules()) { for (const auto& module : mConfig->getFirstModules()->get_module()) { if (module.getName() == name) return &module; } @@ -65,8 +69,10 @@ class PolicyConfig { } bool haveInputProfilesInModule(const std::string& name) const { auto module = getModuleFromName(name); - for (const auto& mixPort : module->getFirstMixPorts()->getMixPort()) { - if (mixPort.getRole() == xsd::Role::sink) return true; + if (module && module->getFirstMixPorts()) { + for (const auto& mixPort : module->getFirstMixPorts()->getMixPort()) { + if (mixPort.getRole() == xsd::Role::sink) return true; + } } return false; } diff --git a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h index 05c9bf7e2e..43c44cbd8d 100644 --- a/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h +++ b/audio/core/all-versions/vts/functional/AudioPrimaryHidlHalTest.h @@ -156,6 +156,21 @@ const PolicyConfig& getCachedPolicyConfig() { return *policyConfig; } +TEST(CheckConfig, audioPolicyConfigurationValidation) { + const auto factories = ::android::hardware::getAllHalInstanceNames(IDevicesFactory::descriptor); + if (factories.size() == 0) { + GTEST_SKIP() << "Skipping audioPolicyConfigurationValidation because no factory instances " + "are found."; + } + RecordProperty("description", + "Verify that the audio policy configuration file " + "is valid according to the schema"); + + const char* xsd = "/data/local/tmp/audio_policy_configuration_" STRINGIFY(CPP_VERSION) ".xsd"; + EXPECT_ONE_VALID_XML_MULTIPLE_LOCATIONS(kConfigFileName, + android::audio_get_configuration_paths(), xsd); +} + ////////////////////////////////////////////////////////////////////////////// //////////////////// Test parameter types and definitions //////////////////// ////////////////////////////////////////////////////////////////////////////// @@ -231,21 +246,6 @@ class AudioHidlTestWithDeviceParameter : public HidlTest, } }; -TEST(CheckConfig, audioPolicyConfigurationValidation) { - auto deviceParameters = getDeviceParametersForFactoryTests(); - if (deviceParameters.size() == 0) { - GTEST_SKIP() << "Skipping audioPolicyConfigurationValidation because no device parameter " - "is found."; - } - RecordProperty("description", - "Verify that the audio policy configuration file " - "is valid according to the schema"); - - const char* xsd = "/data/local/tmp/audio_policy_configuration_" STRINGIFY(CPP_VERSION) ".xsd"; - EXPECT_ONE_VALID_XML_MULTIPLE_LOCATIONS(kConfigFileName, - android::audio_get_configuration_paths(), xsd); -} - class AudioPolicyConfigTest : public AudioHidlTestWithDeviceParameter { public: void SetUp() override {