Make FakeVehicleHardware async.
Use a handler thread to handle all the get/set value requests in
FakeVehicleHardware to make it async. Vendor implementation should
use similar logic to make their VHAL async.
Test: atest FakeVehicleHardwareTest
Bug: 226001897
Change-Id: Iba95125e63566c27597b9d11bff131c229846d49
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
index 892b406..34b2b24 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
@@ -17,6 +17,7 @@
#ifndef android_hardware_automotive_vehicle_aidl_impl_fake_impl_hardware_include_FakeVehicleHardware_H_
#define android_hardware_automotive_vehicle_aidl_impl_fake_impl_hardware_include_FakeVehicleHardware_H_
+#include <ConcurrentQueue.h>
#include <DefaultConfig.h>
#include <FakeObd2Frame.h>
#include <FakeUserHal.h>
@@ -48,6 +49,8 @@
explicit FakeVehicleHardware(std::unique_ptr<VehiclePropValuePool> valuePool);
+ ~FakeVehicleHardware();
+
// Get all the property configs.
std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropConfig>
getAllPropertyConfigs() const override;
@@ -102,6 +105,29 @@
// Expose private methods to unit test.
friend class FakeVehicleHardwareTestHelper;
+ template <class CallbackType, class RequestType>
+ struct RequestWithCallback {
+ RequestType request;
+ std::shared_ptr<const CallbackType> callback;
+ };
+
+ template <class CallbackType, class RequestType>
+ class PendingRequestHandler {
+ public:
+ PendingRequestHandler(FakeVehicleHardware* hardware);
+
+ void addRequest(RequestType request, std::shared_ptr<const CallbackType> callback);
+
+ void stop();
+
+ private:
+ FakeVehicleHardware* mHardware;
+ std::thread mThread;
+ ConcurrentQueue<RequestWithCallback<CallbackType, RequestType>> mRequests;
+
+ void handleRequestsOnce();
+ };
+
const std::unique_ptr<obd2frame::FakeObd2Frame> mFakeObd2Frame;
const std::unique_ptr<FakeUserHal> mFakeUserHal;
// RecurrentTimer is thread-safe.
@@ -111,6 +137,13 @@
std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback GUARDED_BY(mLock);
std::unordered_map<PropIdAreaId, std::shared_ptr<RecurrentTimer::Callback>, PropIdAreaIdHash>
mRecurrentActions GUARDED_BY(mLock);
+ // PendingRequestHandler is thread-safe.
+ mutable PendingRequestHandler<GetValuesCallback,
+ aidl::android::hardware::automotive::vehicle::GetValueRequest>
+ mPendingGetValueRequests;
+ mutable PendingRequestHandler<SetValuesCallback,
+ aidl::android::hardware::automotive::vehicle::SetValueRequest>
+ mPendingSetValueRequests;
void init();
// Stores the initial value to property store.
@@ -170,6 +203,10 @@
android::base::Result<void> checkArgumentsSize(const std::vector<std::string>& options,
size_t minSize);
+ aidl::android::hardware::automotive::vehicle::GetValueResult handleGetValueRequest(
+ const aidl::android::hardware::automotive::vehicle::GetValueRequest& request);
+ aidl::android::hardware::automotive::vehicle::SetValueResult handleSetValueRequest(
+ const aidl::android::hardware::automotive::vehicle::SetValueRequest& request);
};
} // namespace fake
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
index 8f8cc5c..1c2916c 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
@@ -139,10 +139,17 @@
mServerSidePropStore(new VehiclePropertyStore(mValuePool)),
mFakeObd2Frame(new obd2frame::FakeObd2Frame(mServerSidePropStore)),
mFakeUserHal(new FakeUserHal(mValuePool)),
- mRecurrentTimer(new RecurrentTimer()) {
+ mRecurrentTimer(new RecurrentTimer()),
+ mPendingGetValueRequests(this),
+ mPendingSetValueRequests(this) {
init();
}
+FakeVehicleHardware::~FakeVehicleHardware() {
+ mPendingGetValueRequests.stop();
+ mPendingSetValueRequests.stop();
+}
+
void FakeVehicleHardware::init() {
for (auto& it : defaultconfig::getDefaultConfigs()) {
VehiclePropConfig cfg = it.config;
@@ -427,37 +434,25 @@
StatusCode FakeVehicleHardware::setValues(std::shared_ptr<const SetValuesCallback> callback,
const std::vector<SetValueRequest>& requests) {
- std::vector<SetValueResult> results;
for (auto& request : requests) {
- const VehiclePropValue& value = request.value;
- int propId = value.prop;
-
if (FAKE_VEHICLEHARDWARE_DEBUG) {
- ALOGD("Set value for property ID: %d", propId);
+ ALOGD("Set value for property ID: %d", request.value.prop);
}
- SetValueResult setValueResult;
- setValueResult.requestId = request.requestId;
-
- if (auto result = setValue(value); !result.ok()) {
- ALOGE("failed to set value, error: %s, code: %d", getErrorMsg(result).c_str(),
- getIntErrorCode(result));
- setValueResult.status = getErrorCode(result);
- } else {
- setValueResult.status = StatusCode::OK;
- }
-
- results.push_back(std::move(setValueResult));
+ // In a real VHAL implementation, you could either send the setValue request to vehicle bus
+ // here in the binder thread, or you could send the request in setValue which runs in
+ // the handler thread. If you decide to send the setValue request here, you should not
+ // wait for the response here and the handler thread should handle the setValue response.
+ mPendingSetValueRequests.addRequest(request, callback);
}
- // In the real vhal, the values will be sent to Car ECU. We just pretend it is done here and
- // send back the updated property values to client.
- (*callback)(std::move(results));
-
return StatusCode::OK;
}
VhalResult<void> FakeVehicleHardware::setValue(const VehiclePropValue& value) {
+ // In a real VHAL implementation, this will send the request to vehicle bus if not already
+ // sent in setValues, and wait for the response from vehicle bus.
+ // Here we are just updating mValuePool.
bool isSpecialValue = false;
auto setSpecialValueResult = maybeSetSpecialValue(value, &isSpecialValue);
@@ -484,41 +479,59 @@
return {};
}
-StatusCode FakeVehicleHardware::getValues(std::shared_ptr<const GetValuesCallback> callback,
- const std::vector<GetValueRequest>& requests) const {
- std::vector<GetValueResult> results;
- for (auto& request : requests) {
- const VehiclePropValue& value = request.prop;
+SetValueResult FakeVehicleHardware::handleSetValueRequest(const SetValueRequest& request) {
+ SetValueResult setValueResult;
+ setValueResult.requestId = request.requestId;
- if (FAKE_VEHICLEHARDWARE_DEBUG) {
- ALOGD("getValues(%d)", value.prop);
- }
-
- GetValueResult getValueResult;
- getValueResult.requestId = request.requestId;
-
- auto result = getValue(value);
- if (!result.ok()) {
- ALOGE("failed to get value, error: %s, code: %d", getErrorMsg(result).c_str(),
- getIntErrorCode(result));
- getValueResult.status = getErrorCode(result);
- } else {
- getValueResult.status = StatusCode::OK;
- getValueResult.prop = *result.value();
- }
- results.push_back(std::move(getValueResult));
+ if (auto result = setValue(request.value); !result.ok()) {
+ ALOGE("failed to set value, error: %s, code: %d", getErrorMsg(result).c_str(),
+ getIntErrorCode(result));
+ setValueResult.status = getErrorCode(result);
+ } else {
+ setValueResult.status = StatusCode::OK;
}
- // In a real VHAL implementation, getValue would be async and we would call the callback after
- // we actually received the values from vehicle bus. Here we are getting the result
- // synchronously so we could call the callback here.
- (*callback)(std::move(results));
+ return setValueResult;
+}
+
+StatusCode FakeVehicleHardware::getValues(std::shared_ptr<const GetValuesCallback> callback,
+ const std::vector<GetValueRequest>& requests) const {
+ for (auto& request : requests) {
+ if (FAKE_VEHICLEHARDWARE_DEBUG) {
+ ALOGD("getValues(%d)", request.prop.prop);
+ }
+
+ // In a real VHAL implementation, you could either send the getValue request to vehicle bus
+ // here in the binder thread, or you could send the request in getValue which runs in
+ // the handler thread. If you decide to send the getValue request here, you should not
+ // wait for the response here and the handler thread should handle the getValue response.
+ mPendingGetValueRequests.addRequest(request, callback);
+ }
return StatusCode::OK;
}
+GetValueResult FakeVehicleHardware::handleGetValueRequest(const GetValueRequest& request) {
+ GetValueResult getValueResult;
+ getValueResult.requestId = request.requestId;
+
+ auto result = getValue(request.prop);
+ if (!result.ok()) {
+ ALOGE("failed to get value, error: %s, code: %d", getErrorMsg(result).c_str(),
+ getIntErrorCode(result));
+ getValueResult.status = getErrorCode(result);
+ } else {
+ getValueResult.status = StatusCode::OK;
+ getValueResult.prop = *result.value();
+ }
+ return getValueResult;
+}
+
FakeVehicleHardware::ValueResultType FakeVehicleHardware::getValue(
const VehiclePropValue& value) const {
+ // In a real VHAL implementation, this will send the request to vehicle bus if not already
+ // sent in getValues, and wait for the response from vehicle bus.
+ // Here we are just reading value from mValuePool.
bool isSpecialValue = false;
auto result = maybeGetSpecialValue(value, &isSpecialValue);
if (isSpecialValue) {
@@ -971,6 +984,60 @@
return bytes;
}
+template <class CallbackType, class RequestType>
+FakeVehicleHardware::PendingRequestHandler<CallbackType, RequestType>::PendingRequestHandler(
+ FakeVehicleHardware* hardware)
+ : mHardware(hardware), mThread([this] {
+ while (mRequests.waitForItems()) {
+ handleRequestsOnce();
+ }
+ }) {}
+
+template <class CallbackType, class RequestType>
+void FakeVehicleHardware::PendingRequestHandler<CallbackType, RequestType>::addRequest(
+ RequestType request, std::shared_ptr<const CallbackType> callback) {
+ mRequests.push({
+ request,
+ callback,
+ });
+}
+
+template <class CallbackType, class RequestType>
+void FakeVehicleHardware::PendingRequestHandler<CallbackType, RequestType>::stop() {
+ mRequests.deactivate();
+ if (mThread.joinable()) {
+ mThread.join();
+ }
+}
+
+template <>
+void FakeVehicleHardware::PendingRequestHandler<FakeVehicleHardware::GetValuesCallback,
+ GetValueRequest>::handleRequestsOnce() {
+ std::unordered_map<std::shared_ptr<const GetValuesCallback>, std::vector<GetValueResult>>
+ callbackToResults;
+ for (const auto& rwc : mRequests.flush()) {
+ auto result = mHardware->handleGetValueRequest(rwc.request);
+ callbackToResults[rwc.callback].push_back(std::move(result));
+ }
+ for (const auto& [callback, results] : callbackToResults) {
+ (*callback)(std::move(results));
+ }
+}
+
+template <>
+void FakeVehicleHardware::PendingRequestHandler<FakeVehicleHardware::SetValuesCallback,
+ SetValueRequest>::handleRequestsOnce() {
+ std::unordered_map<std::shared_ptr<const SetValuesCallback>, std::vector<SetValueResult>>
+ callbackToResults;
+ for (const auto& rwc : mRequests.flush()) {
+ auto result = mHardware->handleSetValueRequest(rwc.request);
+ callbackToResults[rwc.callback].push_back(std::move(result));
+ }
+ for (const auto& [callback, results] : callbackToResults) {
+ (*callback)(std::move(results));
+ }
+}
+
} // namespace fake
} // namespace vehicle
} // namespace automotive
diff --git a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
index 8839c18..504940b 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
@@ -34,6 +34,8 @@
#include <inttypes.h>
#include <chrono>
#include <condition_variable>
+#include <unordered_map>
+#include <unordered_set>
#include <vector>
namespace android {
@@ -99,11 +101,51 @@
FakeVehicleHardware* getHardware() { return &mHardware; }
StatusCode setValues(const std::vector<SetValueRequest>& requests) {
- return getHardware()->setValues(mSetValuesCallback, requests);
+ {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ for (const auto& request : requests) {
+ mPendingSetValueRequests.insert(request.requestId);
+ }
+ }
+ if (StatusCode status = getHardware()->setValues(mSetValuesCallback, requests);
+ status != StatusCode::OK) {
+ return status;
+ }
+ std::unique_lock<std::mutex> lk(mLock);
+ // Wait for the onSetValueResults.
+ bool result = mCv.wait_for(lk, milliseconds(1000), [this] {
+ ScopedLockAssertion lockAssertion(mLock);
+ return mPendingSetValueRequests.size() == 0;
+ });
+ if (!result) {
+ ALOGE("wait for callbacks for setValues timed-out");
+ return StatusCode::INTERNAL_ERROR;
+ }
+ return StatusCode::OK;
}
StatusCode getValues(const std::vector<GetValueRequest>& requests) {
- return getHardware()->getValues(mGetValuesCallback, requests);
+ {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ for (const auto& request : requests) {
+ mPendingGetValueRequests.insert(request.requestId);
+ }
+ }
+ if (StatusCode status = getHardware()->getValues(mGetValuesCallback, requests);
+ status != StatusCode::OK) {
+ return status;
+ }
+ std::unique_lock<std::mutex> lk(mLock);
+ // Wait for the onGetValueResults.
+ bool result = mCv.wait_for(lk, milliseconds(1000), [this] {
+ ScopedLockAssertion lockAssertion(mLock);
+ return mPendingGetValueRequests.size() == 0;
+ });
+ if (!result) {
+ ALOGE("wait for callbacks for getValues timed-out");
+ return StatusCode::INTERNAL_ERROR;
+ }
+ return StatusCode::OK;
}
StatusCode setValue(const VehiclePropValue& value) {
@@ -167,7 +209,9 @@
std::scoped_lock<std::mutex> lockGuard(mLock);
for (auto& result : results) {
mSetValueResults.push_back(result);
+ mPendingSetValueRequests.erase(result.requestId);
}
+ mCv.notify_all();
}
const std::vector<SetValueResult>& getSetValueResults() {
@@ -179,7 +223,9 @@
std::scoped_lock<std::mutex> lockGuard(mLock);
for (auto& result : results) {
mGetValueResults.push_back(result);
+ mPendingGetValueRequests.erase(result.requestId);
}
+ mCv.notify_all();
}
const std::vector<GetValueResult>& getGetValueResults() {
@@ -197,7 +243,7 @@
};
mEventCount[propIdAreaId]++;
}
- mCv.notify_one();
+ mCv.notify_all();
}
const std::vector<VehiclePropValue>& getChangedProperties() {
@@ -295,6 +341,8 @@
std::vector<SetValueResult> mSetValueResults GUARDED_BY(mLock);
std::vector<GetValueResult> mGetValueResults GUARDED_BY(mLock);
std::vector<VehiclePropValue> mChangedProperties GUARDED_BY(mLock);
+ std::unordered_set<int64_t> mPendingSetValueRequests GUARDED_BY(mLock);
+ std::unordered_set<int64_t> mPendingGetValueRequests GUARDED_BY(mLock);
};
TEST_F(FakeVehicleHardwareTest, testGetAllPropertyConfigs) {