From bc49029114079dbbec614c8cf652fa5992b58552 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 21 May 2021 00:34:09 +0000 Subject: [PATCH 01/35] DO NOT MERGE: Disable current matrix. The current matrix should not be used in Android S. However, it is left here so that any late fixes to the S matrix can be made in a way that will flow downstream. Reason for 'DO NOT MERGE' - avoid landing this change in goog/master, since the current matrix has HALs added there, some devices may be relying on it. In order to allow the (AOSP subset of) the S manifest to be frozen in AOSP, and the current matrix to be enabled from aosp/master -..-> goog/master, this will be reverted in sc-dev-plus-aosp immediately after it merges. Bug: 178221726 Test: boot devices and vts_treble_vintf_vendor_test passes Change-Id: I5716db7cfd629527dd33daac94244e5df41520f5 (cherry picked from commit e0bb84be37c5acc8ff84ad657242eca075a75726) --- compatibility_matrices/Android.bp | 1 + compatibility_matrices/Android.mk | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/compatibility_matrices/Android.bp b/compatibility_matrices/Android.bp index da55347d25..31fa1ae403 100644 --- a/compatibility_matrices/Android.bp +++ b/compatibility_matrices/Android.bp @@ -75,6 +75,7 @@ vintf_compatibility_matrix { vintf_compatibility_matrix { name: "framework_compatibility_matrix.current.xml", + enabled: false, stem: "compatibility_matrix.current.xml", srcs: [ "compatibility_matrix.current.xml", diff --git a/compatibility_matrices/Android.mk b/compatibility_matrices/Android.mk index 9e715bf37d..4cefb5506e 100644 --- a/compatibility_matrices/Android.mk +++ b/compatibility_matrices/Android.mk @@ -102,7 +102,6 @@ my_system_matrix_deps := \ framework_compatibility_matrix.4.xml \ framework_compatibility_matrix.5.xml \ framework_compatibility_matrix.6.xml \ - framework_compatibility_matrix.current.xml \ framework_compatibility_matrix.device.xml \ my_framework_matrix_deps += \ From 165d173e94b99553bb69b6fdb97ea46176694aec Mon Sep 17 00:00:00 2001 From: Keith Mok Date: Tue, 1 Jun 2021 18:51:57 +0000 Subject: [PATCH 02/35] Move vehicle hal to start in early_hal Vehicle HAL required before storage encryption can get unlocked when using vehicle seed binding with ECU. Test: manual, check vehicle-hal starts in early_hal from logcat Bug: 157501579 Change-Id: I88db77fbb1d04577b999b47e6f8dc4df1b300832 --- .../default/android.hardware.automotive.vehicle@2.0-service.rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-service.rc b/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-service.rc index c8c89dc821..44f9134714 100644 --- a/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-service.rc +++ b/automotive/vehicle/2.0/default/android.hardware.automotive.vehicle@2.0-service.rc @@ -1,4 +1,4 @@ service vendor.vehicle-hal-2.0 /vendor/bin/hw/android.hardware.automotive.vehicle@2.0-service - class hal + class early_hal user vehicle_network group system inet From 1b32420affbbb9f6b28d2ec8d55c3c5534e79993 Mon Sep 17 00:00:00 2001 From: Keith Mok Date: Wed, 14 Jul 2021 01:17:28 +0000 Subject: [PATCH 03/35] Remove vehicle binding seed property in emulator Emulator does not support vehicle binding seed property, but it reports it supports it. Remove it for now. Otherwise the vehicle binding utilities will try to set the vehicle binding seed but that will not be saved anywhere in the emulator. Test: manual Bug: 157501579 Change-Id: I9e8ad382a49e1108e0e25d6eee94b24d766264e8 Merged-In: I874e94b06fe675a96e5b15c9bff087023b4ea109 --- .../vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h index a85cdf06a5..f98962b81e 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h @@ -1024,14 +1024,6 @@ const ConfigDeclaration kVehicleProperties[]{ .changeMode = VehiclePropertyChangeMode::ON_CHANGE, }, }, - { - .config = - { - .prop = toInt(VehicleProperty::STORAGE_ENCRYPTION_BINDING_SEED), - .access = VehiclePropertyAccess::READ_WRITE, - .changeMode = VehiclePropertyChangeMode::ON_CHANGE, - }, - }, { .config = { From 8f836b94992576a85b15d9666f4da193008e0597 Mon Sep 17 00:00:00 2001 From: Andy Hung Date: Wed, 16 Jun 2021 09:40:07 -0700 Subject: [PATCH 04/35] Audio: Add memory leak checking for HAL $ adb shell setprop libc.debug.malloc.program android.hardware.audio.service $ adb shell setprop libc.debug.malloc.options backtrace=8 $ adb shell setenforce 0 $ adb shell pkill audioserver $ adb shell dumpsys media.audio_flinger Test: Check the audio flinger dumpsys as above. Bug: 186054996 Bug: 187462632 Change-Id: I2e8db14b816cc4cd7e1420c538505bf71fa58c97 --- audio/core/all-versions/default/Android.bp | 2 ++ audio/core/all-versions/default/Device.cpp | 29 ++++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/audio/core/all-versions/default/Android.bp b/audio/core/all-versions/default/Android.bp index 901b7eeab9..392642d383 100644 --- a/audio/core/all-versions/default/Android.bp +++ b/audio/core/all-versions/default/Android.bp @@ -48,6 +48,8 @@ cc_defaults { "libhidlbase", "liblog", "libmedia_helper", + "libmediautils_vendor", + "libmemunreachable", "libutils", "android.hardware.audio.common-util", ], diff --git a/audio/core/all-versions/default/Device.cpp b/audio/core/all-versions/default/Device.cpp index 130dfba95b..c33e6f30fa 100644 --- a/audio/core/all-versions/default/Device.cpp +++ b/audio/core/all-versions/default/Device.cpp @@ -30,6 +30,8 @@ #include #include +#include +#include #include @@ -456,9 +458,32 @@ Return Device::debugDump(const hidl_handle& fd) { } #endif -Return Device::debug(const hidl_handle& fd, const hidl_vec& /* options */) { +Return Device::debug(const hidl_handle& fd, const hidl_vec& options) { if (fd.getNativeHandle() != nullptr && fd->numFds == 1) { - analyzeStatus("dump", mDevice->dump(mDevice, fd->data[0])); + const int fd0 = fd->data[0]; + bool dumpMem = false; + bool unreachableMemory = false; + for (const auto& option : options) { + if (option == "-m") { + dumpMem = true; + } else if (option == "--unreachable") { + unreachableMemory = true; + } + } + + if (dumpMem) { + dprintf(fd0, "\nDumping memory:\n"); + std::string s = dumpMemoryAddresses(100 /* limit */); + write(fd0, s.c_str(), s.size()); + } + if (unreachableMemory) { + dprintf(fd0, "\nDumping unreachable memory:\n"); + // TODO - should limit be an argument parameter? + std::string s = GetUnreachableMemoryString(true /* contents */, 100 /* limit */); + write(fd0, s.c_str(), s.size()); + } + + analyzeStatus("dump", mDevice->dump(mDevice, fd0)); } return Void(); } From 27946f0b00daf39f8770d67dc22321eabd849542 Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Tue, 1 Jun 2021 15:44:43 -0700 Subject: [PATCH 05/35] Support more types for set property cmd. Now support int64, float and bytes. This CL also allows setting read-only properties or reading write-only properties through cmdline. The cmdline debug interface is for test only and does not need to enforce these restrictions. Test: unit tests. Bug: b/189969547 Merged-In: Ib0e085f7a68e3d54782c21fb12caa500a0ad82ec Change-Id: Ib0e085f7a68e3d54782c21fb12caa500a0ad82ec (cherry picked from commit 8ae3ea981335007de5e837bcb64d3c917e1edcce) --- automotive/vehicle/2.0/default/Android.bp | 1 + .../include/vhal_v2_0/VehicleHalManager.h | 26 +- .../default/common/src/VehicleHalManager.cpp | 304 +++++++++++++----- .../default/tests/VehicleHalManager_test.cpp | 207 ++++++++++-- 4 files changed, 422 insertions(+), 116 deletions(-) diff --git a/automotive/vehicle/2.0/default/Android.bp b/automotive/vehicle/2.0/default/Android.bp index ffa0c13958..0b49ef9d2c 100644 --- a/automotive/vehicle/2.0/default/Android.bp +++ b/automotive/vehicle/2.0/default/Android.bp @@ -182,6 +182,7 @@ cc_test { ], shared_libs: [ "libbase", + "libcutils", ], header_libs: ["libbase_headers"], test_suites: ["general-tests"], diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h index fcfe7612ac..6706258717 100644 --- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h +++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h @@ -76,6 +76,9 @@ public: Return debug(const hidl_handle& fd, const hidl_vec& options) override; private: + // Set unit test class as friend class to test private functions. + friend class VehicleHalManagerTestHelper; + using VehiclePropValuePtr = VehicleHal::VehiclePropValuePtr; // Returns true if needs to call again shortly. using RetriableAction = std::function; @@ -105,14 +108,20 @@ public: void cmdDumpOneProperty(int fd, int32_t prop, int32_t areaId); void cmdDumpOneProperty(int fd, int rowNumber, const VehiclePropConfig& config); + bool cmdSetOneProperty(int fd, const hidl_vec& options); + static bool checkArgumentsSize(int fd, const hidl_vec& options, size_t minSize); static bool checkCallerHasWritePermissions(int fd); - static bool safelyParseInt(int fd, int index, std::string s, int* out); + template + static bool safelyParseInt(int fd, int index, const std::string& s, T* out); + static bool safelyParseFloat(int fd, int index, const std::string& s, float* out); + // Parses "s" as a hex string and populate "*bytes". The hex string must be in the format of + // valid hex format, e.g. "0xABCD". + static bool parseHexString(int fd, const std::string& s, std::vector* bytes); void cmdHelp(int fd) const; void cmdListAllProperties(int fd) const; void cmdDumpAllProperties(int fd); void cmdDumpSpecificProperties(int fd, const hidl_vec& options); - void cmdSetOneProperty(int fd, const hidl_vec& options); static bool isSubscribable(const VehiclePropConfig& config, SubscribeFlags flags); @@ -120,7 +129,18 @@ public: static float checkSampleRate(const VehiclePropConfig& config, float sampleRate); static ClientId getClientId(const sp& callback); -private: + + // Parses the cmdline options for "--set" command. "*prop" would be populated with the + // the properties to be set. Returns true when the cmdline options are valid, false otherwise. + static bool parseSetPropOptions(int fd, const hidl_vec& options, + VehiclePropValue* prop); + // Parses the options and get the values for the current option specified by "*index". "*index" + // would advance to the next option field (e.g., the next "-f"). Returns a list of values for + // the current option. + static std::vector getOptionValues(const hidl_vec& options, + size_t* index); + + private: VehicleHal* mHal; std::unique_ptr mConfigIndex; SubscriptionManager mSubscriptionManager; diff --git a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp index dc5d3d300e..e34e692bfc 100644 --- a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp +++ b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp @@ -20,7 +20,9 @@ #include #include +#include +#include #include #include #include @@ -44,15 +46,34 @@ using ::android::base::EqualsIgnoreCase; using ::android::hardware::hidl_handle; using ::android::hardware::hidl_string; +namespace { + constexpr std::chrono::milliseconds kHalEventBatchingTimeWindow(10); const VehiclePropValue kEmptyValue{}; +// A list of supported options for "--set" command. +const std::unordered_set kSetPropOptions = { + // integer. + "-i", + // 64bit integer. + "-i64", + // float. + "-f", + // string. + "-s", + // bytes in hex format, e.g. 0xDEADBEEF. + "-b", + // Area id in integer. + "-a"}; + +} // namespace + /** * Indicates what's the maximum size of hidl_vec we want * to store in reusable object pool. */ -constexpr auto kMaxHidlVecOfVehiclPropValuePoolSize = 20; +constexpr auto kMaxHidlVecOfVehiclePropValuePoolSize = 20; Return VehicleHalManager::getAllPropConfigs(getAllPropConfigs_cb _hidl_cb) { ALOGI("getAllPropConfigs called"); @@ -213,6 +234,11 @@ void VehicleHalManager::cmdDump(int fd, const hidl_vec& options) { } else if (EqualsIgnoreCase(option, "--get")) { cmdDumpSpecificProperties(fd, options); } else if (EqualsIgnoreCase(option, "--set")) { + if (!checkCallerHasWritePermissions(fd)) { + dprintf(fd, "Caller does not have write permission\n"); + return; + } + // Ignore the return value for this. cmdSetOneProperty(fd, options); } else { dprintf(fd, "Invalid option: %s\n", option.c_str()); @@ -239,7 +265,8 @@ bool VehicleHalManager::checkArgumentsSize(int fd, const hidl_vec& return false; } -bool VehicleHalManager::safelyParseInt(int fd, int index, std::string s, int* out) { +template +bool VehicleHalManager::safelyParseInt(int fd, int index, const std::string& s, T* out) { if (!android::base::ParseInt(s, out)) { dprintf(fd, "non-integer argument at index %d: %s\n", index, s.c_str()); return false; @@ -247,19 +274,27 @@ bool VehicleHalManager::safelyParseInt(int fd, int index, std::string s, int* ou return true; } +bool VehicleHalManager::safelyParseFloat(int fd, int index, const std::string& s, float* out) { + if (!android::base::ParseFloat(s, out)) { + dprintf(fd, "non-float argument at index %d: %s\n", index, s.c_str()); + return false; + } + return true; +} + void VehicleHalManager::cmdHelp(int fd) const { dprintf(fd, "Usage: \n\n"); dprintf(fd, "[no args]: dumps (id and value) all supported properties \n"); dprintf(fd, "--help: shows this help\n"); dprintf(fd, "--list: lists the ids of all supported properties\n"); dprintf(fd, "--get [PROP2] [PROPN]: dumps the value of specific properties \n"); - // TODO: support other formats (int64, float, bytes) dprintf(fd, - "--set [ ] [a AREA_ID] : sets the value of " - "property PROP, using arbitrary number of key/value parameters (i for int32, " - "s for string) and an optional area.\n" - "Notice that the string value can be set just once, while the other can have multiple " - "values (so they're used in the respective array)\n"); + "--set [-i INT_VALUE [INT_VALUE ...]] [-i64 INT64_VALUE [INT64_VALUE ...]] " + "[-f FLOAT_VALUE [FLOAT_VALUE ...]] [-s STR_VALUE] " + "[-b BYTES_VALUE] [-a AREA_ID] : sets the value of property PROP. " + "Notice that the string, bytes and area value can be set just once, while the other can" + " have multiple values (so they're used in the respective array), " + "BYTES_VALUE is in the form of 0xXXXX, e.g. 0xdeadbeef.\n"); } void VehicleHalManager::cmdListAllProperties(int fd) const { @@ -337,102 +372,49 @@ void VehicleHalManager::cmdDumpOneProperty(int fd, int32_t prop, int32_t areaId) VehiclePropValue input; input.prop = prop; input.areaId = areaId; - auto callback = [&](StatusCode status, const VehiclePropValue& output) { + auto callback = [&fd, &prop](StatusCode status, const VehiclePropValue& output) { if (status == StatusCode::OK) { dprintf(fd, "%s\n", toString(output).c_str()); } else { dprintf(fd, "Could not get property %d. Error: %s\n", prop, toString(status).c_str()); } }; - get(input, callback); + + StatusCode status; + auto value = mHal->get(input, &status); + callback(status, value.get() ? *value : kEmptyValue); } -void VehicleHalManager::cmdSetOneProperty(int fd, const hidl_vec& options) { - if (!checkCallerHasWritePermissions(fd) || !checkArgumentsSize(fd, options, 3)) return; - - size_t size = options.size(); - - // Syntax is --set PROP Type1 Value1 TypeN ValueN, so number of arguments must be even - if (size % 2 != 0) { - dprintf(fd, "must pass even number of arguments (passed %zu)\n", size); - return; +bool VehicleHalManager::cmdSetOneProperty(int fd, const hidl_vec& options) { + if (!checkArgumentsSize(fd, options, 4)) { + dprintf(fd, "Requires at least 4 options, see help\n"); + return false; } - int numberValues = (size - 2) / 2; - VehiclePropValue prop; - if (!safelyParseInt(fd, 1, options[1], &prop.prop)) return; - prop.timestamp = elapsedRealtimeNano(); - prop.status = VehiclePropertyStatus::AVAILABLE; - - // First pass: calculate sizes - int sizeInt32 = 0; - int stringIndex = 0; - int areaIndex = 0; - for (int i = 2, kv = 1; kv <= numberValues; kv++) { - // iterate through the kv=1..n key/value pairs, accessing indexes i / i+1 at each step - std::string type = options[i]; - std::string value = options[i + 1]; - if (EqualsIgnoreCase(type, "i")) { - sizeInt32++; - } else if (EqualsIgnoreCase(type, "s")) { - if (stringIndex != 0) { - dprintf(fd, - "defining string value (%s) again at index %d (already defined at %d=%s" - ")\n", - value.c_str(), i, stringIndex, options[stringIndex + 1].c_str()); - return; - } - stringIndex = i; - } else if (EqualsIgnoreCase(type, "a")) { - if (areaIndex != 0) { - dprintf(fd, - "defining area value (%s) again at index %d (already defined at %d=%s" - ")\n", - value.c_str(), i, areaIndex, options[areaIndex + 1].c_str()); - return; - } - areaIndex = i; - } else { - dprintf(fd, "invalid (%s) type at index %d\n", type.c_str(), i); - return; - } - i += 2; - } - prop.value.int32Values.resize(sizeInt32); - - // Second pass: populate it - int indexInt32 = 0; - for (int i = 2, kv = 1; kv <= numberValues; kv++) { - // iterate through the kv=1..n key/value pairs, accessing indexes i / i+1 at each step - int valueIndex = i + 1; - std::string type = options[i]; - std::string value = options[valueIndex]; - if (EqualsIgnoreCase(type, "i")) { - int safeInt; - if (!safelyParseInt(fd, valueIndex, value, &safeInt)) return; - prop.value.int32Values[indexInt32++] = safeInt; - } else if (EqualsIgnoreCase(type, "s")) { - prop.value.stringValue = value; - } else if (EqualsIgnoreCase(type, "a")) { - if (!safelyParseInt(fd, valueIndex, value, &prop.areaId)) return; - } - i += 2; + VehiclePropValue prop = {}; + if (!parseSetPropOptions(fd, options, &prop)) { + return false; } ALOGD("Setting prop %s", toString(prop).c_str()); - auto status = set(prop); + + // Do not use VehicleHalManager::set here because we don't want to check write permission. + // Caller should be able to use the debug interface to set read-only properties. + handlePropertySetEvent(prop); + auto status = mHal->set(prop); + if (status == StatusCode::OK) { dprintf(fd, "Set property %s\n", toString(prop).c_str()); - } else { - dprintf(fd, "Failed to set property %s: %s\n", toString(prop).c_str(), - toString(status).c_str()); + return true; } + dprintf(fd, "Failed to set property %s: %s\n", toString(prop).c_str(), + toString(status).c_str()); + return false; } void VehicleHalManager::init() { ALOGI("VehicleHalManager::init"); - mHidlVecOfVehiclePropValuePool.resize(kMaxHidlVecOfVehiclPropValuePoolSize); - + mHidlVecOfVehiclePropValuePool.resize(kMaxHidlVecOfVehiclePropValuePoolSize); mBatchingConsumer.run(&mEventQueue, kHalEventBatchingTimeWindow, @@ -486,7 +468,7 @@ void VehicleHalManager::onBatchHalEvent(const std::vector& for (const HalClientValues& cv : clientValues) { auto vecSize = cv.values.size(); hidl_vec vec; - if (vecSize < kMaxHidlVecOfVehiclPropValuePoolSize) { + if (vecSize < kMaxHidlVecOfVehiclePropValuePoolSize) { vec.setToExternal(&mHidlVecOfVehiclePropValuePool[0], vecSize); } else { vec.resize(vecSize); @@ -595,6 +577,158 @@ ClientId VehicleHalManager::getClientId(const sp& callback) { } } +std::vector VehicleHalManager::getOptionValues(const hidl_vec& options, + size_t* index) { + std::vector values; + while (*index < options.size()) { + std::string option = options[*index]; + if (kSetPropOptions.find(option) != kSetPropOptions.end()) { + return std::move(values); + } + values.push_back(option); + (*index)++; + } + return std::move(values); +} + +bool VehicleHalManager::parseSetPropOptions(int fd, const hidl_vec& options, + VehiclePropValue* prop) { + // Options format: + // --set PROP [-f f1 f2...] [-i i1 i2...] [-i64 i1 i2...] [-s s1 s2...] [-b b1 b2...] [-a a] + size_t optionIndex = 1; + int propValue; + if (!safelyParseInt(fd, optionIndex, options[optionIndex], &propValue)) { + dprintf(fd, "property value: \"%s\" is not a valid int\n", options[optionIndex].c_str()); + return false; + } + prop->prop = propValue; + prop->timestamp = elapsedRealtimeNano(); + prop->status = VehiclePropertyStatus::AVAILABLE; + optionIndex++; + std::unordered_set parsedOptions; + + while (optionIndex < options.size()) { + std::string type = options[optionIndex]; + optionIndex++; + size_t currentIndex = optionIndex; + std::vector values = getOptionValues(options, &optionIndex); + if (parsedOptions.find(type) != parsedOptions.end()) { + dprintf(fd, "duplicate \"%s\" options\n", type.c_str()); + return false; + } + parsedOptions.insert(type); + if (EqualsIgnoreCase(type, "-i")) { + if (values.size() == 0) { + dprintf(fd, "no values specified when using \"-i\"\n"); + return false; + } + prop->value.int32Values.resize(values.size()); + for (size_t i = 0; i < values.size(); i++) { + int32_t safeInt; + if (!safelyParseInt(fd, currentIndex + i, values[i], &safeInt)) { + dprintf(fd, "value: \"%s\" is not a valid int\n", values[i].c_str()); + return false; + } + prop->value.int32Values[i] = safeInt; + } + } else if (EqualsIgnoreCase(type, "-i64")) { + if (values.size() == 0) { + dprintf(fd, "no values specified when using \"-i64\"\n"); + return false; + } + prop->value.int64Values.resize(values.size()); + for (size_t i = 0; i < values.size(); i++) { + int64_t safeInt; + if (!safelyParseInt(fd, currentIndex + i, values[i], &safeInt)) { + dprintf(fd, "value: \"%s\" is not a valid int64\n", values[i].c_str()); + return false; + } + prop->value.int64Values[i] = safeInt; + } + } else if (EqualsIgnoreCase(type, "-f")) { + if (values.size() == 0) { + dprintf(fd, "no values specified when using \"-f\"\n"); + return false; + } + prop->value.floatValues.resize(values.size()); + for (size_t i = 0; i < values.size(); i++) { + float safeFloat; + if (!safelyParseFloat(fd, currentIndex + i, values[i], &safeFloat)) { + dprintf(fd, "value: \"%s\" is not a valid float\n", values[i].c_str()); + return false; + } + prop->value.floatValues[i] = safeFloat; + } + } else if (EqualsIgnoreCase(type, "-s")) { + if (values.size() != 1) { + dprintf(fd, "expect exact one value when using \"-s\"\n"); + return false; + } + prop->value.stringValue = values[0]; + } else if (EqualsIgnoreCase(type, "-b")) { + if (values.size() != 1) { + dprintf(fd, "expect exact one value when using \"-b\"\n"); + return false; + } + std::vector bytes; + if (!parseHexString(fd, values[0], &bytes)) { + dprintf(fd, "value: \"%s\" is not a valid hex string\n", values[0].c_str()); + return false; + } + prop->value.bytes = bytes; + } else if (EqualsIgnoreCase(type, "-a")) { + if (values.size() != 1) { + dprintf(fd, "expect exact one value when using \"-a\"\n"); + return false; + } + if (!safelyParseInt(fd, currentIndex, values[0], &(prop->areaId))) { + dprintf(fd, "area ID: \"%s\" is not a valid int\n", values[0].c_str()); + return false; + } + } else { + dprintf(fd, "unknown option: %s\n", type.c_str()); + return false; + } + } + + return true; +} + +bool VehicleHalManager::parseHexString(int fd, const std::string& s, std::vector* bytes) { + if (s.size() % 2 != 0) { + dprintf(fd, "invalid hex string: %s, should have even size\n", s.c_str()); + return false; + } + if (strncmp(s.substr(0, 2).c_str(), "0x", 2)) { + dprintf(fd, "hex string should start with \"0x\", got %s\n", s.c_str()); + return false; + } + std::string subs = s.substr(2); + std::transform(subs.begin(), subs.end(), subs.begin(), + [](unsigned char c) { return std::tolower(c); }); + + bool highDigit = true; + for (size_t i = 0; i < subs.size(); i++) { + char c = subs[i]; + uint8_t v; + if (c >= '0' && c <= '9') { + v = c - '0'; + } else if (c >= 'a' && c <= 'f') { + v = c - 'a' + 10; + } else { + dprintf(fd, "invalid character %c in hex string %s\n", c, subs.c_str()); + return false; + } + if (highDigit) { + (*bytes).push_back(v * 16); + } else { + (*bytes)[bytes->size() - 1] += v; + } + highDigit = !highDigit; + } + return true; +} + } // namespace V2_0 } // namespace vehicle } // namespace automotive diff --git a/automotive/vehicle/2.0/default/tests/VehicleHalManager_test.cpp b/automotive/vehicle/2.0/default/tests/VehicleHalManager_test.cpp index 09750718a0..bdf46fb24c 100644 --- a/automotive/vehicle/2.0/default/tests/VehicleHalManager_test.cpp +++ b/automotive/vehicle/2.0/default/tests/VehicleHalManager_test.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -32,6 +33,18 @@ namespace automotive { namespace vehicle { namespace V2_0 { +// A simple helper class to expose 'cmdSetOneProperty' to the unit tests. +class VehicleHalManagerTestHelper { + public: + VehicleHalManagerTestHelper(VehicleHalManager* manager) { mManager = manager; } + bool cmdSetOneProperty(int fd, const hidl_vec& options) { + return mManager->cmdSetOneProperty(fd, options); + } + + public: + VehicleHalManager* mManager; +}; + namespace { using namespace std::placeholders; @@ -57,33 +70,21 @@ public: auto property = static_cast(requestedPropValue.prop); int32_t areaId = requestedPropValue.areaId; - switch (property) { - case VehicleProperty::INFO_MAKE: - pValue = getValuePool()->obtainString(kCarMake); - break; - case VehicleProperty::INFO_FUEL_CAPACITY: - if (fuelCapacityAttemptsLeft-- > 0) { - // Emulate property not ready yet. - *outStatus = StatusCode::TRY_AGAIN; - } else { - pValue = getValuePool()->obtainFloat(42.42); - } - break; - default: - if (requestedPropValue.prop == kCustomComplexProperty) { - pValue = getValuePool()->obtainComplex(); - pValue->value.int32Values = hidl_vec { 10, 20 }; - pValue->value.int64Values = hidl_vec { 30, 40 }; - pValue->value.floatValues = hidl_vec { 1.1, 2.2 }; - pValue->value.bytes = hidl_vec { 1, 2, 3 }; - pValue->value.stringValue = kCarMake; - break; - } - auto key = makeKey(toInt(property), areaId); - if (mValues.count(key) == 0) { - ALOGW(""); - } - pValue = getValuePool()->obtain(mValues[key]); + if (property == VehicleProperty::INFO_FUEL_CAPACITY) { + if (fuelCapacityAttemptsLeft-- > 0) { + // Emulate property not ready yet. + *outStatus = StatusCode::TRY_AGAIN; + } else { + pValue = getValuePool()->obtainFloat(42.42); + } + } else { + auto key = makeKey(requestedPropValue); + if (mValues.count(key) == 0) { + ALOGW("key not found\n"); + *outStatus = StatusCode::INVALID_ARG; + return pValue; + } + pValue = getValuePool()->obtain(mValues[key]); } if (*outStatus == StatusCode::OK && pValue.get() != nullptr) { @@ -100,7 +101,6 @@ public: && mirrorFoldAttemptsLeft-- > 0) { return StatusCode::TRY_AGAIN; } - mValues[makeKey(propValue)] = propValue; return StatusCode::OK; } @@ -181,6 +181,18 @@ public: actualStatusCode = refStatus; } + MockedVehicleHal::VehiclePropValuePtr getComplexProperty() { + auto pValue = objectPool->obtainComplex(); + pValue->prop = kCustomComplexProperty; + pValue->areaId = 0; + pValue->value.int32Values = hidl_vec{10, 20}; + pValue->value.int64Values = hidl_vec{30, 40}; + pValue->value.floatValues = hidl_vec{1.1, 2.2}; + pValue->value.bytes = hidl_vec{1, 2, 3}; + pValue->value.stringValue = kCarMake; + return pValue; + } + public: VehiclePropValue actualValue; StatusCode actualStatusCode; @@ -308,6 +320,8 @@ TEST_F(VehicleHalManagerTest, subscribe_WriteOnly) { } TEST_F(VehicleHalManagerTest, get_Complex) { + ASSERT_EQ(StatusCode::OK, hal->set(*getComplexProperty().get())); + invokeGet(kCustomComplexProperty, 0); ASSERT_EQ(StatusCode::OK, actualStatusCode); @@ -334,6 +348,11 @@ TEST_F(VehicleHalManagerTest, get_Complex) { } TEST_F(VehicleHalManagerTest, get_StaticString) { + auto pValue = objectPool->obtainString(kCarMake); + pValue->prop = toInt(VehicleProperty::INFO_MAKE); + pValue->areaId = 0; + ASSERT_EQ(StatusCode::OK, hal->set(*pValue.get())); + invokeGet(toInt(VehicleProperty::INFO_MAKE), 0); ASSERT_EQ(StatusCode::OK, actualStatusCode); @@ -458,6 +477,138 @@ TEST(HalClientVectorTest, basic) { ASSERT_TRUE(clients.isEmpty()); } +TEST_F(VehicleHalManagerTest, debug) { + hidl_handle fd = {}; + fd.setTo(native_handle_create(/*numFds=*/1, /*numInts=*/0), /*shouldOwn=*/true); + + // Because debug function returns void, so no way to check return value. + manager->debug(fd, {}); + manager->debug(fd, {"--help"}); + manager->debug(fd, {"--list"}); + manager->debug(fd, {"--get"}); + manager->debug(fd, {"--set"}); + manager->debug(fd, {"invalid"}); +} + +struct SetPropTestCase { + std::string test_name; + const hidl_vec configs; + bool success; +}; + +class VehicleHalManagerSetPropTest : public VehicleHalManagerTest, + public testing::WithParamInterface {}; + +TEST_P(VehicleHalManagerSetPropTest, cmdSetOneProperty) { + const SetPropTestCase& tc = GetParam(); + VehicleHalManagerTestHelper helper(manager.get()); + ASSERT_EQ(tc.success, helper.cmdSetOneProperty(STDERR_FILENO, tc.configs)); +} + +std::vector GenSetPropParams() { + char infoMakeProperty[100] = {}; + snprintf(infoMakeProperty, sizeof(infoMakeProperty), "%d", toInt(VehicleProperty::INFO_MAKE)); + return { + {"success_set_string", {"--set", infoMakeProperty, "-s", kCarMake}, true}, + {"success_set_bytes", {"--set", infoMakeProperty, "-b", "0xdeadbeef"}, true}, + {"success_set_bytes_caps", {"--set", infoMakeProperty, "-b", "0xDEADBEEF"}, true}, + {"success_set_int", {"--set", infoMakeProperty, "-i", "2147483647"}, true}, + {"success_set_ints", + {"--set", infoMakeProperty, "-i", "2147483647", "0", "-2147483648"}, + true}, + {"success_set_int64", + {"--set", infoMakeProperty, "-i64", "-9223372036854775808"}, + true}, + {"success_set_int64s", + {"--set", infoMakeProperty, "-i64", "-9223372036854775808", "0", + "9223372036854775807"}, + true}, + {"success_set_float", {"--set", infoMakeProperty, "-f", "1.175494351E-38"}, true}, + {"success_set_floats", + {"--set", infoMakeProperty, "-f", "-3.402823466E+38", "0", "3.402823466E+38"}, + true}, + {"success_set_area", {"--set", infoMakeProperty, "-a", "2147483647"}, true}, + {"fail_no_options", {}, false}, + {"fail_less_than_4_options", {"--set", infoMakeProperty, "-i"}, false}, + {"fail_unknown_options", {"--set", infoMakeProperty, "-s", kCarMake, "-abcd"}, false}, + {"fail_invalid_property", {"--set", "not valid", "-s", kCarMake}, false}, + {"fail_duplicate_string", + {"--set", infoMakeProperty, "-s", kCarMake, "-s", kCarMake}, + false}, + {"fail_multiple_strings", {"--set", infoMakeProperty, "-s", kCarMake, kCarMake}, false}, + {"fail_no_string_value", {"--set", infoMakeProperty, "-s", "-a", "1234"}, false}, + {"fail_duplicate_bytes", + {"--set", infoMakeProperty, "-b", "0xdeadbeef", "-b", "0xdeadbeef"}, + false}, + {"fail_multiple_bytes", + {"--set", infoMakeProperty, "-b", "0xdeadbeef", "0xdeadbeef"}, + false}, + {"fail_invalid_bytes", {"--set", infoMakeProperty, "-b", "0xgood"}, false}, + {"fail_invalid_bytes_no_prefix", {"--set", infoMakeProperty, "-b", "deadbeef"}, false}, + {"fail_invalid_int", {"--set", infoMakeProperty, "-i", "abc"}, false}, + {"fail_int_out_of_range", {"--set", infoMakeProperty, "-i", "2147483648"}, false}, + {"fail_no_int_value", {"--set", infoMakeProperty, "-i", "-s", kCarMake}, false}, + {"fail_invalid_int64", {"--set", infoMakeProperty, "-i64", "abc"}, false}, + {"fail_int64_out_of_range", + {"--set", infoMakeProperty, "-i64", "-9223372036854775809"}, + false}, + {"fail_no_int64_value", {"--set", infoMakeProperty, "-i64", "-s", kCarMake}, false}, + {"fail_invalid_float", {"--set", infoMakeProperty, "-f", "abc"}, false}, + {"fail_float_out_of_range", + {"--set", infoMakeProperty, "-f", "-3.402823466E+39"}, + false}, + {"fail_no_float_value", {"--set", infoMakeProperty, "-f", "-s", kCarMake}, false}, + {"fail_multiple_areas", {"--set", infoMakeProperty, "-a", "2147483648", "0"}, false}, + {"fail_invalid_area", {"--set", infoMakeProperty, "-a", "abc"}, false}, + {"fail_area_out_of_range", {"--set", infoMakeProperty, "-a", "2147483648"}, false}, + {"fail_no_area_value", {"--set", infoMakeProperty, "-a", "-s", kCarMake}, false}, + }; +} + +INSTANTIATE_TEST_SUITE_P( + VehicleHalManagerSetPropTests, VehicleHalManagerSetPropTest, + testing::ValuesIn(GenSetPropParams()), + [](const testing::TestParamInfo& info) { + return info.param.test_name; + }); + +TEST_F(VehicleHalManagerTest, SetComplexPropTest) { + char infoMakeProperty[100] = {}; + snprintf(infoMakeProperty, sizeof(infoMakeProperty), "%d", toInt(VehicleProperty::INFO_MAKE)); + VehicleHalManagerTestHelper helper(manager.get()); + ASSERT_TRUE(helper.cmdSetOneProperty( + STDERR_FILENO, {"--set", infoMakeProperty, "-s", kCarMake, + "-b", "0xdeadbeef", "-i", "2147483647", + "0", "-2147483648", "-i64", "-9223372036854775808", + "0", "9223372036854775807", "-f", "-3.402823466E+38", + "0", "3.402823466E+38", "-a", "123"})); + StatusCode status = StatusCode::OK; + VehiclePropValue requestProp; + requestProp.prop = toInt(VehicleProperty::INFO_MAKE); + requestProp.areaId = 123; + auto value = hal->get(requestProp, &status); + ASSERT_EQ(StatusCode::OK, status); + ASSERT_EQ(value->prop, toInt(VehicleProperty::INFO_MAKE)); + ASSERT_EQ(value->areaId, 123); + ASSERT_STREQ(kCarMake, value->value.stringValue.c_str()); + uint8_t bytes[] = {0xde, 0xad, 0xbe, 0xef}; + ASSERT_FALSE(memcmp(bytes, value->value.bytes.data(), sizeof(bytes))); + ASSERT_EQ(3u, value->value.int32Values.size()); + ASSERT_EQ(2147483647, value->value.int32Values[0]); + ASSERT_EQ(0, value->value.int32Values[1]); + ASSERT_EQ(-2147483648, value->value.int32Values[2]); + ASSERT_EQ(3u, value->value.int64Values.size()); + // -9223372036854775808 is not a valid literal since '-' and '9223372036854775808' would be two + // tokens and the later does not fit in unsigned long long. + ASSERT_EQ(-9223372036854775807 - 1, value->value.int64Values[0]); + ASSERT_EQ(0, value->value.int64Values[1]); + ASSERT_EQ(9223372036854775807, value->value.int64Values[2]); + ASSERT_EQ(3u, value->value.floatValues.size()); + ASSERT_EQ(-3.402823466E+38f, value->value.floatValues[0]); + ASSERT_EQ(0.0f, value->value.floatValues[1]); + ASSERT_EQ(3.402823466E+38f, value->value.floatValues[2]); +} + } // namespace anonymous } // namespace V2_0 From b22a8678f8d36fbfb65cd9e98483f3e6207ffe42 Mon Sep 17 00:00:00 2001 From: Kai Date: Thu, 5 Aug 2021 14:02:33 -0700 Subject: [PATCH 06/35] Add placeholder properties for emulator Add some placeholder properties in google VHAL. Developers who use the aae emulator can use them in developing new features without implementing a real property. Bug: 193460353 Test: build aae emulator, use adb lshal to change property values Change-Id: I6131a4892495c4c1e1c73e078572a666a33f984e Merged-In: I6131a4892495c4c1e1c73e078572a666a33f984e (cherry picked from commit 4d1fe81bee2a1ef01b1a14638484a2d7528afcc3) --- .../default/impl/vhal_v2_0/DefaultConfig.h | 36 +++++++++++++++++++ .../default/impl/vhal_v2_0/PropertyUtils.h | 13 +++++++ 2 files changed, 49 insertions(+) diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h index 7588f7001d..a3e7606189 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h @@ -1097,6 +1097,42 @@ const ConfigDeclaration kVehicleProperties[]{ .changeMode = VehiclePropertyChangeMode::ON_CHANGE, }, }, + { + .config = + { + .prop = PLACEHOLDER_PROPERTY_INT, + .access = VehiclePropertyAccess::READ_WRITE, + .changeMode = VehiclePropertyChangeMode::ON_CHANGE, + }, + .initialValue = {.int32Values = {0}}, + }, + { + .config = + { + .prop = PLACEHOLDER_PROPERTY_FLOAT, + .access = VehiclePropertyAccess::READ_WRITE, + .changeMode = VehiclePropertyChangeMode::ON_CHANGE, + }, + .initialValue = {.floatValues = {0.0f}}, + }, + { + .config = + { + .prop = PLACEHOLDER_PROPERTY_BOOLEAN, + .access = VehiclePropertyAccess::READ_WRITE, + .changeMode = VehiclePropertyChangeMode::ON_CHANGE, + }, + .initialValue = {.int32Values = {0 /* false */}}, + }, + { + .config = + { + .prop = PLACEHOLDER_PROPERTY_STRING, + .access = VehiclePropertyAccess::READ_WRITE, + .changeMode = VehiclePropertyChangeMode::ON_CHANGE, + }, + .initialValue = {.stringValue = {"Test"}}, + }, #ifdef ENABLE_VENDOR_CLUSTER_PROPERTY_FOR_TESTING // Vendor propetry for E2E ClusterHomeService testing. { diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/PropertyUtils.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/PropertyUtils.h index d5f6a1841e..51251a7e70 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/PropertyUtils.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/PropertyUtils.h @@ -189,6 +189,19 @@ enum class FakeDataCommand : int32_t { KeyPress = 100, }; +/** + * These properties are placeholder properties for developers to test new features without + * implementing a real property. + */ +constexpr int32_t PLACEHOLDER_PROPERTY_INT = + 0x2a11 | VehiclePropertyGroup::VENDOR | VehicleArea::GLOBAL | VehiclePropertyType::INT32; +constexpr int32_t PLACEHOLDER_PROPERTY_FLOAT = + 0x2a11 | VehiclePropertyGroup::VENDOR | VehicleArea::GLOBAL | VehiclePropertyType::FLOAT; +constexpr int32_t PLACEHOLDER_PROPERTY_BOOLEAN = + 0x2a11 | VehiclePropertyGroup::VENDOR | VehicleArea::GLOBAL | VehiclePropertyType::BOOLEAN; +constexpr int32_t PLACEHOLDER_PROPERTY_STRING = + 0x2a11 | VehiclePropertyGroup::VENDOR | VehicleArea::GLOBAL | VehiclePropertyType::STRING; + const int32_t kHvacPowerProperties[] = { toInt(VehicleProperty::HVAC_FAN_SPEED), toInt(VehicleProperty::HVAC_FAN_DIRECTION), From 58880c783c11d28047681927633e38fbc638ff5c Mon Sep 17 00:00:00 2001 From: Roman Kiryanov Date: Thu, 19 Aug 2021 17:59:03 -0700 Subject: [PATCH 07/35] Provide defaults for android.hardware.audio@7.0-impl see aosp/1366502. Bug: 196868480 Bug: 161485545 Test: presubmit Signed-off-by: Roman Kiryanov Change-Id: Ie582038f2212fbf881497ed3db62ef04bac286f3 Merged-In: Ie582038f2212fbf881497ed3db62ef04bac286f3 --- audio/core/all-versions/default/Android.bp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/audio/core/all-versions/default/Android.bp b/audio/core/all-versions/default/Android.bp index 392642d383..b170ff4d42 100644 --- a/audio/core/all-versions/default/Android.bp +++ b/audio/core/all-versions/default/Android.bp @@ -140,8 +140,8 @@ cc_library_shared { defaults: ["android.hardware.audio@6.0-impl_default"], } -cc_library_shared { - name: "android.hardware.audio@7.0-impl", +cc_defaults { + name: "android.hardware.audio@7.0-impl_default", defaults: ["android.hardware.audio-impl_default"], shared_libs: [ "android.hardware.audio@7.0", @@ -157,3 +157,8 @@ cc_library_shared { "-include common/all-versions/VersionMacro.h", ], } + +cc_library_shared { + name: "android.hardware.audio@7.0-impl", + defaults: ["android.hardware.audio@7.0-impl_default"], +} From 1c2eaeae770663dec5b7a81d0585cf6f3af6d733 Mon Sep 17 00:00:00 2001 From: Yuchen He Date: Mon, 23 Aug 2021 16:40:24 +0000 Subject: [PATCH 08/35] Report default location when location is not available in /dev/gnss0 Test: atest VtsHalGnssTargetTest Bug: 197579774 Bug: 197825053 Change-Id: I08e761d6023df2954d8a265b4af3e5b5d0aec09c (cherry picked from commit 1aac7fa45c69ef13f93f3f79f28c5c6424b22e99) --- gnss/common/utils/default/include/v2_1/GnssTemplate.h | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/gnss/common/utils/default/include/v2_1/GnssTemplate.h b/gnss/common/utils/default/include/v2_1/GnssTemplate.h index 131af24fbe..48cab99a10 100644 --- a/gnss/common/utils/default/include/v2_1/GnssTemplate.h +++ b/gnss/common/utils/default/include/v2_1/GnssTemplate.h @@ -223,14 +223,8 @@ Return GnssTemplate::start() { this->reportSvStatus(svStatus); auto currentLocation = getLocationFromHW(); notePowerConsumption(); - if (mGnssFd != -1) { - // Only report location if the return from hardware is valid - // note that we can not merge these two "if" together, if didn't - // get location from hardware, we shouldn't report location, not - // report the "default" one. - if (currentLocation != nullptr) { - this->reportLocation(*currentLocation); - } + if (currentLocation != nullptr) { + this->reportLocation(*currentLocation); } else { if (sGnssCallback_2_1 != nullptr || sGnssCallback_2_0 != nullptr) { const auto location = Utils::getMockLocationV2_0(); From e509bf1f76521ee47e69342f3662e049b0c4db2b Mon Sep 17 00:00:00 2001 From: Kai Wang Date: Fri, 17 Sep 2021 22:43:44 +0000 Subject: [PATCH 09/35] Add areaId for CRITICALLY_LOW_TIRE_PRESSURE CRITICALLY_LOW_TIRE_PRESSURE is a seat type property. Added areaId into the property config. Bug: 174585018 Test: atest AtsCarHostTests Change-Id: I260b9e79844f44cad31efcf510301af0ee6227a1 --- .../2.0/default/impl/vhal_v2_0/DefaultConfig.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h index ca080d81ec..aa945142ba 100644 --- a/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h +++ b/automotive/vehicle/2.0/default/impl/vhal_v2_0/DefaultConfig.h @@ -307,6 +307,18 @@ const ConfigDeclaration kVehicleProperties[]{ .prop = toInt(VehicleProperty::CRITICALLY_LOW_TIRE_PRESSURE), .access = VehiclePropertyAccess::READ, .changeMode = VehiclePropertyChangeMode::STATIC, + .areaConfigs = {VehicleAreaConfig{ + .areaId = WHEEL_FRONT_LEFT, + }, + VehicleAreaConfig{ + .areaId = WHEEL_FRONT_RIGHT, + }, + VehicleAreaConfig{ + .areaId = WHEEL_REAR_LEFT, + }, + VehicleAreaConfig{ + .areaId = WHEEL_REAR_RIGHT, + }}, }, .initialAreaValues = {{WHEEL_FRONT_LEFT, {.floatValues = {137.0f}}}, {WHEEL_FRONT_RIGHT, {.floatValues = {137.0f}}}, From 3387f1c84e375860b2bd921792cd2763ad435aec Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Wed, 29 Sep 2021 15:57:35 -0700 Subject: [PATCH 10/35] Camera: Add static metadata that can map device state to orientation Allow camera providers to advertise the mapping between device state and camera orientation. Bug: 201005727 Test: VtsHalCameraProviderV2_4TargetTest --gtest_filter=PerInstance/CameraHidlTest.getCameraCharacteristics/0_internal_0 Change-Id: Ibb035f4dc3d8af1106ac08f86e43b953ddcbf55b --- camera/metadata/3.7/types.hal | 50 +++++++++++++++++++ .../VtsHalCameraProviderV2_4TargetTest.cpp | 14 ++++++ current.txt | 3 ++ 3 files changed, 67 insertions(+) create mode 100644 camera/metadata/3.7/types.hal diff --git a/camera/metadata/3.7/types.hal b/camera/metadata/3.7/types.hal new file mode 100644 index 0000000000..a09bdf9977 --- /dev/null +++ b/camera/metadata/3.7/types.hal @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2021 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. + */ + +/* + * Autogenerated from camera metadata definitions in + * /system/media/camera/docs/metadata_definitions.xml + * *** DO NOT EDIT BY HAND *** + */ + +package android.hardware.camera.metadata@3.7; + +import android.hardware.camera.metadata@3.2; +import android.hardware.camera.metadata@3.3; +import android.hardware.camera.metadata@3.4; +import android.hardware.camera.metadata@3.5; +import android.hardware.camera.metadata@3.6; + +// No new metadata sections added in this revision + +/** + * Main enumeration for defining camera metadata tags added in this revision + * + *

Partial documentation is included for each tag; for complete documentation, reference + * '/system/media/camera/docs/docs.html' in the corresponding Android source tree.

+ */ +enum CameraMetadataTag : @3.6::CameraMetadataTag { + /** android.info.deviceStateOrientations [static, int64[], ndk_public] + */ + ANDROID_INFO_DEVICE_STATE_ORIENTATIONS = android.hardware.camera.metadata@3.4::CameraMetadataTag:ANDROID_INFO_END_3_4, + + ANDROID_INFO_END_3_7, + +}; + +/* + * Enumeration definitions for the various entries that need them + */ diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index 7727547e7d..ad3da48aab 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -8161,6 +8161,20 @@ void CameraHidlTest::verifyCameraCharacteristics(Status status, const CameraMeta poseReference >= ANDROID_LENS_POSE_REFERENCE_PRIMARY_CAMERA); } + retcode = find_camera_metadata_ro_entry(metadata, + ANDROID_INFO_DEVICE_STATE_ORIENTATIONS, &entry); + if (0 == retcode && entry.count > 0) { + ASSERT_TRUE((entry.count % 2) == 0); + uint64_t maxPublicState = ((uint64_t) provider::V2_5::DeviceState::FOLDED) << 1; + uint64_t vendorStateStart = 1UL << 31; // Reserved for vendor specific states + uint64_t stateMask = (1 << vendorStateStart) - 1; + stateMask &= ~((1 << maxPublicState) - 1); + for (int i = 0; i < entry.count; i += 2){ + ASSERT_TRUE((entry.data.i64[i] & stateMask) == 0); + ASSERT_TRUE((entry.data.i64[i+1] % 90) == 0); + } + } + verifyExtendedSceneModeCharacteristics(metadata); verifyZoomCharacteristics(metadata); } diff --git a/current.txt b/current.txt index e70aef0c57..4841c97f4c 100644 --- a/current.txt +++ b/current.txt @@ -900,4 +900,7 @@ c8a57364f6ad20842be14f4db284df5304f7521ca8eac6bcc1fa6c5b466fb8a6 android.hardwar 0821f516e4d428bc15251969f7e19411c94d8f2ccbd99e1fc8168d8e49e38b0f android.hardware.wifi.supplicant@1.4::types 4a087a308608d146b022ebc15633de989f5f4dfe1491a83fa41763290a82e40d android.hardware.automotive.vehicle@2.0::types +# HALs released in Android SCv2 +77f6fcf3fd0dd3e424d8a0292094ebd17e4c35454bb9abbd3a6cbed1aba70765 android.hardware.camera.metadata@3.7::types + # There should be no more HIDL HALs - please use AIDL instead. From e0f7bd7197969d7b4bc0b05a9b80a8bd0fe27765 Mon Sep 17 00:00:00 2001 From: Jack Nudelman Date: Tue, 28 Sep 2021 14:22:20 -0700 Subject: [PATCH 11/35] Accept NONE as a response for setDataThrottling during VTS. Bug: 199809900 Test: b/199809900 Change-Id: I3b71401c28eb2f06c020f549a28ef5970282c855 --- radio/1.6/vts/functional/radio_hidl_hal_api.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp index f8f5cffd73..d61853b697 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp +++ b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp @@ -515,7 +515,8 @@ TEST_P(RadioHidlTest_v1_6, setDataThrottling) { if (getRadioHalCapabilities()) { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); + {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NONE})); } else { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, @@ -537,7 +538,8 @@ TEST_P(RadioHidlTest_v1_6, setDataThrottling) { if (getRadioHalCapabilities()) { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); + {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NONE})); } else { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, @@ -559,7 +561,8 @@ TEST_P(RadioHidlTest_v1_6, setDataThrottling) { if (getRadioHalCapabilities()) { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); + {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NONE})); } else { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, @@ -580,7 +583,8 @@ TEST_P(RadioHidlTest_v1_6, setDataThrottling) { if (getRadioHalCapabilities()) { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); + {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NONE})); } else { ASSERT_TRUE(CheckAnyOfErrors( radioRsp_v1_6->rspInfo.error, From 0e6752c900fcadfde62b38e8111543a1184b25b0 Mon Sep 17 00:00:00 2001 From: Keith Mok Date: Wed, 13 Oct 2021 00:53:27 +0000 Subject: [PATCH 12/35] Fix fuzzer error for FormatConvertFuzzer The stride was not correct for the fuzzer. And the FormatConvert have some restructions on the width and height. - YUYV width and height must be even nmber - YU12 width mush be divisible by 16 height must be even number Bug: 202641239 Test: FormatConvertFuzzer_copyYV12toBGR32 Change-Id: I45ebea3e22854bdad037abb742fbdbe364b19ec5 --- .../default/test/fuzz/FormatConvertFuzzer.cpp | 42 +++++++++++-------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/automotive/evs/common/utils/default/test/fuzz/FormatConvertFuzzer.cpp b/automotive/evs/common/utils/default/test/fuzz/FormatConvertFuzzer.cpp index 583a455160..58423c8187 100644 --- a/automotive/evs/common/utils/default/test/fuzz/FormatConvertFuzzer.cpp +++ b/automotive/evs/common/utils/default/test/fuzz/FormatConvertFuzzer.cpp @@ -14,6 +14,7 @@ * limitations under the License. */ +#include #include #include #include @@ -21,36 +22,43 @@ #include "FormatConvert.h" extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, std::size_t size) { - if (size < 256) { + // 1 random value (4bytes) + min imagesize = 16*2 times bytes per pixel (worse case 2) + if (size < (4 + 16 * 2 * 2)) { return 0; } + FuzzedDataProvider fdp(data, size); + std::size_t image_pixel_size = size - 4; + image_pixel_size = (image_pixel_size & INT_MAX) / 2; - std::srand(std::time(nullptr)); // use current time as seed for random generator - int random_variable = std::rand() % 10; - int width = (int)sqrt(size); - int height = width * ((float)random_variable / 10.0); + // API have a requirement that width must be divied by 16 except yuyvtorgb + int min_height = 2; + int max_height = (image_pixel_size / 16) & ~(1); // must be even number + int height = fdp.ConsumeIntegralInRange(min_height, max_height); + int width = (image_pixel_size / height) & ~(16); // must be divisible by 16 - uint8_t* src = (uint8_t*)malloc(sizeof(uint8_t) * size); - memcpy(src, data, sizeof(uint8_t) * (size)); - uint32_t* tgt = (uint32_t*)malloc(sizeof(uint32_t) * size); + uint8_t* src = (uint8_t*)(data + 4); + uint32_t* tgt = (uint32_t*)malloc(sizeof(uint32_t) * image_pixel_size); #ifdef COPY_NV21_TO_RGB32 - android::hardware::automotive::evs::common::Utils::copyNV21toRGB32(width, height, src, tgt, 0); + android::hardware::automotive::evs::common::Utils::copyNV21toRGB32(width, height, src, tgt, + width); #elif COPY_NV21_TO_BGR32 - android::hardware::automotive::evs::common::Utils::copyNV21toBGR32(width, height, src, tgt, 0); + android::hardware::automotive::evs::common::Utils::copyNV21toBGR32(width, height, src, tgt, + width); #elif COPY_YV12_TO_RGB32 - android::hardware::automotive::evs::common::Utils::copyYV12toRGB32(width, height, src, tgt, 0); + android::hardware::automotive::evs::common::Utils::copyYV12toRGB32(width, height, src, tgt, + width); #elif COPY_YV12_TO_BGR32 - android::hardware::automotive::evs::common::Utils::copyYV12toBGR32(width, height, src, tgt, 0); + android::hardware::automotive::evs::common::Utils::copyYV12toBGR32(width, height, src, tgt, + width); #elif COPY_YUYV_TO_RGB32 - android::hardware::automotive::evs::common::Utils::copyYUYVtoRGB32(width, height, src, 0, tgt, - 0); + android::hardware::automotive::evs::common::Utils::copyYUYVtoRGB32(width, height, src, width, + tgt, width); #elif COPY_YUYV_TO_BGR32 - android::hardware::automotive::evs::common::Utils::copyYUYVtoBGR32(width, height, src, 0, tgt, - 0); + android::hardware::automotive::evs::common::Utils::copyYUYVtoBGR32(width, height, src, width, + tgt, width); #endif - free(src); free(tgt); return 0; From 82b41e6af9d0b83bcb2349a35e7632f9c7d5ffae Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Wed, 16 Sep 2020 13:51:11 -0700 Subject: [PATCH 13/35] wifi(vts): Precondition AP tests on existence of hostapd HAL cp: aosp/1428863 to sc-v2-dev branch as b/202788915 mentioned This used to be present on the old host side VTS tests, port the feature to target side since host side VTS tests are deprecated. Also, i) Added a separate test: VtsHalWifiV1_4TargetTest to test the wifi chip methods. Putting them in VtsHalWifiApV1_4TargetTest will prevent these from running on devices without AP feature. ii) Ensured all the non-NAN, non-RTT tests disable framework for testing. NAN/RTT tests uses framework to check if the corresponding package manager feature exists. Bug: 166529516 Bug: 202788915 Test: atest \ VtsHalWifiApV1_0TargetTest \ VtsHalWifiApV1_4TargetTest \ VtsHalWifiV1_0TargetTest \ VtsHalWifiV1_4TargetTest Change-Id: I05aab6992277601633a0f926a8262c4c27402e93 Merged-In: I05aab6992277601633a0f926a8262c4c27402e93 --- wifi/1.0/vts/functional/Android.bp | 2 ++ .../functional/wifi_ap_iface_hidl_test.cpp | 7 ++++- .../vts/functional/wifi_chip_hidl_ap_test.cpp | 7 ++++- wifi/1.1/vts/functional/Android.bp | 1 + wifi/1.4/vts/functional/Android.bp | 28 +++++++++++++++++-- .../functional/wifi_ap_iface_hidl_test.cpp | 6 ++++ 6 files changed, 46 insertions(+), 5 deletions(-) diff --git a/wifi/1.0/vts/functional/Android.bp b/wifi/1.0/vts/functional/Android.bp index e4948b4cc2..6c0ebf7637 100644 --- a/wifi/1.0/vts/functional/Android.bp +++ b/wifi/1.0/vts/functional/Android.bp @@ -107,8 +107,10 @@ cc_test { static_libs: [ "VtsHalWifiV1_0TargetTestUtil", "android.hardware.wifi@1.0", + "android.hardware.wifi.hostapd@1.0", "libwifi-system-iface", ], + disable_framework: true, test_suites: [ "general-tests", "vts", diff --git a/wifi/1.0/vts/functional/wifi_ap_iface_hidl_test.cpp b/wifi/1.0/vts/functional/wifi_ap_iface_hidl_test.cpp index 96b4501360..28b16168f5 100644 --- a/wifi/1.0/vts/functional/wifi_ap_iface_hidl_test.cpp +++ b/wifi/1.0/vts/functional/wifi_ap_iface_hidl_test.cpp @@ -15,9 +15,9 @@ */ #include - #include #include +#include #include #include #include @@ -26,6 +26,7 @@ #include "wifi_hidl_test_utils.h" using ::android::sp; +using ::android::hardware::wifi::hostapd::V1_0::IHostapd; using ::android::hardware::wifi::V1_0::IfaceType; using ::android::hardware::wifi::V1_0::IWifi; using ::android::hardware::wifi::V1_0::IWifiApIface; @@ -38,6 +39,10 @@ using ::android::hardware::wifi::V1_0::WifiStatusCode; class WifiApIfaceHidlTest : public ::testing::TestWithParam { public: virtual void SetUp() override { + if (android::hardware::getAllHalInstanceNames(IHostapd::descriptor) + .empty()) { + GTEST_SKIP() << "Device does not support AP"; + } // Make sure test starts with a clean state stopWifi(GetInstanceName()); diff --git a/wifi/1.0/vts/functional/wifi_chip_hidl_ap_test.cpp b/wifi/1.0/vts/functional/wifi_chip_hidl_ap_test.cpp index 2e6ad32d64..66e1a807f0 100644 --- a/wifi/1.0/vts/functional/wifi_chip_hidl_ap_test.cpp +++ b/wifi/1.0/vts/functional/wifi_chip_hidl_ap_test.cpp @@ -15,9 +15,9 @@ */ #include - #include #include +#include #include #include #include @@ -26,6 +26,7 @@ #include "wifi_hidl_test_utils.h" using ::android::sp; +using ::android::hardware::wifi::hostapd::V1_0::IHostapd; using ::android::hardware::wifi::V1_0::ChipModeId; using ::android::hardware::wifi::V1_0::IfaceType; using ::android::hardware::wifi::V1_0::IWifi; @@ -41,6 +42,10 @@ using ::android::hardware::wifi::V1_0::WifiStatusCode; class WifiChipHidlApTest : public ::testing::TestWithParam { public: virtual void SetUp() override { + if (android::hardware::getAllHalInstanceNames(IHostapd::descriptor) + .empty()) { + GTEST_SKIP() << "Device does not support AP"; + } // Make sure test starts with a clean state stopWifi(GetInstanceName()); diff --git a/wifi/1.1/vts/functional/Android.bp b/wifi/1.1/vts/functional/Android.bp index 80486425c1..a8f3470c1a 100644 --- a/wifi/1.1/vts/functional/Android.bp +++ b/wifi/1.1/vts/functional/Android.bp @@ -39,6 +39,7 @@ cc_test { "android.hardware.wifi@1.5", "libwifi-system-iface", ], + disable_framework: true, test_suites: [ "general-tests", "vts", diff --git a/wifi/1.4/vts/functional/Android.bp b/wifi/1.4/vts/functional/Android.bp index 14ebbe3bb8..f86869bf61 100644 --- a/wifi/1.4/vts/functional/Android.bp +++ b/wifi/1.4/vts/functional/Android.bp @@ -14,7 +14,6 @@ // limitations under the License. // -// SoftAP-specific tests, similar to VtsHalWifiApV1_0TargetTest. package { // See: http://go/android-license-faq // A large-scale-change added 'default_applicable_licenses' to import @@ -25,10 +24,9 @@ package { } cc_test { - name: "VtsHalWifiApV1_4TargetTest", + name: "VtsHalWifiV1_4TargetTest", defaults: ["VtsHalTargetTestDefaults"], srcs: [ - "wifi_ap_iface_hidl_test.cpp", "wifi_chip_hidl_test.cpp", ], static_libs: [ @@ -46,6 +44,30 @@ cc_test { ], } +// SoftAP-specific tests, similar to VtsHalWifiApV1_0TargetTest. +cc_test { + name: "VtsHalWifiApV1_4TargetTest", + defaults: ["VtsHalTargetTestDefaults"], + srcs: [ + "wifi_ap_iface_hidl_test.cpp", + ], + static_libs: [ + "VtsHalWifiV1_0TargetTestUtil", + "android.hardware.wifi@1.0", + "android.hardware.wifi@1.1", + "android.hardware.wifi@1.2", + "android.hardware.wifi@1.3", + "android.hardware.wifi@1.4", + "android.hardware.wifi.hostapd@1.0", + "libwifi-system-iface", + ], + disable_framework: true, + test_suites: [ + "general-tests", + "vts", + ], +} + // These tests are split out so that they can be conditioned on presence of the // "android.hardware.wifi.aware" feature. cc_test { diff --git a/wifi/1.4/vts/functional/wifi_ap_iface_hidl_test.cpp b/wifi/1.4/vts/functional/wifi_ap_iface_hidl_test.cpp index 5b0f1736ea..756afa5122 100644 --- a/wifi/1.4/vts/functional/wifi_ap_iface_hidl_test.cpp +++ b/wifi/1.4/vts/functional/wifi_ap_iface_hidl_test.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -25,6 +26,7 @@ using ::android::sp; using ::android::hardware::hidl_array; +using ::android::hardware::wifi::hostapd::V1_0::IHostapd; using ::android::hardware::wifi::V1_0::WifiStatus; using ::android::hardware::wifi::V1_0::WifiStatusCode; using ::android::hardware::wifi::V1_4::IWifi; @@ -36,6 +38,10 @@ using ::android::hardware::wifi::V1_4::IWifiApIface; class WifiApIfaceHidlTest : public ::testing::TestWithParam { public: virtual void SetUp() override { + if (android::hardware::getAllHalInstanceNames(IHostapd::descriptor) + .empty()) { + GTEST_SKIP() << "Device does not support AP"; + } // Make sure to start with a clean state stopWifi(GetInstanceName()); From 9c4930dfd9edad2503a5142aa0e003acb7246787 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Wed, 4 Nov 2020 09:31:02 -0800 Subject: [PATCH 14/35] wifi: remove disable_framework option to pass the stopWifiFramework procedure Stopping entire framework can cause other essential services to be stopped. When wifi is stopped, it does not interact with any of the wifi HAL's. Bug: 201266536 Bug: 201184673 Test: atest --iterations 10 \ VtsHalWifiSupplicantV1_0TargetTest \ VtsHalWifiSupplicantV1_1TargetTest \ VtsHalWifiSupplicantV1_2TargetTest \ VtsHalWifiSupplicantV1_3TargetTest \ VtsHalWifiSupplicantV1_4TargetTest \ VtsHalWifiSupplicantP2pV1_0TargetTest \ VtsHalWifiSupplicantP2pV1_2TargetTest \ VtsHalWifiSupplicantP2pV1_4TargetTest Change-Id: Ia4a38c2e942681f323cf76941713c429e14870cc (cherry picked from commit 3a5858a7113491ac8964528a78710412933e50fc) --- wifi/supplicant/1.4/vts/functional/Android.bp | 2 -- 1 file changed, 2 deletions(-) diff --git a/wifi/supplicant/1.4/vts/functional/Android.bp b/wifi/supplicant/1.4/vts/functional/Android.bp index 8cbe04f686..57ee83073b 100644 --- a/wifi/supplicant/1.4/vts/functional/Android.bp +++ b/wifi/supplicant/1.4/vts/functional/Android.bp @@ -77,7 +77,6 @@ cc_test { "general-tests", "vts", ], - disable_framework: true, } cc_test { @@ -108,5 +107,4 @@ cc_test { "general-tests", "vts", ], - disable_framework: true, } From 01c2259e1979f91246c077a2173e63b45b473497 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Mon, 27 Sep 2021 17:01:07 +0800 Subject: [PATCH 15/35] wifi: extending framework restart waiting time 10s is not enough for low-end devices, extending waiting time to avoid false alarm for low-end devices. Bug: 201184673 Test: atest VtsHalWifiSupplicantP2pV1_4TargetTest Change-Id: I9baa53a462b97738e6dc471cf06c2b9230b92c1c (cherry picked from commit ef3f77f831445a75636b8dfb2ef4ca29db2d64e5) --- .../1.0/vts/functional/supplicant_hidl_test_utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wifi/supplicant/1.0/vts/functional/supplicant_hidl_test_utils.cpp b/wifi/supplicant/1.0/vts/functional/supplicant_hidl_test_utils.cpp index 8cb7e22f0e..114fe4f32c 100644 --- a/wifi/supplicant/1.0/vts/functional/supplicant_hidl_test_utils.cpp +++ b/wifi/supplicant/1.0/vts/functional/supplicant_hidl_test_utils.cpp @@ -317,7 +317,7 @@ bool turnOnExcessiveLogging(const sp& supplicant) { } bool waitForFrameworkReady() { - int waitCount = 10; + int waitCount = 15; do { // Check whether package service is ready or not. if (!testing::checkSubstringInCommandOutput( From 02499d65fc68be423405a2c6f9e5bcf5f6a48e72 Mon Sep 17 00:00:00 2001 From: Changyeon Jo Date: Sun, 3 Oct 2021 17:11:03 -0700 Subject: [PATCH 16/35] Update VtsHalEvsV1_1TargetTest This CL makes following changes to fix VTS failures reported on Seahawk reference platform. * Correct the definition of RawStreamConfig and its size constant. * Update all test cases to use a stream configuration reported by the target camera device. * Update CameraStreamExternalBuffering test cases to take internally allocated buffers into account. Bug: 192460757 Bug: 199626993 Test: atest VtsHalEvsV1_1TargetTest Change-Id: Ia03775ae543617ba5057e91bbbb5aed9221d1a30 --- .../functional/VtsHalEvsV1_1TargetTest.cpp | 274 +++++++++--------- 1 file changed, 129 insertions(+), 145 deletions(-) diff --git a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp index 8cc18822f2..1216d36eda 100644 --- a/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp +++ b/automotive/evs/1.1/vts/functional/VtsHalEvsV1_1TargetTest.cpp @@ -79,19 +79,24 @@ using IEvsCamera_1_1 = ::android::hardware::automotive::evs::V1_1::IEvsCamera; using IEvsDisplay_1_0 = ::android::hardware::automotive::evs::V1_0::IEvsDisplay; using IEvsDisplay_1_1 = ::android::hardware::automotive::evs::V1_1::IEvsDisplay; +namespace { + /* * Plese note that this is different from what is defined in * libhardware/modules/camera/3_4/metadata/types.h; this has one additional * field to store a framerate. */ -const size_t kStreamCfgSz = 5; typedef struct { + int32_t id; int32_t width; int32_t height; int32_t format; int32_t direction; int32_t framerate; } RawStreamConfig; +constexpr const size_t kStreamCfgSz = sizeof(RawStreamConfig) / sizeof(int32_t); + +} // anonymous namespace // The main test class for EVS @@ -236,6 +241,28 @@ protected: return physicalCameras; } + Stream getFirstStreamConfiguration(camera_metadata_t* metadata) { + Stream targetCfg = {}; + camera_metadata_entry_t streamCfgs; + if (!find_camera_metadata_entry(metadata, + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, + &streamCfgs)) { + // Stream configurations are found in metadata + RawStreamConfig *ptr = reinterpret_cast(streamCfgs.data.i32); + for (unsigned offset = 0; offset < streamCfgs.count; offset += kStreamCfgSz) { + if (ptr->direction == ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT && + ptr->format == HAL_PIXEL_FORMAT_RGBA_8888) { + targetCfg.width = ptr->width; + targetCfg.height = ptr->height; + targetCfg.format = static_cast(ptr->format); + break; + } + ++ptr; + } + } + + return targetCfg; + } sp pEnumerator; // Every test needs access to the service std::vector cameraInfo; // Empty unless/until loadCameraList() is called @@ -265,10 +292,6 @@ TEST_P(EvsHidlTest, CameraOpenClean) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Open and close each camera twice for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -278,8 +301,14 @@ TEST_P(EvsHidlTest, CameraOpenClean) { continue; } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + for (int pass = 0; pass < 2; pass++) { - sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg); + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); for (auto&& devName : devices) { @@ -343,10 +372,6 @@ TEST_P(EvsHidlTest, CameraOpenAggressive) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Open and close each camera twice for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -356,10 +381,14 @@ TEST_P(EvsHidlTest, CameraOpenAggressive) { continue; } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + activeCameras.clear(); - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -372,9 +401,7 @@ TEST_P(EvsHidlTest, CameraOpenAggressive) { } ); - sp pCam2 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam2 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam2, nullptr); // Store a camera handle for a clean-up @@ -422,10 +449,6 @@ TEST_P(EvsHidlTest, CameraStreamPerformance) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -435,9 +458,13 @@ TEST_P(EvsHidlTest, CameraStreamPerformance) { continue; } - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -519,10 +546,6 @@ TEST_P(EvsHidlTest, CameraStreamBuffering) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -532,9 +555,13 @@ TEST_P(EvsHidlTest, CameraStreamBuffering) { continue; } - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -601,10 +628,6 @@ TEST_P(EvsHidlTest, CameraToDisplayRoundTrip) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Request available display IDs uint8_t targetDisplayId = 0; pEnumerator->getDisplayIdList([&targetDisplayId](auto ids) { @@ -642,9 +665,13 @@ TEST_P(EvsHidlTest, CameraToDisplayRoundTrip) { continue; } - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -708,24 +735,22 @@ TEST_P(EvsHidlTest, MultiCameraStream) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera for (auto&& cam: cameraInfo) { + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + // Create two camera clients. - sp pCam0 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam0 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam0, nullptr); // Store a camera handle for a clean-up activeCameras.push_back(pCam0); - sp pCam1 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam1 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam1, nullptr); // Store a camera handle for a clean-up @@ -812,10 +837,6 @@ TEST_P(EvsHidlTest, CameraParameter) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera Return result = EvsResult::OK; for (auto&& cam: cameraInfo) { @@ -828,10 +849,14 @@ TEST_P(EvsHidlTest, CameraParameter) { continue; } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + // Create a camera client - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera @@ -961,10 +986,6 @@ TEST_P(EvsHidlTest, CameraPrimaryClientRelease) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -976,18 +997,20 @@ TEST_P(EvsHidlTest, CameraPrimaryClientRelease) { continue; } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + // Create two camera clients. - sp pCamPrimary = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCamPrimary = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCamPrimary, nullptr); // Store a camera handle for a clean-up activeCameras.push_back(pCamPrimary); - sp pCamSecondary = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCamSecondary = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCamSecondary, nullptr); // Store a camera handle for a clean-up @@ -1142,10 +1165,6 @@ TEST_P(EvsHidlTest, MultiCameraParameter) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Test each reported camera for (auto&& cam: cameraInfo) { bool isLogicalCam = false; @@ -1157,18 +1176,20 @@ TEST_P(EvsHidlTest, MultiCameraParameter) { continue; } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + // Create two camera clients. - sp pCamPrimary = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCamPrimary = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCamPrimary, nullptr); // Store a camera handle for a clean-up activeCameras.push_back(pCamPrimary); - sp pCamSecondary = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCamSecondary = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCamSecondary, nullptr); // Store a camera handle for a clean-up @@ -1615,28 +1636,26 @@ TEST_P(EvsHidlTest, HighPriorityCameraClient) { // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Request exclusive access to the EVS display sp pDisplay = pEnumerator->openDisplay(); ASSERT_NE(pDisplay, nullptr); // Test each reported camera for (auto&& cam: cameraInfo) { + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); + // Create two clients - sp pCam0 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam0 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam0, nullptr); // Store a camera handle for a clean-up activeCameras.push_back(pCam0); - sp pCam1 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam1 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam1, nullptr); // Store a camera handle for a clean-up @@ -2001,7 +2020,7 @@ TEST_P(EvsHidlTest, CameraUseStreamConfigToDisplay) { &streamCfgs)) { // Stream configurations are found in metadata RawStreamConfig *ptr = reinterpret_cast(streamCfgs.data.i32); - for (unsigned idx = 0; idx < streamCfgs.count; idx += kStreamCfgSz) { + for (unsigned offset = 0; offset < streamCfgs.count; offset += kStreamCfgSz) { if (ptr->direction == ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT && ptr->format == HAL_PIXEL_FORMAT_RGBA_8888) { @@ -2026,9 +2045,7 @@ TEST_P(EvsHidlTest, CameraUseStreamConfigToDisplay) { continue; } - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg)) - .withDefault(nullptr); + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -2106,7 +2123,7 @@ TEST_P(EvsHidlTest, MultiCameraStreamUseConfig) { &streamCfgs)) { // Stream configurations are found in metadata RawStreamConfig *ptr = reinterpret_cast(streamCfgs.data.i32); - for (unsigned idx = 0; idx < streamCfgs.count; idx += kStreamCfgSz) { + for (unsigned offset = 0; offset < streamCfgs.count; offset += kStreamCfgSz) { if (ptr->direction == ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT && ptr->format == HAL_PIXEL_FORMAT_RGBA_8888) { @@ -2132,9 +2149,7 @@ TEST_P(EvsHidlTest, MultiCameraStreamUseConfig) { } // Create the first camera client with a selected stream configuration. - sp pCam0 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg)) - .withDefault(nullptr); + sp pCam0 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam0, nullptr); // Store a camera handle for a clean-up @@ -2144,9 +2159,7 @@ TEST_P(EvsHidlTest, MultiCameraStreamUseConfig) { // configuration. int32_t id = targetCfg.id; targetCfg.id += 1; // EVS manager sees only the stream id. - sp pCam1 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg)) - .withDefault(nullptr); + sp pCam1 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_EQ(pCam1, nullptr); // Store a camera handle for a clean-up @@ -2154,9 +2167,7 @@ TEST_P(EvsHidlTest, MultiCameraStreamUseConfig) { // Try again with same stream configuration. targetCfg.id = id; - pCam1 = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg)) - .withDefault(nullptr); + pCam1 = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam1, nullptr); // Set up per-client frame receiver objects which will fire up its own thread @@ -2258,52 +2269,23 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { LOG(INFO) << "Starting CameraStreamExternalBuffering test"; // Arbitrary constant (should be > 1 and not too big) - static const unsigned int kBuffersToHold = 6; + static const unsigned int kBuffersToHold = 3; // Get the camera list loadCameraList(); - // Using null stream configuration makes EVS uses the default resolution and - // output format. - Stream nullCfg = {}; - // Acquire the graphics buffer allocator android::GraphicBufferAllocator& alloc(android::GraphicBufferAllocator::get()); const auto usage = GRALLOC_USAGE_HW_TEXTURE | GRALLOC_USAGE_SW_READ_RARELY | GRALLOC_USAGE_SW_WRITE_OFTEN; - const auto format = HAL_PIXEL_FORMAT_RGBA_8888; - uint32_t width = 640; - uint32_t height = 360; - camera_metadata_entry_t streamCfgs; // Test each reported camera for (auto&& cam : cameraInfo) { - bool foundCfg = false; - if (!find_camera_metadata_entry(reinterpret_cast(cam.metadata.data()), - ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, - &streamCfgs)) { - // Stream configurations are found in metadata - RawStreamConfig* ptr = reinterpret_cast(streamCfgs.data.i32); - - LOG(DEBUG) << __LINE__ << " start searching " << streamCfgs.count; - for (unsigned idx = 0; idx < streamCfgs.count; idx++) { - LOG(DEBUG) << "ptr->direction= " << ptr->direction - << " ptr->format= " << ptr->format; - if (ptr->direction == ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT && - ptr->format == HAL_PIXEL_FORMAT_RGBA_8888) { - width = ptr->width; - height = ptr->height; - foundCfg = true; - // Always use the 1st available configuration - break; - } - ++ptr; - } - } - - if (!foundCfg) { - LOG(INFO) << "No configuration found. Use default stream configurations."; - } + // Read a target resolution from the metadata + Stream targetCfg = + getFirstStreamConfiguration(reinterpret_cast(cam.metadata.data())); + ASSERT_GT(targetCfg.width, 0); + ASSERT_GT(targetCfg.height, 0); // Allocate buffers to use hidl_vec buffers; @@ -2312,8 +2294,11 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { unsigned pixelsPerLine; buffer_handle_t memHandle = nullptr; android::status_t result = - alloc.allocate(width, height, format, 1, usage, &memHandle, &pixelsPerLine, 0, - "CameraStreamExternalBufferingTest"); + alloc.allocate(targetCfg.width, targetCfg.height, + (android::PixelFormat)targetCfg.format, + /* layerCount = */ 1, usage, &memHandle, &pixelsPerLine, + /* graphicBufferId = */ 0, + /* requestorName = */ "CameraStreamExternalBufferingTest"); if (result != android::NO_ERROR) { LOG(ERROR) << __FUNCTION__ << " failed to allocate memory."; // Release previous allocated buffers @@ -2325,10 +2310,10 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { BufferDesc buf; AHardwareBuffer_Desc* pDesc = reinterpret_cast(&buf.buffer.description); - pDesc->width = width; - pDesc->height = height; + pDesc->width = targetCfg.width; + pDesc->height = targetCfg.height; pDesc->layers = 1; - pDesc->format = format; + pDesc->format = static_cast(targetCfg.format); pDesc->usage = usage; pDesc->stride = pixelsPerLine; buf.buffer.nativeHandle = memHandle; @@ -2340,9 +2325,7 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { bool isLogicalCam = false; getPhysicalCameraIds(cam.v1.cameraId, isLogicalCam); - sp pCam = - IEvsCamera_1_1::castFrom(pEnumerator->openCamera_1_1(cam.v1.cameraId, nullCfg)) - .withDefault(nullptr); + sp pCam = pEnumerator->openCamera_1_1(cam.v1.cameraId, targetCfg); ASSERT_NE(pCam, nullptr); // Store a camera handle for a clean-up @@ -2362,7 +2345,7 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { } EXPECT_EQ(result, EvsResult::OK); - EXPECT_GE(delta, 0); + EXPECT_GE(delta, kBuffersToHold); // Set up a frame receiver object which will fire up its own thread. sp frameHandler = new FrameHandler(pCam, cam, @@ -2378,7 +2361,7 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { sleep(1); // 1 second should be enough for at least 5 frames to be delivered worst case unsigned framesReceived = 0; frameHandler->getFramesCounters(&framesReceived, nullptr); - ASSERT_EQ(kBuffersToHold, framesReceived) << "Stream didn't stall at expected buffer limit"; + ASSERT_LE(kBuffersToHold, framesReceived) << "Stream didn't stall at expected buffer limit"; // Give back one buffer @@ -2387,9 +2370,10 @@ TEST_P(EvsHidlTest, CameraStreamExternalBuffering) { // Once we return a buffer, it shouldn't take more than 1/10 second to get a new one // filled since we require 10fps minimum -- but give a 10% allowance just in case. + unsigned framesReceivedAfter = 0; usleep(110 * kMillisecondsToMicroseconds); - frameHandler->getFramesCounters(&framesReceived, nullptr); - EXPECT_EQ(kBuffersToHold+1, framesReceived) << "Stream should've resumed"; + frameHandler->getFramesCounters(&framesReceivedAfter, nullptr); + EXPECT_EQ(framesReceived + 1, framesReceivedAfter) << "Stream should've resumed"; // Even when the camera pointer goes out of scope, the FrameHandler object will // keep the stream alive unless we tell it to shutdown. From 6adbab13ee2c0a99b03a9dc66bc741b706dc7bf2 Mon Sep 17 00:00:00 2001 From: Sunil Ravi Date: Fri, 15 Oct 2021 16:55:53 -0700 Subject: [PATCH 17/35] Update the p2p device interface name In some implementations P2P device interface is created under primary interface(wlan0 by default). In those implementations p2p device name is predefined in system property wifi.direct.interface. And the interface is created by supplicant with primary interface as the parent interface. The naming of p2p device interface is p2p-dev- ("p2p-dev-wlan0"). With STA+STA feature, wlan0 interface gets deleted in certain scenarios and wlan1 becomes the active interface. In such scenarios P2P fails to create the interface as parent interface wlan0 is deleted. To fix the issue update the p2p device interface from system property based on the current active wlan interface. ie First get the p2p parent interface name from p2p device interface name set in property. Check if the parent interface derived from p2p device interface name is the current active interface. If not, get the current active interface and update the name as p2p-dev- ("p2p-dev-wlan1"). This helps HIDL/supplicant to get an active wlan interface from p2p device interface name and succeed in creating the p2p interface under the active interface. Bug: 203434193 Test: Manual - Tested STA+STA which ended up deleting wlan0 interface. Then ran p2p tests and confirmed that p2p scan and connection works. Change-Id: I522cec02a662c057e21d434f3ed98c7e7a4ca8f0 Merged-In: I522cec02a662c057e21d434f3ed98c7e7a4ca8f0 (cherry picked from commit 7f2822aff735e3c4f727de0759f867791db129ed) --- wifi/1.5/default/wifi_chip.cpp | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/wifi/1.5/default/wifi_chip.cpp b/wifi/1.5/default/wifi_chip.cpp index 82d794ca16..6bdff42149 100644 --- a/wifi/1.5/default/wifi_chip.cpp +++ b/wifi/1.5/default/wifi_chip.cpp @@ -28,6 +28,8 @@ #include "wifi_chip.h" #include "wifi_status_util.h" +#define P2P_MGMT_DEVICE_PREFIX "p2p-dev-" + namespace { using android::sp; using android::base::unique_fd; @@ -126,8 +128,37 @@ std::vector getPredefinedApIfaceNames(bool is_bridged) { } std::string getPredefinedP2pIfaceName() { + std::array primaryIfaceName; + char p2pParentIfname[100]; + std::string p2pDevIfName = ""; std::array buffer; property_get("wifi.direct.interface", buffer.data(), "p2p0"); + if (strncmp(buffer.data(), P2P_MGMT_DEVICE_PREFIX, + strlen(P2P_MGMT_DEVICE_PREFIX)) == 0) { + /* Get the p2p parent interface name from p2p device interface name set + * in property */ + strncpy(p2pParentIfname, buffer.data() + strlen(P2P_MGMT_DEVICE_PREFIX), + strlen(buffer.data()) - strlen(P2P_MGMT_DEVICE_PREFIX)); + if (property_get(kActiveWlanIfaceNameProperty, primaryIfaceName.data(), + nullptr) == 0) { + return buffer.data(); + } + /* Check if the parent interface derived from p2p device interface name + * is active */ + if (strncmp(p2pParentIfname, primaryIfaceName.data(), + strlen(buffer.data()) - strlen(P2P_MGMT_DEVICE_PREFIX)) != + 0) { + /* + * Update the predefined p2p device interface parent interface name + * with current active wlan interface + */ + p2pDevIfName += P2P_MGMT_DEVICE_PREFIX; + p2pDevIfName += primaryIfaceName.data(); + LOG(INFO) << "update the p2p device interface name to " + << p2pDevIfName.c_str(); + return p2pDevIfName; + } + } return buffer.data(); } From 3ccc2de9c43430e4f9b54ed589bee65e0e69dc52 Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Wed, 20 Oct 2021 18:54:16 +0000 Subject: [PATCH 18/35] Revert "VTS: Test that configs in a group differ only by vsync period" This reverts commit 6f36dd6e31ac1c2d62cd9283a4d07c7c684e5872. Differing only by vsync period was not a requirement on Android R, and it shouldn't be added after-the-fact to an existing HAL version. There are devices on the market that switch seamlessly between configs that differ in resolution/dpi, and therefore have those configs in a common group. Bug: 200184776 Merged-In: Id0bfc67e55d5139fddb2b359cabafd9281c33734 Change-Id: Ib6e6a8c5f45c9829ea30d031e1cfa9a4fb775535 --- .../VtsHalGraphicsComposerV2_4TargetTest.cpp | 55 ------------------- 1 file changed, 55 deletions(-) diff --git a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp index 5aceda721e..b071f71e85 100644 --- a/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp +++ b/graphics/composer/2.4/vts/functional/VtsHalGraphicsComposerV2_4TargetTest.cpp @@ -19,8 +19,6 @@ #include #include #include -#include -#include #include #include @@ -317,59 +315,6 @@ TEST_P(GraphicsComposerHidlTest, GetDisplayAttribute_2_4) { } } -TEST_P(GraphicsComposerHidlTest, GetDisplayAttribute_2_4_ConfigsInAGroupDifferOnlyByVsyncPeriod) { - struct Resolution { - int32_t width, height; - }; - struct Dpi { - int32_t x, y; - }; - for (const auto& display : mDisplays) { - std::vector configs = mComposerClient->getDisplayConfigs(display.get()); - std::unordered_map configGroupToResolutionMap; - std::unordered_map configGroupToDpiMap; - for (auto config : configs) { - const auto configGroup = mComposerClient->getDisplayAttribute_2_4( - display.get(), config, IComposerClient::Attribute::CONFIG_GROUP); - const auto width = mComposerClient->getDisplayAttribute_2_4( - display.get(), config, IComposerClient::Attribute::WIDTH); - const auto height = mComposerClient->getDisplayAttribute_2_4( - display.get(), config, IComposerClient::Attribute::HEIGHT); - if (configGroupToResolutionMap.find(configGroup) == configGroupToResolutionMap.end()) { - configGroupToResolutionMap[configGroup] = {width, height}; - } - EXPECT_EQ(configGroupToResolutionMap[configGroup].width, width); - EXPECT_EQ(configGroupToResolutionMap[configGroup].height, height); - - int32_t dpiX = -1; - mComposerClient->getRaw()->getDisplayAttribute_2_4( - display.get(), config, IComposerClient::Attribute::DPI_X, - [&](const auto& tmpError, const auto& value) { - if (tmpError == Error::NONE) { - dpiX = value; - } - }); - int32_t dpiY = -1; - mComposerClient->getRaw()->getDisplayAttribute_2_4( - display.get(), config, IComposerClient::Attribute::DPI_Y, - [&](const auto& tmpError, const auto& value) { - if (tmpError == Error::NONE) { - dpiY = value; - } - }); - if (dpiX == -1 && dpiY == -1) { - continue; - } - - if (configGroupToDpiMap.find(configGroup) == configGroupToDpiMap.end()) { - configGroupToDpiMap[configGroup] = {dpiX, dpiY}; - } - EXPECT_EQ(configGroupToDpiMap[configGroup].x, dpiX); - EXPECT_EQ(configGroupToDpiMap[configGroup].y, dpiY); - } - } -} - TEST_P(GraphicsComposerHidlTest, getDisplayVsyncPeriod_BadDisplay) { VsyncPeriodNanos vsyncPeriodNanos; EXPECT_EQ(Error::BAD_DISPLAY, From 849d924d6d20b569ec8ba71e423f9476b9e7a087 Mon Sep 17 00:00:00 2001 From: Yomna Nasser Date: Mon, 4 Oct 2021 21:55:12 +0000 Subject: [PATCH 19/35] VTS fix for getAllowedNetworkTypesBitmap and setAllowedNetworkTypesBitmap Fix VTS for getAllowedNetworkTypesBitmap and setAllowedNetworkTypesBitmap, which should not rely on getRadioHalCapabilities. Bug: b/199809900 Test: atest VtsHalRadioV1_6TargetTest Change-Id: I0b50e7e6c0d406a62f7a1e2ea8ac9bc3a71d8aeb Merged-In: I0b50e7e6c0d406a62f7a1e2ea8ac9bc3a71d8aeb --- .../1.6/vts/functional/radio_hidl_hal_api.cpp | 56 ++++++++----------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp index d61853b697..f607b24fef 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp +++ b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp @@ -32,23 +32,17 @@ TEST_P(RadioHidlTest_v1_6, setAllowedNetworkTypesBitmap) { EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_v1_6->rspInfo.type); EXPECT_EQ(serial, radioRsp_v1_6->rspInfo.serial); - - if (getRadioHalCapabilities()) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); - } else { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::NONE, - ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE, - ::android::hardware::radio::V1_6::RadioError::OPERATION_NOT_ALLOWED, - ::android::hardware::radio::V1_6::RadioError::MODE_NOT_SUPPORTED, - ::android::hardware::radio::V1_6::RadioError::INTERNAL_ERR, - ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, - ::android::hardware::radio::V1_6::RadioError::MODEM_ERR, - ::android::hardware::radio::V1_6::RadioError::NO_RESOURCES})); - } + ASSERT_TRUE( + CheckAnyOfErrors(radioRsp_v1_6->rspInfo.error, + {::android::hardware::radio::V1_6::RadioError::NONE, + ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE, + ::android::hardware::radio::V1_6::RadioError::OPERATION_NOT_ALLOWED, + ::android::hardware::radio::V1_6::RadioError::MODE_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::INTERNAL_ERR, + ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, + ::android::hardware::radio::V1_6::RadioError::MODEM_ERR, + ::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NO_RESOURCES})); } /* @@ -74,23 +68,17 @@ TEST_P(RadioHidlTest_v1_6, getAllowedNetworkTypesBitmap) { EXPECT_EQ(std::cv_status::no_timeout, wait()); EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_v1_6->rspInfo.type); EXPECT_EQ(serial, radioRsp_v1_6->rspInfo.serial); - - if (getRadioHalCapabilities()) { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED})); - } else { - ASSERT_TRUE(CheckAnyOfErrors( - radioRsp_v1_6->rspInfo.error, - {::android::hardware::radio::V1_6::RadioError::NONE, - ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE, - ::android::hardware::radio::V1_6::RadioError::OPERATION_NOT_ALLOWED, - ::android::hardware::radio::V1_6::RadioError::MODE_NOT_SUPPORTED, - ::android::hardware::radio::V1_6::RadioError::INTERNAL_ERR, - ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, - ::android::hardware::radio::V1_6::RadioError::MODEM_ERR, - ::android::hardware::radio::V1_6::RadioError::NO_RESOURCES})); - } + ASSERT_TRUE(CheckAnyOfErrors( + radioRsp_v1_6->rspInfo.error, + {::android::hardware::radio::V1_6::RadioError::NONE, + ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE, + ::android::hardware::radio::V1_6::RadioError::OPERATION_NOT_ALLOWED, + ::android::hardware::radio::V1_6::RadioError::MODE_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::INTERNAL_ERR, + ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, + ::android::hardware::radio::V1_6::RadioError::MODEM_ERR, + ::android::hardware::radio::V1_6::RadioError::REQUEST_NOT_SUPPORTED, + ::android::hardware::radio::V1_6::RadioError::NO_RESOURCES})); } } From 9b3c9adb71145056dd8cbfd417b0b017106b17a3 Mon Sep 17 00:00:00 2001 From: Changyeon Jo Date: Mon, 18 Oct 2021 08:47:03 -0700 Subject: [PATCH 20/35] Using a manifest fragment Fix: 203414344 Test: m -j and start CAN services Change-Id: I423ab883d959f234a52adf680fae91bf264bea8b --- automotive/can/1.0/default/Android.bp | 1 + ...st_android.hardware.automotive.can@1.0.xml | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 automotive/can/1.0/default/manifest_android.hardware.automotive.can@1.0.xml diff --git a/automotive/can/1.0/default/Android.bp b/automotive/can/1.0/default/Android.bp index c0c17e27f5..163fdb7265 100644 --- a/automotive/can/1.0/default/Android.bp +++ b/automotive/can/1.0/default/Android.bp @@ -64,4 +64,5 @@ cc_binary { "android.hardware.automotive@libc++fs", "libnl++", ], + vintf_fragments: ["manifest_android.hardware.automotive.can@1.0.xml"], } diff --git a/automotive/can/1.0/default/manifest_android.hardware.automotive.can@1.0.xml b/automotive/can/1.0/default/manifest_android.hardware.automotive.can@1.0.xml new file mode 100644 index 0000000000..2078ce54df --- /dev/null +++ b/automotive/can/1.0/default/manifest_android.hardware.automotive.can@1.0.xml @@ -0,0 +1,22 @@ + + + + + android.hardware.automotive.can + hwbinder + @1.0::ICanController/socketcan + + From f7069c658c9a11b15c50f9deb0139e6cfbd86fca Mon Sep 17 00:00:00 2001 From: Jordan Liu Date: Wed, 20 Oct 2021 11:34:23 -0700 Subject: [PATCH 21/35] Do not assert CardState::PRESENT on sim power down Bug: 203031664 Test: manual Change-Id: I6c9cbad7cd4fd19eb0b77c55ff37298b97f32050 --- radio/1.6/vts/functional/radio_hidl_hal_api.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp index d61853b697..5964f757ac 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp +++ b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp @@ -618,7 +618,8 @@ TEST_P(RadioHidlTest_v1_6, setSimCardPower_1_6) { if (radioRsp_v1_6->rspInfo.error == ::android::hardware::radio::V1_6::RadioError::NONE) { /* Wait some time for setting sim power down and then verify it */ updateSimCardStatus(); - EXPECT_EQ(CardState::PRESENT, cardStatus.base.base.base.cardState); + // We cannot assert the consistency of CardState here due to b/203031664 + // EXPECT_EQ(CardState::PRESENT, cardStatus.base.base.base.cardState); // applications should be an empty vector of AppStatus EXPECT_EQ(0, cardStatus.applications.size()); } From 5c0ec3f1debcf5c395609c951cf748315b52219a Mon Sep 17 00:00:00 2001 From: Yu Shan Date: Wed, 20 Oct 2021 14:01:42 -0700 Subject: [PATCH 22/35] Parse user flags as flags instead of enum. The flags field in UserInfo is a int32_t that contains multiple UserFlags 'or'ed together. We should not parse it as enum. Test: atest android.hardware.automotive.vehicle@2.0-utils-unit-tests Bug: 202520478 Change-Id: Ie7e81a8a5f39f6070e35f2e77bce88a211fd526b --- .../vehicle/2.0/utils/UserHalHelper.cpp | 19 ++++-- .../2.0/utils/tests/UserHalHelper_test.cpp | 58 ++++++++++++++++++- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/automotive/vehicle/2.0/utils/UserHalHelper.cpp b/automotive/vehicle/2.0/utils/UserHalHelper.cpp index abf59b74e3..dccdb2b845 100644 --- a/automotive/vehicle/2.0/utils/UserHalHelper.cpp +++ b/automotive/vehicle/2.0/utils/UserHalHelper.cpp @@ -60,11 +60,22 @@ Result parseUserInfo(const hidl_vec& int32Values, size_t startPos << int32Values.size(); } userInfo->userId = int32Values[startPos]; - auto userFlags = verifyAndCast(int32Values[startPos + 1]); - if (!userFlags.ok()) { - return Error() << "Invalid user flags: " << userFlags.error(); + int32_t intUserFlags = int32Values[startPos + 1]; + int32_t expectedUserFlags = 0; + for (const auto& v : hidl_enum_range()) { + int32_t intEnumUserFlag = static_cast(v); + if ((intUserFlags & intEnumUserFlag) != 0) { + expectedUserFlags |= intEnumUserFlag; + } } - userInfo->flags = *userFlags; + if (intUserFlags != expectedUserFlags) { + return Error() << "Invalid user flags: " << intUserFlags << ", must be '|' of UserFlags"; + } + // intUserFlags is actually not a valid UserFlags enum, instead, it is a 'bit or' of possible + // multiple UserFlags. However, because the HAL interface was defined incorrectly, we have to + // cast it to UserFlags here, which is defined behavior because the underlying type for + // UserFlags is int32_t and our intUserFlags is within the range of int32_t. + userInfo->flags = static_cast(intUserFlags); return {}; } diff --git a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp index 7da87a23c8..0562a54cec 100644 --- a/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp +++ b/automotive/vehicle/2.0/utils/tests/UserHalHelper_test.cpp @@ -54,6 +54,10 @@ constexpr int32_t VEHICLE_REQUEST = static_cast(SwitchUserMessageType:: constexpr int32_t GUEST_USER = static_cast(UserFlags::GUEST); constexpr int32_t NONE_USER = static_cast(UserFlags::NONE); constexpr int32_t SYSTEM_USER = static_cast(UserFlags::SYSTEM); +constexpr int32_t ADMIN_USER = static_cast(UserFlags::ADMIN); +constexpr int32_t SYSTEM_ADMIN_USER = static_cast(UserFlags::SYSTEM | UserFlags::ADMIN); +// 0x1111 is not a valid UserFlags combination. +constexpr int32_t INVALID_USER_FLAG = 0x1111; constexpr int32_t USER_ID_ASSOC_KEY_FOB = static_cast(UserIdentificationAssociationType::KEY_FOB); @@ -72,7 +76,7 @@ constexpr int32_t USER_ID_ASSOC_NO_USER = } // namespace -TEST(UserHalHelperTest, TestToInitialUserInfoRequest) { +TEST(UserHalHelperTest, TestToInitialUserInfoRequestSystemUser) { VehiclePropValue propValue{ .prop = INITIAL_USER_INFO, .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, SYSTEM_USER, @@ -92,6 +96,58 @@ TEST(UserHalHelperTest, TestToInitialUserInfoRequest) { EXPECT_THAT(actual.value(), Eq(expected)); } +TEST(UserHalHelperTest, TestToInitialUserInfoRequestAdminUser) { + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, ADMIN_USER, 10, + NONE_USER}}, + }; + InitialUserInfoRequest expected{ + .requestId = 23, + .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA, + .usersInfo = {{10, UserFlags::NONE}, 2, {{0, UserFlags::ADMIN}, {10, UserFlags::NONE}}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + ASSERT_TRUE(actual.ok()) << actual.error().message(); + EXPECT_THAT(actual.value(), Eq(expected)); +} + +TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserFlagsBitCombination) { + // SYSTEM_ADMIN_USER is two UserFlags combined and is itself not a defined UserFlags enum. + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, + SYSTEM_ADMIN_USER, 10, NONE_USER}}, + }; + InitialUserInfoRequest expected{ + .requestId = 23, + .requestType = InitialUserInfoRequestType::FIRST_BOOT_AFTER_OTA, + .usersInfo = {{10, UserFlags::NONE}, + 2, + {{0, static_cast(SYSTEM_ADMIN_USER)}, {10, UserFlags::NONE}}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + ASSERT_TRUE(actual.ok()) << actual.error().message(); + EXPECT_THAT(actual.value(), Eq(expected)); +} + +TEST(UserHalHelperTest, TestToInitialUserInfoRequestUserInvalidUserFlag) { + // 0x1111 is not a valid UserFlags flag combination. + VehiclePropValue propValue{ + .prop = INITIAL_USER_INFO, + .value = {.int32Values = {23, FIRST_BOOT_AFTER_OTA, 10, NONE_USER, 2, 0, + INVALID_USER_FLAG, 10, NONE_USER}}, + }; + + auto actual = toInitialUserInfoRequest(propValue); + + EXPECT_FALSE(actual.ok()) << "No error returned on invalid user flags"; +} + TEST(UserHalHelperTest, TestFailsToInitialUserInfoRequestWithMismatchingPropType) { VehiclePropValue propValue{ .prop = INT32_MAX, From 06ec563f55fc6c0b90d631ba50e97ab04cac56d0 Mon Sep 17 00:00:00 2001 From: Emilian Peev Date: Thu, 28 Oct 2021 17:49:19 -0700 Subject: [PATCH 23/35] VtsHalCameraProviderV2_4TargetTest: Override rotate&crop As per documentation the default auto rotate&crop mode must be overriden before passing the capture request back to the camera provider. Bug: 204407427 Test: atest VtsHalCameraProviderV2_4TargetTest:PerInstance/CameraHidlTest#processCaptureRequestPreview/0_internal_0 atest VtsHalCameraProviderV2_4TargetTest:PerInstance/CameraHidlTest#processMultiCaptureRequestPreview/0_internal_0 atest VtsHalCameraProviderV2_4TargetTest:PerInstance/CameraHidlTest#processCaptureRequestBurstISO/0_internal_0 atest VtsHalCameraProviderV2_4TargetTest:PerInstance/CameraHidlTest#flushPreviewRequest/0_internal_0 Change-Id: I1e1d387a04add8be9a281fcc3f78a867990248b3 --- .../VtsHalCameraProviderV2_4TargetTest.cpp | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index ad3da48aab..464cea646b 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -882,6 +882,7 @@ public: camera_metadata* oldSessionParams, camera_metadata* newSessionParams); void verifyRequestTemplate(const camera_metadata_t* metadata, RequestTemplate requestTemplate); + static void overrideRotateAndCrop(::android::hardware::hidl_vec *settings /*in/out*/); static bool isDepthOnly(const camera_metadata_t* staticMeta); @@ -4660,6 +4661,7 @@ void CameraHidlTest::processCaptureRequestInternal(uint64_t bufferUsage, settings = req; }); ASSERT_TRUE(ret.isOk()); + overrideRotateAndCrop(&settings); hidl_handle buffer_handle; StreamBuffer outputBuffer; @@ -4836,6 +4838,7 @@ TEST_P(CameraHidlTest, processMultiCaptureRequestPreview) { settings.setToExternal( reinterpret_cast (const_cast (settingsBuffer)), get_camera_metadata_size(settingsBuffer)); + overrideRotateAndCrop(&settings); free_camera_metadata(staticMeta); ret = session->close(); @@ -4913,6 +4916,7 @@ TEST_P(CameraHidlTest, processMultiCaptureRequestPreview) { reinterpret_cast (const_cast ( filteredSettingsBuffer)), get_camera_metadata_size(filteredSettingsBuffer)); + overrideRotateAndCrop(&camSettings[0].settings); camSettings[0].fmqSettingsSize = 0; camSettings[0].physicalCameraId = physicalDeviceId; @@ -5070,6 +5074,7 @@ TEST_P(CameraHidlTest, processUltraHighResolutionRequest) { settings.setToExternal( reinterpret_cast(const_cast(settingsBuffer)), get_camera_metadata_size(settingsBuffer)); + overrideRotateAndCrop(&settings); free_camera_metadata(staticMeta); ret = session->close(); @@ -5305,6 +5310,7 @@ TEST_P(CameraHidlTest, processCaptureRequestBurstISO) { camera_metadata_t *metaBuffer = requestMeta.release(); requestSettings[i].setToExternal(reinterpret_cast (metaBuffer), get_camera_metadata_size(metaBuffer), true); + overrideRotateAndCrop(&requestSettings[i]); requests[i] = {frameNumber + i, 0 /* fmqSettingsSize */, requestSettings[i], emptyInputBuffer, {outputBuffers[i]}}; @@ -5531,6 +5537,7 @@ TEST_P(CameraHidlTest, switchToOffline) { camera_metadata_t *metaBuffer = requestMeta.release(); requestSettings[i].setToExternal(reinterpret_cast (metaBuffer), get_camera_metadata_size(metaBuffer), true); + overrideRotateAndCrop(&requestSettings[i]); requests[i] = {frameNumber + i, 0 /* fmqSettingsSize */, requestSettings[i], emptyInputBuffer, {outputBuffers[i]}}; @@ -5671,6 +5678,7 @@ TEST_P(CameraHidlTest, processCaptureRequestInvalidBuffer) { settings = req; }); ASSERT_TRUE(ret.isOk()); + overrideRotateAndCrop(&settings); ::android::hardware::hidl_vec emptyOutputBuffers; StreamBuffer emptyInputBuffer = {-1, 0, nullptr, BufferStatus::ERROR, nullptr, @@ -5755,6 +5763,7 @@ TEST_P(CameraHidlTest, flushPreviewRequest) { settings = req; }); ASSERT_TRUE(ret.isOk()); + overrideRotateAndCrop(&settings); hidl_handle buffer_handle; if (useHalBufManager) { @@ -8935,6 +8944,25 @@ void CameraHidlTest::verifyRequestTemplate(const camera_metadata_t* metadata, } } +void CameraHidlTest::overrideRotateAndCrop( + ::android::hardware::hidl_vec *settings /*in/out*/) { + if (settings == nullptr) { + return; + } + + ::android::hardware::camera::common::V1_0::helper::CameraMetadata requestMeta; + requestMeta.append(reinterpret_cast (settings->data())); + auto entry = requestMeta.find(ANDROID_SCALER_ROTATE_AND_CROP); + if ((entry.count > 0) && (entry.data.u8[0] == ANDROID_SCALER_ROTATE_AND_CROP_AUTO)) { + uint8_t disableRotateAndCrop = ANDROID_SCALER_ROTATE_AND_CROP_NONE; + requestMeta.update(ANDROID_SCALER_ROTATE_AND_CROP, &disableRotateAndCrop, 1); + settings->releaseData(); + camera_metadata_t *metaBuffer = requestMeta.release(); + settings->setToExternal(reinterpret_cast (metaBuffer), + get_camera_metadata_size(metaBuffer), true); + } +} + GTEST_ALLOW_UNINSTANTIATED_PARAMETERIZED_TEST(CameraHidlTest); INSTANTIATE_TEST_SUITE_P( PerInstance, CameraHidlTest, From dc6da704eda36e050812357a647a59304d73d17b Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Wed, 27 Oct 2021 09:21:44 -0700 Subject: [PATCH 24/35] Camera: Adjust VTS test to relax multi-camera requirement The multi-camera Grf requirement is now only applicable to rear facing camera. Test: Run test on Pixel devices Bug: 204252005 Change-Id: Idb7735eebdc104f6ee6a5946b50c11aea809eb85 --- .../VtsHalCameraProviderV2_4TargetTest.cpp | 64 ++++++------------- 1 file changed, 19 insertions(+), 45 deletions(-) diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index ad3da48aab..0f55bc1ed9 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -6199,14 +6199,13 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { return; } - // Test that if more than one color cameras facing the same direction are - // supported, there must be at least one logical camera facing that - // direction. + // Test that if more than one rear-facing color camera is + // supported, there must be at least one rear-facing logical camera. hidl_vec cameraDeviceNames = getCameraDeviceNames(mProvider); - // Front and back facing non-logical color cameras - std::set frontColorCameras, rearColorCameras; - // Front and back facing logical cameras' physical camera Id sets - std::set> frontPhysicalIds, rearPhysicalIds; + // Back facing non-logical color cameras + std::set rearColorCameras; + // Back facing logical cameras' physical camera Id sets + std::set> rearPhysicalIds; for (const auto& name : cameraDeviceNames) { std::string cameraId; int deviceVersion = getCameraDeviceVersionAndId(name, mProviderType, &cameraId); @@ -6238,8 +6237,8 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { return; } - // Check camera facing. Skip if facing is neither FRONT - // nor BACK. If this is not a logical camera, only note down + // Check camera facing. Skip if facing is not BACK. + // If this is not a logical camera, only note down // the camera ID, and skip. camera_metadata_ro_entry entry; int retcode = find_camera_metadata_ro_entry( @@ -6248,18 +6247,12 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { ASSERT_GT(entry.count, 0); uint8_t facing = entry.data.u8[0]; bool isLogicalCamera = (isLogicalMultiCamera(metadata) == Status::OK); - if (facing == ANDROID_LENS_FACING_FRONT) { - if (!isLogicalCamera) { - frontColorCameras.insert(cameraId); - return; - } - } else if (facing == ANDROID_LENS_FACING_BACK) { - if (!isLogicalCamera) { - rearColorCameras.insert(cameraId); - return; - } - } else { - // Not FRONT or BACK facing. Skip. + if (facing != ANDROID_LENS_FACING_BACK) { + // Not BACK facing. Skip. + return; + } + if (!isLogicalCamera) { + rearColorCameras.insert(cameraId); return; } @@ -6268,11 +6261,7 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { std::unordered_set physicalCameraIds; Status s = getPhysicalCameraIds(metadata, &physicalCameraIds); ASSERT_EQ(Status::OK, s); - if (facing == ANDROID_LENS_FACING_FRONT) { - frontPhysicalIds.emplace(physicalCameraIds.begin(), physicalCameraIds.end()); - } else { - rearPhysicalIds.emplace(physicalCameraIds.begin(), physicalCameraIds.end()); - } + rearPhysicalIds.emplace(physicalCameraIds.begin(), physicalCameraIds.end()); for (const auto& physicalId : physicalCameraIds) { // Skip if the physicalId is publicly available for (auto& deviceName : cameraDeviceNames) { @@ -6299,11 +6288,7 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { (camera_metadata_t*)chars.data(); if (CameraHidlTest::isColorCamera(physicalMetadata)) { - if (facing == ANDROID_LENS_FACING_FRONT) { - frontColorCameras.insert(physicalId); - } else if (facing == ANDROID_LENS_FACING_BACK) { - rearColorCameras.insert(physicalId); - } + rearColorCameras.insert(physicalId); } }); ASSERT_TRUE(ret.isOk()); @@ -6321,20 +6306,9 @@ TEST_P(CameraHidlTest, grfSMultiCameraTest) { } } - // If there are more than one color cameras facing one direction, a logical - // multi-camera must be defined consisting of all color cameras facing that - // direction. - if (frontColorCameras.size() > 1) { - bool hasFrontLogical = false; - for (const auto& physicalIds : frontPhysicalIds) { - if (std::includes(physicalIds.begin(), physicalIds.end(), - frontColorCameras.begin(), frontColorCameras.end())) { - hasFrontLogical = true; - break; - } - } - ASSERT_TRUE(hasFrontLogical); - } + // If there are more than one rear-facing color camera, a logical + // multi-camera must be defined consisting of all rear-facing color + // cameras. if (rearColorCameras.size() > 1) { bool hasRearLogical = false; for (const auto& physicalIds : rearPhysicalIds) { From 5ee43a6d246304b53cbf131b9be12907398a4263 Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Thu, 28 Oct 2021 23:39:27 -0700 Subject: [PATCH 25/35] Camera: VTS for test_pattern tag requirement for physical camera Ensure that if TEST_PATTERN_MODE is listed in physical request keys, the corresponding physical camera must support the privacy related test pattern modes. Test: Run Camera VTS with Cuttlefish emulator Bug: 204108650 Change-Id: I8f378ab642c7c010a2ba73a8e89e65c91ba780eb --- .../VtsHalCameraProviderV2_4TargetTest.cpp | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index 62de3afec3..d02547c81e 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -936,6 +936,9 @@ public: camera_metadata_ro_entry* streamConfigs, camera_metadata_ro_entry* maxResolutionStreamConfigs, const camera_metadata_t* staticMetadata); + void getPrivacyTestPatternModes( + const camera_metadata_t* staticMetadata, + std::unordered_set* privacyTestPatternModes/*out*/); static bool isColorCamera(const camera_metadata_t *metadata); static V3_2::DataspaceFlags getDataspace(PixelFormat format); @@ -6762,6 +6765,25 @@ void CameraHidlTest::getMultiResolutionStreamConfigurations( ASSERT_TRUE(-ENOENT == retcode || 0 == retcode); } +void CameraHidlTest::getPrivacyTestPatternModes( + const camera_metadata_t* staticMetadata, + std::unordered_set* privacyTestPatternModes/*out*/) { + ASSERT_NE(staticMetadata, nullptr); + ASSERT_NE(privacyTestPatternModes, nullptr); + + camera_metadata_ro_entry entry; + int retcode = find_camera_metadata_ro_entry( + staticMetadata, ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES, &entry); + ASSERT_TRUE(0 == retcode); + + for (auto i = 0; i < entry.count; i++) { + if (entry.data.i32[i] == ANDROID_SENSOR_TEST_PATTERN_MODE_SOLID_COLOR || + entry.data.i32[i] == ANDROID_SENSOR_TEST_PATTERN_MODE_BLACK) { + privacyTestPatternModes->insert(entry.data.i32[i]); + } + } +} + // Select an appropriate dataspace given a specific pixel format. V3_2::DataspaceFlags CameraHidlTest::getDataspace(PixelFormat format) { switch (format) { @@ -7816,6 +7838,16 @@ void CameraHidlTest::verifyLogicalOrUltraHighResCameraMetadata( ASSERT_TRUE(isUltraHighResCamera && !isMultiCamera); physicalIds.insert(cameraId); } + + std::unordered_set physicalRequestKeyIDs; + rc = getSupportedKeys(const_cast(metadata), + ANDROID_REQUEST_AVAILABLE_PHYSICAL_CAMERA_REQUEST_KEYS, &physicalRequestKeyIDs); + ASSERT_TRUE(Status::OK == rc); + bool hasTestPatternPhysicalRequestKey = physicalRequestKeyIDs.find( + ANDROID_SENSOR_TEST_PATTERN_MODE) != physicalRequestKeyIDs.end(); + std::unordered_set privacyTestPatternModes; + getPrivacyTestPatternModes(metadata, &privacyTestPatternModes); + // Map from image format to number of multi-resolution sizes for that format std::unordered_map multiResOutputFormatCounterMap; std::unordered_map multiResInputFormatCounterMap; @@ -7837,6 +7869,7 @@ void CameraHidlTest::verifyLogicalOrUltraHighResCameraMetadata( camera_metadata_ro_entry physicalStreamConfigs; camera_metadata_ro_entry physicalMaxResolutionStreamConfigs; bool isUltraHighRes = false; + std::unordered_set subCameraPrivacyTestPatterns; if (isPublicId) { ::android::sp<::android::hardware::camera::device::V3_2::ICameraDevice> subDevice; Return ret; @@ -7867,6 +7900,8 @@ void CameraHidlTest::verifyLogicalOrUltraHighResCameraMetadata( &physicalMultiResStreamConfigs, &physicalStreamConfigs, &physicalMaxResolutionStreamConfigs, staticMetadata); isUltraHighRes = isUltraHighResolution(staticMetadata); + + getPrivacyTestPatternModes(staticMetadata, &subCameraPrivacyTestPatterns); }); ASSERT_TRUE(ret.isOk()); } else { @@ -7893,6 +7928,7 @@ void CameraHidlTest::verifyLogicalOrUltraHighResCameraMetadata( &physicalMultiResStreamConfigs, &physicalStreamConfigs, &physicalMaxResolutionStreamConfigs, staticMetadata); isUltraHighRes = isUltraHighResolution(staticMetadata); + getPrivacyTestPatternModes(staticMetadata, &subCameraPrivacyTestPatterns); }); ASSERT_TRUE(ret.isOk()); @@ -7909,6 +7945,10 @@ void CameraHidlTest::verifyLogicalOrUltraHighResCameraMetadata( ASSERT_TRUE(ret.isOk()); } + if (hasTestPatternPhysicalRequestKey) { + ASSERT_TRUE(privacyTestPatternModes == subCameraPrivacyTestPatterns); + } + if (physicalMultiResStreamConfigs.count > 0) { ASSERT_GE(deviceVersion, CAMERA_DEVICE_API_VERSION_3_7); ASSERT_EQ(physicalMultiResStreamConfigs.count % 4, 0); From 2f361c15718b660957712c2992653af5751da024 Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Mon, 11 Oct 2021 12:49:50 +0100 Subject: [PATCH 26/35] KeyMint VTS: check INCLUDE_UNIQUE_ID works Bug: 202487002 Test: atest VtsAidlKeyMintTargetTest (on CF, O6) Merged-In: I8bc674b47549aa1133f816c510289774db752e04 Change-Id: I8bc674b47549aa1133f816c510289774db752e04 Ignore-AOSP-First: already in aosp/master --- .../vts/functional/KeyMintAidlTestBase.cpp | 7 +- .../aidl/vts/functional/KeyMintAidlTestBase.h | 3 +- .../aidl/vts/functional/KeyMintTest.cpp | 88 +++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp index 20324117b9..8e35c91b66 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.cpp @@ -1298,7 +1298,8 @@ bool verify_attestation_record(const string& challenge, // AuthorizationSet expected_sw_enforced, // AuthorizationSet expected_hw_enforced, // SecurityLevel security_level, - const vector& attestation_cert) { + const vector& attestation_cert, + vector* unique_id) { X509_Ptr cert(parse_cert_blob(attestation_cert)); EXPECT_TRUE(!!cert.get()); if (!cert.get()) return false; @@ -1458,6 +1459,10 @@ bool verify_attestation_record(const string& challenge, // expected_hw_enforced.Sort(); EXPECT_EQ(filtered_tags(expected_hw_enforced), filtered_tags(att_hw_enforced)); + if (unique_id != nullptr) { + *unique_id = att_unique_id; + } + return true; } diff --git a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h index ec3fcf6a3e..7b3b9d4b4b 100644 --- a/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h +++ b/security/keymint/aidl/vts/functional/KeyMintAidlTestBase.h @@ -338,7 +338,8 @@ bool verify_attestation_record(const string& challenge, // AuthorizationSet expected_sw_enforced, // AuthorizationSet expected_hw_enforced, // SecurityLevel security_level, - const vector& attestation_cert); + const vector& attestation_cert, + vector* unique_id = nullptr); string bin2hex(const vector& data); X509_Ptr parse_cert_blob(const vector& blob); diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 2a0ee7fd3e..5647db6a07 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -1549,6 +1549,94 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationTags) { } } +/* + * NewKeyGenerationTest.EcdsaAttestationUniqueId + * + * Verifies that creation of an attested ECDSA key with a UNIQUE_ID included. + */ +TEST_P(NewKeyGenerationTest, EcdsaAttestationUniqueId) { + auto get_unique_id = [this](const std::string& app_id, uint64_t datetime, + vector* unique_id) { + auto challenge = "hello"; + auto subject = "cert subj 2"; + vector subject_der(make_name_from_str(subject)); + uint64_t serial_int = 0x1010; + vector serial_blob(build_serial_blob(serial_int)); + const AuthorizationSetBuilder builder = + AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .Authorization(TAG_INCLUDE_UNIQUE_ID) + .EcdsaSigningKey(EcCurve::P_256) + .Digest(Digest::NONE) + .AttestationChallenge(challenge) + .Authorization(TAG_CERTIFICATE_SERIAL, serial_blob) + .Authorization(TAG_CERTIFICATE_SUBJECT, subject_der) + .AttestationApplicationId(app_id) + .Authorization(TAG_CREATION_DATETIME, datetime) + .SetDefaultValidity(); + + ASSERT_EQ(ErrorCode::OK, GenerateKey(builder)); + ASSERT_GT(key_blob_.size(), 0U); + + EXPECT_TRUE(ChainSignaturesAreValid(cert_chain_)); + ASSERT_GT(cert_chain_.size(), 0); + verify_subject_and_serial(cert_chain_[0], serial_int, subject, /* self_signed = */ false); + + AuthorizationSet hw_enforced = HwEnforcedAuthorizations(key_characteristics_); + AuthorizationSet sw_enforced = SwEnforcedAuthorizations(key_characteristics_); + + // Check that the unique ID field in the extension is non-empty. + EXPECT_TRUE(verify_attestation_record(challenge, app_id, sw_enforced, hw_enforced, + SecLevel(), cert_chain_[0].encodedCertificate, + unique_id)); + EXPECT_GT(unique_id->size(), 0); + CheckedDeleteKey(); + }; + + // Generate unique ID + auto app_id = "foo"; + uint64_t cert_date = 1619621648000; // Wed Apr 28 14:54:08 2021 in ms since epoch + vector unique_id; + get_unique_id(app_id, cert_date, &unique_id); + + // Generating a new key with the same parameters should give the same unique ID. + vector unique_id2; + get_unique_id(app_id, cert_date, &unique_id2); + EXPECT_EQ(unique_id, unique_id2); + + // Generating a new key with a slightly different date should give the same unique ID. + uint64_t rounded_date = cert_date / 2592000000LLU; + uint64_t min_date = rounded_date * 2592000000LLU; + uint64_t max_date = ((rounded_date + 1) * 2592000000LLU) - 1; + + vector unique_id3; + get_unique_id(app_id, min_date, &unique_id3); + EXPECT_EQ(unique_id, unique_id3); + + vector unique_id4; + get_unique_id(app_id, max_date, &unique_id4); + EXPECT_EQ(unique_id, unique_id4); + + // A different attestation application ID should yield a different unique ID. + auto app_id2 = "different_foo"; + vector unique_id5; + get_unique_id(app_id2, cert_date, &unique_id5); + EXPECT_NE(unique_id, unique_id5); + + // A radically different date should yield a different unique ID. + vector unique_id6; + get_unique_id(app_id, 1611621648000, &unique_id6); + EXPECT_NE(unique_id, unique_id6); + + vector unique_id7; + get_unique_id(app_id, max_date + 1, &unique_id7); + EXPECT_NE(unique_id, unique_id7); + + vector unique_id8; + get_unique_id(app_id, min_date - 1, &unique_id8); + EXPECT_NE(unique_id, unique_id8); +} + /* * NewKeyGenerationTest.EcdsaAttestationTagNoApplicationId * From e26fab78ed1ecc6e6360f418201b3c55b7d00d0d Mon Sep 17 00:00:00 2001 From: David Drysdale Date: Mon, 1 Nov 2021 11:40:08 +0000 Subject: [PATCH 27/35] KeyMint VTS: extra unique ID test Test that specifying RESET_SINCE_ID_ROTATION results in a different unique ID value. Test: VtsAidlKeyMintTargetTest Bug: 202487002 Change-Id: I2aed96514bf9e4802f0ef756f880cac79fa09554 --- .../DeviceUniqueAttestationTest.cpp | 40 +++++++++++-------- .../aidl/vts/functional/KeyMintTest.cpp | 12 +++++- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp index d7abf0790c..79716b1354 100644 --- a/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp +++ b/security/keymint/aidl/vts/functional/DeviceUniqueAttestationTest.cpp @@ -76,6 +76,7 @@ TEST_P(DeviceUniqueAttestationTest, RsaNonStrongBoxUnimplemented) { .Digest(Digest::SHA_2_256) .Padding(PaddingMode::RSA_PKCS1_1_5_SIGN) .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) .AttestationChallenge("challenge") .AttestationApplicationId("foo") .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), @@ -102,6 +103,7 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaNonStrongBoxUnimplemented) { .EcdsaSigningKey(EcCurve::P_256) .Digest(Digest::SHA_2_256) .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) .AttestationChallenge("challenge") .AttestationApplicationId("foo") .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), @@ -129,6 +131,7 @@ TEST_P(DeviceUniqueAttestationTest, RsaDeviceUniqueAttestation) { .Digest(Digest::SHA_2_256) .Padding(PaddingMode::RSA_PKCS1_1_5_SIGN) .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) .AttestationChallenge("challenge") .AttestationApplicationId("foo") .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), @@ -184,6 +187,7 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaDeviceUniqueAttestation) { .EcdsaSigningKey(EcCurve::P_256) .Digest(Digest::SHA_2_256) .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) .AttestationChallenge("challenge") .AttestationApplicationId("foo") .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION), @@ -242,14 +246,16 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaDeviceUniqueAttestationID) { for (const KeyParameter& tag : attestation_id_tags) { SCOPED_TRACE(testing::Message() << "+tag-" << tag); - AuthorizationSetBuilder builder = AuthorizationSetBuilder() - .Authorization(TAG_NO_AUTH_REQUIRED) - .EcdsaSigningKey(EcCurve::P_256) - .Digest(Digest::SHA_2_256) - .Authorization(TAG_INCLUDE_UNIQUE_ID) - .AttestationChallenge("challenge") - .AttestationApplicationId("foo") - .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION); + AuthorizationSetBuilder builder = + AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .EcdsaSigningKey(EcCurve::P_256) + .Digest(Digest::SHA_2_256) + .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) + .AttestationChallenge("challenge") + .AttestationApplicationId("foo") + .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION); builder.push_back(tag); auto result = GenerateKey(builder, &key_blob, &key_characteristics); @@ -310,14 +316,16 @@ TEST_P(DeviceUniqueAttestationTest, EcdsaDeviceUniqueAttestationMismatchID) { for (const KeyParameter& invalid_tag : attestation_id_tags) { SCOPED_TRACE(testing::Message() << "+tag-" << invalid_tag); - AuthorizationSetBuilder builder = AuthorizationSetBuilder() - .Authorization(TAG_NO_AUTH_REQUIRED) - .EcdsaSigningKey(EcCurve::P_256) - .Digest(Digest::SHA_2_256) - .Authorization(TAG_INCLUDE_UNIQUE_ID) - .AttestationChallenge("challenge") - .AttestationApplicationId("foo") - .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION); + AuthorizationSetBuilder builder = + AuthorizationSetBuilder() + .Authorization(TAG_NO_AUTH_REQUIRED) + .EcdsaSigningKey(EcCurve::P_256) + .Digest(Digest::SHA_2_256) + .Authorization(TAG_INCLUDE_UNIQUE_ID) + .Authorization(TAG_CREATION_DATETIME, 1619621648000) + .AttestationChallenge("challenge") + .AttestationApplicationId("foo") + .Authorization(TAG_DEVICE_UNIQUE_ATTESTATION); // 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); diff --git a/security/keymint/aidl/vts/functional/KeyMintTest.cpp b/security/keymint/aidl/vts/functional/KeyMintTest.cpp index 5647db6a07..4d7f1b8eec 100644 --- a/security/keymint/aidl/vts/functional/KeyMintTest.cpp +++ b/security/keymint/aidl/vts/functional/KeyMintTest.cpp @@ -1556,13 +1556,13 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationTags) { */ TEST_P(NewKeyGenerationTest, EcdsaAttestationUniqueId) { auto get_unique_id = [this](const std::string& app_id, uint64_t datetime, - vector* unique_id) { + vector* unique_id, bool reset = false) { auto challenge = "hello"; auto subject = "cert subj 2"; vector subject_der(make_name_from_str(subject)); uint64_t serial_int = 0x1010; vector serial_blob(build_serial_blob(serial_int)); - const AuthorizationSetBuilder builder = + AuthorizationSetBuilder builder = AuthorizationSetBuilder() .Authorization(TAG_NO_AUTH_REQUIRED) .Authorization(TAG_INCLUDE_UNIQUE_ID) @@ -1574,6 +1574,9 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationUniqueId) { .AttestationApplicationId(app_id) .Authorization(TAG_CREATION_DATETIME, datetime) .SetDefaultValidity(); + if (reset) { + builder.Authorization(TAG_RESET_SINCE_ID_ROTATION); + } ASSERT_EQ(ErrorCode::OK, GenerateKey(builder)); ASSERT_GT(key_blob_.size(), 0U); @@ -1635,6 +1638,11 @@ TEST_P(NewKeyGenerationTest, EcdsaAttestationUniqueId) { vector unique_id8; get_unique_id(app_id, min_date - 1, &unique_id8); EXPECT_NE(unique_id, unique_id8); + + // Marking RESET_SINCE_ID_ROTATION should give a different unique ID. + vector unique_id9; + get_unique_id(app_id, cert_date, &unique_id9, /* reset_id = */ true); + EXPECT_NE(unique_id, unique_id9); } /* From 4b18fff621677efc0a8c8c0bdfc48bfca934a650 Mon Sep 17 00:00:00 2001 From: Jayachandran C Date: Sun, 7 Nov 2021 23:45:29 -0800 Subject: [PATCH 28/35] Add some delay for SIM power up and down delay for setSimCardPower_1_6 API Bug: 203031664 Test: VTS Change-Id: I68e7352ed95a8487dee291de493c78cf1491d569 Merged-In: I68e7352ed95a8487dee291de493c78cf1491d569 --- radio/1.6/vts/functional/radio_hidl_hal_api.cpp | 6 ++++++ radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h | 1 + 2 files changed, 7 insertions(+) diff --git a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp index cdfcc94cb8..76bbebbfef 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_api.cpp +++ b/radio/1.6/vts/functional/radio_hidl_hal_api.cpp @@ -601,6 +601,9 @@ TEST_P(RadioHidlTest_v1_6, setSimCardPower_1_6) { ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE})); + // Give some time for modem to fully power up the SIM card + sleep(MODEM_SET_SIM_POWER_DELAY_IN_SECONDS); + // setSimCardPower_1_6 does not return until the request is handled, and should not trigger // CardState::ABSENT when turning off power if (radioRsp_v1_6->rspInfo.error == ::android::hardware::radio::V1_6::RadioError::NONE) { @@ -624,6 +627,9 @@ TEST_P(RadioHidlTest_v1_6, setSimCardPower_1_6) { ::android::hardware::radio::V1_6::RadioError::INVALID_ARGUMENTS, ::android::hardware::radio::V1_6::RadioError::RADIO_NOT_AVAILABLE})); + // Give some time for modem to fully power up the SIM card + sleep(MODEM_SET_SIM_POWER_DELAY_IN_SECONDS); + // setSimCardPower_1_6 does not return until the request is handled. Just verify that we still // have CardState::PRESENT after turning the power back on if (radioRsp_v1_6->rspInfo.error == ::android::hardware::radio::V1_6::RadioError::NONE) { diff --git a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h index 54c297719f..f0418652fd 100644 --- a/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h +++ b/radio/1.6/vts/functional/radio_hidl_hal_utils_v1_6.h @@ -47,6 +47,7 @@ using ::android::hardware::Void; #define MODEM_EMERGENCY_CALL_ESTABLISH_TIME 3 #define MODEM_EMERGENCY_CALL_DISCONNECT_TIME 3 +#define MODEM_SET_SIM_POWER_DELAY_IN_SECONDS 2 #define RADIO_SERVICE_SLOT1_NAME "slot1" // HAL instance name for SIM slot 1 or single SIM device #define RADIO_SERVICE_SLOT2_NAME "slot2" // HAL instance name for SIM slot 2 on dual SIM device From 79ab13a1a67f1abfdabf8405f241dfd62f29ab37 Mon Sep 17 00:00:00 2001 From: Mikhail Naganov Date: Wed, 3 Nov 2021 20:15:36 +0000 Subject: [PATCH 29/35] audio: Fix handling of dynamic profiles in VtsHalAudioV7_0TargetTest The profiles generator wasn't loading correctly from the Audio Policy Manager config file profiles with unpopulated attributes, which is typical for dynamic profiles. Bug: 204314749 Test: atest HalAudioV7_0GeneratorTest Change-Id: I514a4e03da165cacb5dbaaa16470130895681484 Merged-In: I514a4e03da165cacb5dbaaa16470130895681484 --- .../vts/functional/7.0/Generators.cpp | 6 + .../all-versions/vts/functional/Android.bp | 1 + .../tests/HalAudioV7_0GeneratorTest.xml | 1 + .../tests/apm_config_b_204314749_7_0.xml | 263 ++++++++++++++++++ .../vts/functional/tests/generators_tests.cpp | 8 +- 5 files changed, 278 insertions(+), 1 deletion(-) create mode 100644 audio/core/all-versions/vts/functional/tests/apm_config_b_204314749_7_0.xml diff --git a/audio/core/all-versions/vts/functional/7.0/Generators.cpp b/audio/core/all-versions/vts/functional/7.0/Generators.cpp index d2ba3397af..8c92cbd161 100644 --- a/audio/core/all-versions/vts/functional/7.0/Generators.cpp +++ b/audio/core/all-versions/vts/functional/7.0/Generators.cpp @@ -102,6 +102,9 @@ std::vector generateOutputDeviceConfigParameters(bool one if (mixPort.getRole() != xsd::Role::source) continue; // not an output profile auto [flags, isOffload] = generateOutFlags(mixPort); for (const auto& profile : mixPort.getProfile()) { + if (!profile.hasFormat() || !profile.hasSamplingRates() || + !profile.hasChannelMasks()) + continue; auto configs = combineAudioConfig(profile.getChannelMasks(), profile.getSamplingRates(), profile.getFormat()); for (auto& config : configs) { @@ -231,6 +234,9 @@ std::vector generateInputDeviceConfigParameters(bool oneP std::back_inserter(flags), [](auto flag) { return toString(flag); }); } for (const auto& profile : mixPort.getProfile()) { + if (!profile.hasFormat() || !profile.hasSamplingRates() || + !profile.hasChannelMasks()) + continue; auto configs = combineAudioConfig(profile.getChannelMasks(), profile.getSamplingRates(), profile.getFormat()); for (const auto& config : configs) { diff --git a/audio/core/all-versions/vts/functional/Android.bp b/audio/core/all-versions/vts/functional/Android.bp index e446a7f2cf..c576060b2e 100644 --- a/audio/core/all-versions/vts/functional/Android.bp +++ b/audio/core/all-versions/vts/functional/Android.bp @@ -239,6 +239,7 @@ cc_test { data: [ "tests/apm_config_no_vx_7_0.xml", "tests/apm_config_with_vx_7_0.xml", + "tests/apm_config_b_204314749_7_0.xml", ], test_config: "tests/HalAudioV7_0GeneratorTest.xml", } diff --git a/audio/core/all-versions/vts/functional/tests/HalAudioV7_0GeneratorTest.xml b/audio/core/all-versions/vts/functional/tests/HalAudioV7_0GeneratorTest.xml index 2e794554c8..3dc5b33e40 100644 --- a/audio/core/all-versions/vts/functional/tests/HalAudioV7_0GeneratorTest.xml +++ b/audio/core/all-versions/vts/functional/tests/HalAudioV7_0GeneratorTest.xml @@ -24,6 +24,7 @@