Change IVehicleHardware callbacks to shared_ptr.
Use shared_ptr for hardware callbacks so that same callback could
be reused for multiple hardware calls.
Test: atest DefaultVehicleHalTest
atest FakeVehicleHardwareTest
Bug: 200737967
Change-Id: I2a005bbf77241fe2c85f871690c8aef18e770b69
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 46a526c..cab184b 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
@@ -39,14 +39,6 @@
class FakeVehicleHardware final : public IVehicleHardware {
public:
- using SetValuesCallback = std::function<void(
- const std::vector<::aidl::android::hardware::automotive::vehicle::SetValueResult>&)>;
- using GetValuesCallback = std::function<void(
- const std::vector<::aidl::android::hardware::automotive::vehicle::GetValueResult>&)>;
- using OnPropertyChangeCallback = std::function<void(
- const std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>&)>;
- using OnPropertySetErrorCallback = std::function<void(const std::vector<SetValueErrorEvent>&)>;
-
FakeVehicleHardware();
explicit FakeVehicleHardware(std::unique_ptr<VehiclePropValuePool> valuePool);
@@ -59,7 +51,7 @@
// are sent to vehicle bus or before property set confirmation is received. The callback is
// safe to be called after the function returns and is safe to be called in a different thread.
::aidl::android::hardware::automotive::vehicle::StatusCode setValues(
- SetValuesCallback&& callback,
+ std::shared_ptr<const SetValuesCallback> callback,
const std::vector<::aidl::android::hardware::automotive::vehicle::SetValueRequest>&
requests) override;
@@ -67,7 +59,7 @@
// The callback is safe to be called after the function returns and is safe to be called in a
// different thread.
::aidl::android::hardware::automotive::vehicle::StatusCode getValues(
- GetValuesCallback&& callback,
+ std::shared_ptr<const GetValuesCallback> callback,
const std::vector<::aidl::android::hardware::automotive::vehicle::GetValueRequest>&
requests) const override;
@@ -78,11 +70,13 @@
::aidl::android::hardware::automotive::vehicle::StatusCode checkHealth() override;
// Register a callback that would be called when there is a property change event from vehicle.
- void registerOnPropertyChangeEvent(OnPropertyChangeCallback&& callback) override;
+ void registerOnPropertyChangeEvent(
+ std::unique_ptr<const PropertyChangeCallback> callback) override;
// Register a callback that would be called when there is a property set error event from
// vehicle.
- void registerOnPropertySetErrorEvent(OnPropertySetErrorCallback&& callback) override;
+ void registerOnPropertySetErrorEvent(
+ std::unique_ptr<const PropertySetErrorCallback> callback) override;
private:
// Expose private methods to unit test.
@@ -94,8 +88,10 @@
const std::unique_ptr<obd2frame::FakeObd2Frame> mFakeObd2Frame;
const std::unique_ptr<FakeUserHal> mFakeUserHal;
std::mutex mCallbackLock;
- OnPropertyChangeCallback mOnPropertyChangeCallback GUARDED_BY(mCallbackLock);
- OnPropertySetErrorCallback mOnPropertySetErrorCallback GUARDED_BY(mCallbackLock);
+ std::unique_ptr<const PropertyChangeCallback> mOnPropertyChangeCallback
+ GUARDED_BY(mCallbackLock);
+ std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback
+ GUARDED_BY(mCallbackLock);
void init();
// Stores the initial value to property store.
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 5b2003e..e75f0e7 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
@@ -379,7 +379,7 @@
return {};
}
-StatusCode FakeVehicleHardware::setValues(FakeVehicleHardware::SetValuesCallback&& callback,
+StatusCode FakeVehicleHardware::setValues(std::shared_ptr<const SetValuesCallback> callback,
const std::vector<SetValueRequest>& requests) {
std::vector<VehiclePropValue> updatedValues;
std::vector<SetValueResult> results;
@@ -424,12 +424,12 @@
// 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));
+ (*callback)(std::move(results));
return StatusCode::OK;
}
-StatusCode FakeVehicleHardware::getValues(FakeVehicleHardware::GetValuesCallback&& callback,
+StatusCode FakeVehicleHardware::getValues(std::shared_ptr<const GetValuesCallback> callback,
const std::vector<GetValueRequest>& requests) const {
std::vector<GetValueResult> results;
for (auto& request : requests) {
@@ -471,7 +471,7 @@
results.push_back(std::move(getValueResult));
}
- callback(std::move(results));
+ (*callback)(std::move(results));
return StatusCode::OK;
}
@@ -487,23 +487,28 @@
return StatusCode::OK;
}
-void FakeVehicleHardware::registerOnPropertyChangeEvent(OnPropertyChangeCallback&& callback) {
+void FakeVehicleHardware::registerOnPropertyChangeEvent(
+ std::unique_ptr<const PropertyChangeCallback> callback) {
std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
mOnPropertyChangeCallback = std::move(callback);
}
-void FakeVehicleHardware::registerOnPropertySetErrorEvent(OnPropertySetErrorCallback&& callback) {
+void FakeVehicleHardware::registerOnPropertySetErrorEvent(
+ std::unique_ptr<const PropertySetErrorCallback> callback) {
std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
mOnPropertySetErrorCallback = std::move(callback);
}
void FakeVehicleHardware::onValueChangeCallback(const VehiclePropValue& value) {
std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
- if (mOnPropertyChangeCallback != nullptr) {
- std::vector<VehiclePropValue> updatedValues;
- updatedValues.push_back(value);
- mOnPropertyChangeCallback(std::move(updatedValues));
+
+ if (mOnPropertyChangeCallback == nullptr) {
+ return;
}
+
+ std::vector<VehiclePropValue> updatedValues;
+ updatedValues.push_back(value);
+ (*mOnPropertyChangeCallback)(std::move(updatedValues));
}
void FakeVehicleHardware::maybeOverrideProperties(const char* overrideDir) {
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 88834f3..f8df6e6 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
@@ -76,24 +76,25 @@
class FakeVehicleHardwareTest : public ::testing::Test {
protected:
void SetUp() override {
- getHardware()->registerOnPropertyChangeEvent(
+ auto callback = std::make_unique<IVehicleHardware::PropertyChangeCallback>(
[this](const std::vector<VehiclePropValue>& values) {
- return onPropertyChangeEvent(values);
+ onPropertyChangeEvent(values);
});
+ getHardware()->registerOnPropertyChangeEvent(std::move(callback));
+ mSetValuesCallback = std::make_shared<IVehicleHardware::SetValuesCallback>(
+ [this](std::vector<SetValueResult> results) { onSetValues(results); });
+ mGetValuesCallback = std::make_shared<IVehicleHardware::GetValuesCallback>(
+ [this](std::vector<GetValueResult> results) { onGetValues(results); });
}
FakeVehicleHardware* getHardware() { return &mHardware; }
StatusCode setValues(const std::vector<SetValueRequest>& requests) {
- return getHardware()->setValues(
- [this](const std::vector<SetValueResult> results) { return onSetValues(results); },
- requests);
+ return getHardware()->setValues(mSetValuesCallback, requests);
}
StatusCode getValues(const std::vector<GetValueRequest>& requests) {
- return getHardware()->getValues(
- [this](const std::vector<GetValueResult> results) { return onGetValues(results); },
- requests);
+ return getHardware()->getValues(mGetValuesCallback, requests);
}
StatusCode setValue(const VehiclePropValue& value) {
@@ -245,6 +246,8 @@
std::vector<SetValueResult> mSetValueResults;
std::vector<GetValueResult> mGetValueResults;
std::vector<VehiclePropValue> mChangedProperties;
+ std::shared_ptr<IVehicleHardware::SetValuesCallback> mSetValuesCallback;
+ std::shared_ptr<IVehicleHardware::GetValuesCallback> mGetValuesCallback;
};
TEST_F(FakeVehicleHardwareTest, testGetAllPropertyConfigs) {
@@ -367,9 +370,9 @@
TEST_F(FakeVehicleHardwareTest, testRegisterOnPropertyChangeEvent) {
// We have already registered this callback in Setup, here we are registering again.
- getHardware()->registerOnPropertyChangeEvent(std::bind(
- &FakeVehicleHardwareTest_testRegisterOnPropertyChangeEvent_Test::onPropertyChangeEvent,
- this, std::placeholders::_1));
+ auto callback = std::make_unique<IVehicleHardware::PropertyChangeCallback>(
+ [this](const std::vector<VehiclePropValue>& values) { onPropertyChangeEvent(values); });
+ getHardware()->registerOnPropertyChangeEvent(std::move(callback));
auto testValues = getTestPropValues();
std::vector<SetValueRequest> requests;
diff --git a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
index 2e12327..4b9de2d 100644
--- a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
@@ -19,6 +19,7 @@
#include <VehicleHalTypes.h>
+#include <memory>
#include <vector>
namespace android {
@@ -49,6 +50,14 @@
// with a VehicleHardware through this interface.
class IVehicleHardware {
public:
+ using SetValuesCallback = std::function<void(
+ std::vector<::aidl::android::hardware::automotive::vehicle::SetValueResult>)>;
+ using GetValuesCallback = std::function<void(
+ std::vector<::aidl::android::hardware::automotive::vehicle::GetValueResult>)>;
+ using PropertyChangeCallback = std::function<void(
+ std::vector<::aidl::android::hardware::automotive::vehicle::VehiclePropValue>)>;
+ using PropertySetErrorCallback = std::function<void(std::vector<SetValueErrorEvent>)>;
+
virtual ~IVehicleHardware() = default;
// Get all the property configs.
@@ -59,9 +68,7 @@
// are sent to vehicle bus or before property set confirmation is received. The callback is
// safe to be called after the function returns and is safe to be called in a different thread.
virtual ::aidl::android::hardware::automotive::vehicle::StatusCode setValues(
- std::function<void(const std::vector<
- ::aidl::android::hardware::automotive::vehicle::SetValueResult>&)>&&
- callback,
+ std::shared_ptr<const SetValuesCallback> callback,
const std::vector<::aidl::android::hardware::automotive::vehicle::SetValueRequest>&
requests) = 0;
@@ -69,9 +76,7 @@
// The callback is safe to be called after the function returns and is safe to be called in a
// different thread.
virtual ::aidl::android::hardware::automotive::vehicle::StatusCode getValues(
- std::function<void(const std::vector<
- ::aidl::android::hardware::automotive::vehicle::GetValueResult>&)>&&
- callback,
+ std::shared_ptr<const GetValuesCallback> callback,
const std::vector<::aidl::android::hardware::automotive::vehicle::GetValueRequest>&
requests) const = 0;
@@ -83,13 +88,12 @@
// Register a callback that would be called when there is a property change event from vehicle.
virtual void registerOnPropertyChangeEvent(
- std::function<void(const std::vector<::aidl::android::hardware::automotive::vehicle::
- VehiclePropValue>&)>&& callback) = 0;
+ std::unique_ptr<const PropertyChangeCallback> callback) = 0;
// Register a callback that would be called when there is a property set error event from
// vehicle.
virtual void registerOnPropertySetErrorEvent(
- std::function<void(const std::vector<SetValueErrorEvent>&)>&& callback) = 0;
+ std::unique_ptr<const PropertySetErrorCallback> callback) = 0;
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 62a7098..2b5ca70 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -19,6 +19,7 @@
#include <IVehicleHardware.h>
#include <LargeParcelableBase.h>
#include <aidl/android/hardware/automotive/vehicle/IVehicle.h>
+#include <android-base/thread_annotations.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
@@ -53,16 +54,17 @@
class MockVehicleHardware final : public IVehicleHardware {
public:
std::vector<VehiclePropConfig> getAllPropertyConfigs() const override {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
return mPropertyConfigs;
}
- StatusCode setValues(std::function<void(const std::vector<SetValueResult>&)>&&,
+ StatusCode setValues(std::shared_ptr<const SetValuesCallback>,
const std::vector<SetValueRequest>&) override {
// TODO(b/200737967): mock this.
return StatusCode::OK;
}
- StatusCode getValues(std::function<void(const std::vector<GetValueResult>&)>&&,
+ StatusCode getValues(std::shared_ptr<const GetValuesCallback>,
const std::vector<GetValueRequest>&) const override {
// TODO(b/200737967): mock this.
return StatusCode::OK;
@@ -78,23 +80,23 @@
return StatusCode::OK;
}
- void registerOnPropertyChangeEvent(
- std::function<void(const std::vector<VehiclePropValue>&)>&&) override {
+ void registerOnPropertyChangeEvent(std::unique_ptr<const PropertyChangeCallback>) override {
// TODO(b/200737967): mock this.
}
- void registerOnPropertySetErrorEvent(
- std::function<void(const std::vector<SetValueErrorEvent>&)>&&) override {
+ void registerOnPropertySetErrorEvent(std::unique_ptr<const PropertySetErrorCallback>) override {
// TODO(b/200737967): mock this.
}
// Test functions.
void setPropertyConfigs(const std::vector<VehiclePropConfig>& configs) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
mPropertyConfigs = configs;
}
private:
- std::vector<VehiclePropConfig> mPropertyConfigs;
+ mutable std::mutex mLock;
+ std::vector<VehiclePropConfig> mPropertyConfigs GUARDED_BY(mLock);
};
struct PropConfigCmp {