From cce1a0ae74affaad86495e8cfaef67e31bf9af85 Mon Sep 17 00:00:00 2001 From: shrikar Date: Tue, 2 Apr 2024 22:31:12 +0000 Subject: [PATCH] Add examples to area access documentation Also add VTS checks to ensure WRITE_ONLY configs don't exist along with READ_ONLY and READ_WRITE configs Bug: 332598311 Test: atest VtsHalAutomotiveVehicle_TargetTest Change-Id: Ie3d38fc2fd582d8949736739e83d277d75d69e62 --- .../automotive/vehicle/VehicleAreaConfig.aidl | 6 +++- .../automotive/vehicle/VehiclePropConfig.aidl | 30 +++++++++++++++++++ .../VtsHalAutomotiveVehicle_TargetTest.cpp | 8 ++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehicleAreaConfig.aidl b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehicleAreaConfig.aidl index 726d4191b9..9387965df3 100644 --- a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehicleAreaConfig.aidl +++ b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehicleAreaConfig.aidl @@ -62,7 +62,9 @@ parcelable VehicleAreaConfig { * For example, if a property is defined as READ_WRITE, but the OEM wants to specify certain * area Ids as READ-only, the corresponding areaIds should have an access set to READ, while the * others must be set to READ_WRITE. We do not support setting specific area Ids to WRITE-only - * when the property is READ-WRITE. + * when the property is READ-WRITE. If any one area config has access + * VehiclePropertyAccess::WRITE, then all VehicleAreaConfig.access values and + * VehiclePropConfig.access must be set to WRITE for the property. * * VehiclePropConfig.access should be equal the maximal subset of the accesses set in * VehiclePropConfig.areaConfigs, excluding those with access == VehiclePropertyAccess.NONE. For @@ -73,6 +75,8 @@ parcelable VehicleAreaConfig { * In the scenario where the OEM actually wants to set VehicleAreaConfig.access = * VehiclePropertyAccess.NONE, the maximal subset rule should apply with this area config * included, making the VehiclePropConfig.access = VehiclePropertyAccess.NONE. + * + * See VehiclePropConfig.access for more information. */ VehiclePropertyAccess access = VehiclePropertyAccess.NONE; diff --git a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehiclePropConfig.aidl b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehiclePropConfig.aidl index 31096219f4..d8304f6fb3 100644 --- a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehiclePropConfig.aidl +++ b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/VehiclePropConfig.aidl @@ -44,6 +44,36 @@ parcelable VehiclePropConfig { * VehiclePropertyAccess.NONE for a particular area config, the maximal subset rule should apply * with this area config included, making the VehiclePropConfig.access = * VehiclePropertyAccess.NONE. + * + * Currently we do not support scenarios where some areaIds are WRITE while others are + * READ_WRITE. See the documentation for VehicleAreaConfig.access for more details. + * + * Examples: + * Suppose we have a property with two areaIds which we will call "LEFT" and "RIGHT". Here + * are some scenarios that can describe what the VehiclePropConfig.access value should be for + * this property. + * 1. LEFT is READ and RIGHT is READ_WRITE. VehiclePropConfig.access must be READ as that is + * the maximal common access across all areaIds. + * 2. LEFT is READ_WRITE and RIGHT is READ_WRITE. VehiclePropConfig.access must be READ_WRITE + * as that is the maximal common access across all areaIds. + * 3. LEFT is WRITE and RIGHT is WRITE. VehiclePropConfig.access must be WRITE as that is the + * maximal common access across all areaIds. + * 4. LEFT is READ_WRITE and RIGHT is not set (i.e. defaults to NONE)/is set to NONE, with the + * expectation that RIGHT should be populated with the default access mode of the property. + * VehiclePropConfig.access can be set to READ or READ_WRITE, whatever the OEM feels is the + * appropriate default access for the property. + * 5. LEFT is READ and RIGHT is not set (i.e. defaults to NONE)/is set to NONE, with the + * expectation that RIGHT should be populated with the default access mode of the property. + * VehiclePropConfig.access must be set to READ because setting to READ_WRITE breaks the + * rule of having the global access being the maximal subset of the area config accesses. + * If the OEM wants RIGHT to be READ_WRITE in this scenario, the config should be rewritten + * such that LEFT is not set/is set to NONE and RIGHT is set to READ_WRITE with + * VehiclePropConfig.access set to READ. + * 6. LEFT is READ_WRITE and RIGHT is set to NONE with the intention of RIGHT to specifically + * have no access. VehiclePropConfig.access must be NONE to support RIGHT maintaining its + * NONE access. + * 7. LEFT is READ_WRITE and RIGHT is WRITE. This is unsupported behaviour and the config + * should not be defined this way. */ VehiclePropertyAccess access = VehiclePropertyAccess.NONE; diff --git a/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp b/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp index 4ea6dfe5f2..30661a2bb2 100644 --- a/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp +++ b/automotive/vehicle/vts/src/VtsHalAutomotiveVehicle_TargetTest.cpp @@ -752,9 +752,15 @@ void VtsHalAutomotiveVehicleTargetTest::verifyGlobalAccessIsMaximalAreaAccessSub } } - if (readOnlyPresent && !writeOnlyPresent) { + if (readOnlyPresent) { + ASSERT_FALSE(writeOnlyPresent) << StringPrintf( + "Found both READ_ONLY and WRITE_ONLY access modes in area configs, which is not " + "supported"); maximalAreaAccessSubset = toInt(VehiclePropertyAccess::READ); } else if (writeOnlyPresent) { + ASSERT_FALSE(readWritePresent) << StringPrintf( + "Found both WRITE_ONLY and READ_WRITE access modes in area configs, which is not " + "supported"); maximalAreaAccessSubset = toInt(VehiclePropertyAccess::WRITE); } else if (readWritePresent) { maximalAreaAccessSubset = toInt(VehiclePropertyAccess::READ_WRITE);