Implement setValues in DefaultVHAL.
Test: atest DefaultVehicleHalTest
Bug: 200737967
Change-Id: I702d139d2f1c0eb647559dde88deb0486c986c66
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
index c3e8168..49b33d5 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
@@ -213,7 +213,7 @@
::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>
::ndk::ScopedAStatus toScopedAStatus(const ::android::base::Result<T>& result,
- std::string additionalErrorMsg) {
+ const std::string& additionalErrorMsg) {
return toScopedAStatus(result, getErrorCode(result), additionalErrorMsg);
}
diff --git a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
index da83e94..4ee3ee9 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
@@ -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 @@
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 @@
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 @@
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
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index 41753a6..e98f021 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -35,13 +35,18 @@
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 @@
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 @@
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();
}
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 6c99ceb..8934a7b 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -53,6 +53,7 @@
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 ::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 @@
return std::nullopt;
}
+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:
- std::vector<VehiclePropConfig> getAllPropertyConfigs() const override {
- std::scoped_lock<std::mutex> lockGuard(mLock);
- return mPropertyConfigs;
- }
-
~MockVehicleHardware() {
std::scoped_lock<std::mutex> lockGuard(mLock);
for (auto& thread : mThreads) {
@@ -92,6 +97,11 @@
}
}
+ 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 @@
}
} 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 @@
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 @@
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 @@
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