Merge "Implement setValues in DefaultVHAL." (ffd7f9ce) · Commits · e / os / android_hardware_interfaces

automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h

+2 −2

Original line number	Diff line number	Diff line
		@@ -213,7 +213,7 @@ template <class T>
		::ndk::ScopedAStatus toScopedAStatus(
		const ::android::base::Result<T>& result,
		::aidl::android::hardware::automotive::vehicle::StatusCode status,
		std::string additionalErrorMsg) {
		const std::string& additionalErrorMsg) {
		if (result.ok()) {
		return ::ndk::ScopedAStatus::ok();
		}
		@@ -236,7 +236,7 @@ template <class T>

		template <class T>
		::ndk::ScopedAStatus toScopedAStatus(const ::android::base::Result<T>& result,
		std::string additionalErrorMsg) {
		const std::string& additionalErrorMsg) {
		return toScopedAStatus(result, getErrorCode(result), additionalErrorMsg);
		}

automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h

+11 −31

Original line number	Diff line number	Diff line
		@@ -21,7 +21,6 @@
		#include "ParcelableUtils.h"

		#include <IVehicleHardware.h>
		#include <LargeParcelableBase.h>
		#include <VehicleUtils.h>
		#include <aidl/android/hardware/automotive/vehicle/BnVehicle.h>
		#include <android-base/expected.h>
		@@ -42,36 +41,6 @@ namespace defaultvehiclehal_impl {

		constexpr int INVALID_MEMORY_FD = -1;

		template <class T>
		::ndk::ScopedAStatus toScopedAStatus(
		const ::android::base::Result<T>& result,
		::aidl::android::hardware::automotive::vehicle::StatusCode status,
		const std::string& additionalErrorMsg) {
		if (result.ok()) {
		return ::ndk::ScopedAStatus::ok();
		}
		return ::ndk::ScopedAStatus::fromServiceSpecificErrorWithMessage(
		toInt(status), (additionalErrorMsg + getErrorMsg(result)).c_str());
		}

		template <class T>
		::ndk::ScopedAStatus toScopedAStatus(
		const ::android::base::Result<T>& result,
		::aidl::android::hardware::automotive::vehicle::StatusCode status) {
		return toScopedAStatus(result, status, "");
		}

		template <class T>
		::ndk::ScopedAStatus toScopedAStatus(const ::android::base::Result<T>& result) {
		return toScopedAStatus(result, getErrorCode(result));
		}

		template <class T>
		::ndk::ScopedAStatus toScopedAStatus(const ::android::base::Result<T>& result,
		const std::string& additionalErrorMsg) {
		return toScopedAStatus(result, getErrorCode(result), additionalErrorMsg);
		}

		} // namespace defaultvehiclehal_impl

		class DefaultVehicleHal final : public ::aidl::android::hardware::automotive::vehicle::BnVehicle {
		@@ -115,8 +84,14 @@ class DefaultVehicleHal final : public ::aidl::android::hardware::automotive::ve
		using GetValuesClient =
		GetSetValuesClient<::aidl::android::hardware::automotive::vehicle::GetValueResult,
		::aidl::android::hardware::automotive::vehicle::GetValueResults>;
		using SetValuesClient =
		GetSetValuesClient<::aidl::android::hardware::automotive::vehicle::SetValueResult,
		::aidl::android::hardware::automotive::vehicle::SetValueResults>;

		const std::unique_ptr<IVehicleHardware> mVehicleHardware;

		// mConfigsByPropId and mConfigFile are only modified during initialization, so no need to
		// lock guard them.
		std::unordered_map<int32_t, ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig>
		mConfigsByPropId;
		std::unique_ptr<::ndk::ScopedFileDescriptor> mConfigFile;
		@@ -124,11 +99,16 @@ class DefaultVehicleHal final : public ::aidl::android::hardware::automotive::ve
		std::mutex mLock;
		std::unordered_map<CallbackType, std::shared_ptr<GetValuesClient>> mGetValuesClients
		GUARDED_BY(mLock);
		std::unordered_map<CallbackType, std::shared_ptr<SetValuesClient>> mSetValuesClients
		GUARDED_BY(mLock);

		template <class T>
		std::shared_ptr<T> getOrCreateClient(
		std::unordered_map<CallbackType, std::shared_ptr<T>>* clients,
		const CallbackType& callback) REQUIRES(mLock);

		::android::base::Result<void> checkProperty(
		const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);
		};

		} // namespace vehicle

automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp

+89 −2

Original line number	Diff line number	Diff line
		@@ -35,13 +35,18 @@ using ::aidl::android::hardware::automotive::vehicle::GetValueRequests;
		using ::aidl::android::hardware::automotive::vehicle::GetValueResult;
		using ::aidl::android::hardware::automotive::vehicle::GetValueResults;
		using ::aidl::android::hardware::automotive::vehicle::IVehicleCallback;
		using ::aidl::android::hardware::automotive::vehicle::SetValueRequest;
		using ::aidl::android::hardware::automotive::vehicle::SetValueRequests;
		using ::aidl::android::hardware::automotive::vehicle::SetValueResult;
		using ::aidl::android::hardware::automotive::vehicle::SetValueResults;
		using ::aidl::android::hardware::automotive::vehicle::StatusCode;
		using ::aidl::android::hardware::automotive::vehicle::SubscribeOptions;
		using ::aidl::android::hardware::automotive::vehicle::VehicleAreaConfig;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfigs;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
		using ::android::automotive::car_binder_lib::LargeParcelableBase;
		using ::android::base::Error;
		using ::android::base::expected;
		using ::android::base::Result;
		using ::ndk::ScopedAStatus;
		@@ -92,6 +97,36 @@ DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::GetValuesClient>(
		std::unordered_map<CallbackType, std::shared_ptr<GetValuesClient>>* clients,
		const CallbackType& callback);

		template std::shared_ptr<DefaultVehicleHal::SetValuesClient>
		DefaultVehicleHal::getOrCreateClient<DefaultVehicleHal::SetValuesClient>(
		std::unordered_map<CallbackType, std::shared_ptr<SetValuesClient>>* clients,
		const CallbackType& callback);

		Result<void> DefaultVehicleHal::checkProperty(const VehiclePropValue& propValue) {
		int32_t propId = propValue.prop;
		auto it = mConfigsByPropId.find(propId);
		if (it == mConfigsByPropId.end()) {
		return Error() << "no config for property, ID: " << propId;
		}
		const VehiclePropConfig& config = it->second;
		const VehicleAreaConfig* areaConfig = getAreaConfig(propValue, config);
		if (!isGlobalProp(propId) && areaConfig == nullptr) {
		// Ignore areaId for global property. For non global property, check whether areaId is
		// allowed. areaId must appear in areaConfig.
		return Error() << "invalid area ID: " << propValue.areaId << " for prop ID: " << propId
		<< ", not listed in config";
		}
		if (auto result = checkPropValue(propValue, &config); !result.ok()) {
		return Error() << "invalid property value: " << propValue.toString()
		<< ", error: " << result.error().message();
		}
		if (auto result = checkValueRange(propValue, areaConfig); !result.ok()) {
		return Error() << "property value out of range: " << propValue.toString()
		<< ", error: " << result.error().message();
		}
		return {};
		}

		ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
		const GetValueRequests& requests) {
		// TODO(b/203713317): check for duplicate properties and duplicate request IDs.
		@@ -127,8 +162,60 @@ ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
		return ScopedAStatus::ok();
		}

		ScopedAStatus DefaultVehicleHal::setValues(const CallbackType&, const SetValueRequests&) {
		// TODO(b/200737967): implement this.
		ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
		const SetValueRequests& requests) {
		// TODO(b/203713317): check for duplicate properties and duplicate request IDs.

		const std::vector<SetValueRequest>* setValueRequests;
		// Define deserializedResults here because we need it to have the same lifetime as
		// setValueRequests.
		expected<std::vector<SetValueRequest>, ScopedAStatus> deserializedResults;
		if (!requests.payloads.empty()) {
		setValueRequests = &requests.payloads;
		} else {
		deserializedResults = stableLargeParcelableToVector<SetValueRequest>(requests);
		if (!deserializedResults.ok()) {
		ALOGE("failed to parse setValues requests");
		return std::move(deserializedResults.error());
		}
		setValueRequests = &deserializedResults.value();
		}

		// A list of failed result we already know before sending to hardware.
		std::vector<SetValueResult> failedResults;
		// The list of requests that we would send to hardware.
		std::vector<SetValueRequest> hardwareRequests;

		for (auto& request : *setValueRequests) {
		int64_t requestId = request.requestId;
		if (auto result = checkProperty(request.value); !result.ok()) {
		ALOGW("property not valid: %s", result.error().message().c_str());
		failedResults.push_back(SetValueResult{
		.requestId = requestId,
		.status = StatusCode::INVALID_ARG,
		});
		continue;
		}
		hardwareRequests.push_back(request);
		}

		std::shared_ptr<SetValuesClient> client;
		{
		std::scoped_lock<std::mutex> lockGuard(mLock);
		client = getOrCreateClient(&mSetValuesClients, callback);
		}

		if (!failedResults.empty()) {
		client->sendResults(failedResults);
		}

		if (StatusCode status =
		mVehicleHardware->setValues(client->getResultCallback(), hardwareRequests);
		status != StatusCode::OK) {
		return ScopedAStatus::fromServiceSpecificErrorWithMessage(
		toInt(status), "failed to set value to VehicleHardware");
		}

		return ScopedAStatus::ok();
		}

automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp

+229 −7

Original line number	Diff line number	Diff line
		@@ -53,6 +53,7 @@ using ::aidl::android::hardware::automotive::vehicle::SetValueRequests;
		using ::aidl::android::hardware::automotive::vehicle::SetValueResult;
		using ::aidl::android::hardware::automotive::vehicle::SetValueResults;
		using ::aidl::android::hardware::automotive::vehicle::StatusCode;
		using ::aidl::android::hardware::automotive::vehicle::VehicleAreaWindow;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfigs;
		using ::aidl::android::hardware::automotive::vehicle::VehiclePropErrors;
		@@ -68,6 +69,10 @@ using ::ndk::ScopedFileDescriptor;
		using ::testing::Eq;
		using ::testing::WhenSortedBy;

		constexpr int32_t INVALID_PROP_ID = 0;
		// VehiclePropertyGroup:SYSTEM,VehicleArea:WINDOW,VehiclePropertyType:INT32
		constexpr int32_t INT32_WINDOW_PROP = 10001 + 0x10000000 + 0x03000000 + 0x00400000;

		template <class T>
		std::optional<T> pop(std::list<T>& items) {
		if (items.size() > 0) {
		@@ -78,13 +83,13 @@ std::optional<T> pop(std::list<T>& items) {
		return std::nullopt;
		}

		class MockVehicleHardware final : public IVehicleHardware {
		public:
		std::vector<VehiclePropConfig> getAllPropertyConfigs() const override {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		return mPropertyConfigs;
		int32_t testInt32VecProp(size_t i) {
		// VehiclePropertyGroup:SYSTEM,VehicleArea:GLOBAL,VehiclePropertyType:INT32_VEC
		return static_cast<int32_t>(i) + 0x10000000 + 0x01000000 + 0x00410000;
		}

		class MockVehicleHardware final : public IVehicleHardware {
		public:
		~MockVehicleHardware() {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		for (auto& thread : mThreads) {
		@@ -92,6 +97,11 @@ class MockVehicleHardware final : public IVehicleHardware {
		}
		}

		std::vector<VehiclePropConfig> getAllPropertyConfigs() const override {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		return mPropertyConfigs;
		}

		StatusCode setValues(std::shared_ptr<const SetValuesCallback> callback,
		const std::vector<SetValueRequest>& requests) override {
		std::scoped_lock<std::mutex> lockGuard(mLock);
		@@ -262,12 +272,83 @@ struct PropConfigCmp {
		}
		} propConfigCmp;

		struct SetValuesInvalidRequestTestCase {
		std::string name;
		VehiclePropValue request;
		StatusCode expectedStatus;
		};

		std::vector<SetValuesInvalidRequestTestCase> getSetValuesInvalidRequestTestCases() {
		return {{
		.name = "config_not_found",
		.request =
		{
		// No config for INVALID_PROP_ID.
		.prop = INVALID_PROP_ID,
		},
		.expectedStatus = StatusCode::INVALID_ARG,
		},
		{
		.name = "invalid_prop_value",
		.request =
		{
		.prop = testInt32VecProp(0),
		// No int32Values for INT32_VEC property.
		.value.int32Values = {},
		},
		.expectedStatus = StatusCode::INVALID_ARG,
		},
		{
		.name = "value_out_of_range",
		.request =
		{
		.prop = testInt32VecProp(0),
		// We configured the range to be 0-100.
		.value.int32Values = {0, -1},
		},
		.expectedStatus = StatusCode::INVALID_ARG,
		},
		{
		.name = "invalid_area",
		.request =
		{
		.prop = INT32_WINDOW_PROP,
		.value.int32Values = {0},
		// Only ROW_1_LEFT is allowed.
		.areaId = toInt(VehicleAreaWindow::ROW_1_RIGHT),
		},
		.expectedStatus = StatusCode::INVALID_ARG,
		}};
		}

		} // namespace

		class DefaultVehicleHalTest : public ::testing::Test {
		public:
		void SetUp() override {
		auto hardware = std::make_unique<MockVehicleHardware>();
		std::vector<VehiclePropConfig> testConfigs;
		for (size_t i = 0; i < 10000; i++) {
		testConfigs.push_back(VehiclePropConfig{
		.prop = testInt32VecProp(i),
		.areaConfigs =
		{
		{
		.areaId = 0,
		.minInt32Value = 0,
		.maxInt32Value = 100,
		},
		},
		});
		}
		testConfigs.push_back(
		VehiclePropConfig{.prop = INT32_WINDOW_PROP,
		.areaConfigs = {{
		.areaId = toInt(VehicleAreaWindow::ROW_1_LEFT),
		.minInt32Value = 0,
		.maxInt32Value = 100,
		}}});
		hardware->setPropertyConfigs(testConfigs);
		mHardwarePtr = hardware.get();
		mVhal = ndk::SharedRefBase::make<DefaultVehicleHal>(std::move(hardware));
		mVhalClient = IVehicle::fromBinder(mVhal->asBinder());
		@@ -289,7 +370,7 @@ class DefaultVehicleHalTest : public ::testing::Test {
		expectedHardwareRequests.clear();
		for (size_t i = 0; i < size; i++) {
		int64_t requestId = static_cast<int64_t>(i);
		int32_t propId = static_cast<int32_t>(i);
		int32_t propId = testInt32VecProp(i);
		expectedHardwareRequests.push_back(GetValueRequest{
		.prop =
		VehiclePropValue{
		@@ -321,9 +402,43 @@ class DefaultVehicleHalTest : public ::testing::Test {
		return {};
		}

		static Result<void> setValuesTestCases(size_t size, SetValueRequests& requests,
		std::vector<SetValueResult>& expectedResults,
		std::vector<SetValueRequest>& expectedHardwareRequests) {
		expectedHardwareRequests.clear();
		for (size_t i = 0; i < size; i++) {
		int64_t requestId = static_cast<int64_t>(i);
		int32_t propId = testInt32VecProp(i);
		expectedHardwareRequests.push_back(SetValueRequest{
		.value =
		VehiclePropValue{
		.prop = propId,
		.value.int32Values = {1, 2, 3, 4},
		},
		.requestId = requestId,
		});
		expectedResults.push_back(SetValueResult{
		.requestId = requestId,
		.status = StatusCode::OK,
		});
		}

		auto result = LargeParcelableBase::parcelableVectorToStableLargeParcelable(
		expectedHardwareRequests);
		if (!result.ok()) {
		return result.error();
		}
		if (result.value() == nullptr) {
		requests.payloads = expectedHardwareRequests;
		} else {
		requests.sharedMemoryFd = std::move(*result.value());
		}
		return {};
		}

		size_t countClients() {
		std::scoped_lock<std::mutex> lockGuard(mVhal->mLock);
		return mVhal->mGetValuesClients.size();
		return mVhal->mGetValuesClients.size() + mVhal->mSetValuesClients.size();
		}

		private:
		@@ -462,6 +577,113 @@ TEST_F(DefaultVehicleHalTest, testGetValuesInvalidLargeParcelableInput) {
		ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::INVALID_ARG));
		}

		TEST_F(DefaultVehicleHalTest, testSetValuesSmall) {
		SetValueRequests requests;
		std::vector<SetValueResult> expectedResults;
		std::vector<SetValueRequest> expectedHardwareRequests;

		ASSERT_TRUE(setValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());

		getHardware()->addSetValueResponses(expectedResults);

		auto status = getClient()->setValues(getCallbackClient(), requests);

		ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();

		EXPECT_EQ(getHardware()->nextSetValueRequests(), expectedHardwareRequests)
		<< "requests to hardware mismatch";

		auto maybeSetValueResults = getCallback()->nextSetValueResults();
		ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";
		ASSERT_EQ(maybeSetValueResults.value().payloads, expectedResults) << "results mismatch";
		EXPECT_EQ(countClients(), static_cast<size_t>(1));
		}

		TEST_F(DefaultVehicleHalTest, testSetValuesLarge) {
		SetValueRequests requests;
		std::vector<SetValueResult> expectedResults;
		std::vector<SetValueRequest> expectedHardwareRequests;

		ASSERT_TRUE(setValuesTestCases(5000, requests, expectedResults, expectedHardwareRequests).ok());

		getHardware()->addSetValueResponses(expectedResults);

		auto status = getClient()->setValues(getCallbackClient(), requests);

		ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();

		EXPECT_EQ(getHardware()->nextSetValueRequests(), expectedHardwareRequests)
		<< "requests to hardware mismatch";

		auto maybeSetValueResults = getCallback()->nextSetValueResults();
		ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";
		const SetValueResults& setValueResults = maybeSetValueResults.value();
		ASSERT_TRUE(setValueResults.payloads.empty())
		<< "payload should be empty, shared memory file should be used";

		auto result = LargeParcelableBase::stableLargeParcelableToParcelableVector<SetValueResult>(
		setValueResults.sharedMemoryFd);
		ASSERT_TRUE(result.ok()) << "failed to parse shared memory file";
		ASSERT_TRUE(result.value().has_value()) << "no parsed value";
		ASSERT_EQ(result.value().value(), expectedResults) << "results mismatch";
		EXPECT_EQ(countClients(), static_cast<size_t>(1));
		}

		class SetValuesInvalidRequestTest
		: public DefaultVehicleHalTest,
		public testing::WithParamInterface<SetValuesInvalidRequestTestCase> {};

		INSTANTIATE_TEST_SUITE_P(
		SetValuesInvalidRequestTests, SetValuesInvalidRequestTest,
		::testing::ValuesIn(getSetValuesInvalidRequestTestCases()),
		[](const testing::TestParamInfo<SetValuesInvalidRequestTest::ParamType>& info) {
		return info.param.name;
		});

		TEST_P(SetValuesInvalidRequestTest, testSetValuesInvalidRequest) {
		SetValuesInvalidRequestTestCase tc = GetParam();
		std::vector<SetValueResult> expectedHardwareResults{
		SetValueResult{
		.requestId = 1,
		.status = StatusCode::OK,
		},
		};
		getHardware()->addSetValueResponses(expectedHardwareResults);

		SetValueRequests requests;
		SetValueRequest invalidRequest{
		.requestId = 0,
		.value = tc.request,
		};
		SetValueRequest normalRequest{.requestId = 1,
		.value = {
		.prop = testInt32VecProp(0),
		.value.int32Values = {0},
		}};
		requests.payloads = {invalidRequest, normalRequest};
		auto status = getClient()->setValues(getCallbackClient(), requests);

		ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();

		EXPECT_EQ(getHardware()->nextSetValueRequests(), std::vector<SetValueRequest>({normalRequest}))
		<< "requests to hardware mismatch";

		auto maybeSetValueResults = getCallback()->nextSetValueResults();
		ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";
		EXPECT_EQ(maybeSetValueResults.value().payloads, std::vector<SetValueResult>({
		{
		.requestId = 0,
		.status = tc.expectedStatus,
		},
		}))
		<< "invalid argument result mismatch";

		maybeSetValueResults = getCallback()->nextSetValueResults();
		ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results from hardware in callback";
		EXPECT_EQ(maybeSetValueResults.value().payloads, expectedHardwareResults)
		<< "results from hardware mismatch";
		}

		} // namespace vehicle
		} // namespace automotive
		} // namespace hardware