From 8b39328dccd796888aeb566f60f145a70e6d503a Mon Sep 17 00:00:00 2001 From: mike liao Date: Mon, 26 Jun 2023 14:12:48 +0800 Subject: [PATCH 01/39] Add fix vts fail when configureMonitorEvent [Description] VTS StartFilterInDemux failed when configureMonitorEvent is called [Root Cause] Scrambling status event is not notified when configureMonitorEvent is called so test case failed. [Solution] Scrambling status event is not notified because of no input data. Add input setting and check event notified or not after data is input. Test: Manual bug: 288193021 Change-Id: If5875d064fd67b72f8299205a5e35b1a2bd61934 (cherry picked from commit def46527927875efad5a3fbfb5e6a695a9106d51) --- tv/tuner/1.1/vts/functional/FilterTests.cpp | 17 +++++++++++------ tv/tuner/1.1/vts/functional/FilterTests.h | 1 + .../functional/VtsHalTvTunerV1_1TargetTest.cpp | 5 +++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tv/tuner/1.1/vts/functional/FilterTests.cpp b/tv/tuner/1.1/vts/functional/FilterTests.cpp index 8bdf8f68b8..a24cd63f42 100644 --- a/tv/tuner/1.1/vts/functional/FilterTests.cpp +++ b/tv/tuner/1.1/vts/functional/FilterTests.cpp @@ -311,12 +311,6 @@ AssertionResult FilterTests::configureMonitorEvent(uint64_t filterId, uint32_t m android::hardware::tv::tuner::V1_1::IFilter::castFrom(mFilters[filterId]); if (filter_v1_1 != NULL) { status = filter_v1_1->configureMonitorEvent(monitorEventTypes); - if (monitorEventTypes & DemuxFilterMonitorEventType::SCRAMBLING_STATUS) { - mFilterCallbacks[filterId]->testFilterScramblingEvent(); - } - if (monitorEventTypes & DemuxFilterMonitorEventType::IP_CID_CHANGE) { - mFilterCallbacks[filterId]->testFilterIpCidEvent(); - } } else { ALOGW("[vts] Can't cast IFilter into v1_1."); return failure(); @@ -324,6 +318,17 @@ AssertionResult FilterTests::configureMonitorEvent(uint64_t filterId, uint32_t m return AssertionResult(status == Result::SUCCESS); } +AssertionResult FilterTests::testMonitorEvent(uint64_t filterId, uint32_t monitorEventTypes) { + EXPECT_TRUE(mFilterCallbacks[filterId]) << "Test with getNewlyOpenedFilterId first."; + if (monitorEventTypes & DemuxFilterMonitorEventType::SCRAMBLING_STATUS) { + mFilterCallbacks[filterId]->testFilterScramblingEvent(); + } + if (monitorEventTypes & DemuxFilterMonitorEventType::IP_CID_CHANGE) { + mFilterCallbacks[filterId]->testFilterIpCidEvent(); + } + return AssertionResult(true); +} + AssertionResult FilterTests::startIdTest(uint64_t filterId) { EXPECT_TRUE(mFilterCallbacks[filterId]) << "Test with getNewlyOpenedFilterId first."; mFilterCallbacks[filterId]->testStartIdAfterReconfigure(); diff --git a/tv/tuner/1.1/vts/functional/FilterTests.h b/tv/tuner/1.1/vts/functional/FilterTests.h index 1a1273ecb4..e652154b36 100644 --- a/tv/tuner/1.1/vts/functional/FilterTests.h +++ b/tv/tuner/1.1/vts/functional/FilterTests.h @@ -159,6 +159,7 @@ class FilterTests { AssertionResult configAvFilterStreamType(AvStreamType type, uint64_t filterId); AssertionResult configIpFilterCid(uint32_t ipCid, uint64_t filterId); AssertionResult configureMonitorEvent(uint64_t filterId, uint32_t monitorEventTypes); + AssertionResult testMonitorEvent(uint64_t filterId, uint32_t monitorEventTypes); AssertionResult getFilterMQDescriptor(uint64_t filterId, bool getMqDesc); AssertionResult startFilter(uint64_t filterId); AssertionResult stopFilter(uint64_t filterId); diff --git a/tv/tuner/1.1/vts/functional/VtsHalTvTunerV1_1TargetTest.cpp b/tv/tuner/1.1/vts/functional/VtsHalTvTunerV1_1TargetTest.cpp index 41acaa1704..fccd2ede9b 100644 --- a/tv/tuner/1.1/vts/functional/VtsHalTvTunerV1_1TargetTest.cpp +++ b/tv/tuner/1.1/vts/functional/VtsHalTvTunerV1_1TargetTest.cpp @@ -48,6 +48,11 @@ void TunerFilterHidlTest::configSingleFilterInDemuxTest(FilterConfig1_1 filterCo } ASSERT_TRUE(mFilterTests.getFilterMQDescriptor(filterId, filterConf.config1_0.getMqDesc)); ASSERT_TRUE(mFilterTests.startFilter(filterId)); + ASSERT_TRUE(mFrontendTests.tuneFrontend(frontendConf, true /*testWithDemux*/)); + if (filterConf.monitorEventTypes > 0) { + ASSERT_TRUE(mFilterTests.testMonitorEvent(filterId, filterConf.monitorEventTypes)); + } + ASSERT_TRUE(mFrontendTests.stopTuneFrontend(true /*testWithDemux*/)); ASSERT_TRUE(mFilterTests.stopFilter(filterId)); ASSERT_TRUE(mFilterTests.closeFilter(filterId)); ASSERT_TRUE(mDemuxTests.closeDemux()); From 54bbf07e061ccd2c50254856a52514b960b541f9 Mon Sep 17 00:00:00 2001 From: Yuyang Huang Date: Thu, 18 May 2023 17:48:27 +0900 Subject: [PATCH 02/39] Add TV devices that have MdnsOffloadManagerService to APF exempt list For Panel TV devices, the vendor can implemented TV specific mDNS offload instead of APF in U. If TV specific mDNS offload is implemented. MdnsOffloadManagerService will exist in the system ext partition. The APF vts will be skipped if MdnsOffloadManagerService exist. Bug: 283038712 Test: atest VtsHalWifiStaIfaceTargetTest Change-Id: If55ec42507460b9a2c6eee683d85b8109f2af236 --- .../functional/wifi_sta_iface_aidl_test.cpp | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/wifi/aidl/vts/functional/wifi_sta_iface_aidl_test.cpp b/wifi/aidl/vts/functional/wifi_sta_iface_aidl_test.cpp index f12d873f0c..1ea1237a2d 100644 --- a/wifi/aidl/vts/functional/wifi_sta_iface_aidl_test.cpp +++ b/wifi/aidl/vts/functional/wifi_sta_iface_aidl_test.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include @@ -68,6 +69,50 @@ class WifiStaIfaceAidlTest : public testing::TestWithParam { std::shared_ptr wifi_sta_iface_; + // Checks if the MdnsOffloadManagerService is installed. + bool isMdnsOffloadServicePresent() { + int status = + // --query-flags MATCH_SYSTEM_ONLY(1048576) will only return matched service + // installed on system or system_ext partition. The MdnsOffloadManagerService should + // be installed on system_ext partition. + // NOLINTNEXTLINE(cert-env33-c) + system("pm query-services --query-flags 1048576" + " com.android.tv.mdnsoffloadmanager/" + "com.android.tv.mdnsoffloadmanager.MdnsOffloadManagerService" + " | egrep -q mdnsoffloadmanager"); + return status == 0; + } + + // Detected panel TV device by using ro.oem.key1 property. + // https://docs.partner.android.com/tv/build/platform/props-vars/ro-oem-key1 + bool isPanelTvDevice() { + const std::string oem_key1 = getPropertyString("ro.oem.key1"); + if (oem_key1.size() < 9) { + return false; + } + if (oem_key1.substr(0, 3) != "ATV") { + return false; + } + const std::string psz_string = oem_key1.substr(6, 3); + // If PSZ string contains non digit, then it is not a panel TV device. + for (char ch : psz_string) { + if (!isdigit(ch)) { + return false; + } + } + // If PSZ is "000", then it is not a panel TV device. + if (psz_string == "000") { + return false; + } + return true; + } + + std::string getPropertyString(const char* property_name) { + char property_string_raw_bytes[PROPERTY_VALUE_MAX] = {}; + int len = property_get(property_name, property_string_raw_bytes, ""); + return std::string(property_string_raw_bytes, len); + } + private: const char* getInstanceName() { return GetParam().c_str(); } }; @@ -99,6 +144,11 @@ TEST_P(WifiStaIfaceAidlTest, GetFeatureSet) { */ // @VsrTest = 5.3.12 TEST_P(WifiStaIfaceAidlTest, CheckApfIsSupported) { + // Flat panel TV devices that support MDNS offload do not have to implement APF if the WiFi + // chipset does not have sufficient RAM to do so. + if (isPanelTvDevice() && isMdnsOffloadServicePresent()) { + GTEST_SKIP() << "Panel TV supports mDNS offload. It is not required to support APF"; + } int vendor_api_level = property_get_int32("ro.vendor.api_level", 0); // Before VSR 14, APF support is optional. if (vendor_api_level < __ANDROID_API_U__) { From 263e36209101faa1c2f5bb02f944aee2b3153712 Mon Sep 17 00:00:00 2001 From: Austin Borger Date: Thu, 15 Jun 2023 11:32:04 -0700 Subject: [PATCH 03/39] Camera VTS: Properly initialize Stream in various places useCase and colorSpace are not explicitly initialized. colorSpace must be explicitly initialized to UNSPECIFIED, as its default value is not zero. Without initialization, the HAL will receive incorrect Stream data. Bug: 287305593 Test: Ran full VTS test on Cuttlefish / OEM testing. Change-Id: I6a29600b5dc06ebdc61b38e0585204fe52d590c2 --- .../VtsAidlHalCameraProvider_TargetTest.cpp | 5 + camera/provider/aidl/vts/camera_aidl_test.cpp | 104 ++++++++++-------- camera/provider/aidl/vts/camera_aidl_test.h | 38 +++---- 3 files changed, 82 insertions(+), 65 deletions(-) diff --git a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp index aee53664b0..2845180727 100644 --- a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp +++ b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp @@ -551,6 +551,11 @@ TEST_P(CameraAidlTest, configureStreamsAvailableOutputs) { stream.rotation = StreamRotation::ROTATION_0; stream.dynamicRangeProfile = RequestAvailableDynamicRangeProfilesMap:: ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD; + stream.useCase = ScalerAvailableStreamUseCases:: + ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT; + stream.colorSpace = static_cast( + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED); std::vector streams = {stream}; StreamConfiguration config; diff --git a/camera/provider/aidl/vts/camera_aidl_test.cpp b/camera/provider/aidl/vts/camera_aidl_test.cpp index 7665f797c1..08ad0bbaa0 100644 --- a/camera/provider/aidl/vts/camera_aidl_test.cpp +++ b/camera/provider/aidl/vts/camera_aidl_test.cpp @@ -45,8 +45,6 @@ using ::aidl::android::hardware::camera::common::TorchModeStatus; using ::aidl::android::hardware::camera::device::CameraMetadata; using ::aidl::android::hardware::camera::device::ICameraDevice; using ::aidl::android::hardware::camera::metadata::CameraMetadataTag; -using ::aidl::android::hardware::camera::metadata::RequestAvailableColorSpaceProfilesMap; -using ::aidl::android::hardware::camera::metadata::RequestAvailableDynamicRangeProfilesMap; using ::aidl::android::hardware::camera::metadata::SensorInfoColorFilterArrangement; using ::aidl::android::hardware::camera::metadata::SensorPixelMode; using ::aidl::android::hardware::camera::provider::BnCameraProviderCallback; @@ -2237,21 +2235,26 @@ void CameraAidlTest::configureStreamUseCaseInternal(const AvailableStream &thres } std::vector streams(1); - streams[0] = {0, - StreamType::OUTPUT, - outputPreviewStreams[0].width, - outputPreviewStreams[0].height, - static_cast(outputPreviewStreams[0].format), - static_cast<::aidl::android::hardware::graphics::common::BufferUsage>( - GRALLOC1_CONSUMER_USAGE_CPU_READ), - Dataspace::UNKNOWN, - StreamRotation::ROTATION_0, - std::string(), - 0, - -1, - {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, - RequestAvailableDynamicRangeProfilesMap:: - ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD}; + streams[0] = { + 0, + StreamType::OUTPUT, + outputPreviewStreams[0].width, + outputPreviewStreams[0].height, + static_cast(outputPreviewStreams[0].format), + static_cast<::aidl::android::hardware::graphics::common::BufferUsage>( + GRALLOC1_CONSUMER_USAGE_CPU_READ), + Dataspace::UNKNOWN, + StreamRotation::ROTATION_0, + std::string(), + 0, + -1, + {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, + RequestAvailableDynamicRangeProfilesMap:: + ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD, + ScalerAvailableStreamUseCases::ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT, + static_cast( + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED)}; int32_t streamConfigCounter = 0; CameraMetadata req; @@ -2395,7 +2398,11 @@ void CameraAidlTest::configureSingleStream( /*groupId*/ -1, {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, RequestAvailableDynamicRangeProfilesMap:: - ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD}; + ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD, + ScalerAvailableStreamUseCases::ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT, + static_cast( + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED)}; StreamConfiguration config; config.streams = streams; @@ -2726,21 +2733,26 @@ void CameraAidlTest::configurePreviewStreams( std::vector streams(physicalIds.size()); int32_t streamId = 0; for (auto const& physicalId : physicalIds) { - streams[streamId] = {streamId, - StreamType::OUTPUT, - outputPreviewStreams[0].width, - outputPreviewStreams[0].height, - static_cast(outputPreviewStreams[0].format), - static_cast( - GRALLOC1_CONSUMER_USAGE_HWCOMPOSER), - Dataspace::UNKNOWN, - StreamRotation::ROTATION_0, - physicalId, - 0, - -1, - {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, - RequestAvailableDynamicRangeProfilesMap:: - ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD}; + streams[streamId] = { + streamId, + StreamType::OUTPUT, + outputPreviewStreams[0].width, + outputPreviewStreams[0].height, + static_cast(outputPreviewStreams[0].format), + static_cast( + GRALLOC1_CONSUMER_USAGE_HWCOMPOSER), + Dataspace::UNKNOWN, + StreamRotation::ROTATION_0, + physicalId, + 0, + -1, + {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, + RequestAvailableDynamicRangeProfilesMap:: + ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD, + ScalerAvailableStreamUseCases::ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT, + static_cast( + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED)}; streamId++; } @@ -2799,7 +2811,8 @@ void CameraAidlTest::configureStreams(const std::string& name, bool* supportsPartialResults, int32_t* partialResultCount, bool* useHalBufManager, std::shared_ptr* outCb, uint32_t streamConfigCounter, bool maxResolution, - RequestAvailableDynamicRangeProfilesMap prof) { + RequestAvailableDynamicRangeProfilesMap dynamicRangeProf, + RequestAvailableColorSpaceProfilesMap colorSpaceProf) { ASSERT_NE(nullptr, session); ASSERT_NE(nullptr, halStreams); ASSERT_NE(nullptr, previewStream); @@ -2881,7 +2894,9 @@ void CameraAidlTest::configureStreams(const std::string& name, -1, {maxResolution ? SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_MAXIMUM_RESOLUTION : SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, - prof}; + dynamicRangeProf, + ScalerAvailableStreamUseCases::ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT, + static_cast(colorSpaceProf)}; StreamConfiguration config; config.streams = streams; @@ -3332,7 +3347,11 @@ void CameraAidlTest::configureOfflineStillStream( /*groupId*/ 0, {SensorPixelMode::ANDROID_SENSOR_PIXEL_MODE_DEFAULT}, RequestAvailableDynamicRangeProfilesMap:: - ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD}; + ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD, + ScalerAvailableStreamUseCases::ANDROID_SCALER_AVAILABLE_STREAM_USE_CASES_DEFAULT, + static_cast( + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED)}; StreamConfiguration config = {streams, StreamConfigurationMode::NORMAL_MODE, CameraMetadata()}; @@ -3447,15 +3466,12 @@ void CameraAidlTest::processColorSpaceRequest( Stream previewStream; std::shared_ptr cb; - previewStream.usage = - static_cast( - GRALLOC1_CONSUMER_USAGE_HWCOMPOSER); - previewStream.dataSpace = getDataspace(PixelFormat::IMPLEMENTATION_DEFINED); - previewStream.colorSpace = static_cast(colorSpace); + previewStream.usage = static_cast( + GRALLOC1_CONSUMER_USAGE_HWCOMPOSER); configureStreams(name, mProvider, PixelFormat::IMPLEMENTATION_DEFINED, &mSession, - &previewStream, &halStreams, &supportsPartialResults, - &partialResultCount, &useHalBufManager, &cb, 0, - /*maxResolution*/ false, dynamicRangeProfile); + &previewStream, &halStreams, &supportsPartialResults, &partialResultCount, + &useHalBufManager, &cb, 0, + /*maxResolution*/ false, dynamicRangeProfile, colorSpace); ASSERT_NE(mSession, nullptr); ::aidl::android::hardware::common::fmq::MQDescriptor< diff --git a/camera/provider/aidl/vts/camera_aidl_test.h b/camera/provider/aidl/vts/camera_aidl_test.h index 588cb503b9..6f8f380fa4 100644 --- a/camera/provider/aidl/vts/camera_aidl_test.h +++ b/camera/provider/aidl/vts/camera_aidl_test.h @@ -77,6 +77,9 @@ using ::aidl::android::hardware::camera::device::StreamBuffer; using ::aidl::android::hardware::camera::device::StreamBufferRet; using ::aidl::android::hardware::camera::device::StreamConfiguration; using ::aidl::android::hardware::camera::device::StreamConfigurationMode; +using ::aidl::android::hardware::camera::metadata::RequestAvailableColorSpaceProfilesMap; +using ::aidl::android::hardware::camera::metadata::RequestAvailableDynamicRangeProfilesMap; +using ::aidl::android::hardware::camera::metadata::ScalerAvailableStreamUseCases; using ::aidl::android::hardware::camera::provider::ConcurrentCameraIdCombination; using ::aidl::android::hardware::camera::provider::ICameraProvider; @@ -205,10 +208,12 @@ class CameraAidlTest : public ::testing::TestWithParam { bool* supportsPartialResults /*out*/, int32_t* partialResultCount /*out*/, bool* useHalBufManager /*out*/, std::shared_ptr* outCb /*out*/, uint32_t streamConfigCounter, bool maxResolution, - aidl::android::hardware::camera::metadata::RequestAvailableDynamicRangeProfilesMap - prof = ::aidl::android::hardware::camera::metadata:: - RequestAvailableDynamicRangeProfilesMap:: - ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD); + RequestAvailableDynamicRangeProfilesMap dynamicRangeProf = + RequestAvailableDynamicRangeProfilesMap:: + ANDROID_REQUEST_AVAILABLE_DYNAMIC_RANGE_PROFILES_MAP_STANDARD, + RequestAvailableColorSpaceProfilesMap colorSpaceProf = + RequestAvailableColorSpaceProfilesMap:: + ANDROID_REQUEST_AVAILABLE_COLOR_SPACE_PROFILES_MAP_UNSPECIFIED); void configurePreviewStreams( const std::string& name, const std::shared_ptr& provider, @@ -376,8 +381,7 @@ class CameraAidlTest : public ::testing::TestWithParam { static void get10BitDynamicRangeProfiles( const camera_metadata_t* staticMeta, - std::vector* profiles); + std::vector* profiles); static bool reportsColorSpaces(const camera_metadata_t* staticMeta); @@ -387,17 +391,13 @@ class CameraAidlTest : public ::testing::TestWithParam { RequestAvailableColorSpaceProfilesMap>* profiles); static bool isColorSpaceCompatibleWithDynamicRangeAndPixelFormat( - const camera_metadata_t* staticMeta, - aidl::android::hardware::camera::metadata:: - RequestAvailableColorSpaceProfilesMap colorSpace, - aidl::android::hardware::camera::metadata:: + const camera_metadata_t* staticMeta, RequestAvailableColorSpaceProfilesMap colorSpace, RequestAvailableDynamicRangeProfilesMap dynamicRangeProfile, aidl::android::hardware::graphics::common::PixelFormat pixelFormat); - static const char* getColorSpaceProfileString(aidl::android::hardware::camera::metadata:: - RequestAvailableColorSpaceProfilesMap colorSpace); + static const char* getColorSpaceProfileString(RequestAvailableColorSpaceProfilesMap colorSpace); - static const char* getDynamicRangeProfileString(aidl::android::hardware::camera::metadata:: + static const char* getDynamicRangeProfileString( RequestAvailableDynamicRangeProfilesMap dynamicRangeProfile); static int32_t halFormatToPublicFormat( @@ -408,10 +408,8 @@ class CameraAidlTest : public ::testing::TestWithParam { static Size getMinSize(Size a, Size b); - void processColorSpaceRequest(aidl::android::hardware::camera::metadata:: - RequestAvailableColorSpaceProfilesMap colorSpace, - aidl::android::hardware::camera::metadata:: - RequestAvailableDynamicRangeProfilesMap dynamicRangeProfile); + void processColorSpaceRequest(RequestAvailableColorSpaceProfilesMap colorSpace, + RequestAvailableDynamicRangeProfilesMap dynamicRangeProfile); void processZoomSettingsOverrideRequests( int32_t frameCount, const bool *overrideSequence, const bool *expectedResults); @@ -571,10 +569,8 @@ class CameraAidlTest : public ::testing::TestWithParam { static bool matchDeviceName(const std::string& deviceName, const std::string& providerType, std::string* deviceVersion, std::string* cameraId); - static void verify10BitMetadata( - HandleImporter& importer, const InFlightRequest& request, - aidl::android::hardware::camera::metadata::RequestAvailableDynamicRangeProfilesMap - profile); + static void verify10BitMetadata(HandleImporter& importer, const InFlightRequest& request, + RequestAvailableDynamicRangeProfilesMap profile); static void waitForReleaseFence( std::vector& resultOutputBuffers); From 558dbedf51dda8ed8b70aacda3a8f11138e66833 Mon Sep 17 00:00:00 2001 From: Subrahmanyaman Date: Fri, 28 Apr 2023 23:37:02 +0000 Subject: [PATCH 04/39] Strongbox may not support 1024 bit key size for RSA. Strongbox may not support 1024 bit key size for RSA. So in NoUserConfirmation test updated the key size to 2048 so that the test works for both TEE and Strongbox. Bug: 280117495 Test: run VtsAidlKeyMintTarget (cherry picked from https://android-review.googlesource.com/q/commit:ce2bebdd79cf7536b06c2d67cdee8867475a3b10) Merged-In: I32bb28001aca9b69eedb1bd3d0bcff43052d06e4 Change-Id: I32bb28001aca9b69eedb1bd3d0bcff43052d06e4 --- security/keymint/aidl/vts/functional/KeyMintTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index e99149bf17..bdec4d3fb4 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -3119,7 +3119,7 @@ TEST_P(SigningOperationsTest, RsaPaddingNoneDoesNotAllowOther) { */ TEST_P(SigningOperationsTest, NoUserConfirmation) { ASSERT_EQ(ErrorCode::OK, GenerateKey(AuthorizationSetBuilder() - .RsaSigningKey(1024, 65537) + .RsaSigningKey(2048, 65537) .Digest(Digest::NONE) .Padding(PaddingMode::NONE) .Authorization(TAG_NO_AUTH_REQUIRED) From 997efaa19115ef3b5a655323b35118d1be1736fd Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 9 May 2023 08:10:36 +0100 Subject: [PATCH 05/39] Skip ATTEST_KEY using variant on waivered devices Bug: 281452355 Bug: 289451966 Test: VtsAidlKeyMintTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:c3de1caf4327dc367a95f7416cba19827428bd1b) Merged-In: Id448edae88569518deb2db4ab7bf50d16f33709a Change-Id: Id448edae88569518deb2db4ab7bf50d16f33709a --- .../vts/functional/KeyBlobUpgradeTest.cpp | 25 +++++++++++- .../vts/functional/KeyMintAidlTestBase.cpp | 40 ++++++++++++++----- .../aidl/vts/functional/KeyMintAidlTestBase.h | 1 + 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp index 68924422d1..fb014cfe0b 100644 --- a/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyBlobUpgradeTest.cpp @@ -71,6 +71,9 @@ namespace aidl::android::hardware::security::keymint::test { namespace { +// Names for individual key types to create and use. Note that some the names +// induce specific behaviour, as indicated by the functions below. + std::vector keyblob_names_tee = { "aes-key", "aes-key-rr", "des-key", "hmac-key", "rsa-key", "p256-key", "ed25519-key", "x25519-key", @@ -84,6 +87,11 @@ std::vector keyblob_names_sb = {"aes-key", "aes-key-rr", "hmac-key", "rsa-key", "p256-key", "rsa-attest-key", "p256-attest-key"}; +// Helper functions to detect particular key types based on the name. +bool requires_attest_key(const std::string& name) { + return name.find("-attest-key") != std::string::npos; +} + bool requires_rr(const std::string& name) { return name.find("-rr") != std::string::npos; } @@ -207,6 +215,11 @@ class KeyBlobUpgradeTest : public KeyMintAidlTestBase { } for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } for (bool with_hidden : {false, true}) { std::string app_id; std::string app_data; @@ -355,6 +368,11 @@ TEST_P(KeyBlobUpgradeTest, CreateKeyBlobsBefore) { }}; for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } auto entry = keys_info.find(name); ASSERT_NE(entry, keys_info.end()) << "no builder for " << name; auto builder = entry->second; @@ -432,6 +450,11 @@ TEST_P(KeyBlobUpgradeTest, UseKeyBlobsBeforeOrAfter) { } for (std::string name : keyblob_names()) { + if (requires_attest_key(name) && shouldSkipAttestKeyTest()) { + std::cerr << "Skipping variant '" << name + << "' which requires ATTEST_KEY support that has been waivered\n"; + continue; + } for (bool with_hidden : {false, true}) { auto builder = AuthorizationSetBuilder(); if (with_hidden) { @@ -531,7 +554,7 @@ TEST_P(KeyBlobUpgradeTest, UseKeyBlobsBeforeOrAfter) { // Both ways round should agree. EXPECT_EQ(keymint_data, local_data); - } else if (name.find("-attest-key") != std::string::npos) { + } else if (requires_attest_key(name)) { // Covers rsa-attest-key, p256-attest-key, ed25519-attest-key. // Use attestation key to sign RSA signing key diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index a8ea407e44..ec7256e23f 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1617,17 +1617,39 @@ bool KeyMintAidlTestBase::is_chipset_allowed_km4_strongbox(void) const { return false; } -// Skip the test if all the following conditions hold: -// 1. ATTEST_KEY feature is disabled -// 2. STRONGBOX is enabled -// 3. The device is running one of the chipsets that have received a waiver -// allowing it to be launched with Android S (or later) with Keymaster 4.0 +// Indicate whether a test that involves use of the ATTEST_KEY feature should be +// skipped. +// +// In general, every KeyMint implementation should support ATTEST_KEY; +// however, there is a waiver for some specific devices that ship with a +// combination of Keymaster/StrongBox and KeyMint/TEE. On these devices, the +// ATTEST_KEY feature is disabled in the KeyMint/TEE implementation so that +// the device has consistent ATTEST_KEY behavior (ie. UNIMPLEMENTED) across both +// HAL implementations. +// +// This means that a test involving ATTEST_KEY test should be skipped if all of +// the following conditions hold: +// 1. The device is running one of the chipsets that have received a waiver +// allowing it to be launched with Android S or T with Keymaster 4.0 // in StrongBox -void KeyMintAidlTestBase::skipAttestKeyTest(void) const { +// 2. The device has a STRONGBOX implementation present. +// 3. ATTEST_KEY feature is advertised as disabled. +// +// Note that in this scenario, ATTEST_KEY tests should be skipped for both +// the StrongBox implementation (which is Keymaster, therefore not tested here) +// and for the TEE implementation (which is adjusted to return UNIMPLEMENTED +// specifically for this waiver). +bool KeyMintAidlTestBase::shouldSkipAttestKeyTest(void) const { // Check the chipset first as that doesn't require a round-trip to Package Manager. - if (is_chipset_allowed_km4_strongbox() && is_strongbox_enabled() && - is_attest_key_feature_disabled()) { - GTEST_SKIP() << "Test is not applicable"; + return (is_chipset_allowed_km4_strongbox() && is_strongbox_enabled() && + is_attest_key_feature_disabled()); +} + +// Skip a test that involves use of the ATTEST_KEY feature in specific configurations +// where ATTEST_KEY is not supported (for either StrongBox or TEE). +void KeyMintAidlTestBase::skipAttestKeyTest(void) const { + if (shouldSkipAttestKeyTest()) { + GTEST_SKIP() << "Test using ATTEST_KEY is not applicable on waivered device"; } } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 30ac452bab..1763e943a7 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -359,6 +359,7 @@ class KeyMintAidlTestBase : public ::testing::TestWithParam { bool is_attest_key_feature_disabled(void) const; bool is_strongbox_enabled(void) const; bool is_chipset_allowed_km4_strongbox(void) const; + bool shouldSkipAttestKeyTest(void) const; void skipAttestKeyTest(void) const; protected: From 94042a987c692b97c500abd7eaa9d932362367a1 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 15 Jun 2023 09:41:05 +0100 Subject: [PATCH 06/39] Allow extra error code in device ID attestation Generalize the existing helper function to allow more variants. Remove a couple of pointless invocations of the existing helper. Bug: 286733800 Test: VtsAidlKeyMintTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:f42238c99ffe0df2e51cec84a96ed859a878b2b0) Merged-In: Ic01c53cbe79f55c2d403a66acbfd04029395c287 Change-Id: Ic01c53cbe79f55c2d403a66acbfd04029395c287 --- .../aidl/vts/functional/AttestKeyTest.cpp | 9 +------ .../DeviceUniqueAttestationTest.cpp | 4 ++-- .../vts/functional/KeyMintAidlTestBase.cpp | 24 ++++++++++++++++--- .../aidl/vts/functional/KeyMintAidlTestBase.h | 2 +- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp index a868c966e6..6d289ecda8 100644 --- a/security/keymint/aidl/vts/functional/AttestKeyTest.cpp +++ b/security/keymint/aidl/vts/functional/AttestKeyTest.cpp @@ -961,10 +961,7 @@ TEST_P(AttestKeyTest, EcdsaAttestationMismatchID) { vector attested_key_cert_chain; auto result = GenerateKey(builder, attest_key, &attested_key_blob, &attested_key_characteristics, &attested_key_cert_chain); - - ASSERT_TRUE(result == ErrorCode::CANNOT_ATTEST_IDS || result == ErrorCode::INVALID_TAG) - << "result = " << result; - device_id_attestation_vsr_check(result); + device_id_attestation_check_acceptable_error(invalid_tag.tag, result); } CheckedDeleteKey(&attest_key.keyBlob); } @@ -1026,8 +1023,6 @@ TEST_P(AttestKeyTest, SecondIMEIAttestationIDSuccess) { ASSERT_EQ(result, ErrorCode::OK); - device_id_attestation_vsr_check(result); - CheckedDeleteKey(&attested_key_blob); AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); @@ -1107,8 +1102,6 @@ TEST_P(AttestKeyTest, MultipleIMEIAttestationIDSuccess) { ASSERT_EQ(result, ErrorCode::OK); - device_id_attestation_vsr_check(result); - CheckedDeleteKey(&attested_key_blob); AuthorizationSet hw_enforced = HwEnforcedAuthorizations(attested_key_characteristics); diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index 55bb5b4fab..8e9adedf5c 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -374,8 +374,8 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaDeviceUniqueAttestationMismatchID) { // Add the tag that doesn't match the local device's real ID. builder.push_back(invalid_tag); auto result = GenerateKey(builder, &key_blob, &key_characteristics); - ASSERT_TRUE(result == ErrorCode::CANNOT_ATTEST_IDS || result == ErrorCode::INVALID_TAG); - device_id_attestation_vsr_check(result); + + device_id_attestation_check_acceptable_error(invalid_tag.tag, result); } } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index a8ea407e44..a8b1d06f47 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -2153,14 +2153,32 @@ void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey) *signingKey = std::move(pubKey); } -void device_id_attestation_vsr_check(const ErrorCode& result) { - if (get_vsr_api_level() > __ANDROID_API_T__) { - ASSERT_FALSE(result == ErrorCode::INVALID_TAG) +// Check the error code from an attempt to perform device ID attestation with an invalid value. +void device_id_attestation_check_acceptable_error(Tag tag, const ErrorCode& result) { + // Standard/default error code for ID mismatch. + if (result == ErrorCode::CANNOT_ATTEST_IDS) { + return; + } + + // Depending on the situation, other error codes may be acceptable. First, allow older + // implementations to use INVALID_TAG. + if (result == ErrorCode::INVALID_TAG) { + ASSERT_FALSE(get_vsr_api_level() > __ANDROID_API_T__) << "It is a specification violation for INVALID_TAG to be returned due to ID " << "mismatch in a Device ID Attestation call. INVALID_TAG is only intended to " << "be used for a case where updateAad() is called after update(). As of " << "VSR-14, this is now enforced as an error."; } + + // If the device is not a phone, it will not have IMEI/MEID values available. Allow + // ATTESTATION_IDS_NOT_PROVISIONED in this case. + if (result == ErrorCode::ATTESTATION_IDS_NOT_PROVISIONED) { + ASSERT_TRUE((tag == TAG_ATTESTATION_ID_IMEI || tag == TAG_ATTESTATION_ID_MEID || + tag == TAG_ATTESTATION_ID_SECOND_IMEI)) + << "incorrect error code on attestation ID mismatch"; + } + ADD_FAILURE() << "Error code " << result + << " returned on attestation ID mismatch, should be CANNOT_ATTEST_IDS"; } // Check whether the given named feature is available. diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index 30ac452bab..6318514d6b 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -421,7 +421,7 @@ vector make_name_from_str(const string& name); void check_maced_pubkey(const MacedPublicKey& macedPubKey, bool testMode, vector* payload_value); void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey); -void device_id_attestation_vsr_check(const ErrorCode& result); +void device_id_attestation_check_acceptable_error(Tag tag, const ErrorCode& result); bool check_feature(const std::string& name); AuthorizationSet HwEnforcedAuthorizations(const vector& key_characteristics); From 35621098def02699788da6775ba56066d650a3f2 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Tue, 4 Jul 2023 13:08:30 +0100 Subject: [PATCH 07/39] Fix attestation error checks Avoid the ADD_FAILURE at the end if attestion ID failure uses an allowed return code. Test: VtsAidlKeyMintTargetTest Bug: 286733800 (cherry picked from https://android-review.googlesource.com/q/commit:810fbcffed8e86a3b53e8212ce4fdb64971d812f) Change-Id: I0dcac312ac4516a078b2742721e3a19074da52b1 Merged-In: I0dcac312ac4516a078b2742721e3a19074da52b1 --- .../vts/functional/KeyMintAidlTestBase.cpp | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index a8b1d06f47..284af941b1 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -2155,30 +2155,26 @@ void p256_pub_key(const vector& coseKeyData, EVP_PKEY_Ptr* signingKey) // Check the error code from an attempt to perform device ID attestation with an invalid value. void device_id_attestation_check_acceptable_error(Tag tag, const ErrorCode& result) { - // Standard/default error code for ID mismatch. if (result == ErrorCode::CANNOT_ATTEST_IDS) { - return; - } - - // Depending on the situation, other error codes may be acceptable. First, allow older - // implementations to use INVALID_TAG. - if (result == ErrorCode::INVALID_TAG) { + // Standard/default error code for ID mismatch. + } else if (result == ErrorCode::INVALID_TAG) { + // Depending on the situation, other error codes may be acceptable. First, allow older + // implementations to use INVALID_TAG. ASSERT_FALSE(get_vsr_api_level() > __ANDROID_API_T__) << "It is a specification violation for INVALID_TAG to be returned due to ID " << "mismatch in a Device ID Attestation call. INVALID_TAG is only intended to " << "be used for a case where updateAad() is called after update(). As of " << "VSR-14, this is now enforced as an error."; - } - - // If the device is not a phone, it will not have IMEI/MEID values available. Allow - // ATTESTATION_IDS_NOT_PROVISIONED in this case. - if (result == ErrorCode::ATTESTATION_IDS_NOT_PROVISIONED) { + } else if (result == ErrorCode::ATTESTATION_IDS_NOT_PROVISIONED) { + // If the device is not a phone, it will not have IMEI/MEID values available. Allow + // ATTESTATION_IDS_NOT_PROVISIONED in this case. ASSERT_TRUE((tag == TAG_ATTESTATION_ID_IMEI || tag == TAG_ATTESTATION_ID_MEID || tag == TAG_ATTESTATION_ID_SECOND_IMEI)) << "incorrect error code on attestation ID mismatch"; + } else { + ADD_FAILURE() << "Error code " << result + << " returned on attestation ID mismatch, should be CANNOT_ATTEST_IDS"; } - ADD_FAILURE() << "Error code " << result - << " returned on attestation ID mismatch, should be CANNOT_ATTEST_IDS"; } // Check whether the given named feature is available. From 3f7bfd2dc5a079548a98f217ef7b76396531b96e Mon Sep 17 00:00:00 2001 From: Gabriel Biren Date: Tue, 11 Jul 2023 19:59:22 +0000 Subject: [PATCH 08/39] Represent NAN callback events as a bitmap to better handle overlapping events. Reimplementation of aosp/2473685 for the AIDL VTS tests. Bug: 290318208 Test: atest VtsHalWifiNanIfaceTargetTest Change-Id: I1a0e6ab8dd529c10fcec8f26424a7181e7d91568 --- .../functional/wifi_nan_iface_aidl_test.cpp | 171 +++++++----------- 1 file changed, 65 insertions(+), 106 deletions(-) diff --git a/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp b/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp index bebad7ca08..95bcec7029 100644 --- a/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp +++ b/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp @@ -76,18 +76,10 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { void TearDown() override { stopWifiService(getInstanceName()); } - // Used as a mechanism to inform the test about data/event callbacks. - inline void notify() { - std::unique_lock lock(mtx_); - count_++; - cv_.notify_one(); - } - enum CallbackType { - INVALID = -2, - ANY_CALLBACK = -1, + INVALID = 0, - NOTIFY_CAPABILITIES_RESPONSE = 0, + NOTIFY_CAPABILITIES_RESPONSE = 1, NOTIFY_ENABLE_RESPONSE, NOTIFY_CONFIG_RESPONSE, NOTIFY_DISABLE_RESPONSE, @@ -128,310 +120,278 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { EVENT_SUSPENSION_MODE_CHANGE, }; + // Used as a mechanism to inform the test about data/event callbacks. + inline void notify(CallbackType callbackType) { + std::unique_lock lock(mtx_); + callback_event_bitmap_ |= (0x1 << callbackType); + cv_.notify_one(); + } + // Test code calls this function to wait for data/event callback. - // Must set callbackType = INVALID before calling this function. + // Must set callback_event_bitmap_ to 0 before calling this function. inline std::cv_status wait(CallbackType waitForCallbackType) { std::unique_lock lock(mtx_); EXPECT_NE(INVALID, waitForCallbackType); std::cv_status status = std::cv_status::no_timeout; auto now = std::chrono::system_clock::now(); - while (count_ == 0) { + while (!(receivedCallback(waitForCallbackType))) { status = cv_.wait_until(lock, now + std::chrono::seconds(TIMEOUT_PERIOD)); if (status == std::cv_status::timeout) return status; - if (waitForCallbackType != ANY_CALLBACK && callback_type_ != INVALID && - callback_type_ != waitForCallbackType) { - count_--; - } } - count_--; return status; } + inline bool receivedCallback(CallbackType waitForCallbackType) { + return callback_event_bitmap_ & (0x1 << waitForCallbackType); + } + class WifiNanIfaceEventCallback : public BnWifiNanIfaceEventCallback { public: WifiNanIfaceEventCallback(WifiNanIfaceAidlTest& parent) : parent_(parent){}; ::ndk::ScopedAStatus eventClusterEvent(const NanClusterEventInd& event) override { - parent_.callback_type_ = EVENT_CLUSTER_EVENT; parent_.nan_cluster_event_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_CLUSTER_EVENT); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventDataPathConfirm(const NanDataPathConfirmInd& event) override { - parent_.callback_type_ = EVENT_DATA_PATH_CONFIRM; parent_.nan_data_path_confirm_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_DATA_PATH_CONFIRM); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventDataPathRequest(const NanDataPathRequestInd& event) override { - parent_.callback_type_ = EVENT_DATA_PATH_REQUEST; parent_.nan_data_path_request_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_DATA_PATH_REQUEST); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventDataPathScheduleUpdate( const NanDataPathScheduleUpdateInd& event) override { - parent_.callback_type_ = EVENT_DATA_PATH_SCHEDULE_UPDATE; parent_.nan_data_path_schedule_update_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_DATA_PATH_SCHEDULE_UPDATE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventDataPathTerminated(int32_t ndpInstanceId) override { - parent_.callback_type_ = EVENT_DATA_PATH_TERMINATED; parent_.ndp_instance_id_ = ndpInstanceId; - parent_.notify(); + parent_.notify(EVENT_DATA_PATH_TERMINATED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventDisabled(const NanStatus& status) override { - parent_.callback_type_ = EVENT_DISABLED; parent_.status_ = status; - parent_.notify(); + parent_.notify(EVENT_DISABLED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventFollowupReceived(const NanFollowupReceivedInd& event) override { - parent_.callback_type_ = EVENT_FOLLOWUP_RECEIVED; parent_.nan_followup_received_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_FOLLOWUP_RECEIVED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventMatch(const NanMatchInd& event) override { - parent_.callback_type_ = EVENT_MATCH; parent_.nan_match_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_MATCH); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventMatchExpired(int8_t discoverySessionId, int32_t peerId) override { - parent_.callback_type_ = EVENT_MATCH_EXPIRED; parent_.session_id_ = discoverySessionId; parent_.peer_id_ = peerId; - parent_.notify(); + parent_.notify(EVENT_MATCH_EXPIRED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventPublishTerminated(int8_t sessionId, const NanStatus& status) override { - parent_.callback_type_ = EVENT_PUBLISH_TERMINATED; parent_.session_id_ = sessionId; parent_.status_ = status; - parent_.notify(); + parent_.notify(EVENT_PUBLISH_TERMINATED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventSubscribeTerminated(int8_t sessionId, const NanStatus& status) override { - parent_.callback_type_ = EVENT_SUBSCRIBE_TERMINATED; parent_.session_id_ = sessionId; parent_.status_ = status; - parent_.notify(); + parent_.notify(EVENT_SUBSCRIBE_TERMINATED); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventTransmitFollowup(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = EVENT_TRANSMIT_FOLLOWUP; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(EVENT_TRANSMIT_FOLLOWUP); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventPairingConfirm(const NanPairingConfirmInd& event) override { - parent_.callback_type_ = EVENT_PAIRING_CONFIRM; parent_.nan_pairing_confirm_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_PAIRING_CONFIRM); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventPairingRequest(const NanPairingRequestInd& event) override { - parent_.callback_type_ = EVENT_PAIRING_REQUEST; parent_.nan_pairing_request_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_PAIRING_REQUEST); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventBootstrappingConfirm( const NanBootstrappingConfirmInd& event) override { - parent_.callback_type_ = EVENT_BOOTSTRAPPING_CONFIRM; parent_.nan_bootstrapping_confirm_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_BOOTSTRAPPING_CONFIRM); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventBootstrappingRequest( const NanBootstrappingRequestInd& event) override { - parent_.callback_type_ = EVENT_BOOTSTRAPPING_REQUEST; parent_.nan_bootstrapping_request_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_BOOTSTRAPPING_REQUEST); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus eventSuspensionModeChanged( const NanSuspensionModeChangeInd& event) override { - parent_.callback_type_ = EVENT_SUSPENSION_MODE_CHANGE; parent_.nan_suspension_mode_change_ind_ = event; - parent_.notify(); + parent_.notify(EVENT_SUSPENSION_MODE_CHANGE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyCapabilitiesResponse( char16_t id, const NanStatus& status, const NanCapabilities& capabilities) override { - parent_.callback_type_ = NOTIFY_CAPABILITIES_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.capabilities_ = capabilities; - parent_.notify(); + parent_.notify(NOTIFY_CAPABILITIES_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyConfigResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_CONFIG_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_CONFIG_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyCreateDataInterfaceResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_CREATE_DATA_INTERFACE_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_CREATE_DATA_INTERFACE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyDeleteDataInterfaceResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_DELETE_DATA_INTERFACE_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_DELETE_DATA_INTERFACE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyDisableResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_DISABLE_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_DISABLE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyEnableResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_ENABLE_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_ENABLE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyInitiateDataPathResponse(char16_t id, const NanStatus& status, int32_t ndpInstanceId) override { - parent_.callback_type_ = NOTIFY_INITIATE_DATA_PATH_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.ndp_instance_id_ = ndpInstanceId; - parent_.notify(); + parent_.notify(NOTIFY_INITIATE_DATA_PATH_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyRespondToDataPathIndicationResponse( char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_RESPOND_TO_DATA_PATH_INDICATION_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_RESPOND_TO_DATA_PATH_INDICATION_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyStartPublishResponse(char16_t id, const NanStatus& status, int8_t sessionId) override { - parent_.callback_type_ = NOTIFY_START_PUBLISH_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.session_id_ = sessionId; - parent_.notify(); + parent_.notify(NOTIFY_START_PUBLISH_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyStartSubscribeResponse(char16_t id, const NanStatus& status, int8_t sessionId) override { - parent_.callback_type_ = NOTIFY_START_SUBSCRIBE_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.session_id_ = sessionId; - parent_.notify(); + parent_.notify(NOTIFY_START_SUBSCRIBE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyStopPublishResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_STOP_PUBLISH_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_STOP_PUBLISH_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyStopSubscribeResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_STOP_SUBSCRIBE_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_STOP_SUBSCRIBE_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyTerminateDataPathResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_TERMINATE_DATA_PATH_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_TERMINATE_DATA_PATH_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifySuspendResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_SUSPEND_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_SUSPEND_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyResumeResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_RESUME_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_RESUME_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyTransmitFollowupResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_TRANSMIT_FOLLOWUP_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_TRANSMIT_FOLLOWUP_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyInitiatePairingResponse(char16_t id, const NanStatus& status, int32_t pairingInstanceId) override { - parent_.callback_type_ = NOTIFY_INITIATE_PAIRING_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.pairing_instance_id_ = pairingInstanceId; - parent_.notify(); + parent_.notify(NOTIFY_INITIATE_PAIRING_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyRespondToPairingIndicationResponse( char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_RESPOND_TO_PAIRING_INDICATION_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_RESPOND_TO_PAIRING_INDICATION_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyInitiateBootstrappingResponse( char16_t id, const NanStatus& status, int32_t bootstrapppingInstanceId) override { - parent_.callback_type_ = NOTIFY_INITIATE_BOOTSTRAPPING_RESPONSE; parent_.id_ = id; parent_.status_ = status; parent_.bootstrappping_instance_id_ = bootstrapppingInstanceId; - parent_.notify(); + parent_.notify(NOTIFY_INITIATE_BOOTSTRAPPING_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyRespondToBootstrappingIndicationResponse( char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_RESPOND_TO_BOOTSTRAPPING_INDICATION_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_RESPOND_TO_BOOTSTRAPPING_INDICATION_RESPONSE); return ndk::ScopedAStatus::ok(); } ::ndk::ScopedAStatus notifyTerminatePairingResponse(char16_t id, const NanStatus& status) override { - parent_.callback_type_ = NOTIFY_TERMINATE_PAIRING_RESPONSE; parent_.id_ = id; parent_.status_ = status; - parent_.notify(); + parent_.notify(NOTIFY_TERMINATE_PAIRING_RESPONSE); return ndk::ScopedAStatus::ok(); } @@ -441,7 +401,7 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { protected: std::shared_ptr wifi_nan_iface_; - CallbackType callback_type_; + uint64_t callback_event_bitmap_; uint16_t id_; uint8_t session_id_; uint32_t ndp_instance_id_; @@ -468,7 +428,6 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { // synchronization objects std::mutex mtx_; std::condition_variable cv_; - int count_ = 0; }; /* @@ -488,7 +447,7 @@ TEST_P(WifiNanIfaceAidlTest, FailOnIfaceInvalid) { */ TEST_P(WifiNanIfaceAidlTest, EnableRequest_InvalidArgs) { uint16_t inputCmdId = 10; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; NanEnableRequest nanEnableRequest = {}; NanConfigRequestSupplemental nanConfigRequestSupp = {}; auto status = @@ -498,7 +457,7 @@ TEST_P(WifiNanIfaceAidlTest, EnableRequest_InvalidArgs) { // Wait for a callback. ASSERT_EQ(std::cv_status::no_timeout, wait(NOTIFY_ENABLE_RESPONSE)); - ASSERT_EQ(NOTIFY_ENABLE_RESPONSE, callback_type_); + ASSERT_TRUE(receivedCallback(NOTIFY_ENABLE_RESPONSE)); ASSERT_EQ(id_, inputCmdId); ASSERT_EQ(status_.status, NanStatusCode::INVALID_ARGS); } @@ -509,7 +468,7 @@ TEST_P(WifiNanIfaceAidlTest, EnableRequest_InvalidArgs) { */ TEST_P(WifiNanIfaceAidlTest, ConfigRequest_InvalidArgs) { uint16_t inputCmdId = 10; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; NanConfigRequest nanConfigRequest = {}; NanConfigRequestSupplemental nanConfigRequestSupp = {}; auto status = @@ -520,7 +479,7 @@ TEST_P(WifiNanIfaceAidlTest, ConfigRequest_InvalidArgs) { // Wait for a callback. ASSERT_EQ(std::cv_status::no_timeout, wait(NOTIFY_CONFIG_RESPONSE)); - ASSERT_EQ(NOTIFY_CONFIG_RESPONSE, callback_type_); + ASSERT_TRUE(receivedCallback(NOTIFY_CONFIG_RESPONSE)); ASSERT_EQ(id_, inputCmdId); ASSERT_EQ(status_.status, NanStatusCode::INVALID_ARGS); } @@ -561,12 +520,12 @@ TEST_P(WifiNanIfaceAidlTest, ConfigRequest_InvalidShimArgs) { */ TEST_P(WifiNanIfaceAidlTest, NotifyCapabilitiesResponse) { uint16_t inputCmdId = 10; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; EXPECT_TRUE(wifi_nan_iface_->getCapabilitiesRequest(inputCmdId).isOk()); // Wait for a callback. ASSERT_EQ(std::cv_status::no_timeout, wait(NOTIFY_CAPABILITIES_RESPONSE)); - ASSERT_EQ(NOTIFY_CAPABILITIES_RESPONSE, callback_type_); + ASSERT_TRUE(receivedCallback(NOTIFY_CAPABILITIES_RESPONSE)); ASSERT_EQ(id_, inputCmdId); ASSERT_EQ(status_.status, NanStatusCode::SUCCESS); @@ -654,14 +613,14 @@ TEST_P(WifiNanIfaceAidlTest, StartPublishRequest) { nanConfigRequestSupp.numberOfSpatialStreamsInDiscovery = 0; nanConfigRequestSupp.enableDiscoveryWindowEarlyTermination = false; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; auto status = wifi_nan_iface_->enableRequest(inputCmdId, req, nanConfigRequestSupp); if (!checkStatusCode(&status, WifiStatusCode::ERROR_NOT_SUPPORTED)) { ASSERT_TRUE(status.isOk()); // Wait for a callback. ASSERT_EQ(std::cv_status::no_timeout, wait(NOTIFY_ENABLE_RESPONSE)); - ASSERT_EQ(NOTIFY_ENABLE_RESPONSE, callback_type_); + ASSERT_TRUE(receivedCallback(NOTIFY_ENABLE_RESPONSE)); ASSERT_EQ(id_, inputCmdId); ASSERT_EQ(status_.status, NanStatusCode::SUCCESS); } @@ -688,7 +647,7 @@ TEST_P(WifiNanIfaceAidlTest, StartPublishRequest) { // Wait for a callback. ASSERT_EQ(std::cv_status::no_timeout, wait(NOTIFY_START_PUBLISH_RESPONSE)); - ASSERT_EQ(NOTIFY_START_PUBLISH_RESPONSE, callback_type_); + ASSERT_TRUE(receivedCallback(NOTIFY_START_PUBLISH_RESPONSE)); ASSERT_EQ(id_, inputCmdId + 1); ASSERT_EQ(status_.status, NanStatusCode::SUCCESS); } @@ -699,7 +658,7 @@ TEST_P(WifiNanIfaceAidlTest, StartPublishRequest) { */ TEST_P(WifiNanIfaceAidlTest, RespondToDataPathIndicationRequest_InvalidArgs) { uint16_t inputCmdId = 10; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; NanRespondToDataPathIndicationRequest nanRespondToDataPathIndicationRequest = {}; nanRespondToDataPathIndicationRequest.ifaceName = "AwareInterfaceNameTooLong"; auto status = wifi_nan_iface_->respondToDataPathIndicationRequest( @@ -716,7 +675,7 @@ TEST_P(WifiNanIfaceAidlTest, RespondToDataPathIndicationRequest_InvalidArgs) { */ TEST_P(WifiNanIfaceAidlTest, InitiateDataPathRequest_InvalidArgs) { uint16_t inputCmdId = 10; - callback_type_ = INVALID; + callback_event_bitmap_ = 0; NanInitiateDataPathRequest nanInitiateDataPathRequest = {}; nanInitiateDataPathRequest.ifaceName = "AwareInterfaceNameTooLong"; auto status = wifi_nan_iface_->initiateDataPathRequest(inputCmdId, nanInitiateDataPathRequest); From 5aadf9ad17c356fd1b0a90905684869482261e4a Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Tue, 11 Jul 2023 16:08:27 -0700 Subject: [PATCH 09/39] BluetoothAudio: Statically link HAL dependencies Bug: 290666670 Bug: 289857524 Bug: 284091286 Bug: 289489060 Test: mma -j32 (cherry picked from https://android-review.googlesource.com/q/commit:44628f38d2f60a8e9c283315485d2d62605f677d) Merged-In: I2b3c32b133e208ec2cfb1b244ccb24ad249f22df Change-Id: I2b3c32b133e208ec2cfb1b244ccb24ad249f22df The 32-bit libraries may not be present on the device. This makes the test more self-contained. --- bluetooth/audio/aidl/vts/Android.bp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bluetooth/audio/aidl/vts/Android.bp b/bluetooth/audio/aidl/vts/Android.bp index e03fb5873d..5a604a1960 100644 --- a/bluetooth/audio/aidl/vts/Android.bp +++ b/bluetooth/audio/aidl/vts/Android.bp @@ -15,11 +15,14 @@ cc_test { ], tidy_timeout_srcs: ["VtsHalBluetoothAudioTargetTest.cpp"], srcs: ["VtsHalBluetoothAudioTargetTest.cpp"], - shared_libs: [ + static_libs: [ "android.hardware.audio.common-V1-ndk", "android.hardware.bluetooth.audio-V3-ndk", "android.hardware.common-V2-ndk", "android.hardware.common.fmq-V1-ndk", + "android.media.audio.common.types-V2-ndk", + ], + shared_libs: [ "libbase", "libbinder_ndk", "libcutils", From 4c3eeb417ee28249061be9d00ff9522f59cedddb Mon Sep 17 00:00:00 2001 From: Gabriel Biren Date: Thu, 13 Jul 2023 18:24:14 +0000 Subject: [PATCH 10/39] Use a 64-bit instance of 0x1 when creating the event bitmask. Comments on ag/24025770 suggest that the bit shift will lead to undefined behavior for event codes greater than 31. Bug: 290318208 Test: atest VtsHalWifiNanIfaceTargetTest # manually set some of the expected # events codes to > 31 to verify that # the bit shift works as expected Change-Id: I09e11dac8acf90baf047e24cebe1d01970b1dc8a --- wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp b/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp index 95bcec7029..738e72cf00 100644 --- a/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp +++ b/wifi/aidl/vts/functional/wifi_nan_iface_aidl_test.cpp @@ -123,7 +123,7 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { // Used as a mechanism to inform the test about data/event callbacks. inline void notify(CallbackType callbackType) { std::unique_lock lock(mtx_); - callback_event_bitmap_ |= (0x1 << callbackType); + callback_event_bitmap_ |= (UINT64_C(0x1) << callbackType); cv_.notify_one(); } @@ -143,7 +143,7 @@ class WifiNanIfaceAidlTest : public testing::TestWithParam { } inline bool receivedCallback(CallbackType waitForCallbackType) { - return callback_event_bitmap_ & (0x1 << waitForCallbackType); + return callback_event_bitmap_ & (UINT64_C(0x1) << waitForCallbackType); } class WifiNanIfaceEventCallback : public BnWifiNanIfaceEventCallback { From 3729b0bd6074edccb00976690f22a3c7572d652d Mon Sep 17 00:00:00 2001 From: Weilin Xu Date: Wed, 21 Jun 2023 22:49:04 +0000 Subject: [PATCH 11/39] Fix wrong timeout and mock method in radio VTS Fixed wrong time unit convension for timeout values used in broadcast radio HAL VTS. Also replaced tuner callback mock method which caused segmentation fault when unimplemented callback methods are called. In addition, antenna connection state was checked at the end of the each test instead of at the beginning. Bug: 277531858 Test: atest VtsHalBroadcastradioAidlTargetTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4420c1dbbf28cc80ac47cdbafb9cc81034f8fc8f) Merged-In: Ied70fcfd2742ae5d253aec984e2161afa6f65162 Change-Id: Ied70fcfd2742ae5d253aec984e2161afa6f65162 --- .../VtsHalBroadcastradioAidlTargetTest.cpp | 219 +++++++++++++----- 1 file changed, 156 insertions(+), 63 deletions(-) diff --git a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp index 356673f715..8bee1b2d1f 100644 --- a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp +++ b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp @@ -32,11 +32,11 @@ #include #include #include -#include #include #include #include +#include #include #include @@ -61,11 +61,6 @@ using ::testing::SaveArg; namespace bcutils = ::aidl::android::hardware::broadcastradio::utils; -inline constexpr std::chrono::seconds kTuneTimeoutSec = - std::chrono::seconds(IBroadcastRadio::TUNER_TIMEOUT_MS * 1000); -inline constexpr std::chrono::seconds kProgramListScanTimeoutSec = - std::chrono::seconds(IBroadcastRadio::LIST_COMPLETE_TIMEOUT_MS * 1000); - const ConfigFlag kConfigFlagValues[] = { ConfigFlag::FORCE_MONO, ConfigFlag::FORCE_ANALOG, @@ -108,20 +103,68 @@ bool supportsFM(const AmFmRegionConfig& config) { } // namespace -class TunerCallbackMock : public BnTunerCallback { +class CallbackFlag final { public: - TunerCallbackMock(); + CallbackFlag(int timeoutMs) { mTimeoutMs = timeoutMs; } + /** + * Notify that the callback is called. + */ + void notify() { + std::unique_lock lock(mMutex); + mCalled = true; + lock.unlock(); + mCv.notify_all(); + }; + + /** + * Wait for the timeout passed into the constructor. + */ + bool wait() { + std::unique_lock lock(mMutex); + return mCv.wait_for(lock, std::chrono::milliseconds(mTimeoutMs), + [this] { return mCalled; }); + }; + + /** + * Reset the callback to not called. + */ + void reset() { + std::unique_lock lock(mMutex); + mCalled = false; + } + + private: + std::mutex mMutex; + bool mCalled GUARDED_BY(mMutex) = false; + std::condition_variable mCv; + int mTimeoutMs; +}; + +class TunerCallbackImpl final : public BnTunerCallback { + public: + TunerCallbackImpl(); ScopedAStatus onTuneFailed(Result result, const ProgramSelector& selector) override; - MOCK_TIMEOUT_METHOD1(onCurrentProgramInfoChangedMock, ScopedAStatus(const ProgramInfo&)); ScopedAStatus onCurrentProgramInfoChanged(const ProgramInfo& info) override; ScopedAStatus onProgramListUpdated(const ProgramListChunk& chunk) override; - MOCK_METHOD1(onAntennaStateChange, ScopedAStatus(bool connected)); - MOCK_METHOD1(onParametersUpdated, ScopedAStatus(const vector& parameters)); - MOCK_METHOD2(onConfigFlagUpdated, ScopedAStatus(ConfigFlag in_flag, bool in_value)); - MOCK_TIMEOUT_METHOD0(onProgramListReady, void()); + ScopedAStatus onParametersUpdated(const vector& parameters) override; + ScopedAStatus onAntennaStateChange(bool connected) override; + ScopedAStatus onConfigFlagUpdated(ConfigFlag in_flag, bool in_value) override; + bool waitOnCurrentProgramInfoChangedCallback(); + bool waitProgramReady(); + void reset(); + + bool getAntennaConnectionState(); + ProgramInfo getCurrentProgramInfo(); + bcutils::ProgramInfoSet getProgramList(); + + private: std::mutex mLock; + bool mAntennaConnectionState GUARDED_BY(mLock); + ProgramInfo mCurrentProgramInfo GUARDED_BY(mLock); bcutils::ProgramInfoSet mProgramList GUARDED_BY(mLock); + CallbackFlag mOnCurrentProgramInfoChangedFlag = CallbackFlag(IBroadcastRadio::TUNER_TIMEOUT_MS); + CallbackFlag mOnProgramListReadyFlag = CallbackFlag(IBroadcastRadio::LIST_COMPLETE_TIMEOUT_MS); }; struct AnnouncementListenerMock : public BnAnnouncementListener { @@ -139,7 +182,7 @@ class BroadcastRadioHalTest : public testing::TestWithParam { std::shared_ptr mModule; Properties mProperties; - std::shared_ptr mCallback = SharedRefBase::make(); + std::shared_ptr mCallback; }; MATCHER_P(InfoHasId, id, string(negation ? "does not contain" : "contains") + " " + id.toString()) { @@ -147,20 +190,18 @@ MATCHER_P(InfoHasId, id, string(negation ? "does not contain" : "contains") + " return ids.end() != find(ids.begin(), ids.end(), id.value); } -TunerCallbackMock::TunerCallbackMock() { - EXPECT_TIMEOUT_CALL(*this, onCurrentProgramInfoChangedMock, _).Times(AnyNumber()); - - // we expect the antenna is connected through the whole test - EXPECT_CALL(*this, onAntennaStateChange(false)).Times(0); +TunerCallbackImpl::TunerCallbackImpl() { + mAntennaConnectionState = true; } -ScopedAStatus TunerCallbackMock::onTuneFailed(Result result, const ProgramSelector& selector) { +ScopedAStatus TunerCallbackImpl::onTuneFailed(Result result, const ProgramSelector& selector) { LOG(DEBUG) << "Tune failed for selector" << selector.toString(); EXPECT_TRUE(result == Result::CANCELED); return ndk::ScopedAStatus::ok(); } -ScopedAStatus TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& info) { +ScopedAStatus TunerCallbackImpl::onCurrentProgramInfoChanged(const ProgramInfo& info) { + LOG(DEBUG) << "onCurrentProgramInfoChanged called"; for (const auto& id : info.selector) { EXPECT_NE(id.type, IdentifierType::INVALID); } @@ -196,21 +237,75 @@ ScopedAStatus TunerCallbackMock::onCurrentProgramInfoChanged(const ProgramInfo& } } - return onCurrentProgramInfoChangedMock(info); + { + std::lock_guard lk(mLock); + mCurrentProgramInfo = info; + } + + mOnCurrentProgramInfoChangedFlag.notify(); + return ndk::ScopedAStatus::ok(); } -ScopedAStatus TunerCallbackMock::onProgramListUpdated(const ProgramListChunk& chunk) { - std::lock_guard lk(mLock); - - updateProgramList(chunk, &mProgramList); +ScopedAStatus TunerCallbackImpl::onProgramListUpdated(const ProgramListChunk& chunk) { + LOG(DEBUG) << "onProgramListUpdated called"; + { + std::lock_guard lk(mLock); + updateProgramList(chunk, &mProgramList); + } if (chunk.complete) { - onProgramListReady(); + mOnProgramListReadyFlag.notify(); } return ndk::ScopedAStatus::ok(); } +ScopedAStatus TunerCallbackImpl::onParametersUpdated( + [[maybe_unused]] const vector& parameters) { + return ndk::ScopedAStatus::ok(); +} + +ScopedAStatus TunerCallbackImpl::onAntennaStateChange(bool connected) { + if (!connected) { + std::lock_guard lk(mLock); + mAntennaConnectionState = false; + } + return ndk::ScopedAStatus::ok(); +} + +ScopedAStatus TunerCallbackImpl::onConfigFlagUpdated([[maybe_unused]] ConfigFlag in_flag, + [[maybe_unused]] bool in_value) { + return ndk::ScopedAStatus::ok(); +} + +bool TunerCallbackImpl::waitOnCurrentProgramInfoChangedCallback() { + return mOnCurrentProgramInfoChangedFlag.wait(); +} + +bool TunerCallbackImpl::waitProgramReady() { + return mOnProgramListReadyFlag.wait(); +} + +void TunerCallbackImpl::reset() { + mOnCurrentProgramInfoChangedFlag.reset(); + mOnProgramListReadyFlag.reset(); +} + +bool TunerCallbackImpl::getAntennaConnectionState() { + std::lock_guard lk(mLock); + return mAntennaConnectionState; +} + +ProgramInfo TunerCallbackImpl::getCurrentProgramInfo() { + std::lock_guard lk(mLock); + return mCurrentProgramInfo; +} + +bcutils::ProgramInfoSet TunerCallbackImpl::getProgramList() { + std::lock_guard lk(mLock); + return mProgramList; +} + void BroadcastRadioHalTest::SetUp() { EXPECT_EQ(mModule.get(), nullptr) << "Module is already open"; @@ -228,6 +323,8 @@ void BroadcastRadioHalTest::SetUp() { EXPECT_FALSE(mProperties.product.empty()); EXPECT_GT(mProperties.supportedIdentifierTypes.size(), 0u); + mCallback = SharedRefBase::make(); + // set callback EXPECT_TRUE(mModule->setTunerCallback(mCallback).isOk()); } @@ -236,6 +333,11 @@ void BroadcastRadioHalTest::TearDown() { if (mModule) { ASSERT_TRUE(mModule->unsetTunerCallback().isOk()); } + if (mCallback) { + // we expect the antenna is connected through the whole test + EXPECT_TRUE(mCallback->getAntennaConnectionState()); + mCallback = nullptr; + } } bool BroadcastRadioHalTest::getAmFmRegionConfig(bool full, AmFmRegionConfig* config) { @@ -256,7 +358,7 @@ std::optional BroadcastRadioHalTest::getProgramList() { std::optional BroadcastRadioHalTest::getProgramList( const ProgramFilter& filter) { - EXPECT_TIMEOUT_CALL(*mCallback, onProgramListReady).Times(AnyNumber()); + mCallback->reset(); auto startResult = mModule->startProgramListUpdates(filter); @@ -268,13 +370,13 @@ std::optional BroadcastRadioHalTest::getProgramList( if (!startResult.isOk()) { return std::nullopt; } - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onProgramListReady, kProgramListScanTimeoutSec); + EXPECT_TRUE(mCallback->waitProgramReady()); auto stopResult = mModule->stopProgramListUpdates(); EXPECT_TRUE(stopResult.isOk()); - return mCallback->mProgramList; + return mCallback->getProgramList(); } /** @@ -456,7 +558,7 @@ TEST_P(BroadcastRadioHalTest, TuneFailsWithoutTunerCallback) { * - if it is supported, the test is ignored; */ TEST_P(BroadcastRadioHalTest, TuneFailsWithNotSupported) { - LOG(DEBUG) << "TuneFailsWithInvalid Test"; + LOG(DEBUG) << "TuneFailsWithNotSupported Test"; vector supportTestId = { makeIdentifier(IdentifierType::AMFM_FREQUENCY_KHZ, 0), // invalid @@ -477,9 +579,9 @@ TEST_P(BroadcastRadioHalTest, TuneFailsWithNotSupported) { for (const auto& id : supportTestId) { ProgramSelector sel{id, {}}; - auto result = mModule->tune(sel); - if (!bcutils::isSupported(mProperties, sel)) { + auto result = mModule->tune(sel); + EXPECT_EQ(result.getServiceSpecificError(), notSupportedError); } } @@ -508,9 +610,9 @@ TEST_P(BroadcastRadioHalTest, TuneFailsWithInvalid) { for (const auto& id : invalidId) { ProgramSelector sel{id, {}}; - auto result = mModule->tune(sel); - if (bcutils::isSupported(mProperties, sel)) { + auto result = mModule->tune(sel); + EXPECT_EQ(result.getServiceSpecificError(), invalidArgumentsError); } } @@ -549,13 +651,7 @@ TEST_P(BroadcastRadioHalTest, FmTune) { int64_t freq = 90900; // 90.9 FM ProgramSelector sel = makeSelectorAmfm(freq); // try tuning - ProgramInfo infoCb = {}; - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, - InfoHasId(makeIdentifier(IdentifierType::AMFM_FREQUENCY_KHZ, freq))) - .Times(AnyNumber()) - .WillOnce(DoAll(SaveArg<0>(&infoCb), testing::Return(ByMove(ndk::ScopedAStatus::ok())))) - .WillRepeatedly(testing::InvokeWithoutArgs([] { return ndk::ScopedAStatus::ok(); })); - + mCallback->reset(); auto result = mModule->tune(sel); // expect a failure if it's not supported @@ -566,7 +662,8 @@ TEST_P(BroadcastRadioHalTest, FmTune) { // expect a callback if it succeeds EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); + ProgramInfo infoCb = mCallback->getCurrentProgramInfo(); LOG(DEBUG) << "Current program info: " << infoCb.toString(); @@ -638,12 +735,6 @@ TEST_P(BroadcastRadioHalTest, DabTune) { } // try tuning - ProgramInfo infoCb = {}; - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, - InfoHasId(makeIdentifier(IdentifierType::DAB_FREQUENCY_KHZ, freq))) - .Times(AnyNumber()) - .WillOnce( - DoAll(SaveArg<0>(&infoCb), testing::Return(ByMove(ndk::ScopedAStatus::ok())))); auto result = mModule->tune(sel); @@ -655,7 +746,9 @@ TEST_P(BroadcastRadioHalTest, DabTune) { // expect a callback if it succeeds EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); + ProgramInfo infoCb = mCallback->getCurrentProgramInfo(); + LOG(DEBUG) << "Current program info: " << infoCb.toString(); // it should tune exactly to what was requested @@ -669,13 +762,13 @@ TEST_P(BroadcastRadioHalTest, DabTune) { * * Verifies that: * - the method succeeds; - * - the program info is changed within kTuneTimeoutSec; + * - the program info is changed within kTuneTimeoutMs; * - works both directions and with or without skipping sub-channel. */ TEST_P(BroadcastRadioHalTest, Seek) { LOG(DEBUG) << "Seek Test"; - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, _).Times(AnyNumber()); + mCallback->reset(); auto result = mModule->seek(/* in_directionUp= */ true, /* in_skipSubChannel= */ true); @@ -685,14 +778,14 @@ TEST_P(BroadcastRadioHalTest, Seek) { } EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, _).Times(AnyNumber()); + mCallback->reset(); result = mModule->seek(/* in_directionUp= */ false, /* in_skipSubChannel= */ false); EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); } /** @@ -720,13 +813,13 @@ TEST_P(BroadcastRadioHalTest, SeekFailsWithoutTunerCallback) { * * Verifies that: * - the method succeeds or returns NOT_SUPPORTED; - * - the program info is changed within kTuneTimeoutSec if the method succeeded; + * - the program info is changed within kTuneTimeoutMs if the method succeeded; * - works both directions. */ TEST_P(BroadcastRadioHalTest, Step) { LOG(DEBUG) << "Step Test"; - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, _).Times(AnyNumber()); + mCallback->reset(); auto result = mModule->step(/* in_directionUp= */ true); @@ -735,14 +828,14 @@ TEST_P(BroadcastRadioHalTest, Step) { return; } EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); - EXPECT_TIMEOUT_CALL(*mCallback, onCurrentProgramInfoChangedMock, _).Times(AnyNumber()); + mCallback->reset(); result = mModule->step(/* in_directionUp= */ false); EXPECT_TRUE(result.isOk()); - EXPECT_TIMEOUT_CALL_WAIT(*mCallback, onCurrentProgramInfoChangedMock, kTuneTimeoutSec); + EXPECT_TRUE(mCallback->waitOnCurrentProgramInfoChangedCallback()); } /** @@ -955,7 +1048,7 @@ TEST_P(BroadcastRadioHalTest, SetConfigFlags) { * * Verifies that: * - startProgramListUpdates either succeeds or returns NOT_SUPPORTED; - * - the complete list is fetched within kProgramListScanTimeoutSec; + * - the complete list is fetched within kProgramListScanTimeoutMs; * - stopProgramListUpdates does not crash. */ TEST_P(BroadcastRadioHalTest, GetProgramListFromEmptyFilter) { @@ -969,7 +1062,7 @@ TEST_P(BroadcastRadioHalTest, GetProgramListFromEmptyFilter) { * * Verifies that: * - startProgramListUpdates either succeeds or returns NOT_SUPPORTED; - * - the complete list is fetched within kProgramListScanTimeoutSec; + * - the complete list is fetched within kProgramListScanTimeoutMs; * - stopProgramListUpdates does not crash; * - result for startProgramListUpdates using a filter with AMFM_FREQUENCY_KHZ value of the first * AMFM program matches the expected result. @@ -1017,7 +1110,7 @@ TEST_P(BroadcastRadioHalTest, GetProgramListFromAmFmFilter) { * * Verifies that: * - startProgramListUpdates either succeeds or returns NOT_SUPPORTED; - * - the complete list is fetched within kProgramListScanTimeoutSec; + * - the complete list is fetched within kProgramListScanTimeoutMs; * - stopProgramListUpdates does not crash; * - result for startProgramListUpdates using a filter with DAB_ENSEMBLE value of the first DAB * program matches the expected result. From 360c71ddabcfb4ee978f3dfa48af94a432e6b4d1 Mon Sep 17 00:00:00 2001 From: "xiaoshun.xu" Date: Tue, 11 Jul 2023 17:30:04 +0800 Subject: [PATCH 12/39] Fix vts sco test fail for usb interface [Description] If bt controller device is usb interface, sco loopback single pkt and bandwidth test can not pass because pkt size is limited accroding to core spec [Root Cause] USB sco packet size is limited in spec [Solution] If is bt usb chip, continuous receive sco packets from controller until size is enough [Test Report] Test Pass Bug: 286355871 (cherry picked from https://android-review.googlesource.com/q/commit:4e85c099593a7d337bb277afc178eda3f7e7a818) Merged-In: Idb150d8a72149f7e1dfaccfd7bc82a91710da59c Change-Id: Idb150d8a72149f7e1dfaccfd7bc82a91710da59c --- .../aidl/vts/VtsHalBluetoothTargetTest.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/bluetooth/aidl/vts/VtsHalBluetoothTargetTest.cpp b/bluetooth/aidl/vts/VtsHalBluetoothTargetTest.cpp index e5222a764d..24eb4d0871 100644 --- a/bluetooth/aidl/vts/VtsHalBluetoothTargetTest.cpp +++ b/bluetooth/aidl/vts/VtsHalBluetoothTargetTest.cpp @@ -222,6 +222,8 @@ class BluetoothAidlTest : public ::testing::TestWithParam { int wait_for_completed_packets_event(uint16_t handle); void send_and_wait_for_cmd_complete(std::unique_ptr cmd, std::vector& cmd_complete); + void reassemble_sco_loopback_pkt(std::vector& scoPackets, + size_t size); // A simple test implementation of BluetoothHciCallbacks. class BluetoothHciCallbacks @@ -569,6 +571,11 @@ void BluetoothAidlTest::sendAndCheckSco(int num_packets, size_t size, ASSERT_TRUE( sco_queue.tryPopWithTimeout(sco_loopback, kWaitForScoDataTimeout)); + if (sco_loopback.size() < size) { + // The packets may have been split for USB. Reassemble before checking. + reassemble_sco_loopback_pkt(sco_loopback, size); + } + ASSERT_EQ(sco_packet, sco_loopback); } logger.setTotalBytes(num_packets * size * 2); @@ -703,6 +710,22 @@ void BluetoothAidlTest::send_and_wait_for_cmd_complete( wait_for_command_complete_event(view.GetOpCode(), cmd_complete)); } +// Handle the loopback packet. +void BluetoothAidlTest::reassemble_sco_loopback_pkt(std::vector& scoPackets, + size_t size) { + std::vector sco_packet_whole; + sco_packet_whole.assign(scoPackets.begin(), scoPackets.end()); + while (size + 3 > sco_packet_whole.size()) { + std::vector sco_packets; + ASSERT_TRUE( + sco_queue.tryPopWithTimeout(sco_packets, kWaitForScoDataTimeout)); + sco_packet_whole.insert(sco_packet_whole.end(), sco_packets.begin() + 3, + sco_packets.end()); + } + scoPackets.assign(sco_packet_whole.begin(), sco_packet_whole.end()); + scoPackets[2] = size; +} + // Empty test: Initialize()/Close() are called in SetUp()/TearDown(). TEST_P(BluetoothAidlTest, InitializeAndClose) {} From cf892db1ef4845b9b2e85ab0481981724037509f Mon Sep 17 00:00:00 2001 From: Weilin Xu Date: Wed, 19 Jul 2023 17:20:19 +0000 Subject: [PATCH 13/39] Fix null pointer crash in AIDL radio HAL VTS Bug: 277531858 Test: atest VtsHalBroadcastradioAidlTargetTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:978de0a8a4968774e81e10e716a46bd9e1369479) Merged-In: Icb67c27b9a747411a9bfbd48647e6e6046cf5e8d Change-Id: Icb67c27b9a747411a9bfbd48647e6e6046cf5e8d --- .../aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp index 8bee1b2d1f..790d60b252 100644 --- a/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp +++ b/broadcastradio/aidl/vts/src/VtsHalBroadcastradioAidlTargetTest.cpp @@ -997,13 +997,12 @@ TEST_P(BroadcastRadioHalTest, SetConfigFlags) { LOG(DEBUG) << "SetConfigFlags Test"; auto get = [&](ConfigFlag flag) -> bool { - bool* gotValue = nullptr; + bool gotValue; - auto halResult = mModule->isConfigFlagSet(flag, gotValue); + auto halResult = mModule->isConfigFlagSet(flag, &gotValue); - EXPECT_FALSE(gotValue == nullptr); EXPECT_TRUE(halResult.isOk()); - return *gotValue; + return gotValue; }; auto notSupportedError = resultToInt(Result::NOT_SUPPORTED); From 0ddd5410e3d4dd7323d0b1b4651c8a7b085806c4 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:58 +0000 Subject: [PATCH 14/39] Update OWNERS for Weaver Weaver does not have a clear owner, but list the people who seem to be most involved with it currently. Bug: 291284381 Test: N/A Change-Id: I69de46f8154bf91272a7197ce71c13c745a7208e (cherry picked from commit f4483a2ce98b8d863c84b4148f788ccd85db7086) Merged-In: I69de46f8154bf91272a7197ce71c13c745a7208e --- weaver/1.0/vts/functional/OWNERS | 3 --- weaver/OWNERS | 6 ++++++ weaver/aidl/vts/OWNERS | 2 -- 3 files changed, 6 insertions(+), 5 deletions(-) delete mode 100644 weaver/1.0/vts/functional/OWNERS create mode 100644 weaver/OWNERS delete mode 100644 weaver/aidl/vts/OWNERS diff --git a/weaver/1.0/vts/functional/OWNERS b/weaver/1.0/vts/functional/OWNERS deleted file mode 100644 index ec8c304b67..0000000000 --- a/weaver/1.0/vts/functional/OWNERS +++ /dev/null @@ -1,3 +0,0 @@ -# Bug component: 186411 -chengyouho@google.com -frankwoo@google.com diff --git a/weaver/OWNERS b/weaver/OWNERS new file mode 100644 index 0000000000..7e579f63cd --- /dev/null +++ b/weaver/OWNERS @@ -0,0 +1,6 @@ +# Bug component: 1081729 +ebiggers@google.com +paulcrowley@google.com +swillden@google.com +wfrichar@google.com +chengyouho@google.com diff --git a/weaver/aidl/vts/OWNERS b/weaver/aidl/vts/OWNERS deleted file mode 100644 index 40d95e4bf0..0000000000 --- a/weaver/aidl/vts/OWNERS +++ /dev/null @@ -1,2 +0,0 @@ -chengyouho@google.com -frankwoo@google.com From 1cd59722b67b7b745a74ea30288d4bd6f754491d Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 15/39] Make VtsHalWeaverTargetTest test both HIDL and AIDL services VtsHalWeaverTargetTest and VtsHalWeaverV1_0TargetTest are identical except for whether they use AIDL or HIDL. Unfortunately, the HIDL test is needed for several more years. For now, we have to make some substantial fixes to both tests. To make continued maintenance of this test easier, update VtsHalWeaverTargetTest to handle both AIDL and HIDL. The test cases are still written in terms of the AIDL API, so it should still be straightforward to remove HIDL support when the time comes. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: I6b760930146ad1b08f17ef810a86c0058601c3bf (cherry picked from commit b59654f239745583f53fda7d62bb06f1144a347d) Merged-In: I6b760930146ad1b08f17ef810a86c0058601c3bf --- weaver/aidl/vts/Android.bp | 5 +- weaver/aidl/vts/VtsHalWeaverTargetTest.cpp | 200 ++++++++++++++++++--- 2 files changed, 181 insertions(+), 24 deletions(-) diff --git a/weaver/aidl/vts/Android.bp b/weaver/aidl/vts/Android.bp index 557fe47aa7..ee03b28136 100644 --- a/weaver/aidl/vts/Android.bp +++ b/weaver/aidl/vts/Android.bp @@ -34,7 +34,10 @@ cc_test { "libbinder_ndk", "libbase", ], - static_libs: ["android.hardware.weaver-V2-ndk"], + static_libs: [ + "android.hardware.weaver-V2-ndk", + "android.hardware.weaver@1.0", + ], test_suites: [ "general-tests", "vts", diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp index f016515a6a..b75a56f039 100644 --- a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp @@ -19,6 +19,9 @@ #include #include #include +#include +#include +#include #include @@ -27,29 +30,162 @@ using ::aidl::android::hardware::weaver::WeaverConfig; using ::aidl::android::hardware::weaver::WeaverReadResponse; using ::aidl::android::hardware::weaver::WeaverReadStatus; -using ::ndk::SpAIBinder; +using HidlIWeaver = ::android::hardware::weaver::V1_0::IWeaver; +using HidlWeaverConfig = ::android::hardware::weaver::V1_0::WeaverConfig; +using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus; +using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; +using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; -struct WeaverAidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::fromBinder( - SpAIBinder(AServiceManager_waitForService(GetParam().c_str()))); - ASSERT_NE(weaver, nullptr); +class WeaverAdapter { + public: + virtual ~WeaverAdapter() {} + virtual bool isReady() = 0; + virtual ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) = 0; + virtual ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) = 0; +}; + +class WeaverAidlAdapter : public WeaverAdapter { + public: + WeaverAidlAdapter(const std::string& param) + : aidl_weaver_(IWeaver::fromBinder( + ::ndk::SpAIBinder(AServiceManager_waitForService(param.c_str())))) {} + ~WeaverAidlAdapter() {} + + bool isReady() { return aidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + return aidl_weaver_->getConfig(_aidl_return); } - virtual void TearDown() override {} + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + return aidl_weaver_->read(in_slotId, in_key, _aidl_return); + } - std::shared_ptr weaver; + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + return aidl_weaver_->write(in_slotId, in_key, in_value); + } + + private: + std::shared_ptr aidl_weaver_; }; +class WeaverHidlAdapter : public WeaverAdapter { + public: + WeaverHidlAdapter(const std::string& param) : hidl_weaver_(HidlIWeaver::getService(param)) {} + ~WeaverHidlAdapter() {} + + bool isReady() { return hidl_weaver_ != nullptr; } + + ::ndk::ScopedAStatus getConfig(WeaverConfig* _aidl_return) { + bool callbackCalled = false; + HidlWeaverStatus status; + HidlWeaverConfig config; + auto ret = hidl_weaver_->getConfig([&](HidlWeaverStatus s, HidlWeaverConfig c) { + callbackCalled = true; + status = s; + config = c; + }); + if (!ret.isOk() || !callbackCalled || status != HidlWeaverStatus::OK) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + _aidl_return->slots = config.slots; + _aidl_return->keySize = config.keySize; + _aidl_return->valueSize = config.valueSize; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus read(int32_t in_slotId, const std::vector& in_key, + WeaverReadResponse* _aidl_return) { + bool callbackCalled = false; + HidlWeaverReadStatus status; + std::vector value; + uint32_t timeout; + auto ret = hidl_weaver_->read(in_slotId, in_key, + [&](HidlWeaverReadStatus s, HidlWeaverReadResponse r) { + callbackCalled = true; + status = s; + value = r.value; + timeout = r.timeout; + }); + if (!ret.isOk() || !callbackCalled) { + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + switch (status) { + case HidlWeaverReadStatus::OK: + _aidl_return->status = WeaverReadStatus::OK; + break; + case HidlWeaverReadStatus::FAILED: + _aidl_return->status = WeaverReadStatus::FAILED; + break; + case HidlWeaverReadStatus::INCORRECT_KEY: + _aidl_return->status = WeaverReadStatus::INCORRECT_KEY; + break; + case HidlWeaverReadStatus::THROTTLE: + _aidl_return->status = WeaverReadStatus::THROTTLE; + break; + default: + ADD_FAILURE() << "Unknown HIDL read status: " << static_cast(status); + _aidl_return->status = WeaverReadStatus::FAILED; + break; + } + _aidl_return->value = value; + _aidl_return->timeout = timeout; + return ::ndk::ScopedAStatus::ok(); + } + + ::ndk::ScopedAStatus write(int32_t in_slotId, const std::vector& in_key, + const std::vector& in_value) { + auto status = hidl_weaver_->write(in_slotId, in_key, in_value); + switch (status) { + case HidlWeaverStatus::OK: + return ::ndk::ScopedAStatus::ok(); + case HidlWeaverStatus::FAILED: + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + default: + ADD_FAILURE() << "Unknown HIDL write status: " << status.description(); + return ::ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION); + } + } + + private: + android::sp hidl_weaver_; +}; + +class WeaverTest : public ::testing::TestWithParam> { + protected: + void SetUp() override; + void TearDown() override {} + + std::unique_ptr weaver; +}; + +void WeaverTest::SetUp() { + std::string api, instance_name; + std::tie(api, instance_name) = GetParam(); + if (api == "hidl") { + weaver.reset(new WeaverHidlAdapter(instance_name)); + } else if (api == "aidl") { + weaver.reset(new WeaverAidlAdapter(instance_name)); + } else { + FAIL() << "Bad test parameterization"; + } + ASSERT_TRUE(weaver->isReady()); +} + /* * Checks config values are suitably large */ -TEST_P(WeaverAidlTest, GetConfig) { +TEST_P(WeaverTest, GetConfig) { WeaverConfig config; auto ret = weaver->getConfig(&config); @@ -64,7 +200,7 @@ TEST_P(WeaverAidlTest, GetConfig) { /* * Gets the config twice and checks they are the same */ -TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { +TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { WeaverConfig config1; WeaverConfig config2; @@ -80,7 +216,7 @@ TEST_P(WeaverAidlTest, GettingConfigMultipleTimesGivesSameResult) { /* * Gets the number of slots from the config and writes a key and value to the last one */ -TEST_P(WeaverAidlTest, WriteToLastSlot) { +TEST_P(WeaverTest, WriteToLastSlot) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); @@ -95,7 +231,7 @@ TEST_P(WeaverAidlTest, WriteToLastSlot) { * Writes a key and value to a slot * Reads the slot with the same key and receives the value that was previously written */ -TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { +TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -121,7 +257,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadGivesTheSameValue) { * Overwrites the slot with a new key and value * Reads the slot with the new key and receives the new value */ -TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { +TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -149,7 +285,7 @@ TEST_P(WeaverAidlTest, OverwritingSlotUpdatesTheValue) { * Writes a key and value to a slot * Reads the slot with a different key so does not receive the value */ -TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { +TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; const auto ret = weaver->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -171,7 +307,7 @@ TEST_P(WeaverAidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { /* * Writing to an invalid slot fails */ -TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { +TEST_P(WeaverTest, WritingToInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -188,7 +324,7 @@ TEST_P(WeaverAidlTest, WritingToInvalidSlotFails) { /* * Reading from an invalid slot fails rather than incorrect key */ -TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { +TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -218,7 +354,7 @@ TEST_P(WeaverAidlTest, ReadingFromInvalidSlotFails) { /* * Writing a key that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { +TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -233,7 +369,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeKeyFails) { /* * Writing a value that is too large fails */ -TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { +TEST_P(WeaverTest, WriteWithTooLargeValueFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -248,7 +384,7 @@ TEST_P(WeaverAidlTest, WriteWithTooLargeValueFails) { /* * Reading with a key that is loo large fails */ -TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { +TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { WeaverConfig config; const auto configRet = weaver->getConfig(&config); ASSERT_TRUE(configRet.isOk()); @@ -273,11 +409,29 @@ TEST_P(WeaverAidlTest, ReadWithTooLargeKeyFails) { EXPECT_EQ(status, WeaverReadStatus::FAILED); } -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverAidlTest); +// Instantiate the test for each HIDL Weaver service. INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverAidlTest, - testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor)), - android::PrintInstanceNameToString); + PerHidlInstance, WeaverTest, + testing::Combine(testing::Values("hidl"), + testing::ValuesIn(android::hardware::getAllHalInstanceNames( + HidlIWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + return android::hardware::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); + +// Instantiate the test for each AIDL Weaver service. +INSTANTIATE_TEST_SUITE_P( + PerAidlInstance, WeaverTest, + testing::Combine(testing::Values("aidl"), + testing::ValuesIn(android::getAidlHalInstanceNames(IWeaver::descriptor))), + [](const testing::TestParamInfo>& info) { + // This name_generator makes the instance name be included in the test case names, e.g. + // "PerAidlInstance/WeaverTest#GetConfig/0_android_hardware_weaver_IWeaver_default" + // instead of "PerAidlInstance/WeaverTest#GetConfig/0". + return android::PrintInstanceNameToString( + testing::TestParamInfo{std::get<1>(info.param), info.index}); + }); int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv); From a8fc1ea0545a136c59058fb9a2b7103c86f558f1 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 16/39] Move VtsHalWeaverTargetTest to common directory Since VtsHalWeaverTargetTest now handles both AIDL and HIDL, move it from weaver/aidl/vts/ to weaver/vts/. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Icfa0ff3b22b036110df327674fda44820057aabd (cherry picked from commit f0d6907d20a72391762e3e479e4a5833e4091b3b) Merged-In: Icfa0ff3b22b036110df327674fda44820057aabd --- weaver/{aidl => }/vts/Android.bp | 0 weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename weaver/{aidl => }/vts/Android.bp (100%) rename weaver/{aidl => }/vts/VtsHalWeaverTargetTest.cpp (100%) diff --git a/weaver/aidl/vts/Android.bp b/weaver/vts/Android.bp similarity index 100% rename from weaver/aidl/vts/Android.bp rename to weaver/vts/Android.bp diff --git a/weaver/aidl/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp similarity index 100% rename from weaver/aidl/vts/VtsHalWeaverTargetTest.cpp rename to weaver/vts/VtsHalWeaverTargetTest.cpp From c9691dd624fc6a080c95687936184c31c93b5a56 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:33:59 +0000 Subject: [PATCH 17/39] Remove redundant HIDL Weaver VTS test Now that there is a single Weaver VTS test that covers both the HIDL and AIDL services (weaver/vts/), the HIDL-specific test can be deleted. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ie942825c154e6792e6ffdbf0c59248de9de10d92 (cherry picked from commit e2e40d69a6185494855e691e37cea2e40c7edd2f) Merged-In: Ie942825c154e6792e6ffdbf0c59248de9de10d92 --- weaver/1.0/vts/functional/Android.bp | 32 -- .../functional/VtsHalWeaverV1_0TargetTest.cpp | 343 ------------------ 2 files changed, 375 deletions(-) delete mode 100644 weaver/1.0/vts/functional/Android.bp delete mode 100644 weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp diff --git a/weaver/1.0/vts/functional/Android.bp b/weaver/1.0/vts/functional/Android.bp deleted file mode 100644 index cc1d28465d..0000000000 --- a/weaver/1.0/vts/functional/Android.bp +++ /dev/null @@ -1,32 +0,0 @@ -// -// Copyright (C) 2017 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -package { - // See: http://go/android-license-faq - // A large-scale-change added 'default_applicable_licenses' to import - // all of the 'license_kinds' from "hardware_interfaces_license" - // to get the below license kinds: - // SPDX-license-identifier-Apache-2.0 - default_applicable_licenses: ["hardware_interfaces_license"], -} - -cc_test { - name: "VtsHalWeaverV1_0TargetTest", - defaults: ["VtsHalTargetTestDefaults"], - srcs: ["VtsHalWeaverV1_0TargetTest.cpp"], - static_libs: ["android.hardware.weaver@1.0"], - test_suites: ["general-tests", "vts"], -} diff --git a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp b/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp deleted file mode 100644 index 66465a9798..0000000000 --- a/weaver/1.0/vts/functional/VtsHalWeaverV1_0TargetTest.cpp +++ /dev/null @@ -1,343 +0,0 @@ -/* - * Copyright (C) 2017 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include -#include -#include - -#include - -using ::android::hardware::weaver::V1_0::IWeaver; -using ::android::hardware::weaver::V1_0::WeaverConfig; -using ::android::hardware::weaver::V1_0::WeaverReadStatus; -using ::android::hardware::weaver::V1_0::WeaverReadResponse; -using ::android::hardware::weaver::V1_0::WeaverStatus; -using ::android::hardware::Return; -using ::android::sp; - -const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; -const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; -const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; -const std::vector OTHER_VALUE{0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55, 89, 144, 233, 255, 255}; - -struct WeaverHidlTest : public ::testing::TestWithParam { - virtual void SetUp() override { - weaver = IWeaver::getService(GetParam()); - ASSERT_NE(weaver, nullptr); - } - - virtual void TearDown() override {} - - sp weaver; -}; - -/* - * Checks config values are suitably large - */ -TEST_P(WeaverHidlTest, GetConfig) { - WeaverStatus status; - WeaverConfig config; - - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); -} - -/* - * Gets the config twice and checks they are the same - */ -TEST_P(WeaverHidlTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; - WeaverConfig config2; - - WeaverStatus status; - bool callbackCalled = false; - auto ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config1 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - callbackCalled = false; - ret = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - callbackCalled = true; - status = s; - config2 = c; - }); - ASSERT_TRUE(ret.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverStatus::OK); - - EXPECT_EQ(config1, config2); -} - -/* - * Gets the number of slots from the config and writes a key and value to the last one - */ -TEST_P(WeaverHidlTest, WriteToLastSlot) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::OK); -} - -/* - * Writes a key and value to a slot - * Reads the slot with the same key and receives the value that was previously written - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Overwrites the slot with a new key and value - * Reads the slot with the new key and receives the new value - */ -TEST_P(WeaverHidlTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); - ASSERT_TRUE(initialWriteRet.isOk()); - ASSERT_EQ(initialWriteRet, WeaverStatus::OK); - - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); - ASSERT_TRUE(overwriteRet.isOk()); - ASSERT_EQ(overwriteRet, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - uint32_t timeout; - const auto readRet = weaver->read(slotId, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(readRet.isOk()); - ASSERT_TRUE(callbackCalled); - ASSERT_EQ(status, WeaverReadStatus::OK); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writes a key and value to a slot - * Reads the slot with a different key so does not receive the value - */ -TEST_P(WeaverHidlTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(ret, WeaverStatus::OK); - - bool callbackCalled = false; - WeaverReadStatus status; - std::vector readValue; - const auto readRet = - weaver->read(slotId, WRONG_KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - status = s; - readValue = r.value; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(status, WeaverReadStatus::INCORRECT_KEY); - EXPECT_TRUE(readValue.empty()); -} - -/* - * Writing to an invalid slot fails - */ -TEST_P(WeaverHidlTest, WritingToInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - const auto writeRet = weaver->write(config.slots, KEY, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading from an invalid slot fails rather than incorrect key - */ -TEST_P(WeaverHidlTest, ReadingFromInvalidSlotFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - if (config.slots == std::numeric_limits::max()) { - // If there are no invalid slots then pass - return; - } - - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(config.slots, KEY, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -/* - * Writing a key that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Writing a value that is too large fails - */ -TEST_P(WeaverHidlTest, WriteWithTooLargeValueFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigValue(config.valueSize + 1); - - constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); - ASSERT_TRUE(writeRet.isOk()); - ASSERT_EQ(writeRet, WeaverStatus::FAILED); -} - -/* - * Reading with a key that is loo large fails - */ -TEST_P(WeaverHidlTest, ReadWithTooLargeKeyFails) { - WeaverStatus status; - WeaverConfig config; - const auto configRet = weaver->getConfig([&](WeaverStatus s, WeaverConfig c) { - status = s; - config = c; - }); - ASSERT_TRUE(configRet.isOk()); - ASSERT_EQ(status, WeaverStatus::OK); - - std::vector bigKey(config.keySize + 1); - - constexpr uint32_t slotId = 0; - bool callbackCalled = false; - WeaverReadStatus readStatus; - std::vector readValue; - uint32_t timeout; - const auto readRet = - weaver->read(slotId, bigKey, [&](WeaverReadStatus s, WeaverReadResponse r) { - callbackCalled = true; - readStatus = s; - readValue = r.value; - timeout = r.timeout; - }); - ASSERT_TRUE(callbackCalled); - ASSERT_TRUE(readRet.isOk()); - ASSERT_EQ(readStatus, WeaverReadStatus::FAILED); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); -} - -GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverHidlTest); -INSTANTIATE_TEST_SUITE_P( - PerInstance, WeaverHidlTest, - testing::ValuesIn(android::hardware::getAllHalInstanceNames(IWeaver::descriptor)), - android::hardware::PrintInstanceNameToString); From 42d76ae5dfa9aae45f0f971946ec9e962b21c8b6 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 18/39] Simplify Weaver VTS test - Get the config in SetUp() so that it's easily available to test cases. - Rename "weaver" class member to "weaver_" to match coding style. - Eliminate unnecessary variables when checking WeaverReadResponse. - Fix a typo. Bug: 291284381 Test: 'atest VtsHalWeaverTargetTest' on bramble Change-Id: Ia6dca996103057bfdc9002bc9ab2c039e2333ed9 (cherry picked from commit 961a138e47542b46b574f1f9ba94177e89e06c11) Merged-In: Ia6dca996103057bfdc9002bc9ab2c039e2333ed9 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 171 ++++++++------------------ 1 file changed, 51 insertions(+), 120 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index b75a56f039..047481e123 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -166,64 +166,56 @@ class WeaverTest : public ::testing::TestWithParam weaver; + std::unique_ptr weaver_; + WeaverConfig config_; }; void WeaverTest::SetUp() { std::string api, instance_name; std::tie(api, instance_name) = GetParam(); if (api == "hidl") { - weaver.reset(new WeaverHidlAdapter(instance_name)); + weaver_.reset(new WeaverHidlAdapter(instance_name)); } else if (api == "aidl") { - weaver.reset(new WeaverAidlAdapter(instance_name)); + weaver_.reset(new WeaverAidlAdapter(instance_name)); } else { FAIL() << "Bad test parameterization"; } - ASSERT_TRUE(weaver->isReady()); + ASSERT_TRUE(weaver_->isReady()); + + auto ret = weaver_->getConfig(&config_); + ASSERT_TRUE(ret.isOk()); + ASSERT_GT(config_.slots, 0); + GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize + << ", valueSize=" << config_.valueSize; } /* * Checks config values are suitably large */ TEST_P(WeaverTest, GetConfig) { - WeaverConfig config; - - auto ret = weaver->getConfig(&config); - - ASSERT_TRUE(ret.isOk()); - - EXPECT_GE(config.slots, 16u); - EXPECT_GE(config.keySize, 16u); - EXPECT_GE(config.valueSize, 16u); + EXPECT_GE(config_.slots, 16u); + EXPECT_GE(config_.keySize, 16u); + EXPECT_GE(config_.valueSize, 16u); } /* * Gets the config twice and checks they are the same */ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { - WeaverConfig config1; WeaverConfig config2; - auto ret = weaver->getConfig(&config1); + auto ret = weaver_->getConfig(&config2); ASSERT_TRUE(ret.isOk()); - ret = weaver->getConfig(&config2); - ASSERT_TRUE(ret.isOk()); - - EXPECT_EQ(config1, config2); + EXPECT_EQ(config_, config2); } /* * Gets the number of slots from the config and writes a key and value to the last one */ TEST_P(WeaverTest, WriteToLastSlot) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - - ASSERT_TRUE(configRet.isOk()); - - const uint32_t lastSlot = config.slots - 1; - const auto writeRet = weaver->write(lastSlot, KEY, VALUE); + const uint32_t lastSlot = config_.slots - 1; + const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -233,23 +225,15 @@ TEST_P(WeaverTest, WriteToLastSlot) { */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); + const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -259,26 +243,18 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { constexpr uint32_t slotId = 0; - const auto initialWriteRet = weaver->write(slotId, WRONG_KEY, VALUE); + const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); - const auto overwriteRet = weaver->write(slotId, KEY, OTHER_VALUE); + const auto overwriteRet = weaver_->write(slotId, KEY, OTHER_VALUE); ASSERT_TRUE(overwriteRet.isOk()); WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = weaver->read(slotId, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_EQ(readValue, OTHER_VALUE); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::OK); + EXPECT_EQ(response.value, OTHER_VALUE); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::OK); } /* @@ -287,37 +263,26 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { constexpr uint32_t slotId = 0; - const auto ret = weaver->write(slotId, KEY, VALUE); - ASSERT_TRUE(ret.isOk()); + const auto writeRet = weaver_->write(slotId, KEY, VALUE); + ASSERT_TRUE(writeRet.isOk()); WeaverReadResponse response; - std::vector readValue; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, WRONG_KEY, &response); - - readValue = response.value; - status = response.status; - + const auto readRet = weaver_->read(slotId, WRONG_KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(status, WeaverReadStatus::INCORRECT_KEY); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.status, WeaverReadStatus::INCORRECT_KEY); } /* * Writing to an invalid slot fails */ TEST_P(WeaverTest, WritingToInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } - const auto writeRet = weaver->write(config.slots, KEY, VALUE); + const auto writeRet = weaver_->write(config_.slots, KEY, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -325,44 +290,27 @@ TEST_P(WeaverTest, WritingToInvalidSlotFails) { * Reading from an invalid slot fails rather than incorrect key */ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - if (config.slots == std::numeric_limits::max()) { + if (config_.slots == std::numeric_limits::max()) { // If there are no invalid slots then pass return; } WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(config.slots, KEY, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(config_.slots, KEY, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } /* * Writing a key that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(slotId, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -370,43 +318,26 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { * Writing a value that is too large fails */ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigValue(config.valueSize + 1); + std::vector bigValue(config_.valueSize + 1); constexpr uint32_t slotId = 0; - const auto writeRet = weaver->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(slotId, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } /* - * Reading with a key that is loo large fails + * Reading with a key that is too large fails */ TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { - WeaverConfig config; - const auto configRet = weaver->getConfig(&config); - ASSERT_TRUE(configRet.isOk()); - - std::vector bigKey(config.keySize + 1); + std::vector bigKey(config_.keySize + 1); constexpr uint32_t slotId = 0; WeaverReadResponse response; - std::vector readValue; - uint32_t timeout; - WeaverReadStatus status; - const auto readRet = - weaver->read(slotId, bigKey, &response); - - readValue = response.value; - timeout = response.timeout; - status = response.status; - + const auto readRet = weaver_->read(slotId, bigKey, &response); ASSERT_TRUE(readRet.isOk()); - EXPECT_TRUE(readValue.empty()); - EXPECT_EQ(timeout, 0u); - EXPECT_EQ(status, WeaverReadStatus::FAILED); + EXPECT_TRUE(response.value.empty()); + EXPECT_EQ(response.timeout, 0u); + EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } // Instantiate the test for each HIDL Weaver service. From 86b9048544e01f3af3cd055ae841c007979018d3 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Jul 2023 02:34:00 +0000 Subject: [PATCH 19/39] Don't overwrite in-use Weaver slots during VTS VtsHalWeaverTargetTest always overwrote the first and last Weaver slots. Before Android 14, apparently this didn't cause problems because Android didn't use Weaver for users that never had an LSKF set. However, now users get a Weaver slot right away. That typically means that the first Weaver slot will be used by user 0. Fix the test to read /metadata/password_slots/slot_map to determine which slots are in use by the system, and only write to unused slots. Bug: 291284381 Test: 'atest -v VtsHalWeaverTargetTest'. Checked for INFO messages showing that slots 1 and 63 were used by the test. Then rebooted and verified that the device can still be unlocked. Change-Id: Id2cce4240d68999471e7d1e8fc7a8449587eed97 (cherry picked from commit 31380e7bc9122147e7a5d65cc1739ab78f853b24) Merged-In: Id2cce4240d68999471e7d1e8fc7a8449587eed97 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 77 ++++++++++++++++++++++----- 1 file changed, 65 insertions(+), 12 deletions(-) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index 047481e123..9ee6d93cf0 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -17,6 +17,9 @@ #include #include +#include +#include +#include #include #include #include @@ -36,6 +39,7 @@ using HidlWeaverReadStatus = ::android::hardware::weaver::V1_0::WeaverReadStatus using HidlWeaverReadResponse = ::android::hardware::weaver::V1_0::WeaverReadResponse; using HidlWeaverStatus = ::android::hardware::weaver::V1_0::WeaverStatus; +const std::string kSlotMapFile = "/metadata/password_slots/slot_map"; const std::vector KEY{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}; const std::vector WRONG_KEY{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; const std::vector VALUE{16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; @@ -165,9 +169,12 @@ class WeaverTest : public ::testing::TestWithParam weaver_; WeaverConfig config_; + uint32_t first_free_slot_; + uint32_t last_free_slot_; }; void WeaverTest::SetUp() { @@ -187,6 +194,56 @@ void WeaverTest::SetUp() { ASSERT_GT(config_.slots, 0); GTEST_LOG_(INFO) << "WeaverConfig: slots=" << config_.slots << ", keySize=" << config_.keySize << ", valueSize=" << config_.valueSize; + + FindFreeSlots(); + GTEST_LOG_(INFO) << "First free slot is " << first_free_slot_ << ", last free slot is " + << last_free_slot_; +} + +void WeaverTest::FindFreeSlots() { + // Determine which Weaver slots are in use by the system. These slots can't be used by the test. + std::set used_slots; + if (access(kSlotMapFile.c_str(), F_OK) == 0) { + std::string contents; + ASSERT_TRUE(android::base::ReadFileToString(kSlotMapFile, &contents)) + << "Failed to read " << kSlotMapFile; + for (const auto& line : android::base::Split(contents, "\n")) { + auto trimmed_line = android::base::Trim(line); + if (trimmed_line[0] == '#' || trimmed_line[0] == '\0') continue; + auto slot_and_user = android::base::Split(trimmed_line, "="); + uint32_t slot; + ASSERT_TRUE(slot_and_user.size() == 2 && + android::base::ParseUint(slot_and_user[0], &slot)) + << "Error parsing " << kSlotMapFile << " at \"" << line << "\""; + GTEST_LOG_(INFO) << "Slot " << slot << " is in use by " << slot_and_user[1]; + ASSERT_LT(slot, config_.slots); + used_slots.insert(slot); + } + } + // Starting in Android 14, the system will always use at least one Weaver slot if Weaver is + // supported at all. Make sure we saw at least one. + // TODO: uncomment after Android 14 is merged into AOSP + // ASSERT_FALSE(used_slots.empty()) + //<< "Could not determine which Weaver slots are in use by the system"; + + // Find the first free slot. + int found = 0; + for (uint32_t i = 0; i < config_.slots; i++) { + if (used_slots.find(i) == used_slots.end()) { + first_free_slot_ = i; + found++; + break; + } + } + // Find the last free slot. + for (uint32_t i = config_.slots; i > 0; i--) { + if (used_slots.find(i - 1) == used_slots.end()) { + last_free_slot_ = i - 1; + found++; + break; + } + } + ASSERT_EQ(found, 2) << "All Weaver slots are already in use by the system"; } /* @@ -211,11 +268,10 @@ TEST_P(WeaverTest, GettingConfigMultipleTimesGivesSameResult) { } /* - * Gets the number of slots from the config and writes a key and value to the last one + * Writes a key and value to the last free slot */ TEST_P(WeaverTest, WriteToLastSlot) { - const uint32_t lastSlot = config_.slots - 1; - const auto writeRet = weaver_->write(lastSlot, KEY, VALUE); + const auto writeRet = weaver_->write(last_free_slot_, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); } @@ -224,7 +280,7 @@ TEST_P(WeaverTest, WriteToLastSlot) { * Reads the slot with the same key and receives the value that was previously written */ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto ret = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(ret.isOk()); @@ -242,7 +298,7 @@ TEST_P(WeaverTest, WriteFollowedByReadGivesTheSameValue) { * Reads the slot with the new key and receives the new value */ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto initialWriteRet = weaver_->write(slotId, WRONG_KEY, VALUE); ASSERT_TRUE(initialWriteRet.isOk()); @@ -262,7 +318,7 @@ TEST_P(WeaverTest, OverwritingSlotUpdatesTheValue) { * Reads the slot with a different key so does not receive the value */ TEST_P(WeaverTest, WriteFollowedByReadWithWrongKeyDoesNotGiveTheValue) { - constexpr uint32_t slotId = 0; + const uint32_t slotId = first_free_slot_; const auto writeRet = weaver_->write(slotId, KEY, VALUE); ASSERT_TRUE(writeRet.isOk()); @@ -309,8 +365,7 @@ TEST_P(WeaverTest, ReadingFromInvalidSlotFails) { TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, bigKey, VALUE); + const auto writeRet = weaver_->write(first_free_slot_, bigKey, VALUE); ASSERT_FALSE(writeRet.isOk()); } @@ -320,8 +375,7 @@ TEST_P(WeaverTest, WriteWithTooLargeKeyFails) { TEST_P(WeaverTest, WriteWithTooLargeValueFails) { std::vector bigValue(config_.valueSize + 1); - constexpr uint32_t slotId = 0; - const auto writeRet = weaver_->write(slotId, KEY, bigValue); + const auto writeRet = weaver_->write(first_free_slot_, KEY, bigValue); ASSERT_FALSE(writeRet.isOk()); } @@ -331,9 +385,8 @@ TEST_P(WeaverTest, WriteWithTooLargeValueFails) { TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { std::vector bigKey(config_.keySize + 1); - constexpr uint32_t slotId = 0; WeaverReadResponse response; - const auto readRet = weaver_->read(slotId, bigKey, &response); + const auto readRet = weaver_->read(first_free_slot_, bigKey, &response); ASSERT_TRUE(readRet.isOk()); EXPECT_TRUE(response.value.empty()); EXPECT_EQ(response.timeout, 0u); From 821d5c05c2c4086dc07e23f88ee5b119fbb0d9f7 Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Thu, 20 Jul 2023 19:11:37 +0000 Subject: [PATCH 20/39] Support per-port display configs in VTS Bug: 277855934 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:be1b4d6ccec45f47bbb9459bf9aa31b4c6875c7b) Merged-In: I92e1615d8eb9466b40e02f8e2df8b3432e927af6 Change-Id: I92e1615d8eb9466b40e02f8e2df8b3432e927af6 --- .../VtsHalGraphicsComposer3_ReadbackTest.cpp | 80 +++++++++++++------ 1 file changed, 57 insertions(+), 23 deletions(-) diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp index b0472209dc..269abd150d 100644 --- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp +++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp @@ -129,33 +129,20 @@ class GraphicsCompositionTestBase : public ::testing::Test { return {false, graphicBuffer}; } - uint64_t getStableDisplayId(int64_t display) { - const auto& [status, identification] = - mComposerClient->getDisplayIdentificationData(display); - EXPECT_TRUE(status.isOk()); - - if (const auto info = ::android::parseDisplayIdentificationData( - static_cast(identification.port), identification.data)) { - return info->id.value; - } - - return ::android::PhysicalDisplayId::fromPort(static_cast(identification.port)) - .value; - } - // Gets the per-display XML config std::unique_ptr getDisplayConfigXml(int64_t display) { - std::stringstream pathBuilder; - pathBuilder << "/vendor/etc/displayconfig/display_id_" << getStableDisplayId(display) - << ".xml"; - const std::string path = pathBuilder.str(); - auto document = std::make_unique(); - const tinyxml2::XMLError error = document->LoadFile(path.c_str()); - if (error == tinyxml2::XML_SUCCESS) { + + if (auto document = getDisplayConfigXmlByStableId(getStableDisplayId(display)); + document != nullptr) { return document; - } else { - return nullptr; } + + // Fallback to looking up a per-port config if no config exists for the full ID + if (auto document = getDisplayConfigXmlByPort(getPort(display)); document != nullptr) { + return document; + } + + return nullptr; } // Gets the max display brightness for this display. @@ -256,6 +243,53 @@ class GraphicsCompositionTestBase : public ::testing::Test { } } } + + uint8_t getPort(int64_t display) { + const auto& [status, identification] = + mComposerClient->getDisplayIdentificationData(display); + EXPECT_TRUE(status.isOk()); + return static_cast(identification.port); + } + + uint64_t getStableDisplayId(int64_t display) { + const auto& [status, identification] = + mComposerClient->getDisplayIdentificationData(display); + EXPECT_TRUE(status.isOk()); + + if (const auto info = ::android::parseDisplayIdentificationData( + static_cast(identification.port), identification.data)) { + return info->id.value; + } + + return ::android::PhysicalDisplayId::fromPort(static_cast(identification.port)) + .value; + } + + std::unique_ptr loadXml(const std::string& path) { + auto document = std::make_unique(); + const tinyxml2::XMLError error = document->LoadFile(path.c_str()); + if (error != tinyxml2::XML_SUCCESS) { + ALOGD("%s: Failed to load config file: %s", __func__, path.c_str()); + return nullptr; + } + + ALOGD("%s: Successfully loaded config file: %s", __func__, path.c_str()); + return document; + } + + std::unique_ptr getDisplayConfigXmlByPort(uint8_t port) { + std::stringstream pathBuilder; + pathBuilder << "/vendor/etc/displayconfig/display_port_" << static_cast(port) + << ".xml"; + return loadXml(pathBuilder.str()); + } + + std::unique_ptr getDisplayConfigXmlByStableId(uint64_t stableId) { + std::stringstream pathBuilder; + pathBuilder << "/vendor/etc/displayconfig/display_id_" << stableId + << ".xml"; + return loadXml(pathBuilder.str()); + } }; class GraphicsCompositionTest : public GraphicsCompositionTestBase, From 3d1e0b7555ccbc761536256e71d486a0d748f444 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Mon, 24 Jul 2023 17:21:28 +0000 Subject: [PATCH 21/39] Allow uninstantiated WeaverTest Add back GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST, which I had accidentally removed while merging the AIDL and HIDL tests. I think this is still needed on devices that don't support Weaver at all. Bug: 291284381 Test: atest VtsHalWeaverTargetTest Change-Id: Iac1b4476620e51c645e3ad57444ee386cb879029 (cherry picked from commit 47b145a0d8522c41d0aa57e46dbb120bab0590bb) Merged-In: Iac1b4476620e51c645e3ad57444ee386cb879029 --- weaver/vts/VtsHalWeaverTargetTest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/weaver/vts/VtsHalWeaverTargetTest.cpp b/weaver/vts/VtsHalWeaverTargetTest.cpp index 9ee6d93cf0..754d46710b 100644 --- a/weaver/vts/VtsHalWeaverTargetTest.cpp +++ b/weaver/vts/VtsHalWeaverTargetTest.cpp @@ -393,6 +393,8 @@ TEST_P(WeaverTest, ReadWithTooLargeKeyFails) { EXPECT_EQ(response.status, WeaverReadStatus::FAILED); } +GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(WeaverTest); + // Instantiate the test for each HIDL Weaver service. INSTANTIATE_TEST_SUITE_P( PerHidlInstance, WeaverTest, From f0c772199bde052764a697b6de478851126fa596 Mon Sep 17 00:00:00 2001 From: Sarah Chin Date: Thu, 27 Jul 2023 13:34:00 -0700 Subject: [PATCH 22/39] Set per-test timeout for IRadio 1.2-1.6 VTS to 5m To be consistent with the timeout set in IRadio 1.0/1.1 VTS This increases the timeout per-test, but there is still a timeout for all tests in the module to complete. Test: atest VtsHalRadioV1_*TargetTest for all Bug: 292197532 Change-Id: I421c13e65efa565753337dc21e6520682e726e81 --- radio/1.2/vts/functional/AndroidTest.xml | 1 + radio/1.3/vts/functional/AndroidTest.xml | 1 + radio/1.4/vts/functional/AndroidTest.xml | 1 + radio/1.5/vts/functional/AndroidTest.xml | 36 ++++++++++++++++++++++++ radio/1.6/vts/functional/AndroidTest.xml | 36 ++++++++++++++++++++++++ 5 files changed, 75 insertions(+) create mode 100644 radio/1.5/vts/functional/AndroidTest.xml create mode 100644 radio/1.6/vts/functional/AndroidTest.xml diff --git a/radio/1.2/vts/functional/AndroidTest.xml b/radio/1.2/vts/functional/AndroidTest.xml index e25249bda5..4a0be5662d 100644 --- a/radio/1.2/vts/functional/AndroidTest.xml +++ b/radio/1.2/vts/functional/AndroidTest.xml @@ -30,6 +30,7 @@ diff --git a/radio/1.3/vts/functional/AndroidTest.xml b/radio/1.3/vts/functional/AndroidTest.xml index 44b74191b3..a2cd791427 100644 --- a/radio/1.3/vts/functional/AndroidTest.xml +++ b/radio/1.3/vts/functional/AndroidTest.xml @@ -30,6 +30,7 @@ diff --git a/radio/1.4/vts/functional/AndroidTest.xml b/radio/1.4/vts/functional/AndroidTest.xml index d0843e6f6b..54051ed4f0 100644 --- a/radio/1.4/vts/functional/AndroidTest.xml +++ b/radio/1.4/vts/functional/AndroidTest.xml @@ -30,6 +30,7 @@ diff --git a/radio/1.5/vts/functional/AndroidTest.xml b/radio/1.5/vts/functional/AndroidTest.xml new file mode 100644 index 0000000000..d3617c81a9 --- /dev/null +++ b/radio/1.5/vts/functional/AndroidTest.xml @@ -0,0 +1,36 @@ + + + + diff --git a/radio/1.6/vts/functional/AndroidTest.xml b/radio/1.6/vts/functional/AndroidTest.xml new file mode 100644 index 0000000000..c3ecdb0761 --- /dev/null +++ b/radio/1.6/vts/functional/AndroidTest.xml @@ -0,0 +1,36 @@ + + + + From ba1c37e79bbcae50084808ff5a57f21d732e046b Mon Sep 17 00:00:00 2001 From: Sarah Chin Date: Thu, 3 Aug 2023 02:03:51 -0700 Subject: [PATCH 23/39] VTS setGetAllowedNetworkTypesBitmap allow reset If the test fails, the allowed network type bitmap will not be reset to the previous value. Instead of using ASSERT_EQ, change to EXPECT_EQ to allow the test to continue to the reset. Test: atest VtsHalRadioTargetTest Bug: 264913330 Change-Id: I719afa2e0d9ebc41a329f38f3eca597c9381f2f9 --- radio/aidl/vts/radio_network_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/radio/aidl/vts/radio_network_test.cpp b/radio/aidl/vts/radio_network_test.cpp index 2beb249674..253ef82d9f 100644 --- a/radio/aidl/vts/radio_network_test.cpp +++ b/radio/aidl/vts/radio_network_test.cpp @@ -106,7 +106,7 @@ TEST_P(RadioNetworkTest, setGetAllowedNetworkTypesBitmap) { RadioError::REQUEST_NOT_SUPPORTED, RadioError::NO_RESOURCES})); if (radioRsp_network->rspInfo.error == RadioError::NONE) { // verify we get the value we set - ASSERT_EQ(radioRsp_network->networkTypeBitmapResponse, allowedNetworkTypesBitmap); + EXPECT_EQ(radioRsp_network->networkTypeBitmapResponse, allowedNetworkTypesBitmap); } } From e284266e52e62e39a979da5c82d79dd4b5775155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Gaffie?= Date: Tue, 1 Aug 2023 14:04:51 +0200 Subject: [PATCH 24/39] Audio: add system usage to audio policy engine schemas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 293917986 Test: m Signed-off-by: François Gaffie (cherry picked from https://android-review.googlesource.com/q/commit:acd4a677f01f7e9674e07be27aabbc3d8a5cb84d) Merged-In: I49a8d3cfeb45eddc95ad6ab4a33b0adf0585cde2 Change-Id: I49a8d3cfeb45eddc95ad6ab4a33b0adf0585cde2 --- audio/aidl/default/config/audioPolicy/engine/api/current.txt | 4 ++++ .../audioPolicy/engine/audio_policy_engine_configuration.xsd | 4 ++++ audio/policy/1.0/xml/api/current.txt | 4 ++++ audio/policy/1.0/xml/audio_policy_engine_configuration.xsd | 4 ++++ 4 files changed, 16 insertions(+) diff --git a/audio/aidl/default/config/audioPolicy/engine/api/current.txt b/audio/aidl/default/config/audioPolicy/engine/api/current.txt index 59574f3299..063b05dd61 100644 --- a/audio/aidl/default/config/audioPolicy/engine/api/current.txt +++ b/audio/aidl/default/config/audioPolicy/engine/api/current.txt @@ -209,11 +209,13 @@ package android.audio.policy.engine.configuration { public enum UsageEnumType { method @NonNull public String getRawName(); enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ALARM; + enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ANNOUNCEMENT; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ASSISTANCE_ACCESSIBILITY; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ASSISTANCE_NAVIGATION_GUIDANCE; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ASSISTANCE_SONIFICATION; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_ASSISTANT; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_CALL_ASSISTANT; + enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_EMERGENCY; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_GAME; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_MEDIA; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_NOTIFICATION; @@ -222,7 +224,9 @@ package android.audio.policy.engine.configuration { enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_NOTIFICATION_COMMUNICATION_REQUEST; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_NOTIFICATION_EVENT; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_NOTIFICATION_TELEPHONY_RINGTONE; + enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_SAFETY; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_UNKNOWN; + enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_VEHICLE_STATUS; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_VIRTUAL_SOURCE; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_VOICE_COMMUNICATION; enum_constant public static final android.audio.policy.engine.configuration.UsageEnumType AUDIO_USAGE_VOICE_COMMUNICATION_SIGNALLING; diff --git a/audio/aidl/default/config/audioPolicy/engine/audio_policy_engine_configuration.xsd b/audio/aidl/default/config/audioPolicy/engine/audio_policy_engine_configuration.xsd index b58a6c84fb..40396bb9b9 100644 --- a/audio/aidl/default/config/audioPolicy/engine/audio_policy_engine_configuration.xsd +++ b/audio/aidl/default/config/audioPolicy/engine/audio_policy_engine_configuration.xsd @@ -359,6 +359,10 @@ + + + + diff --git a/audio/policy/1.0/xml/api/current.txt b/audio/policy/1.0/xml/api/current.txt index 84a2b710b8..01d77d7725 100644 --- a/audio/policy/1.0/xml/api/current.txt +++ b/audio/policy/1.0/xml/api/current.txt @@ -209,11 +209,13 @@ package audio.policy.V1_0 { public enum UsageEnumType { method public String getRawName(); enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ALARM; + enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ANNOUNCEMENT; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ASSISTANCE_ACCESSIBILITY; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ASSISTANCE_NAVIGATION_GUIDANCE; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ASSISTANCE_SONIFICATION; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_ASSISTANT; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_CALL_ASSISTANT; + enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_EMERGENCY; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_GAME; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_MEDIA; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_NOTIFICATION; @@ -222,7 +224,9 @@ package audio.policy.V1_0 { enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_NOTIFICATION_COMMUNICATION_REQUEST; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_NOTIFICATION_EVENT; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_NOTIFICATION_TELEPHONY_RINGTONE; + enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_SAFETY; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_UNKNOWN; + enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_VEHICLE_STATUS; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_VIRTUAL_SOURCE; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_VOICE_COMMUNICATION; enum_constant public static final audio.policy.V1_0.UsageEnumType AUDIO_USAGE_VOICE_COMMUNICATION_SIGNALLING; diff --git a/audio/policy/1.0/xml/audio_policy_engine_configuration.xsd b/audio/policy/1.0/xml/audio_policy_engine_configuration.xsd index b58a6c84fb..40396bb9b9 100644 --- a/audio/policy/1.0/xml/audio_policy_engine_configuration.xsd +++ b/audio/policy/1.0/xml/audio_policy_engine_configuration.xsd @@ -359,6 +359,10 @@ + + + + From 3454ce179af9123c6f3c5730209f6045bad054d5 Mon Sep 17 00:00:00 2001 From: Gabriel Biren Date: Wed, 19 Jul 2023 17:20:09 +0000 Subject: [PATCH 25/39] Add hostapd_test_utils helper methods for the Hostapd AIDL VTS tests. Main purpose will be to enable/disable the HALs and Wifi framework before and after each test. Bug: 283402709 Test: atest VtsHalHostapdTargetTest Change-Id: Ib3ceca4ad25f472e1b75fa95661235c880a489fd --- .../functional/hostapd_hidl_test_utils.cpp | 12 +- .../vts/functional/hostapd_hidl_test_utils.h | 4 + wifi/hostapd/aidl/vts/functional/Android.bp | 1 + .../vts/functional/hostapd_aidl_test_utils.h | 80 ++++++++++++ .../functional/hostapd_legacy_test_utils.h | 71 ++++++++++ .../aidl/vts/functional/hostapd_test_utils.h | 122 ++++++++++++++++++ 6 files changed, 283 insertions(+), 7 deletions(-) create mode 100644 wifi/hostapd/aidl/vts/functional/hostapd_aidl_test_utils.h create mode 100644 wifi/hostapd/aidl/vts/functional/hostapd_legacy_test_utils.h create mode 100644 wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h diff --git a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp index 75d6252cd2..56f285d147 100644 --- a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp +++ b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp @@ -41,10 +41,9 @@ using ::android::hardware::wifi::V1_0::IWifiChip; using ::android::wifi_system::HostapdManager; using ::android::wifi_system::SupplicantManager; -namespace { // Helper function to initialize the driver and firmware to AP mode // using the vendor HAL HIDL interface. -void initilializeDriverAndFirmware(const std::string& wifi_instance_name) { +void initializeDriverAndFirmware(const std::string& wifi_instance_name) { if (getWifi(wifi_instance_name) != nullptr) { sp wifi_chip = getWifiChip(wifi_instance_name); ChipModeId mode_id; @@ -57,21 +56,20 @@ void initilializeDriverAndFirmware(const std::string& wifi_instance_name) { // Helper function to deinitialize the driver and firmware // using the vendor HAL HIDL interface. -void deInitilializeDriverAndFirmware(const std::string& wifi_instance_name) { +void deInitializeDriverAndFirmware(const std::string& wifi_instance_name) { if (getWifi(wifi_instance_name) != nullptr) { stopWifi(wifi_instance_name); } else { LOG(WARNING) << __func__ << ": Vendor HAL not supported"; } } -} // namespace void stopSupplicantIfNeeded(const std::string& instance_name) { SupplicantManager supplicant_manager; if (supplicant_manager.IsSupplicantRunning()) { LOG(INFO) << "Supplicant is running, stop supplicant first."; ASSERT_TRUE(supplicant_manager.StopSupplicant()); - deInitilializeDriverAndFirmware(instance_name); + deInitializeDriverAndFirmware(instance_name); ASSERT_FALSE(supplicant_manager.IsSupplicantRunning()); } } @@ -80,13 +78,13 @@ void stopHostapd(const std::string& instance_name) { HostapdManager hostapd_manager; ASSERT_TRUE(hostapd_manager.StopHostapd()); - deInitilializeDriverAndFirmware(instance_name); + deInitializeDriverAndFirmware(instance_name); } void startHostapdAndWaitForHidlService( const std::string& wifi_instance_name, const std::string& hostapd_instance_name) { - initilializeDriverAndFirmware(wifi_instance_name); + initializeDriverAndFirmware(wifi_instance_name); HostapdManager hostapd_manager; ASSERT_TRUE(hostapd_manager.StartHostapd()); diff --git a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h index 5cb4f013b1..893de1e334 100644 --- a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h +++ b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h @@ -33,5 +33,9 @@ void startHostapdAndWaitForHidlService( bool is_1_1(const android::sp& hostapd); +// Used to initialize/deinitialize the driver and firmware at the +// beginning and end of each test. +void initializeDriverAndFirmware(const std::string& wifi_instance_name); +void deInitializeDriverAndFirmware(const std::string& wifi_instance_name); #endif /* HOSTAPD_HIDL_TEST_UTILS_H */ diff --git a/wifi/hostapd/aidl/vts/functional/Android.bp b/wifi/hostapd/aidl/vts/functional/Android.bp index 33318a4520..ff35056076 100644 --- a/wifi/hostapd/aidl/vts/functional/Android.bp +++ b/wifi/hostapd/aidl/vts/functional/Android.bp @@ -37,6 +37,7 @@ cc_test { "android.hardware.wifi@1.5", "android.hardware.wifi@1.6", "android.hardware.wifi-V1-ndk", + "libwifi-system", "libwifi-system-iface", "VtsHalWifiTargetTestUtil", ], diff --git a/wifi/hostapd/aidl/vts/functional/hostapd_aidl_test_utils.h b/wifi/hostapd/aidl/vts/functional/hostapd_aidl_test_utils.h new file mode 100644 index 0000000000..93540b2671 --- /dev/null +++ b/wifi/hostapd/aidl/vts/functional/hostapd_aidl_test_utils.h @@ -0,0 +1,80 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#include "wifi_aidl_test_utils.h" + +namespace { + +const std::string kWifiInstanceNameStr = std::string() + IWifi::descriptor + "/default"; +const char* kWifiInstanceName = kWifiInstanceNameStr.c_str(); + +} // namespace + +namespace HostapdAidlTestUtils { + +bool useAidlService() { + return isAidlServiceAvailable(kWifiInstanceName); +} + +void startAndConfigureVendorHal() { + if (getWifi(kWifiInstanceName) != nullptr) { + std::shared_ptr wifi_chip = getWifiChip(kWifiInstanceName); + int mode_id; + EXPECT_TRUE(configureChipToSupportConcurrencyType(wifi_chip, IfaceConcurrencyType::AP, + &mode_id)); + } else { + LOG(ERROR) << "Unable to initialize Vendor HAL"; + } +} + +void stopVendorHal() { + if (getWifi(kWifiInstanceName) != nullptr) { + stopWifiService(kWifiInstanceName); + } else { + LOG(ERROR) << "Unable to stop Vendor HAL"; + } +} + +std::string setupApIfaceAndGetName(bool isBridged) { + std::shared_ptr wifi_ap_iface; + if (isBridged) { + wifi_ap_iface = getBridgedWifiApIface(kWifiInstanceName); + } else { + wifi_ap_iface = getWifiApIface(kWifiInstanceName); + } + + EXPECT_TRUE(wifi_ap_iface.get() != nullptr); + if (!wifi_ap_iface.get()) { + LOG(ERROR) << "Unable to create iface. isBridged=" << isBridged; + return ""; + } + + std::string ap_iface_name; + auto status = wifi_ap_iface->getName(&ap_iface_name); + EXPECT_TRUE(status.isOk()); + if (!status.isOk()) { + LOG(ERROR) << "Unable to retrieve iface name. isBridged=" << isBridged; + return ""; + } + return ap_iface_name; +} + +} // namespace HostapdAidlTestUtils diff --git a/wifi/hostapd/aidl/vts/functional/hostapd_legacy_test_utils.h b/wifi/hostapd/aidl/vts/functional/hostapd_legacy_test_utils.h new file mode 100644 index 0000000000..fb59dc2858 --- /dev/null +++ b/wifi/hostapd/aidl/vts/functional/hostapd_legacy_test_utils.h @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include "hostapd_hidl_test_utils.h" +#include "wifi_hidl_test_utils.h" + +using ::android::hardware::wifi::V1_0::WifiStatus; + +namespace { + +std::string getWifiInstanceName() { + const std::vector instances = android::hardware::getAllHalInstanceNames( + ::android::hardware::wifi::V1_0::IWifi::descriptor); + EXPECT_NE(0, instances.size()); + return instances.size() != 0 ? instances[0] : ""; +} + +} // namespace + +namespace HostapdLegacyTestUtils { + +void startAndConfigureVendorHal() { + initializeDriverAndFirmware(getWifiInstanceName()); +} + +void stopVendorHal() { + deInitializeDriverAndFirmware(getWifiInstanceName()); +} + +std::string setupApIfaceAndGetName(bool isBridged) { + android::sp<::android::hardware::wifi::V1_0::IWifiApIface> wifi_ap_iface; + if (isBridged) { + wifi_ap_iface = getBridgedWifiApIface_1_6(getWifiInstanceName()); + } else { + wifi_ap_iface = getWifiApIface_1_5(getWifiInstanceName()); + } + + EXPECT_TRUE(wifi_ap_iface.get() != nullptr); + if (!wifi_ap_iface.get()) { + LOG(ERROR) << "Unable to create iface. isBridged=" << isBridged; + return ""; + } + + const auto& status_and_name = HIDL_INVOKE(wifi_ap_iface, getName); + EXPECT_TRUE(status_and_name.first.code == + android::hardware::wifi::V1_0::WifiStatusCode::SUCCESS); + if (status_and_name.first.code != android::hardware::wifi::V1_0::WifiStatusCode::SUCCESS) { + LOG(ERROR) << "Unable to retrieve iface name. isBridged=" << isBridged; + return ""; + } + return status_and_name.second; +} + +} // namespace HostapdLegacyTestUtils diff --git a/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h b/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h new file mode 100644 index 0000000000..feee2c89b2 --- /dev/null +++ b/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h @@ -0,0 +1,122 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include "hostapd_aidl_test_utils.h" +#include "hostapd_legacy_test_utils.h" + +using aidl::android::hardware::wifi::hostapd::IHostapd; +using android::wifi_system::HostapdManager; +using android::wifi_system::SupplicantManager; + +namespace { + +void startAndConfigureVendorHal() { + if (HostapdAidlTestUtils::useAidlService()) { + HostapdAidlTestUtils::startAndConfigureVendorHal(); + } else { + HostapdLegacyTestUtils::startAndConfigureVendorHal(); + } +} + +void stopVendorHal() { + if (HostapdAidlTestUtils::useAidlService()) { + HostapdAidlTestUtils::stopVendorHal(); + } else { + HostapdLegacyTestUtils::stopVendorHal(); + } +} + +void stopHostapd() { + HostapdManager hostapd_manager; + ASSERT_TRUE(hostapd_manager.StopHostapd()); +} + +void waitForSupplicantState(bool enable) { + SupplicantManager supplicant_manager; + int count = 50; // wait at most 5 seconds + while (count-- > 0) { + if (supplicant_manager.IsSupplicantRunning() == enable) { + return; + } + usleep(100000); // 100 ms + } + LOG(ERROR) << "Unable to " << (enable ? "start" : "stop") << " supplicant"; +} + +void toggleWifiFramework(bool enable) { + if (enable) { + std::system("svc wifi enable"); + std::system("cmd wifi set-scan-always-available enabled"); + waitForSupplicantState(true); + } else { + std::system("svc wifi disable"); + std::system("cmd wifi set-scan-always-available disabled"); + waitForSupplicantState(false); + } +} + +} // namespace + +std::shared_ptr getHostapd(const std::string& hostapd_instance_name) { + return IHostapd::fromBinder( + ndk::SpAIBinder(AServiceManager_waitForService(hostapd_instance_name.c_str()))); +} + +/** + * Disable the Wifi framework, hostapd, and vendor HAL. + * + * Note: The framework should be disabled to avoid having + * any other clients to the HALs during testing. + */ +void disableHalsAndFramework() { + toggleWifiFramework(false); + stopHostapd(); + stopVendorHal(); + + // Wait for the services to stop. + sleep(3); +} + +void initializeHostapdAndVendorHal(const std::string& hostapd_instance_name) { + startAndConfigureVendorHal(); + HostapdManager hostapd_manager; + ASSERT_TRUE(hostapd_manager.StartHostapd()); + getHostapd(hostapd_instance_name); +} + +void stopHostapdAndVendorHal() { + stopHostapd(); + stopVendorHal(); +} + +void startWifiFramework() { + toggleWifiFramework(true); +} + +std::string setupApIfaceAndGetName(bool isBridged) { + if (HostapdAidlTestUtils::useAidlService()) { + return HostapdAidlTestUtils::setupApIfaceAndGetName(isBridged); + } else { + return HostapdLegacyTestUtils::setupApIfaceAndGetName(isBridged); + } +} From c6cacf6149a8d5655f2aa841597104979e17c622 Mon Sep 17 00:00:00 2001 From: Gabriel Biren Date: Wed, 19 Jul 2023 17:24:21 +0000 Subject: [PATCH 26/39] Use the hostapd_test_utils helper methods in the Hostapd AIDL VTS tests. At the beginning of each test, we now: 1. Disable the Wifi framework, Hostapd, and Vendor HAL. 2. Enable the Hostapd and Vendor HAL. At the end of each test, we now: 1. Disable the Hostapd and Vendor HAL. 2. Enable the Wifi framework. Bug: 283402709 Test: atest VtsHalHostapdTargetTest --iterations 10 # Tested on both an Android T and U device. # All tests passed in both cases. Change-Id: I8b5e24ad44d13fad09d84f1b212e60ca8df3aa7e --- .../functional/VtsHalHostapdTargetTest.cpp | 79 +++---------------- 1 file changed, 11 insertions(+), 68 deletions(-) diff --git a/wifi/hostapd/aidl/vts/functional/VtsHalHostapdTargetTest.cpp b/wifi/hostapd/aidl/vts/functional/VtsHalHostapdTargetTest.cpp index efd1538c7b..137537d18b 100644 --- a/wifi/hostapd/aidl/vts/functional/VtsHalHostapdTargetTest.cpp +++ b/wifi/hostapd/aidl/vts/functional/VtsHalHostapdTargetTest.cpp @@ -32,6 +32,7 @@ #include #include +#include "hostapd_test_utils.h" #include "wifi_aidl_test_utils.h" using aidl::android::hardware::wifi::hostapd::BandMask; @@ -56,10 +57,7 @@ const std::string kInvalidMaxPassphrase = const int kIfaceChannel = 6; const int kIfaceInvalidChannel = 567; const std::vector kTestZeroMacAddr(6, 0x0); -const Ieee80211ReasonCode kTestDisconnectReasonCode = - Ieee80211ReasonCode::WLAN_REASON_UNSPECIFIED; -const std::string kWifiAidlInstanceNameStr = std::string() + IWifi::descriptor + "/default"; -const char* kWifiAidlInstanceName = kWifiAidlInstanceNameStr.c_str(); +const Ieee80211ReasonCode kTestDisconnectReasonCode = Ieee80211ReasonCode::WLAN_REASON_UNSPECIFIED; inline BandMask operator|(BandMask a, BandMask b) { return static_cast(static_cast(a) | @@ -70,10 +68,13 @@ inline BandMask operator|(BandMask a, BandMask b) { class HostapdAidl : public testing::TestWithParam { public: virtual void SetUp() override { - hostapd = IHostapd::fromBinder(ndk::SpAIBinder( - AServiceManager_waitForService(GetParam().c_str()))); + disableHalsAndFramework(); + initializeHostapdAndVendorHal(GetParam()); + + hostapd = getHostapd(GetParam()); ASSERT_NE(hostapd, nullptr); EXPECT_TRUE(hostapd->setDebugParams(DebugLevel::EXCESSIVE).isOk()); + isAcsSupport = testing::checkSubstringInCommandOutput( "/system/bin/cmd wifi get-softap-supported-features", "wifi_softap_acs_supported"); @@ -81,81 +82,23 @@ class HostapdAidl : public testing::TestWithParam { "/system/bin/cmd wifi get-softap-supported-features", "wifi_softap_wpa3_sae_supported"); isBridgedSupport = testing::checkSubstringInCommandOutput( - "/system/bin/cmd wifi get-softap-supported-features", - "wifi_softap_bridged_ap_supported"); - if (!isAidlServiceAvailable(kWifiAidlInstanceName)) { - const std::vector instances = android::hardware::getAllHalInstanceNames( - ::android::hardware::wifi::V1_0::IWifi::descriptor); - EXPECT_NE(0, instances.size()); - wifiHidlInstanceName = instances[0]; - } + "/system/bin/cmd wifi get-softap-supported-features", + "wifi_softap_bridged_ap_supported"); } virtual void TearDown() override { - stopVendorHal(); hostapd->terminate(); // Wait 3 seconds to allow terminate to complete sleep(3); + stopHostapdAndVendorHal(); + startWifiFramework(); } std::shared_ptr hostapd; - std::string wifiHidlInstanceName; bool isAcsSupport; bool isWpa3SaeSupport; bool isBridgedSupport; - void stopVendorHal() { - if (isAidlServiceAvailable(kWifiAidlInstanceName)) { - // HIDL and AIDL versions of getWifi() take different arguments - // i.e. const char* vs string - if (getWifi(kWifiAidlInstanceName) != nullptr) { - stopWifiService(kWifiAidlInstanceName); - } - } else { - if (getWifi(wifiHidlInstanceName) != nullptr) { - stopWifi(wifiHidlInstanceName); - } - } - } - - std::string setupApIfaceAndGetName(bool isBridged) { - if (isAidlServiceAvailable(kWifiAidlInstanceName)) { - return setupApIfaceAndGetNameAidl(isBridged); - } else { - return setupApIfaceAndGetNameHidl(isBridged); - } - } - - std::string setupApIfaceAndGetNameAidl(bool isBridged) { - std::shared_ptr wifi_ap_iface; - if (isBridged) { - wifi_ap_iface = getBridgedWifiApIface(kWifiAidlInstanceName); - } else { - wifi_ap_iface = getWifiApIface(kWifiAidlInstanceName); - } - EXPECT_NE(nullptr, wifi_ap_iface.get()); - - std::string ap_iface_name; - auto status = wifi_ap_iface->getName(&ap_iface_name); - EXPECT_TRUE(status.isOk()); - return ap_iface_name; - } - - std::string setupApIfaceAndGetNameHidl(bool isBridged) { - android::sp<::android::hardware::wifi::V1_0::IWifiApIface> wifi_ap_iface; - if (isBridged) { - wifi_ap_iface = getBridgedWifiApIface_1_6(wifiHidlInstanceName); - } else { - wifi_ap_iface = getWifiApIface_1_5(wifiHidlInstanceName); - } - EXPECT_NE(nullptr, wifi_ap_iface.get()); - - const auto& status_and_name = HIDL_INVOKE(wifi_ap_iface, getName); - EXPECT_EQ(android::hardware::wifi::V1_0::WifiStatusCode::SUCCESS, - status_and_name.first.code); - return status_and_name.second; - } - IfaceParams getIfaceParamsWithoutAcs(std::string iface_name) { IfaceParams iface_params; ChannelParams channelParams; From bd4cdc6a9a80b3f71d17f58788d3d611574cedbd Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Wed, 30 Aug 2023 19:03:09 +0000 Subject: [PATCH 27/39] Camera VTS: Wait for camera provider to become active Use "AServiceManager_waitForService" instead of "AServiceManager_getService" to wait for any camera providers that might be using the lazy mechanism and are not running all the time. Bug: 297965980 Test: atest VtsAidlHalCameraProvider_TargetTest Change-Id: I6374dc768cd1068885668f927f902dcee44a7a42 --- camera/provider/aidl/vts/camera_aidl_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/camera/provider/aidl/vts/camera_aidl_test.cpp b/camera/provider/aidl/vts/camera_aidl_test.cpp index 08ad0bbaa0..5f9d605ef3 100644 --- a/camera/provider/aidl/vts/camera_aidl_test.cpp +++ b/camera/provider/aidl/vts/camera_aidl_test.cpp @@ -120,7 +120,7 @@ void CameraAidlTest::SetUp() { ABinderProcess_startThreadPool(); SpAIBinder cameraProviderBinder = - SpAIBinder(AServiceManager_getService(serviceDescriptor.c_str())); + SpAIBinder(AServiceManager_waitForService(serviceDescriptor.c_str())); ASSERT_NE(cameraProviderBinder.get(), nullptr); std::shared_ptr cameraProvider = From 166d160b78c29527e2dc474f716faa8b31dded48 Mon Sep 17 00:00:00 2001 From: Seth Moore Date: Fri, 25 Aug 2023 11:09:05 -0700 Subject: [PATCH 28/39] Only require RKP on T+ chipsets It turns out we had a bug (b/263844771) in how RKP support was detected, and that was fixed. However, due to this bug, some S chipests shipped without RKP support which is now required by the tests. This change drops the RKP requirement from S chipsets. There should be no new S chipsets, so this effectively grandfathers in the previous ones that were skipped by the RKP VTS tests. T+ tests (both VTS and other suites) will verify that RKP support is there, so there is no gap introduced by this change. Bug: 297139913 Test: VtsAidlKeyMintTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:8be875e0d0c18b8de67744c8b9629f2ff518dd60) Merged-In: I387e5f058ada698747aac103c1745682291f2d1c Change-Id: I387e5f058ada698747aac103c1745682291f2d1c --- .../aidl/vts/functional/KeyMintAidlTestBase.cpp | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 79c6b12f03..433857c66a 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1295,15 +1295,12 @@ std::pair> KeyMintAidlTestBase::UpgradeKey( } bool KeyMintAidlTestBase::IsRkpSupportRequired() const { - if (get_vsr_api_level() >= __ANDROID_API_T__) { - return true; - } - - if (get_vsr_api_level() >= __ANDROID_API_S__) { - return SecLevel() != SecurityLevel::STRONGBOX; - } - - return false; + // This is technically not a match to the requirements for S chipsets, + // however when S shipped there was a bug in the test that skipped the + // tests if KeyMint 2 was not on the system. So we allowed many chipests + // to ship without RKP support. In T we hardened the requirements around + // support for RKP, so relax the test to match. + return get_vsr_api_level() >= __ANDROID_API_T__; } vector KeyMintAidlTestBase::ValidKeySizes(Algorithm algorithm) { From 35bf8e1b500d7ac2e8dffbc0bf7fe5abf384dcad Mon Sep 17 00:00:00 2001 From: Sarah Chin Date: Thu, 7 Sep 2023 16:16:38 -0700 Subject: [PATCH 29/39] Add null check for osAppId Test: atest VtsHalRadioTargetTest Bug: 297467393 Change-Id: Ib94d118765bf527ba51d5e7e29942cec6bff61c4 --- radio/aidl/vts/radio_data_test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/radio/aidl/vts/radio_data_test.cpp b/radio/aidl/vts/radio_data_test.cpp index 0fb2fb404f..f31c254277 100644 --- a/radio/aidl/vts/radio_data_test.cpp +++ b/radio/aidl/vts/radio_data_test.cpp @@ -214,7 +214,8 @@ TEST_P(RadioDataTest, setupDataCall_osAppId) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_data->rspInfo.error, {RadioError::NONE, RadioError::RADIO_NOT_AVAILABLE, RadioError::OP_NOT_ALLOWED_BEFORE_REG_TO_NW})); - if (radioRsp_data->setupDataCallResult.trafficDescriptors.size() <= 0) { + if (radioRsp_data->setupDataCallResult.trafficDescriptors.size() <= 0 || + !radioRsp_data->setupDataCallResult.trafficDescriptors[0].osAppId.has_value()) { return; } EXPECT_EQ(trafficDescriptor.osAppId.value().osAppId, From 3af92ba5637ebfe55bab6f04029345765423b45c Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Thu, 7 Sep 2023 23:59:03 +0000 Subject: [PATCH 30/39] compatibility_matrices: T launching devices allow to use light V1 AIDL. AIDL deprecation checks are only implemented in U, so the existing V2 requirement on the T matrix did not take effect on T branches. Hence, we shouldn't add requirements retroactively. Test: TH Bug: 298318354 (cherry picked from https://android-review.googlesource.com/q/commit:7e2962f03a649dd4eb34c86c0eb93c98a3bc93b4) Merged-In: Iaf38c6d6270f4aa9c7163540a396045be9d7865a Change-Id: Iaf38c6d6270f4aa9c7163540a396045be9d7865a --- compatibility_matrices/compatibility_matrix.7.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compatibility_matrices/compatibility_matrix.7.xml b/compatibility_matrices/compatibility_matrix.7.xml index e5ef954255..bc5a5c41d2 100644 --- a/compatibility_matrices/compatibility_matrix.7.xml +++ b/compatibility_matrices/compatibility_matrix.7.xml @@ -419,7 +419,7 @@ android.hardware.light - 2 + 1-2 ILights default From e63c5bb5c32fbda553637a4d43994e17af2b110c Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 14 Sep 2023 15:16:27 +0100 Subject: [PATCH 31/39] KeyMint: check missing EC_CURVE on v3+ The original change to add this test didn't make it into the Android 13 version of the VTS test, so the version gate needs to be updated to be v3+ Bug: 292318194 Test: VtsAidlKeyMintTargetTest --gtest_filter="*EcdsaMissingCurve*" (cherry picked from https://android-review.googlesource.com/q/commit:9ed7d2c5bfa3958ef399567e12d84a3f67f0cb80) Merged-In: I94bf816688e57c7c04893a23cf0399129de94229 Change-Id: I94bf816688e57c7c04893a23cf0399129de94229 --- security/keymint/aidl/vts/functional/KeyMintTest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 1297d1d978..2e6fa3b492 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -2630,16 +2630,16 @@ TEST_P(NewKeyGenerationTest, EcdsaInvalidCurve) { /* * NewKeyGenerationTest.EcdsaMissingCurve * - * Verifies that EC key generation fails if EC_CURVE not specified after KeyMint V2. + * Verifies that EC key generation fails if EC_CURVE not specified after KeyMint V3. */ TEST_P(NewKeyGenerationTest, EcdsaMissingCurve) { - if (AidlVersion() < 2) { + if (AidlVersion() < 3) { /* * The KeyMint V1 spec required that EC_CURVE be specified for EC keys. * However, this was not checked at the time so we can only be strict about checking this - * for implementations of KeyMint version 2 and above. + * for implementations of KeyMint version 3 and above. */ - GTEST_SKIP() << "Requiring EC_CURVE only strict since KeyMint v2"; + GTEST_SKIP() << "Requiring EC_CURVE only strict since KeyMint v3"; } /* If EC_CURVE not provided, generateKey * must return ErrorCode::UNSUPPORTED_KEY_SIZE or ErrorCode::UNSUPPORTED_EC_CURVE. From 24e594e05e2a5a7c1fa201ee398940241eceb1e5 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Thu, 14 Sep 2023 11:16:27 +0100 Subject: [PATCH 32/39] KeyMint VTS: re-order auth failure arms Allow for devices that claim to need external timestamps, but don't. Test: VtsAidlKeyMintTargetTest Bug: 300211206 (cherry picked from https://android-review.googlesource.com/q/commit:a35699cb5cfef3773afebf51c2dd38530db43bf0) Merged-In: Ie450d9969c337d5274502f3600e14c0b481e8b34 Change-Id: Ie450d9969c337d5274502f3600e14c0b481e8b34 --- .../keymint/aidl/vts/functional/AuthTest.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/security/keymint/aidl/vts/functional/AuthTest.cpp b/security/keymint/aidl/vts/functional/AuthTest.cpp index 78c88f4896..6499f14347 100644 --- a/security/keymint/aidl/vts/functional/AuthTest.cpp +++ b/security/keymint/aidl/vts/functional/AuthTest.cpp @@ -329,14 +329,14 @@ TEST_P(AuthTest, TimeoutAuthentication) { // Wait for long enough that the hardware auth token expires. sleep(timeout_secs + 1); - if (!timestamp_token_required_) { - // KeyMint implementation has its own clock, and can immediately detect timeout. - EXPECT_EQ(ErrorCode::KEY_USER_NOT_AUTHENTICATED, - Begin(KeyPurpose::ENCRYPT, keyblob, params, &out_params, hat)); - } else { - // KeyMint implementation has no clock, so only detects timeout via timestamp token provided - // on update()/finish(). - ASSERT_EQ(ErrorCode::OK, Begin(KeyPurpose::ENCRYPT, keyblob, params, &out_params, hat)); + + auto begin_result = Begin(KeyPurpose::ENCRYPT, keyblob, params, &out_params, hat); + if (begin_result == ErrorCode::OK) { + // If begin() succeeds despite the out-of-date HAT, that must mean that the KeyMint + // device doesn't have its own clock. In that case, it only detects timeout via a + // timestamp token provided on update()/finish() + ASSERT_TRUE(timestamp_token_required_); + secureclock::TimeStampToken time_token; EXPECT_EQ(ErrorCode::OK, GetReturnErrorCode(clock_->generateTimeStamp(challenge_, &time_token))); @@ -344,6 +344,9 @@ TEST_P(AuthTest, TimeoutAuthentication) { string output; EXPECT_EQ(ErrorCode::KEY_USER_NOT_AUTHENTICATED, Finish(message, {} /* signature */, &output, hat, time_token)); + } else { + // The KeyMint implementation may have its own clock that can immediately detect timeout. + ASSERT_EQ(ErrorCode::KEY_USER_NOT_AUTHENTICATED, begin_result); } } From ff914778ef5b4f940bdb19f8c1ac2d2077357ee9 Mon Sep 17 00:00:00 2001 From: Gabriel Biren Date: Thu, 14 Sep 2023 23:41:45 +0000 Subject: [PATCH 33/39] Rename toggleWifiFramework in the Hostapd AIDL test utils in order to avoid having an ambigous function call. aosp/2728874 adds a method of the same name, which is causing pre-submit issues. Bug: 297820612 Test: atest VtsHalHostapdTargetTest Change-Id: Ie4da0a0037e7411e7908401f021b48246ea227bd Merged-In: Ie4da0a0037e7411e7908401f021b48246ea227bd (cherry picked from commit 94950dc20d0c64f458d87d72c895e253e2d4ad11) --- wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h b/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h index feee2c89b2..50a38d33fd 100644 --- a/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h +++ b/wifi/hostapd/aidl/vts/functional/hostapd_test_utils.h @@ -63,7 +63,7 @@ void waitForSupplicantState(bool enable) { LOG(ERROR) << "Unable to " << (enable ? "start" : "stop") << " supplicant"; } -void toggleWifiFramework(bool enable) { +void toggleWifiFrameworkAndScan(bool enable) { if (enable) { std::system("svc wifi enable"); std::system("cmd wifi set-scan-always-available enabled"); @@ -89,7 +89,7 @@ std::shared_ptr getHostapd(const std::string& hostapd_instance_name) { * any other clients to the HALs during testing. */ void disableHalsAndFramework() { - toggleWifiFramework(false); + toggleWifiFrameworkAndScan(false); stopHostapd(); stopVendorHal(); @@ -110,7 +110,7 @@ void stopHostapdAndVendorHal() { } void startWifiFramework() { - toggleWifiFramework(true); + toggleWifiFrameworkAndScan(true); } std::string setupApIfaceAndGetName(bool isBridged) { From 88aa5f6bcdc328288e7fc63b3dd10a815b8e611b Mon Sep 17 00:00:00 2001 From: Sarah Chin Date: Thu, 7 Sep 2023 22:50:11 -0700 Subject: [PATCH 34/39] Update VTS tests with EUTRAN instead of GERAN These tests were created for IRadio 1.2 when all devices supported GSM, and REQUEST_NOT_SUPPORTED was valid for devices that didn't support GSM. Change the VTS logic to test EUTRAN instead of GERAN. Remove REQUEST_NOT_SUPPORTED for tests now without GERAN and add REQUEST_NOT_SUPPORTED for all GERAN-specific tests. Test: atest VtsHalRadioTargetTest Bug: 294965245 Change-Id: Ib36b171e33451bf0c9adc0b065a4c74df357e77e --- radio/aidl/vts/radio_network_test.cpp | 248 +++++++------------------- 1 file changed, 67 insertions(+), 181 deletions(-) diff --git a/radio/aidl/vts/radio_network_test.cpp b/radio/aidl/vts/radio_network_test.cpp index 253ef82d9f..6643c1e7ff 100644 --- a/radio/aidl/vts/radio_network_test.cpp +++ b/radio/aidl/vts/radio_network_test.cpp @@ -23,6 +23,19 @@ #define ASSERT_OK(ret) ASSERT_TRUE(ret.isOk()) +namespace { +const RadioAccessSpecifierBands EUTRAN_BAND_17 = + RadioAccessSpecifierBands::make( + {EutranBands::BAND_17}); +const RadioAccessSpecifierBands EUTRAN_BAND_20 = + RadioAccessSpecifierBands::make( + {EutranBands::BAND_20}); +const RadioAccessSpecifier EUTRAN_SPECIFIER_17 = { + .accessNetwork = AccessNetwork::EUTRAN, .bands = EUTRAN_BAND_17, .channels = {1, 2}}; +const RadioAccessSpecifier EUTRAN_SPECIFIER_20 = { + .accessNetwork = AccessNetwork::EUTRAN, .bands = EUTRAN_BAND_20, .channels = {128, 129}}; +} // namespace + void RadioNetworkTest::SetUp() { RadioServiceTest::SetUp(); std::string serviceName = GetParam(); @@ -289,7 +302,7 @@ TEST_P(RadioNetworkTest, setSignalStrengthReportingCriteria_invalidHysteresisDb) signalThresholdInfo.hysteresisDb = 10; // hysteresisDb too large given threshold list deltas signalThresholdInfo.thresholds = {-109, -103, -97, -89}; signalThresholdInfo.isEnabled = true; - signalThresholdInfo.ran = AccessNetwork::GERAN; + signalThresholdInfo.ran = AccessNetwork::EUTRAN; ndk::ScopedAStatus res = radio_network->setSignalStrengthReportingCriteria(serial, {signalThresholdInfo}); @@ -314,7 +327,7 @@ TEST_P(RadioNetworkTest, setSignalStrengthReportingCriteria_EmptyThresholds) { signalThresholdInfo.hysteresisMs = 0; signalThresholdInfo.hysteresisDb = 0; signalThresholdInfo.isEnabled = true; - signalThresholdInfo.ran = AccessNetwork::GERAN; + signalThresholdInfo.ran = AccessNetwork::EUTRAN; ndk::ScopedAStatus res = radio_network->setSignalStrengthReportingCriteria(serial, {signalThresholdInfo}); @@ -351,7 +364,8 @@ TEST_P(RadioNetworkTest, setSignalStrengthReportingCriteria_Geran) { ALOGI("setSignalStrengthReportingCriteria_Geran, rspInfo.error = %s\n", toString(radioRsp_network->rspInfo.error).c_str()); - ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::NONE})); + ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, + {RadioError::NONE, RadioError::REQUEST_NOT_SUPPORTED})); } /* @@ -677,7 +691,7 @@ TEST_P(RadioNetworkTest, setSignalStrengthReportingCriteria_multiRansPerRequest) ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); if (radioRsp_network->rspInfo.error == RadioError::NONE) { - supportedSignalThresholdInfos.push_back(signalThresholdInfoGeran); + supportedSignalThresholdInfos.push_back(signalThresholdInfoEutran); } else { // Refer to IRadioNetworkResponse#setSignalStrengthReportingCriteriaResponse ASSERT_TRUE(CheckAnyOfErrors( @@ -711,7 +725,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_invalidHysteresisDlKbp ndk::ScopedAStatus res = radio_network->setLinkCapacityReportingCriteria( serial, 5000, 5000, // hysteresisDlKbps too big for thresholds delta - 100, {1000, 5000, 10000, 20000}, {500, 1000, 5000, 10000}, AccessNetwork::GERAN); + 100, {1000, 5000, 10000, 20000}, {500, 1000, 5000, 10000}, AccessNetwork::EUTRAN); ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); @@ -719,11 +733,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_invalidHysteresisDlKbp ALOGI("setLinkCapacityReportingCriteria_invalidHysteresisDlKbps, rspInfo.error = %s\n", toString(radioRsp_network->rspInfo.error).c_str()); - // Allow REQUEST_NOT_SUPPORTED as setLinkCapacityReportingCriteria() may not be supported - // for GERAN - ASSERT_TRUE( - CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } /* @@ -734,7 +744,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_invalidHysteresisUlKbp ndk::ScopedAStatus res = radio_network->setLinkCapacityReportingCriteria( serial, 5000, 500, 1000, // hysteresisUlKbps too big for thresholds delta - {1000, 5000, 10000, 20000}, {500, 1000, 5000, 10000}, AccessNetwork::GERAN); + {1000, 5000, 10000, 20000}, {500, 1000, 5000, 10000}, AccessNetwork::EUTRAN); ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); @@ -742,11 +752,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_invalidHysteresisUlKbp ALOGI("setLinkCapacityReportingCriteria_invalidHysteresisUlKbps, rspInfo.error = %s\n", toString(radioRsp_network->rspInfo.error).c_str()); - // Allow REQUEST_NOT_SUPPORTED as setLinkCapacityReportingCriteria() may not be supported - // for GERAN - ASSERT_TRUE( - CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } /* @@ -756,7 +762,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_emptyParams) { serial = GetRandomSerialNumber(); ndk::ScopedAStatus res = radio_network->setLinkCapacityReportingCriteria( - serial, 0, 0, 0, {}, {}, AccessNetwork::GERAN); + serial, 0, 0, 0, {}, {}, AccessNetwork::EUTRAN); ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); @@ -764,10 +770,7 @@ TEST_P(RadioNetworkTest, setLinkCapacityReportingCriteria_emptyParams) { ALOGI("setLinkCapacityReportingCriteria_emptyParams, rspInfo.error = %s\n", toString(radioRsp_network->rspInfo.error).c_str()); - // Allow REQUEST_NOT_SUPPORTED as setLinkCapacityReportingCriteria() may not be supported - // for GERAN - ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::NONE})); } /* @@ -808,19 +811,9 @@ TEST_P(RadioNetworkTest, setSystemSelectionChannels) { } std::vector originalSpecifiers = radioRsp_network->specifiers; - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - serial = GetRandomSerialNumber(); - res = radio_network->setSystemSelectionChannels(serial, true, {specifierP900, specifier850}); + res = radio_network->setSystemSelectionChannels(serial, true, + {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}); ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); @@ -833,8 +826,8 @@ TEST_P(RadioNetworkTest, setSystemSelectionChannels) { if (radioRsp_network->rspInfo.error == RadioError::NONE) { serial = GetRandomSerialNumber(); - res = radio_network->setSystemSelectionChannels(serial, false, - {specifierP900, specifier850}); + res = radio_network->setSystemSelectionChannels( + serial, false, {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}); ASSERT_OK(res); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); @@ -857,20 +850,9 @@ TEST_P(RadioNetworkTest, setSystemSelectionChannels) { TEST_P(RadioNetworkTest, startNetworkScan) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands band17 = - RadioAccessSpecifierBands::make( - {EutranBands::BAND_17}); - RadioAccessSpecifierBands band20 = - RadioAccessSpecifierBands::make( - {EutranBands::BAND_20}); - RadioAccessSpecifier specifier17 = { - .accessNetwork = AccessNetwork::EUTRAN, .bands = band17, .channels = {1, 2}}; - RadioAccessSpecifier specifier20 = { - .accessNetwork = AccessNetwork::EUTRAN, .bands = band20, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifier17, specifier20}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 60, .incrementalResults = false, .incrementalResultsPeriodicity = 1}; @@ -892,10 +874,9 @@ TEST_P(RadioNetworkTest, startNetworkScan) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::NONE, RadioError::OPERATION_NOT_ALLOWED})); } else { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::OPERATION_NOT_ALLOWED, RadioError::NONE, - RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, + {RadioError::NONE, RadioError::OPERATION_NOT_ALLOWED, + RadioError::INVALID_ARGUMENTS})); } } @@ -925,9 +906,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidArgument) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -937,20 +917,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidArgument) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval1) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_PERIODIC, .interval = 4, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 60, .incrementalResults = false, .incrementalResultsPeriodicity = 1}; @@ -966,9 +935,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval1) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -978,20 +946,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval1) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval2) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_PERIODIC, .interval = 301, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 60, .incrementalResults = false, .incrementalResultsPeriodicity = 1}; @@ -1007,9 +964,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval2) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -1019,20 +975,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidInterval2) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime1) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 59, .incrementalResults = false, .incrementalResultsPeriodicity = 1}; @@ -1048,9 +993,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime1) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -1060,20 +1004,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime1) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime2) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 3601, .incrementalResults = false, .incrementalResultsPeriodicity = 1}; @@ -1089,9 +1022,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime2) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -1101,20 +1033,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidMaxSearchTime2) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity1) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 600, .incrementalResults = true, .incrementalResultsPeriodicity = 0}; @@ -1130,9 +1051,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity1) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -1142,20 +1062,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity1) { TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity2) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 600, .incrementalResults = true, .incrementalResultsPeriodicity = 11}; @@ -1171,9 +1080,8 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity2) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::SIM_ABSENT, RadioError::INVALID_ARGUMENTS})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_network->rspInfo.error, - {RadioError::INVALID_ARGUMENTS, RadioError::REQUEST_NOT_SUPPORTED})); + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_network->rspInfo.error, {RadioError::INVALID_ARGUMENTS})); } } @@ -1183,20 +1091,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_InvalidPeriodicity2) { TEST_P(RadioNetworkTest, startNetworkScan_GoodRequest1) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 360, .incrementalResults = false, .incrementalResultsPeriodicity = 10}; @@ -1213,8 +1110,7 @@ TEST_P(RadioNetworkTest, startNetworkScan_GoodRequest1) { {RadioError::NONE, RadioError::SIM_ABSENT})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::INVALID_ARGUMENTS, - RadioError::REQUEST_NOT_SUPPORTED})); + {RadioError::NONE, RadioError::INVALID_ARGUMENTS})); } if (radioRsp_network->rspInfo.error == RadioError::NONE) { @@ -1229,20 +1125,9 @@ TEST_P(RadioNetworkTest, startNetworkScan_GoodRequest1) { TEST_P(RadioNetworkTest, startNetworkScan_GoodRequest2) { serial = GetRandomSerialNumber(); - RadioAccessSpecifierBands bandP900 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_P900}); - RadioAccessSpecifierBands band850 = - RadioAccessSpecifierBands::make( - {GeranBands::BAND_850}); - RadioAccessSpecifier specifierP900 = { - .accessNetwork = AccessNetwork::GERAN, .bands = bandP900, .channels = {1, 2}}; - RadioAccessSpecifier specifier850 = { - .accessNetwork = AccessNetwork::GERAN, .bands = band850, .channels = {128, 129}}; - NetworkScanRequest request = {.type = NetworkScanRequest::SCAN_TYPE_ONE_SHOT, .interval = 60, - .specifiers = {specifierP900, specifier850}, + .specifiers = {::EUTRAN_SPECIFIER_17, ::EUTRAN_SPECIFIER_20}, .maxSearchTime = 360, .incrementalResults = false, .incrementalResultsPeriodicity = 10, @@ -1260,8 +1145,7 @@ TEST_P(RadioNetworkTest, startNetworkScan_GoodRequest2) { {RadioError::NONE, RadioError::SIM_ABSENT})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::INVALID_ARGUMENTS, - RadioError::REQUEST_NOT_SUPPORTED})); + {RadioError::NONE, RadioError::INVALID_ARGUMENTS})); } if (radioRsp_network->rspInfo.error == RadioError::NONE) { @@ -1278,21 +1162,23 @@ TEST_P(RadioNetworkTest, setNetworkSelectionModeManual) { // can't camp on nonexistent MCCMNC, so we expect this to fail. ndk::ScopedAStatus res = - radio_network->setNetworkSelectionModeManual(serial, "123456", AccessNetwork::GERAN); + radio_network->setNetworkSelectionModeManual(serial, "123456", AccessNetwork::EUTRAN); EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_network->rspInfo.type); EXPECT_EQ(serial, radioRsp_network->rspInfo.serial); if (cardStatus.cardState == CardStatus::STATE_ABSENT) { - ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::ILLEGAL_SIM_OR_ME, - RadioError::INVALID_ARGUMENTS, RadioError::INVALID_STATE}, - CHECK_GENERAL_ERROR)); + ASSERT_TRUE(CheckAnyOfErrors( + radioRsp_network->rspInfo.error, + {RadioError::NONE, RadioError::ILLEGAL_SIM_OR_ME, RadioError::INVALID_ARGUMENTS, + RadioError::INVALID_STATE, RadioError::RADIO_NOT_AVAILABLE, RadioError::NO_MEMORY, + RadioError::INTERNAL_ERR, RadioError::SYSTEM_ERR, RadioError::CANCELLED})); } else if (cardStatus.cardState == CardStatus::STATE_PRESENT) { - ASSERT_TRUE(CheckAnyOfErrors(radioRsp_network->rspInfo.error, - {RadioError::NONE, RadioError::RADIO_NOT_AVAILABLE, - RadioError::INVALID_ARGUMENTS, RadioError::INVALID_STATE}, - CHECK_GENERAL_ERROR)); + ASSERT_TRUE(CheckAnyOfErrors( + radioRsp_network->rspInfo.error, + {RadioError::NONE, RadioError::RADIO_NOT_AVAILABLE, RadioError::INVALID_ARGUMENTS, + RadioError::INVALID_STATE, RadioError::NO_MEMORY, RadioError::INTERNAL_ERR, + RadioError::SYSTEM_ERR, RadioError::CANCELLED})); } } From 95318f2960dbaa48aae4b51457f4a5abaf425d05 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Wed, 10 May 2023 22:08:04 +0000 Subject: [PATCH 35/39] Add security version to config descriptor Introduce a field to the configuration descriptor that provides a standard semantically-defined version number rather than the vendor-defined component version which acts more like a build ID. Test: n/a Bug: 298580435 Bug: 282205139 (cherry picked from https://android-review.googlesource.com/q/commit:0d520e8e1751fde5a3207c6f27be88a8bbc245dc) Merged-In: Idb0c991ab12ae75687236f2489e639e4422a0225 Change-Id: Idb0c991ab12ae75687236f2489e639e4422a0225 --- security/rkp/README.md | 5 +++++ .../security/keymint/IRemotelyProvisionedComponent.aidl | 1 + 2 files changed, 6 insertions(+) diff --git a/security/rkp/README.md b/security/rkp/README.md index 7477f803b3..5a93734c11 100644 --- a/security/rkp/README.md +++ b/security/rkp/README.md @@ -324,6 +324,11 @@ the range \[-70000, -70999\] (these are reserved for future additions here). : : : : boot stage : | Resettable | -70004 | null | If present, key changes on factory| : : : : reset : +| Security version | -70005 | uint | Machine-comparable, monotonically | +: : : : increasing version of the firmware: +: : : : component / boot stage where a : +: : : : greater value indicates a newer : +: : : : version : ``` Please see diff --git a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl index 2a4cba1f0e..7fed3636f6 100644 --- a/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl +++ b/security/rkp/aidl/android/hardware/security/keymint/IRemotelyProvisionedComponent.aidl @@ -427,6 +427,7 @@ interface IRemotelyProvisionedComponent { * ? -70002 : tstr, ; Component name * ? -70003 : int / tstr, ; Component version * ? -70004 : null, ; Resettable + * ? -70005 : uint, ; Security version * }, * -4670549 : bstr, ; Authority Hash * ? -4670550 : bstr, ; Authority Descriptor From d1bc0389e4b86d5f82c7528359247489db3d259a Mon Sep 17 00:00:00 2001 From: Subrahmanyaman Date: Mon, 5 Jun 2023 15:24:38 +0000 Subject: [PATCH 36/39] Support to get EC public key from the UdsCertchain. Bug: 297123463 Bug: 285896470 Test: VtsHalRemotelyProvisionedComponentTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:a18883a58cc9f6b702095bb17bbd0e4e894be49c) Merged-In: I7f829b1346feeab0fd429ad7b9714181b6668b34 Change-Id: I7f829b1346feeab0fd429ad7b9714181b6668b34 --- .../keymint/support/remote_prov_utils.cpp | 75 ++++++++++++++++--- 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/security/keymint/support/remote_prov_utils.cpp b/security/keymint/support/remote_prov_utils.cpp index 3cb783cf0a..c9c3e4d4ae 100644 --- a/security/keymint/support/remote_prov_utils.cpp +++ b/security/keymint/support/remote_prov_utils.cpp @@ -115,6 +115,36 @@ ErrMsgOr> getAffineCoordinates(const bytevec& pubKe return std::make_tuple(std::move(pubX), std::move(pubY)); } +ErrMsgOr getRawPublicKey(const EVP_PKEY_Ptr& pubKey) { + if (pubKey.get() == nullptr) { + return "pkey is null."; + } + int keyType = EVP_PKEY_base_id(pubKey.get()); + switch (keyType) { + case EVP_PKEY_EC: { + auto ecKey = EC_KEY_Ptr(EVP_PKEY_get1_EC_KEY(pubKey.get())); + if (ecKey.get() == nullptr) { + return "Failed to get ec key"; + } + return ecKeyGetPublicKey(ecKey.get()); + } + case EVP_PKEY_ED25519: { + bytevec rawPubKey; + size_t rawKeySize = 0; + if (!EVP_PKEY_get_raw_public_key(pubKey.get(), NULL, &rawKeySize)) { + return "Failed to get raw public key."; + } + rawPubKey.resize(rawKeySize); + if (!EVP_PKEY_get_raw_public_key(pubKey.get(), rawPubKey.data(), &rawKeySize)) { + return "Failed to get raw public key."; + } + return rawPubKey; + } + default: + return "Unknown key type."; + } +} + ErrMsgOr> generateEc256KeyPair() { auto ec_key = EC_KEY_Ptr(EC_KEY_new()); if (ec_key.get() == nullptr) { @@ -706,11 +736,10 @@ std::string getX509SubjectName(const X509_Ptr& cert) { // Validates the certificate chain and returns the leaf public key. ErrMsgOr validateCertChain(const cppbor::Array& chain) { - uint8_t rawPubKey[64]; - size_t rawPubKeySize = sizeof(rawPubKey); + bytevec rawPubKey; for (size_t i = 0; i < chain.size(); ++i) { // Root must be self-signed. - size_t signingCertIndex = (i > 1) ? i - 1 : i; + size_t signingCertIndex = (i > 0) ? i - 1 : i; auto& keyCertItem = chain[i]; auto& signingCertItem = chain[signingCertIndex]; if (!keyCertItem || !keyCertItem->asBstr()) { @@ -724,7 +753,7 @@ ErrMsgOr validateCertChain(const cppbor::Array& chain) { if (!keyCert) { return keyCert.message(); } - auto signingCert = parseX509Cert(keyCertItem->asBstr()->value()); + auto signingCert = parseX509Cert(signingCertItem->asBstr()->value()); if (!signingCert) { return signingCert.message(); } @@ -749,17 +778,16 @@ ErrMsgOr validateCertChain(const cppbor::Array& chain) { return "Certificate " + std::to_string(i) + " has wrong issuer. Signer subject is " + signerSubj + " Issuer subject is " + certIssuer; } - - rawPubKeySize = sizeof(rawPubKey); - if (!EVP_PKEY_get_raw_public_key(pubKey.get(), rawPubKey, &rawPubKeySize)) { - return "Failed to get raw public key."; + if (i == chain.size() - 1) { + auto key = getRawPublicKey(pubKey); + if (!key) key.moveMessage(); + rawPubKey = key.moveValue(); } } - - return bytevec(rawPubKey, rawPubKey + rawPubKeySize); + return rawPubKey; } -std::string validateUdsCerts(const cppbor::Map& udsCerts, const bytevec& udsPub) { +std::string validateUdsCerts(const cppbor::Map& udsCerts, const bytevec& udsCoseKeyBytes) { for (const auto& [signerName, udsCertChain] : udsCerts) { if (!signerName || !signerName->asTstr()) { return "Signer Name must be a Tstr."; @@ -775,8 +803,31 @@ std::string validateUdsCerts(const cppbor::Map& udsCerts, const bytevec& udsPub) if (!leafPubKey) { return leafPubKey.message(); } + auto coseKey = CoseKey::parse(udsCoseKeyBytes); + if (!coseKey) return coseKey.moveMessage(); + + auto curve = coseKey->getIntValue(CoseKey::CURVE); + if (!curve) { + return "CoseKey must contain curve."; + } + bytevec udsPub; + if (curve == CoseKeyCurve::P256 || curve == CoseKeyCurve::P384) { + auto pubKey = coseKey->getEcPublicKey(); + if (!pubKey) return pubKey.moveMessage(); + // convert public key to uncompressed form by prepending 0x04 at begin. + pubKey->insert(pubKey->begin(), 0x04); + udsPub = pubKey.moveValue(); + } else if (curve == CoseKeyCurve::ED25519) { + auto& pubkey = coseKey->getMap().get(cppcose::CoseKey::PUBKEY_X); + if (!pubkey || !pubkey->asBstr()) { + return "Invalid public key."; + } + udsPub = pubkey->asBstr()->value(); + } else { + return "Unknown curve."; + } if (*leafPubKey != udsPub) { - return "Leaf public key in UDS certificat chain doesn't match UDS public key."; + return "Leaf public key in UDS certificate chain doesn't match UDS public key."; } } return ""; From b5cc022359f37474ab36eca630e1cbe133f5b6b0 Mon Sep 17 00:00:00 2001 From: Alisher Alikhodjaev Date: Wed, 20 Sep 2023 15:15:38 -0700 Subject: [PATCH 37/39] Temporary disable one of the transmit checks The transmit without openning a channel passes on some platforms, because a basic channel can be opened by external applications and the state is maintained in the HAL. Bug: 300502872 Test: run vts -m VtsHalSecureElementTargetTest (cherry picked from https://android-review.googlesource.com/q/commit:d0ed43bb7eeb8c2cdc23d23d0d926e083322b7f3) Merged-In: If727c613e8575802b99604f7242e16cf85fc97a0 Change-Id: If727c613e8575802b99604f7242e16cf85fc97a0 --- secure_element/aidl/vts/VtsHalSecureElementTargetTest.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/secure_element/aidl/vts/VtsHalSecureElementTargetTest.cpp b/secure_element/aidl/vts/VtsHalSecureElementTargetTest.cpp index 97b4e27629..9678da4a5b 100644 --- a/secure_element/aidl/vts/VtsHalSecureElementTargetTest.cpp +++ b/secure_element/aidl/vts/VtsHalSecureElementTargetTest.cpp @@ -293,11 +293,13 @@ TEST_P(SecureElementAidl, transmit) { std::vector response; LogicalChannelResponse logical_channel_response; + /* Temporaly disable this check to clarify Basic Channel behavior (b/300502872) // Note: no channel is opened for this test // transmit() will return an empty response with the error // code CHANNEL_NOT_AVAILABLE when the SE cannot be // communicated with. EXPECT_ERR(secure_element_->transmit(kDataApdu, &response)); + */ EXPECT_OK(secure_element_->openLogicalChannel(kSelectableAid, 0x00, &logical_channel_response)); EXPECT_GE(logical_channel_response.selectResponse.size(), 2u); From 2236b96bf451bdf131643c5e64cf627be0f91143 Mon Sep 17 00:00:00 2001 From: Ye Jiao Date: Mon, 28 Aug 2023 15:22:43 +0800 Subject: [PATCH 38/39] Disable Wi-Fi framework during VTS When VTS case kills supplicant in SetUp, Wi-Fi framework will trigger recovery process and causes races between VTS and Wi-Fi framework. Disable Wi-Fi framework using 'cmd wifi' in SetUp. Bug: 297820612 Test: run vts -m VtsHalWifiHostapdV1_0TargetTest with new case (cherry picked from https://android-review.googlesource.com/q/commit:46538346af90bf93dcafa36206fc2ea5c545dd93) Merged-In: I5ea7f00cfebf79021185f66766454d053c973b31 Change-Id: I5ea7f00cfebf79021185f66766454d053c973b31 --- .../1.0/vts/functional/hostapd_hidl_test.cpp | 18 ++++++++++++++-- .../functional/hostapd_hidl_test_utils.cpp | 21 +++++++++++++++++++ .../vts/functional/hostapd_hidl_test_utils.h | 7 +++++-- 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test.cpp b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test.cpp index bb99ae4d6b..46cba93c72 100644 --- a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test.cpp +++ b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test.cpp @@ -48,6 +48,13 @@ class HostapdHidlTest virtual void SetUp() override { wifi_instance_name_ = std::get<0>(GetParam()); hostapd_instance_name_ = std::get<1>(GetParam()); + + // Disable Wi-Fi framework to avoid interference + isWifiEnabled_ = isWifiFrameworkEnabled(); + isScanAlwaysEnabled_ = isWifiScanAlwaysAvailable(); + toggleWifiFramework(false); + toggleWifiScanAlwaysAvailable(false); + stopSupplicantIfNeeded(wifi_instance_name_); startHostapdAndWaitForHidlService(wifi_instance_name_, hostapd_instance_name_); @@ -58,14 +65,21 @@ class HostapdHidlTest virtual void TearDown() override { HIDL_INVOKE_VOID_WITHOUT_ARGUMENTS(hostapd_, terminate); stopHostapd(wifi_instance_name_); + + // Restore Wi-Fi framework state + toggleWifiFramework(isWifiEnabled_); + toggleWifiScanAlwaysAvailable(isScanAlwaysEnabled_); } protected: - std::string getPrimaryWlanIfaceName() { + bool isWifiEnabled_ = false; + bool isScanAlwaysEnabled_ = false; + + std::string getPrimaryWlanIfaceName() { std::array buffer; property_get("wifi.interface", buffer.data(), "wlan0"); return buffer.data(); - } + } IHostapd::IfaceParams getIfaceParamsWithAcs() { IHostapd::IfaceParams iface_params; diff --git a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp index 56f285d147..4c452fbd86 100644 --- a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp +++ b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.cpp @@ -98,3 +98,24 @@ bool is_1_1(const sp& hostapd) { ::android::hardware::wifi::hostapd::V1_1::IHostapd::castFrom(hostapd); return hostapd_1_1.get() != nullptr; } + +void toggleWifiFramework(bool enable) { + std::string cmd = "/system/bin/cmd wifi set-wifi-enabled "; + cmd += enable ? "enabled" : "disabled"; + testing::checkSubstringInCommandOutput(cmd.c_str(), "X"); +} + +void toggleWifiScanAlwaysAvailable(bool enable) { + std::string cmd = "/system/bin/cmd wifi set-scan-always-available "; + cmd += enable ? "enabled" : "disabled"; + testing::checkSubstringInCommandOutput(cmd.c_str(), "X"); +} + +bool isWifiFrameworkEnabled() { + return testing::checkSubstringInCommandOutput("/system/bin/cmd wifi status", "Wifi is enabled"); +} + +bool isWifiScanAlwaysAvailable() { + return testing::checkSubstringInCommandOutput("/system/bin/cmd wifi status", + "Wifi scanning is always available"); +} diff --git a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h index 893de1e334..aa34c9a98f 100644 --- a/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h +++ b/wifi/hostapd/1.0/vts/functional/hostapd_hidl_test_utils.h @@ -17,14 +17,17 @@ #ifndef HOSTAPD_HIDL_TEST_UTILS_H #define HOSTAPD_HIDL_TEST_UTILS_H +#include #include #include // Used to stop the android wifi framework before every test. -void stopWifiFramework(const std::string& instance_name); -void startWifiFramework(const std::string& instance_name); void stopSupplicantIfNeeded(const std::string& instance_name); void stopHostapd(const std::string& instance_name); +void toggleWifiFramework(bool enable); +void toggleWifiScanAlwaysAvailable(bool enable); +bool isWifiFrameworkEnabled(); +bool isWifiScanAlwaysAvailable(); // Used to configure the chip, driver and start wpa_hostapd before every // test. void startHostapdAndWaitForHidlService( From a47d46affbd5e522a426ed5bd5f436fe9904ed2e Mon Sep 17 00:00:00 2001 From: Alec Mouri Date: Fri, 29 Sep 2023 00:21:37 +0000 Subject: [PATCH 39/39] Setting layer brightness doesn't need nit info for readback tests The nit information was used when we set exact nit values rather than a relative brightness per layer. But we only need nit values for the renderengine interface, which isn't tied to any hwc or panel capabilities. Bug: 301261125 Test: VtsHalGraphicsComposer3_TargetTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:712b3d9880b6aff51ab17d539bfcbdf3785cc6ca) Merged-In: I770dc5620648df2eab608e030c5e76cf190f315d Change-Id: I770dc5620648df2eab608e030c5e76cf190f315d --- graphics/composer/aidl/vts/Android.bp | 1 - .../VtsHalGraphicsComposer3_ReadbackTest.cpp | 143 +----------------- 2 files changed, 5 insertions(+), 139 deletions(-) diff --git a/graphics/composer/aidl/vts/Android.bp b/graphics/composer/aidl/vts/Android.bp index 88b5de41bb..60360fd596 100644 --- a/graphics/composer/aidl/vts/Android.bp +++ b/graphics/composer/aidl/vts/Android.bp @@ -54,7 +54,6 @@ cc_test { "libgui", "libhidlbase", "libprocessgroup", - "libtinyxml2", "android.hardware.graphics.mapper@2.0", "android.hardware.graphics.mapper@2.1", "android.hardware.graphics.mapper@3.0", diff --git a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp index 269abd150d..9b849cc133 100644 --- a/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp +++ b/graphics/composer/aidl/vts/VtsHalGraphicsComposer3_ReadbackTest.cpp @@ -31,12 +31,6 @@ #include "RenderEngineVts.h" #include "VtsComposerClient.h" -// tinyxml2 does implicit conversions >:( -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" -#include -#pragma clang diagnostic pop - namespace aidl::android::hardware::graphics::composer3::vts { namespace { @@ -129,63 +123,6 @@ class GraphicsCompositionTestBase : public ::testing::Test { return {false, graphicBuffer}; } - // Gets the per-display XML config - std::unique_ptr getDisplayConfigXml(int64_t display) { - - if (auto document = getDisplayConfigXmlByStableId(getStableDisplayId(display)); - document != nullptr) { - return document; - } - - // Fallback to looking up a per-port config if no config exists for the full ID - if (auto document = getDisplayConfigXmlByPort(getPort(display)); document != nullptr) { - return document; - } - - return nullptr; - } - - // Gets the max display brightness for this display. - // If the display config xml does not exist, then assume that the display is not well-configured - // enough to provide a display brightness, so return nullopt. - std::optional getMaxDisplayBrightnessNits(int64_t display) { - const auto document = getDisplayConfigXml(display); - if (!document) { - // Assume the device doesn't support display brightness - return std::nullopt; - } - - const auto root = document->RootElement(); - if (!root) { - // If there's somehow no root element, then this isn't a valid config - return std::nullopt; - } - - const auto screenBrightnessMap = root->FirstChildElement("screenBrightnessMap"); - if (!screenBrightnessMap) { - // A valid display config must have a screen brightness map - return std::nullopt; - } - - auto point = screenBrightnessMap->FirstChildElement("point"); - float maxNits = -1.f; - while (point != nullptr) { - const auto nits = point->FirstChildElement("nits"); - if (nits) { - maxNits = std::max(maxNits, nits->FloatText(-1.f)); - } - point = point->NextSiblingElement("point"); - } - - if (maxNits < 0.f) { - // If we got here, then there were no point elements containing a nit value, so this - // config isn't valid - return std::nullopt; - } - - return maxNits; - } - void writeLayers(const std::vector>& layers) { for (const auto& layer : layers) { layer->write(*mWriter); @@ -243,53 +180,6 @@ class GraphicsCompositionTestBase : public ::testing::Test { } } } - - uint8_t getPort(int64_t display) { - const auto& [status, identification] = - mComposerClient->getDisplayIdentificationData(display); - EXPECT_TRUE(status.isOk()); - return static_cast(identification.port); - } - - uint64_t getStableDisplayId(int64_t display) { - const auto& [status, identification] = - mComposerClient->getDisplayIdentificationData(display); - EXPECT_TRUE(status.isOk()); - - if (const auto info = ::android::parseDisplayIdentificationData( - static_cast(identification.port), identification.data)) { - return info->id.value; - } - - return ::android::PhysicalDisplayId::fromPort(static_cast(identification.port)) - .value; - } - - std::unique_ptr loadXml(const std::string& path) { - auto document = std::make_unique(); - const tinyxml2::XMLError error = document->LoadFile(path.c_str()); - if (error != tinyxml2::XML_SUCCESS) { - ALOGD("%s: Failed to load config file: %s", __func__, path.c_str()); - return nullptr; - } - - ALOGD("%s: Successfully loaded config file: %s", __func__, path.c_str()); - return document; - } - - std::unique_ptr getDisplayConfigXmlByPort(uint8_t port) { - std::stringstream pathBuilder; - pathBuilder << "/vendor/etc/displayconfig/display_port_" << static_cast(port) - << ".xml"; - return loadXml(pathBuilder.str()); - } - - std::unique_ptr getDisplayConfigXmlByStableId(uint64_t stableId) { - std::stringstream pathBuilder; - pathBuilder << "/vendor/etc/displayconfig/display_id_" << stableId - << ".xml"; - return loadXml(pathBuilder.str()); - } }; class GraphicsCompositionTest : public GraphicsCompositionTestBase, @@ -991,32 +881,6 @@ TEST_P(GraphicsCompositionTest, SetLayerZOrder) { } TEST_P(GraphicsCompositionTest, SetLayerBrightnessDims) { - const auto& [status, capabilities] = - mComposerClient->getDisplayCapabilities(getPrimaryDisplayId()); - ASSERT_TRUE(status.isOk()); - - const bool brightnessSupport = std::find(capabilities.begin(), capabilities.end(), - DisplayCapability::BRIGHTNESS) != capabilities.end(); - - if (!brightnessSupport) { - GTEST_SUCCEED() << "Cannot verify dimming behavior without brightness support"; - return; - } - - const std::optional maxBrightnessNitsOptional = - getMaxDisplayBrightnessNits(getPrimaryDisplayId()); - - ASSERT_TRUE(maxBrightnessNitsOptional.has_value()); - - const float maxBrightnessNits = *maxBrightnessNitsOptional; - - // Preconditions to successfully run are knowing the max brightness and successfully applying - // the max brightness - ASSERT_GT(maxBrightnessNits, 0.f); - mWriter->setDisplayBrightness(getPrimaryDisplayId(), /*brightness*/ 1.f, maxBrightnessNits); - execute(); - ASSERT_TRUE(mReader.takeErrors().empty()); - for (ColorMode mode : mTestColorModes) { EXPECT_TRUE(mComposerClient ->setColorMode(getPrimaryDisplayId(), mode, RenderIntent::COLORIMETRIC) @@ -1033,11 +897,14 @@ TEST_P(GraphicsCompositionTest, SetLayerBrightnessDims) { const common::Rect redRect = {0, 0, getDisplayWidth(), getDisplayHeight() / 2}; const common::Rect dimmerRedRect = {0, getDisplayHeight() / 2, getDisplayWidth(), getDisplayHeight()}; + + static constexpr float kMaxBrightnessNits = 300.f; + const auto redLayer = std::make_shared(mComposerClient, getPrimaryDisplayId()); redLayer->setColor(RED); redLayer->setDisplayFrame(redRect); - redLayer->setWhitePointNits(maxBrightnessNits); + redLayer->setWhitePointNits(kMaxBrightnessNits); redLayer->setBrightness(1.f); const auto dimmerRedLayer = @@ -1047,7 +914,7 @@ TEST_P(GraphicsCompositionTest, SetLayerBrightnessDims) { // Intentionally use a small dimming ratio as some implementations may be more likely to // kick into GPU composition to apply dithering when the dimming ratio is high. static constexpr float kDimmingRatio = 0.9f; - dimmerRedLayer->setWhitePointNits(maxBrightnessNits * kDimmingRatio); + dimmerRedLayer->setWhitePointNits(kMaxBrightnessNits * kDimmingRatio); dimmerRedLayer->setBrightness(kDimmingRatio); const std::vector> layers = {redLayer, dimmerRedLayer};