Manage pending requests in default VHAL.
Use PendingRequestPool in default VHAL to manage pending requests.
It would check for duplicate request IDs, call callbacks when request
timeout.
Test: atest DefaultVehicleHalTest
Bug: 203713317
Change-Id: Ifa42e6f06036c48914c56e357714b6dfb7173538
diff --git a/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h b/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
index 43a9603..97c25e3 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/ConnectedClient.h
@@ -17,6 +17,8 @@
#ifndef android_hardware_automotive_vehicle_aidl_impl_vhal_include_ConnectedClient_H_
#define android_hardware_automotive_vehicle_aidl_impl_vhal_include_ConnectedClient_H_
+#include "PendingRequestPool.h"
+
#include <VehicleHalTypes.h>
#include <aidl/android/hardware/automotive/vehicle/IVehicleCallback.h>
@@ -40,12 +42,31 @@
class ConnectedClient {
public:
ConnectedClient(
+ std::shared_ptr<PendingRequestPool> requestPool,
std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
callback);
virtual ~ConnectedClient() = default;
+ // Gets the unique ID for this client.
+ const void* id();
+
+ // Add client requests. The requests would be registered as pending requests until
+ // {@code tryFinishRequests} is called for them.
+ // Returns {@code INVALID_ARG} error if any of the requestIds are duplicate with one of the
+ // pending request IDs or {@code TRY_AGAIN} error if the pending request pool is full and could
+ // no longer add requests.
+ ::android::base::Result<void> addRequests(const std::unordered_set<int64_t>& requestIds);
+
+ // Mark the requests as finished. Returns a list of request IDs that was pending and has been
+ // finished. It must be a set of the requested request IDs.
+ std::unordered_set<int64_t> tryFinishRequests(const std::unordered_set<int64_t>& requestIds);
+
protected:
+ // Gets the callback to be called when the request for this client has timeout.
+ virtual std::shared_ptr<const PendingRequestPool::TimeoutCallbackFunc> getTimeoutCallback() = 0;
+
+ const std::shared_ptr<PendingRequestPool> mRequestPool;
const std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
mCallback;
};
@@ -56,6 +77,7 @@
class GetSetValuesClient final : public ConnectedClient {
public:
GetSetValuesClient(
+ std::shared_ptr<PendingRequestPool> requestPool,
std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback>
callback);
@@ -69,8 +91,13 @@
// Gets the callback to be called when the request for this client has finished.
std::shared_ptr<const std::function<void(std::vector<ResultType>)>> getResultCallback();
+ protected:
+ // Gets the callback to be called when the request for this client has timeout.
+ std::shared_ptr<const PendingRequestPool::TimeoutCallbackFunc> getTimeoutCallback() override;
+
private:
// The following members are only initialized during construction.
+ std::shared_ptr<const PendingRequestPool::TimeoutCallbackFunc> mTimeoutCallback;
std::shared_ptr<const std::function<void(std::vector<ResultType>)>> mResultCallback;
};
diff --git a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
index f6f7d80..e3e77a3 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
@@ -19,6 +19,7 @@
#include "ConnectedClient.h"
#include "ParcelableUtils.h"
+#include "PendingRequestPool.h"
#include <IVehicleHardware.h>
#include <VehicleUtils.h>
@@ -28,6 +29,7 @@
#include <android/binder_auto_utils.h>
#include <memory>
+#include <mutex>
#include <unordered_map>
#include <vector>
@@ -100,6 +102,8 @@
mConfigsByPropId;
// Only modified in constructor, so thread-safe.
std::unique_ptr<::ndk::ScopedFileDescriptor> mConfigFile;
+ // PendingRequestPool is thread-safe.
+ std::shared_ptr<PendingRequestPool> mPendingRequestPool;
std::mutex mLock;
std::unordered_map<CallbackType, std::shared_ptr<GetValuesClient>> mGetValuesClients
@@ -114,6 +118,18 @@
::android::base::Result<void> checkProperty(
const ::aidl::android::hardware::automotive::vehicle::VehiclePropValue& propValue);
+
+ ::android::base::Result<std::vector<int64_t>> checkDuplicateRequests(
+ const std::vector<::aidl::android::hardware::automotive::vehicle::GetValueRequest>&
+ requests);
+
+ ::android::base::Result<std::vector<int64_t>> checkDuplicateRequests(
+ const std::vector<::aidl::android::hardware::automotive::vehicle::SetValueRequest>&
+ requests);
+
+ // Test-only
+ // Set the default timeout for pending requests.
+ void setTimeout(int64_t timeoutInNano);
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/include/PendingRequestPool.h b/automotive/vehicle/aidl/impl/vhal/include/PendingRequestPool.h
index 6dcfaff..efb3315 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/PendingRequestPool.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/PendingRequestPool.h
@@ -51,7 +51,7 @@
// seconds.
android::base::Result<void> addRequests(const void* clientId,
const std::unordered_set<int64_t>& requestIds,
- std::shared_ptr<TimeoutCallbackFunc> callback);
+ std::shared_ptr<const TimeoutCallbackFunc> callback);
// Checks whether the request is currently pending.
bool isRequestPending(const void* clientId, int64_t requestId) const;
@@ -66,6 +66,8 @@
// Returns how many pending requests in the pool, for testing purpose.
size_t countPendingRequests(const void* clientId) const;
+ size_t countPendingRequests() const;
+
private:
// The maximum number of pending requests allowed per client. If exceeds this number, adding
// more requests would fail. This is to prevent spamming from client.
@@ -74,7 +76,7 @@
struct PendingRequest {
std::unordered_set<int64_t> requestIds;
int64_t timeoutTimestamp;
- std::shared_ptr<TimeoutCallbackFunc> callback;
+ std::shared_ptr<const TimeoutCallbackFunc> callback;
};
int64_t mTimeoutInNano;
diff --git a/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp b/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
index 656dfaf..abc3eb0 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/ConnectedClient.cpp
@@ -102,6 +102,53 @@
sendGetOrSetValueResultsSeparately<ResultType, ResultsType>(callback, results);
}
+// The timeout callback for GetValues/SetValues.
+template <class ResultType, class ResultsType>
+void onTimeout(
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ const std::unordered_set<int64_t>& timeoutIds) {
+ std::vector<ResultType> timeoutResults;
+ for (int64_t requestId : timeoutIds) {
+ ALOGD("hardware request timeout, request ID: %" PRId64, requestId);
+ timeoutResults.push_back({
+ .requestId = requestId,
+ .status = StatusCode::TRY_AGAIN,
+ });
+ }
+ sendGetOrSetValueResults<ResultType, ResultsType>(callback, timeoutResults);
+}
+
+// The on-results callback for GetValues/SetValues.
+template <class ResultType, class ResultsType>
+void getOrSetValuesCallback(
+ const void* clientId,
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ std::vector<ResultType> results, std::shared_ptr<PendingRequestPool> requestPool) {
+ std::unordered_set<int64_t> requestIds;
+ for (const auto& result : results) {
+ requestIds.insert(result.requestId);
+ }
+
+ auto finishedRequests = requestPool->tryFinishRequests(clientId, requestIds);
+
+ auto it = results.begin();
+ while (it != results.end()) {
+ int64_t requestId = it->requestId;
+ if (finishedRequests.find(requestId) == finishedRequests.end()) {
+ ALOGD("no pending request for the result from hardware, "
+ "possibly already time-out, ID: %" PRId64,
+ requestId);
+ it = results.erase(it);
+ } else {
+ it++;
+ }
+ }
+
+ if (!results.empty()) {
+ sendGetOrSetValueResults<ResultType, ResultsType>(callback, results);
+ }
+}
+
// Specify the functions for GetValues and SetValues types.
template void sendGetOrSetValueResult<GetValueResult, GetValueResults>(
std::shared_ptr<IVehicleCallback> callback, const GetValueResult& result);
@@ -118,18 +165,55 @@
template void sendGetOrSetValueResultsSeparately<SetValueResult, SetValueResults>(
std::shared_ptr<IVehicleCallback> callback, const std::vector<SetValueResult>& results);
+template void onTimeout<GetValueResult, GetValueResults>(
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ const std::unordered_set<int64_t>& timeoutIds);
+template void onTimeout<SetValueResult, SetValueResults>(
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ const std::unordered_set<int64_t>& timeoutIds);
+
+template void getOrSetValuesCallback<GetValueResult, GetValueResults>(
+ const void* clientId,
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ std::vector<GetValueResult> results, std::shared_ptr<PendingRequestPool> requestPool);
+template void getOrSetValuesCallback<SetValueResult, SetValueResults>(
+ const void* clientId,
+ std::shared_ptr<::aidl::android::hardware::automotive::vehicle::IVehicleCallback> callback,
+ std::vector<SetValueResult> results, std::shared_ptr<PendingRequestPool> requestPool);
+
} // namespace
-ConnectedClient::ConnectedClient(std::shared_ptr<IVehicleCallback> callback)
- : mCallback(callback) {}
+ConnectedClient::ConnectedClient(std::shared_ptr<PendingRequestPool> requestPool,
+ std::shared_ptr<IVehicleCallback> callback)
+ : mRequestPool(requestPool), mCallback(callback) {}
+
+const void* ConnectedClient::id() {
+ return reinterpret_cast<const void*>(this);
+}
+
+Result<void> ConnectedClient::addRequests(const std::unordered_set<int64_t>& requestIds) {
+ return mRequestPool->addRequests(id(), requestIds, getTimeoutCallback());
+}
+
+std::unordered_set<int64_t> ConnectedClient::tryFinishRequests(
+ const std::unordered_set<int64_t>& requestIds) {
+ return mRequestPool->tryFinishRequests(id(), requestIds);
+}
template <class ResultType, class ResultsType>
GetSetValuesClient<ResultType, ResultsType>::GetSetValuesClient(
- std::shared_ptr<IVehicleCallback> callback)
- : ConnectedClient(callback) {
+ std::shared_ptr<PendingRequestPool> requestPool, std::shared_ptr<IVehicleCallback> callback)
+ : ConnectedClient(requestPool, callback) {
+ mTimeoutCallback = std::make_shared<const PendingRequestPool::TimeoutCallbackFunc>(
+ [callback](const std::unordered_set<int64_t>& timeoutIds) {
+ return onTimeout<ResultType, ResultsType>(callback, timeoutIds);
+ });
+ auto requestPoolCopy = mRequestPool;
+ const void* clientId = id();
mResultCallback = std::make_shared<const std::function<void(std::vector<ResultType>)>>(
- [callback](std::vector<ResultType> results) {
- return sendGetOrSetValueResults<ResultType, ResultsType>(callback, results);
+ [clientId, callback, requestPoolCopy](std::vector<ResultType> results) {
+ return getOrSetValuesCallback<ResultType, ResultsType>(
+ clientId, callback, std::move(results), requestPoolCopy);
});
}
@@ -140,6 +224,12 @@
}
template <class ResultType, class ResultsType>
+std::shared_ptr<const PendingRequestPool::TimeoutCallbackFunc>
+GetSetValuesClient<ResultType, ResultsType>::getTimeoutCallback() {
+ return mTimeoutCallback;
+}
+
+template <class ResultType, class ResultsType>
void GetSetValuesClient<ResultType, ResultsType>::sendResults(
const std::vector<ResultType>& results) {
return sendGetOrSetValueResults<ResultType, ResultsType>(mCallback, results);
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index 7f09a59..3c454f0 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -25,6 +25,9 @@
#include <android-base/result.h>
#include <utils/Log.h>
+#include <set>
+#include <unordered_set>
+
namespace android {
namespace hardware {
namespace automotive {
@@ -52,7 +55,8 @@
using ::ndk::ScopedAStatus;
DefaultVehicleHal::DefaultVehicleHal(std::unique_ptr<IVehicleHardware> hardware)
- : mVehicleHardware(std::move(hardware)) {
+ : mVehicleHardware(std::move(hardware)),
+ mPendingRequestPool(std::make_shared<PendingRequestPool>(TIMEOUT_IN_NANO)) {
auto configs = mVehicleHardware->getAllPropertyConfigs();
for (auto& config : configs) {
mConfigsByPropId[config.prop] = config;
@@ -71,6 +75,10 @@
}
}
+void DefaultVehicleHal::setTimeout(int64_t timeoutInNano) {
+ mPendingRequestPool = std::make_unique<PendingRequestPool>(timeoutInNano);
+}
+
ScopedAStatus DefaultVehicleHal::getAllPropConfigs(VehiclePropConfigs* output) {
if (mConfigFile != nullptr) {
output->payloads.clear();
@@ -90,7 +98,7 @@
const CallbackType& callback) {
if (clients->find(callback) == clients->end()) {
// TODO(b/204943359): Remove client from clients when linkToDeath is implemented.
- (*clients)[callback] = std::make_shared<T>(callback);
+ (*clients)[callback] = std::make_shared<T>(mPendingRequestPool, callback);
}
return (*clients)[callback];
}
@@ -132,8 +140,6 @@
ScopedAStatus DefaultVehicleHal::getValues(const CallbackType& callback,
const GetValueRequests& requests) {
- // TODO(b/203713317): check for duplicate properties and duplicate request IDs.
-
expected<LargeParcelableBase::BorrowedOwnedObject<GetValueRequests>, ScopedAStatus>
deserializedResults = fromStableLargeParcelable(requests);
if (!deserializedResults.ok()) {
@@ -143,26 +149,42 @@
const std::vector<GetValueRequest>& getValueRequests =
deserializedResults.value().getObject()->payloads;
+ auto maybeRequestIds = checkDuplicateRequests(getValueRequests);
+ if (!maybeRequestIds.ok()) {
+ ALOGE("duplicate request ID");
+ return toScopedAStatus(maybeRequestIds, StatusCode::INVALID_ARG);
+ }
+ // The set of request Ids that we would send to hardware.
+ std::unordered_set<int64_t> hardwareRequestIds(maybeRequestIds.value().begin(),
+ maybeRequestIds.value().end());
+
std::shared_ptr<GetValuesClient> client;
{
std::scoped_lock<std::mutex> lockGuard(mLock);
client = getOrCreateClient(&mGetValuesClients, callback);
}
+ // Register the pending hardware requests and also check for duplicate request Ids.
+ if (auto addRequestResult = client->addRequests(hardwareRequestIds); !addRequestResult.ok()) {
+ ALOGE("failed to add pending requests, error: %s",
+ addRequestResult.error().message().c_str());
+ return toScopedAStatus(addRequestResult);
+ }
if (StatusCode status =
mVehicleHardware->getValues(client->getResultCallback(), getValueRequests);
status != StatusCode::OK) {
+ // If the hardware returns error, finish all the pending requests for this request because
+ // we never expect hardware to call callback for these requests.
+ client->tryFinishRequests(hardwareRequestIds);
+ ALOGE("failed to get value from VehicleHardware, status: %d", toInt(status));
return ScopedAStatus::fromServiceSpecificErrorWithMessage(
toInt(status), "failed to get value from VehicleHardware");
}
-
return ScopedAStatus::ok();
}
ScopedAStatus DefaultVehicleHal::setValues(const CallbackType& callback,
const SetValueRequests& requests) {
- // TODO(b/203713317): check for duplicate properties and duplicate request IDs.
-
expected<LargeParcelableBase::BorrowedOwnedObject<SetValueRequests>, ScopedAStatus>
deserializedResults = fromStableLargeParcelable(requests);
if (!deserializedResults.ok()) {
@@ -177,6 +199,12 @@
// The list of requests that we would send to hardware.
std::vector<SetValueRequest> hardwareRequests;
+ auto maybeRequestIds = checkDuplicateRequests(setValueRequests);
+ if (!maybeRequestIds.ok()) {
+ ALOGE("duplicate request ID");
+ return toScopedAStatus(maybeRequestIds, StatusCode::INVALID_ARG);
+ }
+
for (auto& request : setValueRequests) {
int64_t requestId = request.requestId;
if (auto result = checkProperty(request.value); !result.ok()) {
@@ -187,22 +215,41 @@
});
continue;
}
+
hardwareRequests.push_back(request);
}
+ // The set of request Ids that we would send to hardware.
+ std::unordered_set<int64_t> hardwareRequestIds;
+ for (const auto& request : hardwareRequests) {
+ hardwareRequestIds.insert(request.requestId);
+ }
+
std::shared_ptr<SetValuesClient> client;
{
std::scoped_lock<std::mutex> lockGuard(mLock);
client = getOrCreateClient(&mSetValuesClients, callback);
}
+ // Register the pending hardware requests and also check for duplicate request Ids.
+ if (auto addRequestResult = client->addRequests(hardwareRequestIds); !addRequestResult.ok()) {
+ ALOGE("failed to add pending requests, error: %s",
+ addRequestResult.error().message().c_str());
+ return toScopedAStatus(addRequestResult, StatusCode::INVALID_ARG);
+ }
+
if (!failedResults.empty()) {
+ // First send the failed results we already know back to the client.
client->sendResults(failedResults);
}
if (StatusCode status =
mVehicleHardware->setValues(client->getResultCallback(), hardwareRequests);
status != StatusCode::OK) {
+ // If the hardware returns error, finish all the pending requests for this request because
+ // we never expect hardware to call callback for these requests.
+ client->tryFinishRequests(hardwareRequestIds);
+ ALOGE("failed to set value to VehicleHardware, status: %d", toInt(status));
return ScopedAStatus::fromServiceSpecificErrorWithMessage(
toInt(status), "failed to set value to VehicleHardware");
}
@@ -210,6 +257,34 @@
return ScopedAStatus::ok();
}
+#define CHECK_DUPLICATE_REQUESTS(PROP_NAME) \
+ do { \
+ std::vector<int64_t> requestIds; \
+ std::set<::aidl::android::hardware::automotive::vehicle::VehiclePropValue> requestProps; \
+ for (const auto& request : requests) { \
+ const auto& prop = request.PROP_NAME; \
+ if (requestProps.count(prop) != 0) { \
+ return ::android::base::Error() \
+ << "duplicate request for property: " << prop.toString(); \
+ } \
+ requestProps.insert(prop); \
+ requestIds.push_back(request.requestId); \
+ } \
+ return requestIds; \
+ } while (0);
+
+::android::base::Result<std::vector<int64_t>> DefaultVehicleHal::checkDuplicateRequests(
+ const std::vector<GetValueRequest>& requests) {
+ CHECK_DUPLICATE_REQUESTS(prop);
+}
+
+::android::base::Result<std::vector<int64_t>> DefaultVehicleHal::checkDuplicateRequests(
+ const std::vector<SetValueRequest>& requests) {
+ CHECK_DUPLICATE_REQUESTS(value);
+}
+
+#undef CHECK_DUPLICATE_REQUESTS
+
ScopedAStatus DefaultVehicleHal::getPropConfigs(const std::vector<int32_t>& props,
VehiclePropConfigs* output) {
std::vector<VehiclePropConfig> configs;
diff --git a/automotive/vehicle/aidl/impl/vhal/src/PendingRequestPool.cpp b/automotive/vehicle/aidl/impl/vhal/src/PendingRequestPool.cpp
index c2d6f89..23a5403 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/PendingRequestPool.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/PendingRequestPool.cpp
@@ -74,7 +74,7 @@
Result<void> PendingRequestPool::addRequests(const void* clientId,
const std::unordered_set<int64_t>& requestIds,
- std::shared_ptr<TimeoutCallbackFunc> callback) {
+ std::shared_ptr<const TimeoutCallbackFunc> callback) {
std::scoped_lock<std::mutex> lockGuard(mLock);
std::list<PendingRequest>* pendingRequests;
size_t pendingRequestCount = 0;
@@ -117,6 +117,18 @@
return isRequestPendingLocked(clientId, requestId);
}
+size_t PendingRequestPool::countPendingRequests() const {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+
+ size_t count = 0;
+ for (const auto& [clientId, requests] : mPendingRequestsByClient) {
+ for (const auto& request : requests) {
+ count += request.requestIds.size();
+ }
+ }
+ return count;
+}
+
size_t PendingRequestPool::countPendingRequests(const void* clientId) const {
std::scoped_lock<std::mutex> lockGuard(mLock);
diff --git a/automotive/vehicle/aidl/impl/vhal/test/ConnectedClientTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/ConnectedClientTest.cpp
index ddd0c65..bd4a565 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/ConnectedClientTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/ConnectedClientTest.cpp
@@ -39,12 +39,17 @@
void SetUp() override {
mCallback = ndk::SharedRefBase::make<MockVehicleCallback>();
mCallbackClient = IVehicleCallback::fromBinder(mCallback->asBinder());
+ // timeout: 1s.
+ int64_t timeout = 1000000000;
+ mPool = std::make_shared<PendingRequestPool>(timeout);
}
std::shared_ptr<IVehicleCallback> getCallbackClient() { return mCallbackClient; }
MockVehicleCallback* getCallback() { return mCallback.get(); }
+ std::shared_ptr<PendingRequestPool> getPool() { return mPool; }
+
protected:
using GetValuesClient = GetSetValuesClient<GetValueResult, GetValueResults>;
using SetValuesClient = GetSetValuesClient<SetValueResult, SetValueResults>;
@@ -52,6 +57,7 @@
private:
std::shared_ptr<MockVehicleCallback> mCallback;
std::shared_ptr<IVehicleCallback> mCallbackClient;
+ std::shared_ptr<PendingRequestPool> mPool;
};
TEST_F(ConnectedClientTest, testSendGetValueResults) {
@@ -72,7 +78,7 @@
},
}};
- GetValuesClient client(getCallbackClient());
+ GetValuesClient client(getPool(), getCallbackClient());
client.sendResults(results);
@@ -99,7 +105,7 @@
},
}};
- GetValuesClient client(getCallbackClient());
+ GetValuesClient client(getPool(), getCallbackClient());
client.sendResultsSeparately(results);
@@ -131,7 +137,9 @@
},
}};
- GetValuesClient client(getCallbackClient());
+ GetValuesClient client(getPool(), getCallbackClient());
+
+ client.addRequests({0, 1});
(*(client.getResultCallback()))(results);
@@ -150,7 +158,7 @@
.status = StatusCode::OK,
}};
- SetValuesClient client(getCallbackClient());
+ SetValuesClient client(getPool(), getCallbackClient());
client.sendResults(results);
@@ -169,7 +177,7 @@
.status = StatusCode::OK,
}};
- SetValuesClient client(getCallbackClient());
+ SetValuesClient client(getPool(), getCallbackClient());
client.sendResultsSeparately(results);
@@ -193,7 +201,9 @@
.status = StatusCode::OK,
}};
- SetValuesClient client(getCallbackClient());
+ SetValuesClient client(getPool(), getCallbackClient());
+
+ client.addRequests({0, 1});
(*(client.getResultCallback()))(results);
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index ffc08a7..6970e48 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -27,6 +27,7 @@
#include <gtest/gtest.h>
#include <utils/Log.h>
+#include <chrono>
#include <list>
#include <memory>
#include <mutex>
@@ -67,6 +68,7 @@
using ::ndk::ScopedFileDescriptor;
using ::testing::Eq;
+using ::testing::UnorderedElementsAreArray;
using ::testing::WhenSortedBy;
constexpr int32_t INVALID_PROP_ID = 0;
@@ -356,6 +358,11 @@
mCallbackClient = IVehicleCallback::fromBinder(mCallback->asBinder());
}
+ void TearDown() override {
+ ASSERT_EQ(countPendingRequests(), static_cast<size_t>(0))
+ << "must have no pending requests when test finishes";
+ }
+
MockVehicleHardware* getHardware() { return mHardwarePtr; }
std::shared_ptr<IVehicle> getClient() { return mVhal; }
@@ -364,6 +371,12 @@
MockVehicleCallback* getCallback() { return mCallback.get(); }
+ void setTimeout(int64_t timeoutInNano) { mVhal->setTimeout(timeoutInNano); }
+
+ size_t countPendingRequests() { return mVhal->mPendingRequestPool->countPendingRequests(); }
+
+ std::shared_ptr<PendingRequestPool> getPool() { return mVhal->mPendingRequestPool; }
+
static Result<void> getValuesTestCases(size_t size, GetValueRequests& requests,
std::vector<GetValueResult>& expectedResults,
std::vector<GetValueRequest>& expectedHardwareRequests) {
@@ -570,6 +583,145 @@
ASSERT_EQ(status.getServiceSpecificError(), toInt(StatusCode::INVALID_ARG));
}
+TEST_F(DefaultVehicleHalTest, testGetValuesFinishBeforeTimeout) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ GetValueRequests requests;
+ std::vector<GetValueResult> expectedResults;
+ std::vector<GetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());
+
+ // The response would be returned after 0.05s.
+ getHardware()->setSleepTime(timeout / 2);
+ getHardware()->addGetValueResponses(expectedResults);
+
+ auto status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();
+
+ // Wait for the response.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout));
+
+ auto maybeGetValueResults = getCallback()->nextGetValueResults();
+ ASSERT_TRUE(maybeGetValueResults.has_value()) << "no results in callback";
+ EXPECT_EQ(maybeGetValueResults.value().payloads, expectedResults) << "results mismatch";
+ ASSERT_FALSE(getCallback()->nextGetValueResults().has_value()) << "more results than expected";
+}
+
+TEST_F(DefaultVehicleHalTest, testGetValuesFinishAfterTimeout) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ GetValueRequests requests;
+ std::vector<GetValueResult> expectedResults;
+ std::vector<GetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(getValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());
+
+ // The response would be returned after 0.2s.
+ getHardware()->setSleepTime(timeout * 2);
+ getHardware()->addGetValueResponses(expectedResults);
+
+ auto status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();
+
+ // Wait for the response.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));
+
+ for (size_t i = 0; i < expectedResults.size(); i++) {
+ expectedResults[i] = {
+ .requestId = expectedResults[i].requestId,
+ .status = StatusCode::TRY_AGAIN,
+ .prop = std::nullopt,
+ };
+ }
+
+ auto maybeGetValueResults = getCallback()->nextGetValueResults();
+ ASSERT_TRUE(maybeGetValueResults.has_value()) << "no results in callback";
+ ASSERT_THAT(maybeGetValueResults.value().payloads, UnorderedElementsAreArray(expectedResults))
+ << "results mismatch, expect TRY_AGAIN error.";
+ ASSERT_FALSE(getCallback()->nextGetValueResults().has_value()) << "more results than expected";
+}
+
+TEST_F(DefaultVehicleHalTest, testGetValuesDuplicateRequestIdsInTwoRequests) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ GetValueRequests requests;
+ std::vector<GetValueResult> expectedResults;
+ std::vector<GetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(getValuesTestCases(1, requests, expectedResults, expectedHardwareRequests).ok());
+
+ getHardware()->setSleepTime(timeout * 2);
+ getHardware()->addGetValueResponses(expectedResults);
+
+ auto status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "getValues failed: " << status.getMessage();
+
+ // Use the same request ID again.
+ status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk())
+ << "Use the same request ID before the previous request finishes must fail";
+
+ // Wait for the request to finish.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));
+}
+
+TEST_F(DefaultVehicleHalTest, testGetValuesDuplicateRequestIdsInOneRequest) {
+ GetValueRequests requests = {.payloads = {
+ {
+ .requestId = 0,
+ .prop =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ },
+ },
+ {
+ .requestId = 0,
+ .prop =
+ VehiclePropValue{
+ .prop = testInt32VecProp(1),
+ },
+ },
+ }};
+
+ auto status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk()) << "duplicate Ids in one request must fail";
+}
+
+TEST_F(DefaultVehicleHalTest, testGetValuesDuplicateRequestProps) {
+ GetValueRequests requests = {.payloads = {
+ {
+ .requestId = 0,
+ .prop =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ },
+ },
+ {
+ .requestId = 1,
+ .prop =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ },
+ },
+ }};
+
+ auto status = getClient()->getValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk()) << "duplicate request properties in one request must fail";
+}
+
TEST_F(DefaultVehicleHalTest, testSetValuesSmall) {
SetValueRequests requests;
std::vector<SetValueResult> expectedResults;
@@ -675,6 +827,148 @@
<< "results from hardware mismatch";
}
+TEST_F(DefaultVehicleHalTest, testSetValuesFinishBeforeTimeout) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ SetValueRequests requests;
+ std::vector<SetValueResult> expectedResults;
+ std::vector<SetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(setValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());
+
+ // The response would be returned after 0.05s.
+ getHardware()->setSleepTime(timeout / 2);
+ getHardware()->addSetValueResponses(expectedResults);
+
+ auto status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();
+
+ // Wait for the response.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout));
+
+ auto maybeSetValueResults = getCallback()->nextSetValueResults();
+ ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";
+ EXPECT_EQ(maybeSetValueResults.value().payloads, expectedResults) << "results mismatch";
+ ASSERT_FALSE(getCallback()->nextSetValueResults().has_value()) << "more results than expected";
+}
+
+TEST_F(DefaultVehicleHalTest, testSetValuesFinishAfterTimeout) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ SetValueRequests requests;
+ std::vector<SetValueResult> expectedResults;
+ std::vector<SetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(setValuesTestCases(10, requests, expectedResults, expectedHardwareRequests).ok());
+
+ // The response would be returned after 0.2s.
+ getHardware()->setSleepTime(timeout * 2);
+ getHardware()->addSetValueResponses(expectedResults);
+
+ auto status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();
+
+ // Wait for the response.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));
+
+ for (size_t i = 0; i < expectedResults.size(); i++) {
+ expectedResults[i] = {
+ .requestId = expectedResults[i].requestId,
+ .status = StatusCode::TRY_AGAIN,
+ };
+ }
+
+ auto maybeSetValueResults = getCallback()->nextSetValueResults();
+ ASSERT_TRUE(maybeSetValueResults.has_value()) << "no results in callback";
+ ASSERT_THAT(maybeSetValueResults.value().payloads, UnorderedElementsAreArray(expectedResults))
+ << "results mismatch, expect TRY_AGAIN error.";
+ ASSERT_FALSE(getCallback()->nextSetValueResults().has_value()) << "more results than expected";
+}
+
+TEST_F(DefaultVehicleHalTest, testSetValuesDuplicateRequestIdsInTwoRequests) {
+ // timeout: 0.1s
+ int64_t timeout = 100000000;
+ setTimeout(timeout);
+
+ SetValueRequests requests;
+ std::vector<SetValueResult> expectedResults;
+ std::vector<SetValueRequest> expectedHardwareRequests;
+
+ ASSERT_TRUE(setValuesTestCases(1, requests, expectedResults, expectedHardwareRequests).ok());
+
+ getHardware()->setSleepTime(timeout * 2);
+ getHardware()->addSetValueResponses(expectedResults);
+
+ auto status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_TRUE(status.isOk()) << "setValues failed: " << status.getMessage();
+
+ // Use the same request ID again.
+ status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk())
+ << "Use the same request ID before the previous request finishes must fail";
+
+ // Wait for the request to finish.
+ std::this_thread::sleep_for(std::chrono::nanoseconds(timeout * 5));
+}
+
+TEST_F(DefaultVehicleHalTest, testSetValuesDuplicateRequestIdsInOneRequest) {
+ SetValueRequests requests = {.payloads = {
+ {
+ .requestId = 0,
+ .value =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ .value.int32Values = {0},
+ },
+ },
+ {
+ .requestId = 0,
+ .value =
+ VehiclePropValue{
+ .prop = testInt32VecProp(1),
+ .value.int32Values = {0},
+ },
+ },
+ }};
+
+ auto status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk()) << "duplicate Ids in one request must fail";
+}
+
+TEST_F(DefaultVehicleHalTest, testSetValuesDuplicateRequestProps) {
+ SetValueRequests requests = {.payloads = {
+ {
+ .requestId = 0,
+ .value =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ .value.int32Values = {0},
+ },
+ },
+ {
+ .requestId = 1,
+ .value =
+ VehiclePropValue{
+ .prop = testInt32VecProp(0),
+ .value.int32Values = {0},
+ },
+ },
+ }};
+
+ auto status = getClient()->setValues(getCallbackClient(), requests);
+
+ ASSERT_FALSE(status.isOk()) << "duplicate request properties in one request must fail";
+}
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/test/PendingRequestPoolTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/PendingRequestPoolTest.cpp
index 03d795b..9c9e4b9 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/PendingRequestPoolTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/PendingRequestPoolTest.cpp
@@ -53,7 +53,7 @@
int64_t getTimeout() { return TEST_TIMEOUT; }
- void* getTestClientId() { return reinterpret_cast<void*>(0); }
+ const void* getTestClientId() { return reinterpret_cast<const void*>(0); }
private:
// Test timeout is 0.1s.
@@ -239,12 +239,12 @@
auto callback = std::make_shared<PendingRequestPool::TimeoutCallbackFunc>(
[](std::unordered_set<int64_t>) {});
- ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<void*>(0), {0}, callback));
- ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<void*>(1), {1, 2, 0}, callback));
+ ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<const void*>(0), {0}, callback));
+ ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<const void*>(1), {1, 2, 0}, callback));
- ASSERT_THAT(getPool()->tryFinishRequests(reinterpret_cast<void*>(0), {0}),
+ ASSERT_THAT(getPool()->tryFinishRequests(reinterpret_cast<const void*>(0), {0}),
UnorderedElementsAre(0));
- ASSERT_THAT(getPool()->tryFinishRequests(reinterpret_cast<void*>(1), {1, 2, 0}),
+ ASSERT_THAT(getPool()->tryFinishRequests(reinterpret_cast<const void*>(1), {1, 2, 0}),
UnorderedElementsAre(0, 1, 2));
}
@@ -258,14 +258,14 @@
for (size_t i = 0; i < 10000; i++) {
requests.insert(static_cast<int64_t>(i));
}
- ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<void*>(0), requests, callback));
+ ASSERT_RESULT_OK(getPool()->addRequests(reinterpret_cast<const void*>(0), requests, callback));
- auto result = getPool()->addRequests(reinterpret_cast<void*>(0), {static_cast<int64_t>(10000)},
- callback);
+ auto result = getPool()->addRequests(reinterpret_cast<const void*>(0),
+ {static_cast<int64_t>(10000)}, callback);
ASSERT_FALSE(result.ok()) << "adding more pending requests than limit must fail";
ASSERT_EQ(result.error().code(), toInt(StatusCode::TRY_AGAIN));
- getPool()->tryFinishRequests(reinterpret_cast<void*>(0), requests);
+ getPool()->tryFinishRequests(reinterpret_cast<const void*>(0), requests);
}
} // namespace vehicle