Update subscription logic for VHAL ref impl.
Update the implementation for subscription logic. Add clearer
documentation for sampleRate and timestamp behavior. The sampleRate
specified in subscribeOptions is just a guidance to tell VHAL what
the polling rate could be. For timestamp, the timestamp returned
for each property must be the timestamp when that property is
updated, not when the property is retrieved.
Test: atest FakeVehicleHardwareTest
Bug: 225191802, 226000926
Change-Id: I1e886133258236eedfa7fcffe5c4fb49aead4f6f
diff --git a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
index dc9b876..47fc54b 100644
--- a/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
+++ b/automotive/vehicle/aidl/android/hardware/automotive/vehicle/IVehicle.aidl
@@ -90,6 +90,14 @@
* area ID) are not allowed in a single call. This function must return
* {@link StatusCode#INVALID_ARG} for duplicate properties.
*
+ * The {@link VehiclePropValue#timestamp} field in request is ignored. The
+ * {@link VehiclePropValue#timestamp} field in {@link GetValueResult} must
+ * be the system uptime since boot when the value changes for
+ * ON_CHANGE property or when the value is checked according to polling rate
+ * for CONTINUOUS property. Note that for CONTINUOUS property, VHAL client
+ * reading the property multiple times between the polling interval will get
+ * the same timestamp.
+ *
* @param callback A callback interface, whose 'onGetValues' would be called
* after the value is fetched. Caller should use
* {@code android-automotive-large-parcelable} library to parse the
@@ -104,7 +112,7 @@
* Set vehicle property values.
*
* The {@link IVehicleCallback#onSetValues} function would be called after
- * the values set request are sent through vehicle bus or are failed to set.
+ * the values set request are sent through vehicle bus or failed to set.
* If the bus protocol supports confirmation, the callback would be called
* after getting the confirmation.
*
@@ -152,11 +160,36 @@
* Clients must be able to subscribe to multiple properties at a time
* depending on data provided in options argument.
*
- * For one callback, the is only one subscription for one property.
+ * For one callback, there is only one subscription for one property.
* A new subscription with a different sample rate would override the old
* subscription. One property could be subscribed multiple times for
* different callbacks.
*
+ * If error is returned, some of the properties failed to subscribe.
+ * Caller is safe to try again, since subscribing to an already subscribed
+ * property is okay.
+ *
+ * The specified sample rate is just a guidance. It is not guaranteed that
+ * the sample rate is achievable depending on how the polling refresh rate
+ * is. The actual property event rate might be higher/lower than the
+ * specified sampleRate, for example, if the polling rate can be 5 times/s
+ * or 10 times/s, subscribing to a sample rate of 7 might use the 5 times/s
+ * polling rate, thus generating 5 events/s. We only require that on
+ * average, the {@code minSampleRate} and {@code maxSampleRate} can be
+ * achieved, all the sampleRate within min and max would on average
+ * generates events with rate >= {@code minSampleRate} and <=
+ * {@code maxSampleRate}.
+ *
+ * The {@link VehiclePropValue#timestamp} field for each property event must
+ * be the system uptime since boot when the value changes for
+ * ON_CHANGE property or when the value is checked according to polling rate
+ * for CONTINUOUS property. Note that for CONTINUOUS property, VHAL client
+ * reading the property multiple times between the polling interval will get
+ * the same timestamp.
+ * For example, if the polling rate for a property is 10 times/s, no matter
+ * what the sampleRate specified in {@code options}, the timestamp for
+ * the timestamp is updated 10 times/s.
+ *
* @param callback The subscription callbacks.
* {@link IVehicleCallback#onPropertyEvent} would be called when a new
* property event arrives.
@@ -189,8 +222,13 @@
/**
* Unsubscribes from property events.
*
- * If 'callback' is not valid or 'propIds' were not subscribed for this
- * 'callback', this method must return {@link StatusCode#INVALID_ARG}.
+ * If 'callback' is not valid this method must return
+ * {@link StatusCode#INVALID_ARG}. If a specified propId was not subscribed
+ * before, this method must ignore that propId.
+ *
+ * If error is returned, some of the properties failed to unsubscribe.
+ * Caller is safe to try again, since unsubscribing an already unsubscribed
+ * property is okay.
*
* @param callback The callback used in the previous subscription.
* @param propIds The IDs for the properties to unsubscribe.
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 e799a28..892b406 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/include/FakeVehicleHardware.h
@@ -21,6 +21,7 @@
#include <FakeObd2Frame.h>
#include <FakeUserHal.h>
#include <IVehicleHardware.h>
+#include <RecurrentTimer.h>
#include <VehicleHalTypes.h>
#include <VehiclePropertyStore.h>
#include <android-base/parseint.h>
@@ -82,6 +83,10 @@
void registerOnPropertySetErrorEvent(
std::unique_ptr<const PropertySetErrorCallback> callback) override;
+ // Update the sample rate for the [propId, areaId] pair.
+ aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
+ int32_t propId, int32_t areaId, float sampleRate) override;
+
protected:
// mValuePool is also used in mServerSidePropStore.
const std::shared_ptr<VehiclePropValuePool> mValuePool;
@@ -99,11 +104,13 @@
const std::unique_ptr<obd2frame::FakeObd2Frame> mFakeObd2Frame;
const std::unique_ptr<FakeUserHal> mFakeUserHal;
- std::mutex mCallbackLock;
- std::unique_ptr<const PropertyChangeCallback> mOnPropertyChangeCallback
- GUARDED_BY(mCallbackLock);
- std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback
- GUARDED_BY(mCallbackLock);
+ // RecurrentTimer is thread-safe.
+ std::unique_ptr<RecurrentTimer> mRecurrentTimer;
+ std::mutex mLock;
+ std::unique_ptr<const PropertyChangeCallback> mOnPropertyChangeCallback GUARDED_BY(mLock);
+ std::unique_ptr<const PropertySetErrorCallback> mOnPropertySetErrorCallback GUARDED_BY(mLock);
+ std::unordered_map<PropIdAreaId, std::shared_ptr<RecurrentTimer::Callback>, PropIdAreaIdHash>
+ mRecurrentActions GUARDED_BY(mLock);
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 f8b64f2..8f8cc5c 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/src/FakeVehicleHardware.cpp
@@ -66,6 +66,7 @@
using ::android::base::Error;
using ::android::base::ParseFloat;
using ::android::base::Result;
+using ::android::base::ScopedLockAssertion;
using ::android::base::StartsWith;
using ::android::base::StringPrintf;
@@ -131,18 +132,14 @@
}
FakeVehicleHardware::FakeVehicleHardware()
- : mValuePool(new VehiclePropValuePool),
- mServerSidePropStore(new VehiclePropertyStore(mValuePool)),
- mFakeObd2Frame(new obd2frame::FakeObd2Frame(mServerSidePropStore)),
- mFakeUserHal(new FakeUserHal(mValuePool)) {
- init();
-}
+ : FakeVehicleHardware(std::make_unique<VehiclePropValuePool>()) {}
FakeVehicleHardware::FakeVehicleHardware(std::unique_ptr<VehiclePropValuePool> valuePool)
: mValuePool(std::move(valuePool)),
mServerSidePropStore(new VehiclePropertyStore(mValuePool)),
mFakeObd2Frame(new obd2frame::FakeObd2Frame(mServerSidePropStore)),
- mFakeUserHal(new FakeUserHal(mValuePool)) {
+ mFakeUserHal(new FakeUserHal(mValuePool)),
+ mRecurrentTimer(new RecurrentTimer()) {
init();
}
@@ -837,18 +834,57 @@
void FakeVehicleHardware::registerOnPropertyChangeEvent(
std::unique_ptr<const PropertyChangeCallback> callback) {
- std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
+ std::scoped_lock<std::mutex> lockGuard(mLock);
mOnPropertyChangeCallback = std::move(callback);
}
void FakeVehicleHardware::registerOnPropertySetErrorEvent(
std::unique_ptr<const PropertySetErrorCallback> callback) {
- std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
+ std::scoped_lock<std::mutex> lockGuard(mLock);
mOnPropertySetErrorCallback = std::move(callback);
}
+StatusCode FakeVehicleHardware::updateSampleRate(int32_t propId, int32_t areaId, float sampleRate) {
+ // DefaultVehicleHal makes sure that sampleRate must be within minSampleRate and maxSampleRate.
+ // For fake implementation, we would write the same value with a new timestamp into propStore
+ // at sample rate.
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+
+ PropIdAreaId propIdAreaId{
+ .propId = propId,
+ .areaId = areaId,
+ };
+ if (mRecurrentActions.find(propIdAreaId) != mRecurrentActions.end()) {
+ mRecurrentTimer->unregisterTimerCallback(mRecurrentActions[propIdAreaId]);
+ }
+ if (sampleRate == 0) {
+ return StatusCode::OK;
+ }
+ int64_t interval = static_cast<int64_t>(1'000'000'000. / sampleRate);
+ auto action = std::make_shared<RecurrentTimer::Callback>([this, propId, areaId] {
+ // Refresh the property value. In real implementation, this should poll the latest value
+ // from vehicle bus. Here, we are just refreshing the existing value with a new timestamp.
+ auto result = getValue(VehiclePropValue{
+ .prop = propId,
+ .areaId = areaId,
+ });
+ if (!result.ok()) {
+ // Failed to read current value, skip refreshing.
+ return;
+ }
+ result.value()->timestamp = elapsedRealtimeNano();
+ // Must remove the value before writing, otherwise, we would generate no update event since
+ // the value is the same.
+ mServerSidePropStore->removeValue(*result.value());
+ mServerSidePropStore->writeValue(std::move(result.value()));
+ });
+ mRecurrentTimer->registerTimerCallback(interval, action);
+ mRecurrentActions[propIdAreaId] = action;
+ return StatusCode::OK;
+}
+
void FakeVehicleHardware::onValueChangeCallback(const VehiclePropValue& value) {
- std::scoped_lock<std::mutex> lockGuard(mCallbackLock);
+ std::scoped_lock<std::mutex> lockGuard(mLock);
if (mOnPropertyChangeCallback == nullptr) {
return;
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 7a7fb37..8839c18 100644
--- a/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
+++ b/automotive/vehicle/aidl/impl/fake_impl/hardware/test/FakeVehicleHardwareTest.cpp
@@ -25,12 +25,15 @@
#include <android-base/expected.h>
#include <android-base/file.h>
#include <android-base/stringprintf.h>
+#include <android-base/thread_annotations.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <utils/Log.h>
#include <utils/SystemClock.h>
#include <inttypes.h>
+#include <chrono>
+#include <condition_variable>
#include <vector>
namespace android {
@@ -53,6 +56,7 @@
using ::aidl::android::hardware::automotive::vehicle::VehiclePropertyStatus;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
using ::android::base::expected;
+using ::android::base::ScopedLockAssertion;
using ::android::base::StringPrintf;
using ::android::base::unexpected;
using ::testing::ContainerEq;
@@ -60,6 +64,8 @@
using ::testing::Eq;
using ::testing::WhenSortedBy;
+using std::chrono::milliseconds;
+
constexpr int INVALID_PROP_ID = 0;
constexpr char CAR_MAKE[] = "Default Car";
@@ -158,30 +164,65 @@
}
void onSetValues(std::vector<SetValueResult> results) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
for (auto& result : results) {
mSetValueResults.push_back(result);
}
}
- const std::vector<SetValueResult>& getSetValueResults() { return mSetValueResults; }
+ const std::vector<SetValueResult>& getSetValueResults() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ return mSetValueResults;
+ }
void onGetValues(std::vector<GetValueResult> results) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
for (auto& result : results) {
mGetValueResults.push_back(result);
}
}
- const std::vector<GetValueResult>& getGetValueResults() { return mGetValueResults; }
-
- void onPropertyChangeEvent(std::vector<VehiclePropValue> values) {
- for (auto& value : values) {
- mChangedProperties.push_back(value);
- }
+ const std::vector<GetValueResult>& getGetValueResults() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ return mGetValueResults;
}
- const std::vector<VehiclePropValue>& getChangedProperties() { return mChangedProperties; }
+ void onPropertyChangeEvent(std::vector<VehiclePropValue> values) {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ for (auto& value : values) {
+ mChangedProperties.push_back(value);
+ PropIdAreaId propIdAreaId{
+ .propId = value.prop,
+ .areaId = value.areaId,
+ };
+ mEventCount[propIdAreaId]++;
+ }
+ mCv.notify_one();
+ }
- void clearChangedProperties() { mChangedProperties.clear(); }
+ const std::vector<VehiclePropValue>& getChangedProperties() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ return mChangedProperties;
+ }
+
+ bool waitForChangedProperties(int32_t propId, int32_t areaId, size_t count,
+ milliseconds timeout) {
+ PropIdAreaId propIdAreaId{
+ .propId = propId,
+ .areaId = areaId,
+ };
+ std::unique_lock<std::mutex> lk(mLock);
+ return mCv.wait_for(lk, timeout, [this, propIdAreaId, count] {
+ ScopedLockAssertion lockAssertion(mLock);
+ return mEventCount[propIdAreaId] >= count;
+ });
+ }
+
+ void clearChangedProperties() {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ mEventCount.clear();
+ mChangedProperties.clear();
+ }
static void addSetValueRequest(std::vector<SetValueRequest>& requests,
std::vector<SetValueResult>& expectedResults, int64_t requestId,
@@ -246,11 +287,14 @@
private:
FakeVehicleHardware mHardware;
- std::vector<SetValueResult> mSetValueResults;
- std::vector<GetValueResult> mGetValueResults;
- std::vector<VehiclePropValue> mChangedProperties;
std::shared_ptr<IVehicleHardware::SetValuesCallback> mSetValuesCallback;
std::shared_ptr<IVehicleHardware::GetValuesCallback> mGetValuesCallback;
+ std::condition_variable mCv;
+ std::mutex mLock;
+ std::unordered_map<PropIdAreaId, size_t, PropIdAreaIdHash> mEventCount GUARDED_BY(mLock);
+ std::vector<SetValueResult> mSetValueResults GUARDED_BY(mLock);
+ std::vector<GetValueResult> mGetValueResults GUARDED_BY(mLock);
+ std::vector<VehiclePropValue> mChangedProperties GUARDED_BY(mLock);
};
TEST_F(FakeVehicleHardwareTest, testGetAllPropertyConfigs) {
@@ -1510,6 +1554,35 @@
ASSERT_EQ(result.value().value.byteValues, std::vector<uint8_t>({0x04, 0x03, 0x02, 0x01}));
}
+TEST_F(FakeVehicleHardwareTest, testUpdateSampleRate) {
+ int32_t propSpeed = toInt(VehicleProperty::PERF_VEHICLE_SPEED);
+ int32_t propSteering = toInt(VehicleProperty::PERF_STEERING_ANGLE);
+ int32_t areaId = 0;
+ getHardware()->updateSampleRate(propSpeed, areaId, 5);
+
+ ASSERT_TRUE(waitForChangedProperties(propSpeed, areaId, /*count=*/5, milliseconds(1500)))
+ << "not enough events generated for speed";
+
+ getHardware()->updateSampleRate(propSteering, areaId, 10);
+
+ ASSERT_TRUE(waitForChangedProperties(propSteering, areaId, /*count=*/10, milliseconds(1500)))
+ << "not enough events generated for steering";
+
+ int64_t timestamp = elapsedRealtimeNano();
+ // Disable refreshing for propSpeed.
+ getHardware()->updateSampleRate(propSpeed, areaId, 0);
+ clearChangedProperties();
+
+ ASSERT_TRUE(waitForChangedProperties(propSteering, areaId, /*count=*/5, milliseconds(1500)))
+ << "should still receive steering events after disable polling for speed";
+ auto updatedValues = getChangedProperties();
+ for (auto& value : updatedValues) {
+ ASSERT_GE(value.timestamp, timestamp);
+ ASSERT_EQ(value.prop, propSteering);
+ ASSERT_EQ(value.areaId, areaId);
+ }
+}
+
} // namespace fake
} // namespace vehicle
} // namespace automotive
diff --git a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
index 4a38827..759db41 100644
--- a/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/hardware/include/IVehicleHardware.h
@@ -80,6 +80,35 @@
const std::vector<aidl::android::hardware::automotive::vehicle::GetValueRequest>&
requests) const = 0;
+ // Update the sampling rate for the specified property and the specified areaId (0 for global
+ // property) if server supports it. The property must be a continuous property.
+ // {@code sampleRate} means that for this specific property, the server must generate at least
+ // this many OnPropertyChange events per seconds.
+ // A sampleRate of 0 means the property is no longer subscribed and server does not need to
+ // generate any onPropertyEvent for this property.
+ // This would be called if sample rate is updated for a subscriber, a new subscriber is added
+ // or an existing subscriber is removed. For example:
+ // 1. We have no subscriber for speed.
+ // 2. A new subscriber is subscribing speed for 10 times/s, updsateSampleRate would be called
+ // with sampleRate as 10. The impl is now polling vehicle speed from bus 10 times/s.
+ // 3. A new subscriber is subscribing speed for 5 times/s, because it is less than 10
+ // times/sec, updateSampleRate would not be called.
+ // 4. The initial subscriber is removed, updateSampleRate would be called with sampleRate as
+ // 5, because now it only needs to report event 5times/sec. The impl can now poll vehicle
+ // speed 5 times/s. If the impl is still polling at 10 times/s, that is okay as long as
+ // the polling rate is larger than 5times/s. DefaultVehicleHal would ignore the additional
+ // events.
+ // 5. The second subscriber is removed, updateSampleRate would be called with sampleRate as 0.
+ // The impl can optionally disable the polling for vehicle speed.
+ //
+ // If the impl is always polling at {@code maxSampleRate} as specified in config, then this
+ // function can be a no-op.
+ virtual aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
+ [[maybe_unused]] int32_t propId, [[maybe_unused]] int32_t areaId,
+ [[maybe_unused]] float sampleRate) {
+ return aidl::android::hardware::automotive::vehicle::StatusCode::OK;
+ }
+
// Dump debug information in the server.
virtual DumpResult dump(const std::vector<std::string>& options) = 0;
diff --git a/automotive/vehicle/aidl/impl/vhal/include/RecurrentTimer.h b/automotive/vehicle/aidl/impl/utils/common/include/RecurrentTimer.h
similarity index 100%
rename from automotive/vehicle/aidl/impl/vhal/include/RecurrentTimer.h
rename to automotive/vehicle/aidl/impl/utils/common/include/RecurrentTimer.h
diff --git a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
index 6d7d131..c94bad6 100644
--- a/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
+++ b/automotive/vehicle/aidl/impl/utils/common/include/VehicleUtils.h
@@ -21,6 +21,7 @@
#include <android-base/format.h>
#include <android-base/result.h>
+#include <math/HashCombine.h>
#include <utils/Log.h>
namespace android {
@@ -310,6 +311,24 @@
return toScopedAStatus(result, getErrorCode(result), additionalErrorMsg);
}
+struct PropIdAreaId {
+ int32_t propId;
+ int32_t areaId;
+
+ inline bool operator==(const PropIdAreaId& other) const {
+ return areaId == other.areaId && propId == other.propId;
+ }
+};
+
+struct PropIdAreaIdHash {
+ inline size_t operator()(const PropIdAreaId& propIdAreaId) const {
+ size_t res = 0;
+ hashCombine(res, propIdAreaId.propId);
+ hashCombine(res, propIdAreaId.areaId);
+ return res;
+ }
+};
+
} // namespace vehicle
} // namespace automotive
} // namespace hardware
diff --git a/automotive/vehicle/aidl/impl/vhal/src/RecurrentTimer.cpp b/automotive/vehicle/aidl/impl/utils/common/src/RecurrentTimer.cpp
similarity index 100%
rename from automotive/vehicle/aidl/impl/vhal/src/RecurrentTimer.cpp
rename to automotive/vehicle/aidl/impl/utils/common/src/RecurrentTimer.cpp
diff --git a/automotive/vehicle/aidl/impl/vhal/test/RecurrentTimerTest.cpp b/automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp
similarity index 100%
rename from automotive/vehicle/aidl/impl/vhal/test/RecurrentTimerTest.cpp
rename to automotive/vehicle/aidl/impl/utils/common/test/RecurrentTimerTest.cpp
diff --git a/automotive/vehicle/aidl/impl/vhal/Android.bp b/automotive/vehicle/aidl/impl/vhal/Android.bp
index 49f48f7..5abcaf6 100644
--- a/automotive/vehicle/aidl/impl/vhal/Android.bp
+++ b/automotive/vehicle/aidl/impl/vhal/Android.bp
@@ -54,7 +54,6 @@
srcs: [
"src/ConnectedClient.cpp",
"src/DefaultVehicleHal.cpp",
- "src/RecurrentTimer.cpp",
"src/SubscriptionManager.cpp",
],
static_libs: [
diff --git a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
index f646b6b..9c29816 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/DefaultVehicleHal.h
@@ -17,10 +17,11 @@
#ifndef android_hardware_automotive_vehicle_aidl_impl_vhal_include_DefaultVehicleHal_H_
#define android_hardware_automotive_vehicle_aidl_impl_vhal_include_DefaultVehicleHal_H_
-#include "ConnectedClient.h"
-#include "ParcelableUtils.h"
-#include "PendingRequestPool.h"
-#include "SubscriptionManager.h"
+#include <ConnectedClient.h>
+#include <ParcelableUtils.h>
+#include <PendingRequestPool.h>
+#include <RecurrentTimer.h>
+#include <SubscriptionManager.h>
#include <ConcurrentQueue.h>
#include <IVehicleHardware.h>
@@ -163,7 +164,7 @@
static constexpr int64_t TIMEOUT_IN_NANO = 30'000'000'000;
// heart beat event interval: 3s
static constexpr int64_t HEART_BEAT_INTERVAL_IN_NANO = 3'000'000'000;
- const std::shared_ptr<IVehicleHardware> mVehicleHardware;
+ std::unique_ptr<IVehicleHardware> mVehicleHardware;
// mConfigsByPropId and mConfigFile are only modified during initialization, so no need to
// lock guard them.
@@ -188,6 +189,8 @@
// mBinderImpl is only going to be changed in test.
std::unique_ptr<IBinder> mBinderImpl;
+ // Only initialized once.
+ std::shared_ptr<std::function<void()>> mRecurrentAction;
// RecurrentTimer is thread-safe.
RecurrentTimer mRecurrentTimer;
@@ -243,18 +246,12 @@
std::unordered_map<const AIBinder*, std::shared_ptr<T>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
- static void getValueFromHardwareCallCallback(
- std::weak_ptr<IVehicleHardware> vehicleHardware,
- std::shared_ptr<SubscribeIdByClient> subscribeIdByClient,
- std::shared_ptr<SubscriptionClients> subscriptionClients, const CallbackType& callback,
- const aidl::android::hardware::automotive::vehicle::VehiclePropValue& value);
-
static void onPropertyChangeEvent(
std::weak_ptr<SubscriptionManager> subscriptionManager,
const std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropValue>&
updatedValues);
- static void checkHealth(std::weak_ptr<IVehicleHardware> hardware,
+ static void checkHealth(IVehicleHardware* hardware,
std::weak_ptr<SubscriptionManager> subscriptionManager);
static void onBinderDied(void* cookie);
diff --git a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
index b0d6701..7c8f1b4 100644
--- a/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
+++ b/automotive/vehicle/aidl/impl/vhal/include/SubscriptionManager.h
@@ -17,15 +17,16 @@
#ifndef android_hardware_automotive_vehicle_aidl_impl_vhal_include_SubscriptionManager_H_
#define android_hardware_automotive_vehicle_aidl_impl_vhal_include_SubscriptionManager_H_
-#include "RecurrentTimer.h"
-
+#include <IVehicleHardware.h>
#include <VehicleHalTypes.h>
+#include <VehicleUtils.h>
#include <aidl/android/hardware/automotive/vehicle/IVehicleCallback.h>
#include <android-base/result.h>
#include <android-base/thread_annotations.h>
#include <mutex>
+#include <optional>
#include <unordered_map>
#include <unordered_set>
#include <vector>
@@ -35,43 +36,58 @@
namespace automotive {
namespace vehicle {
+// A class to represent all the subscription configs for a continuous [propId, areaId].
+class ContSubConfigs final {
+ public:
+ using ClientIdType = const AIBinder*;
+
+ void addClient(const ClientIdType& clientId, float sampleRate);
+ void removeClient(const ClientIdType& clientId);
+ float getMaxSampleRate();
+
+ private:
+ float mMaxSampleRate = 0.;
+ std::unordered_map<ClientIdType, float> mSampleRates;
+
+ void refreshMaxSampleRate();
+};
+
// A thread-safe subscription manager that manages all VHAL subscriptions.
class SubscriptionManager final {
public:
using ClientIdType = const AIBinder*;
using CallbackType =
std::shared_ptr<aidl::android::hardware::automotive::vehicle::IVehicleCallback>;
- using GetValueFunc = std::function<void(
- const CallbackType& callback,
- const aidl::android::hardware::automotive::vehicle::VehiclePropValue& value)>;
- explicit SubscriptionManager(GetValueFunc&& action);
+ explicit SubscriptionManager(IVehicleHardware* hardware);
~SubscriptionManager();
// Subscribes to properties according to {@code SubscribeOptions}. Note that all option must
// contain non-empty areaIds field, which contains all area IDs to subscribe. As a result,
// the options here is different from the options passed from VHAL client.
- // Returns error if any of the subscribe options is not valid. If error is returned, no
- // properties would be subscribed.
+ // Returns error if any of the subscribe options is not valid or one of the properties failed
+ // to subscribe. Part of the properties maybe be subscribed successfully if this function
+ // returns error. Caller is safe to retry since subscribing to an already subscribed property
+ // is okay.
// Returns ok if all the options are parsed correctly and all the properties are subscribed.
- android::base::Result<void> subscribe(
+ VhalResult<void> subscribe(
const CallbackType& callback,
const std::vector<aidl::android::hardware::automotive::vehicle::SubscribeOptions>&
options,
bool isContinuousProperty);
// Unsubscribes from the properties for the client.
- // Returns error if the client was not subscribed before or one of the given property was not
- // subscribed. If error is returned, no property would be unsubscribed.
+ // Returns error if the client was not subscribed before, or one of the given property was not
+ // subscribed, or one of the property failed to unsubscribe. Caller is safe to retry since
+ // unsubscribing to an already unsubscribed property is okay (it would be ignored).
// Returns ok if all the requested properties for the client are unsubscribed.
- android::base::Result<void> unsubscribe(ClientIdType client,
- const std::vector<int32_t>& propIds);
+ VhalResult<void> unsubscribe(ClientIdType client, const std::vector<int32_t>& propIds);
// Unsubscribes from all the properties for the client.
- // Returns error if the client was not subscribed before. If error is returned, no property
- // would be unsubscribed.
+ // Returns error if the client was not subscribed before or one of the subscribed properties
+ // for the client failed to unsubscribe. Caller is safe to retry.
// Returns ok if all the properties for the client are unsubscribed.
- android::base::Result<void> unsubscribe(ClientIdType client);
+ VhalResult<void> unsubscribe(ClientIdType client);
// For a list of updated properties, returns a map that maps clients subscribing to
// the updated properties to a list of updated values. This would only return on-change property
@@ -83,6 +99,11 @@
const std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropValue>&
updatedValues);
+ // Gets the sample rate for the continuous property. Returns {@code std::nullopt} if the
+ // property has not been subscribed before or is not a continuous property.
+ std::optional<float> getSampleRate(const ClientIdType& clientId, int32_t propId,
+ int32_t areaId);
+
// Checks whether the sample rate is valid.
static bool checkSampleRate(float sampleRate);
@@ -90,65 +111,28 @@
// Friend class for testing.
friend class DefaultVehicleHalTest;
- struct PropIdAreaId {
- int32_t propId;
- int32_t areaId;
-
- bool operator==(const PropIdAreaId& other) const;
- };
-
- struct PropIdAreaIdHash {
- size_t operator()(const PropIdAreaId& propIdAreaId) const;
- };
-
- // A class to represent a registered subscription.
- class Subscription {
- public:
- Subscription() = default;
-
- Subscription(const Subscription&) = delete;
-
- virtual ~Subscription() = default;
-
- virtual bool isOnChange();
- };
-
- // A subscription for OnContinuous property. The registered action would be called recurrently
- // until this class is destructed.
- class RecurrentSubscription final : public Subscription {
- public:
- explicit RecurrentSubscription(std::shared_ptr<RecurrentTimer> timer,
- std::function<void()>&& action, int64_t interval);
- ~RecurrentSubscription();
-
- bool isOnChange() override;
-
- private:
- std::shared_ptr<std::function<void()>> mAction;
- std::shared_ptr<RecurrentTimer> mTimer;
- };
-
- // A subscription for OnChange property.
- class OnChangeSubscription final : public Subscription {
- public:
- bool isOnChange() override;
- };
+ IVehicleHardware* mVehicleHardware;
mutable std::mutex mLock;
std::unordered_map<PropIdAreaId, std::unordered_map<ClientIdType, CallbackType>,
PropIdAreaIdHash>
mClientsByPropIdArea GUARDED_BY(mLock);
- std::unordered_map<ClientIdType, std::unordered_map<PropIdAreaId, std::unique_ptr<Subscription>,
- PropIdAreaIdHash>>
- mSubscriptionsByClient GUARDED_BY(mLock);
- // RecurrentTimer is thread-safe.
- std::shared_ptr<RecurrentTimer> mTimer;
- const GetValueFunc mGetValue;
+ std::unordered_map<ClientIdType, std::unordered_set<PropIdAreaId, PropIdAreaIdHash>>
+ mSubscribedPropsByClient GUARDED_BY(mLock);
+ std::unordered_map<PropIdAreaId, ContSubConfigs, PropIdAreaIdHash> mContSubConfigsByPropIdArea
+ GUARDED_BY(mLock);
- static android::base::Result<int64_t> getInterval(float sampleRate);
+ VhalResult<void> updateSampleRateLocked(const ClientIdType& clientId,
+ const PropIdAreaId& propIdAreaId, float sampleRate)
+ REQUIRES(mLock);
+ VhalResult<void> removeSampleRateLocked(const ClientIdType& clientId,
+ const PropIdAreaId& propIdAreaId) REQUIRES(mLock);
// Checks whether the manager is empty. For testing purpose.
bool isEmpty();
+
+ // Get the interval in nanoseconds accroding to sample rate.
+ static android::base::Result<int64_t> getInterval(float sampleRate);
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
index 82f2c1b..b191aef 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/DefaultVehicleHal.cpp
@@ -144,15 +144,11 @@
}
mSubscriptionClients = std::make_shared<SubscriptionClients>(mPendingRequestPool);
+ mSubscriptionClients = std::make_shared<SubscriptionClients>(mPendingRequestPool);
auto subscribeIdByClient = std::make_shared<SubscribeIdByClient>();
- // Make a weak copy of IVehicleHardware because subscriptionManager uses IVehicleHardware and
- // IVehicleHardware uses subscriptionManager. We want to avoid cyclic reference.
- std::weak_ptr<IVehicleHardware> hardwareCopy = mVehicleHardware;
- SubscriptionManager::GetValueFunc getValueFunc = std::bind(
- &DefaultVehicleHal::getValueFromHardwareCallCallback, hardwareCopy, subscribeIdByClient,
- mSubscriptionClients, std::placeholders::_1, std::placeholders::_2);
- mSubscriptionManager = std::make_shared<SubscriptionManager>(std::move(getValueFunc));
+ IVehicleHardware* hardwarePtr = mVehicleHardware.get();
+ mSubscriptionManager = std::make_shared<SubscriptionManager>(hardwarePtr);
std::weak_ptr<SubscriptionManager> subscriptionManagerCopy = mSubscriptionManager;
mVehicleHardware->registerOnPropertyChangeEvent(
@@ -162,11 +158,11 @@
}));
// Register heartbeat event.
- mRecurrentTimer.registerTimerCallback(
- HEART_BEAT_INTERVAL_IN_NANO,
- std::make_shared<std::function<void()>>([hardwareCopy, subscriptionManagerCopy]() {
- checkHealth(hardwareCopy, subscriptionManagerCopy);
- }));
+ mRecurrentAction =
+ std::make_shared<std::function<void()>>([hardwarePtr, subscriptionManagerCopy]() {
+ checkHealth(hardwarePtr, subscriptionManagerCopy);
+ });
+ mRecurrentTimer.registerTimerCallback(HEART_BEAT_INTERVAL_IN_NANO, mRecurrentAction);
mBinderImpl = std::make_unique<AIBinderImpl>();
mOnBinderDiedUnlinkedHandlerThread = std::thread([this] { onBinderDiedUnlinkedHandler(); });
@@ -183,6 +179,13 @@
if (mOnBinderDiedUnlinkedHandlerThread.joinable()) {
mOnBinderDiedUnlinkedHandlerThread.join();
}
+ // mRecurrentAction uses pointer to mVehicleHardware, so it has to be unregistered before
+ // mVehicleHardware.
+ mRecurrentTimer.unregisterTimerCallback(mRecurrentAction);
+ // mSubscriptionManager uses pointer to mVehicleHardware, so it has to be destroyed before
+ // mVehicleHardware.
+ mSubscriptionManager.reset();
+ mVehicleHardware.reset();
}
void DefaultVehicleHal::onPropertyChangeEvent(
@@ -294,43 +297,6 @@
std::unordered_map<const AIBinder*, std::shared_ptr<SubscriptionClient>>* clients,
const CallbackType& callback, std::shared_ptr<PendingRequestPool> pendingRequestPool);
-void DefaultVehicleHal::getValueFromHardwareCallCallback(
- std::weak_ptr<IVehicleHardware> vehicleHardware,
- std::shared_ptr<SubscribeIdByClient> subscribeIdByClient,
- std::shared_ptr<SubscriptionClients> subscriptionClients, const CallbackType& callback,
- const VehiclePropValue& value) {
- int64_t subscribeId = subscribeIdByClient->getId(callback);
- auto client = subscriptionClients->getClient(callback);
- if (client == nullptr) {
- ALOGW("subscribe[%" PRId64 "]: the client has died", subscribeId);
- return;
- }
- if (auto addRequestResult = client->addRequests({subscribeId}); !addRequestResult.ok()) {
- ALOGE("subscribe[%" PRId64 "]: too many pending requests, ignore the getValue request",
- subscribeId);
- return;
- }
-
- std::vector<GetValueRequest> hardwareRequests = {{
- .requestId = subscribeId,
- .prop = value,
- }};
-
- std::shared_ptr<IVehicleHardware> hardware = vehicleHardware.lock();
- if (hardware == nullptr) {
- ALOGW("the IVehicleHardware is destroyed, DefaultVehicleHal is ending");
- return;
- }
- if (StatusCode status = hardware->getValues(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({subscribeId});
- ALOGE("subscribe[%" PRId64 "]: failed to get value from VehicleHardware, code: %d",
- subscribeId, toInt(status));
- }
-}
-
void DefaultVehicleHal::setTimeout(int64_t timeoutInNano) {
mPendingRequestPool = std::make_unique<PendingRequestPool>(timeoutInNano);
}
@@ -705,12 +671,13 @@
// Since we have already check the sample rates, the following functions must succeed.
if (!onChangeSubscriptions.empty()) {
- mSubscriptionManager->subscribe(callback, onChangeSubscriptions,
- /*isContinuousProperty=*/false);
+ return toScopedAStatus(mSubscriptionManager->subscribe(callback, onChangeSubscriptions,
+ /*isContinuousProperty=*/false));
}
if (!continuousSubscriptions.empty()) {
- mSubscriptionManager->subscribe(callback, continuousSubscriptions,
- /*isContinuousProperty=*/true);
+ return toScopedAStatus(mSubscriptionManager->subscribe(callback,
+ continuousSubscriptions,
+ /*isContinuousProperty=*/true));
}
}
return ScopedAStatus::ok();
@@ -718,8 +685,7 @@
ScopedAStatus DefaultVehicleHal::unsubscribe(const CallbackType& callback,
const std::vector<int32_t>& propIds) {
- return toScopedAStatus(mSubscriptionManager->unsubscribe(callback->asBinder().get(), propIds),
- StatusCode::INVALID_ARG);
+ return toScopedAStatus(mSubscriptionManager->unsubscribe(callback->asBinder().get(), propIds));
}
ScopedAStatus DefaultVehicleHal::returnSharedMemory(const CallbackType&, int64_t) {
@@ -763,15 +729,9 @@
return {};
}
-void DefaultVehicleHal::checkHealth(std::weak_ptr<IVehicleHardware> hardware,
+void DefaultVehicleHal::checkHealth(IVehicleHardware* hardware,
std::weak_ptr<SubscriptionManager> subscriptionManager) {
- auto hardwarePtr = hardware.lock();
- if (hardwarePtr == nullptr) {
- ALOGW("the VehicleHardware is destroyed, DefaultVehicleHal is ending");
- return;
- }
-
- StatusCode status = hardwarePtr->checkHealth();
+ StatusCode status = hardware->checkHealth();
if (status != StatusCode::OK) {
ALOGE("VHAL check health returns non-okay status");
return;
diff --git a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
index 21bfba6..2694401 100644
--- a/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/src/SubscriptionManager.cpp
@@ -16,8 +16,11 @@
#include "SubscriptionManager.h"
-#include <math/HashCombine.h>
+#include <android-base/stringprintf.h>
#include <utils/Log.h>
+#include <utils/SystemClock.h>
+
+#include <inttypes.h>
namespace android {
namespace hardware {
@@ -31,33 +34,21 @@
} // namespace
using ::aidl::android::hardware::automotive::vehicle::IVehicleCallback;
+using ::aidl::android::hardware::automotive::vehicle::StatusCode;
using ::aidl::android::hardware::automotive::vehicle::SubscribeOptions;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
using ::android::base::Error;
using ::android::base::Result;
+using ::android::base::StringPrintf;
using ::ndk::ScopedAStatus;
-bool SubscriptionManager::PropIdAreaId::operator==(const PropIdAreaId& other) const {
- return areaId == other.areaId && propId == other.propId;
-}
-
-size_t SubscriptionManager::PropIdAreaIdHash::operator()(PropIdAreaId const& propIdAreaId) const {
- size_t res = 0;
- hashCombine(res, propIdAreaId.propId);
- hashCombine(res, propIdAreaId.areaId);
- return res;
-}
-
-SubscriptionManager::SubscriptionManager(GetValueFunc&& action)
- : mTimer(std::make_shared<RecurrentTimer>()), mGetValue(std::move(action)) {}
+SubscriptionManager::SubscriptionManager(IVehicleHardware* hardware) : mVehicleHardware(hardware) {}
SubscriptionManager::~SubscriptionManager() {
std::scoped_lock<std::mutex> lockGuard(mLock);
mClientsByPropIdArea.clear();
- // RecurrentSubscription has reference to mGetValue, so it must be destroyed before mGetValue is
- // destroyed.
- mSubscriptionsByClient.clear();
+ mSubscribedPropsByClient.clear();
}
bool SubscriptionManager::checkSampleRate(float sampleRate) {
@@ -76,9 +67,84 @@
return interval;
}
-Result<void> SubscriptionManager::subscribe(const std::shared_ptr<IVehicleCallback>& callback,
- const std::vector<SubscribeOptions>& options,
- bool isContinuousProperty) {
+void ContSubConfigs::refreshMaxSampleRate() {
+ float maxSampleRate = 0.;
+ // This is not called frequently so a brute-focre is okay. More efficient way exists but this
+ // is simpler.
+ for (const auto& [_, sampleRate] : mSampleRates) {
+ if (sampleRate > maxSampleRate) {
+ maxSampleRate = sampleRate;
+ }
+ }
+ mMaxSampleRate = maxSampleRate;
+}
+
+void ContSubConfigs::addClient(const ClientIdType& clientId, float sampleRate) {
+ mSampleRates[clientId] = sampleRate;
+ refreshMaxSampleRate();
+}
+
+void ContSubConfigs::removeClient(const ClientIdType& clientId) {
+ mSampleRates.erase(clientId);
+ refreshMaxSampleRate();
+}
+
+float ContSubConfigs::getMaxSampleRate() {
+ return mMaxSampleRate;
+}
+
+VhalResult<void> SubscriptionManager::updateSampleRateLocked(const ClientIdType& clientId,
+ const PropIdAreaId& propIdAreaId,
+ float sampleRate) {
+ // Make a copy so that we don't modify 'mContSubConfigsByPropIdArea' on failure cases.
+ ContSubConfigs infoCopy = mContSubConfigsByPropIdArea[propIdAreaId];
+ infoCopy.addClient(clientId, sampleRate);
+ if (infoCopy.getMaxSampleRate() ==
+ mContSubConfigsByPropIdArea[propIdAreaId].getMaxSampleRate()) {
+ mContSubConfigsByPropIdArea[propIdAreaId] = infoCopy;
+ return {};
+ }
+ float newRate = infoCopy.getMaxSampleRate();
+ int32_t propId = propIdAreaId.propId;
+ int32_t areaId = propIdAreaId.areaId;
+ if (auto status = mVehicleHardware->updateSampleRate(propId, areaId, newRate);
+ status != StatusCode::OK) {
+ return StatusError(status) << StringPrintf("failed to update sample rate for prop: %" PRId32
+ ", area"
+ ": %" PRId32 ", sample rate: %f",
+ propId, areaId, newRate);
+ }
+ mContSubConfigsByPropIdArea[propIdAreaId] = infoCopy;
+ return {};
+}
+
+VhalResult<void> SubscriptionManager::removeSampleRateLocked(const ClientIdType& clientId,
+ const PropIdAreaId& propIdAreaId) {
+ // Make a copy so that we don't modify 'mContSubConfigsByPropIdArea' on failure cases.
+ ContSubConfigs infoCopy = mContSubConfigsByPropIdArea[propIdAreaId];
+ infoCopy.removeClient(clientId);
+ if (infoCopy.getMaxSampleRate() ==
+ mContSubConfigsByPropIdArea[propIdAreaId].getMaxSampleRate()) {
+ mContSubConfigsByPropIdArea[propIdAreaId] = infoCopy;
+ return {};
+ }
+ float newRate = infoCopy.getMaxSampleRate();
+ int32_t propId = propIdAreaId.propId;
+ int32_t areaId = propIdAreaId.areaId;
+ if (auto status = mVehicleHardware->updateSampleRate(propId, areaId, newRate);
+ status != StatusCode::OK) {
+ return StatusError(status) << StringPrintf("failed to update sample rate for prop: %" PRId32
+ ", area"
+ ": %" PRId32 ", sample rate: %f",
+ propId, areaId, newRate);
+ }
+ mContSubConfigsByPropIdArea[propIdAreaId] = infoCopy;
+ return {};
+}
+
+VhalResult<void> SubscriptionManager::subscribe(const std::shared_ptr<IVehicleCallback>& callback,
+ const std::vector<SubscribeOptions>& options,
+ bool isContinuousProperty) {
std::scoped_lock<std::mutex> lockGuard(mLock);
std::vector<int64_t> intervals;
@@ -88,109 +154,108 @@
if (isContinuousProperty) {
auto intervalResult = getInterval(sampleRate);
if (!intervalResult.ok()) {
- return intervalResult.error();
+ return StatusError(StatusCode::INVALID_ARG) << intervalResult.error().message();
}
- intervals.push_back(intervalResult.value());
}
if (option.areaIds.empty()) {
ALOGE("area IDs to subscribe must not be empty");
- return Error() << "area IDs to subscribe must not be empty";
+ return StatusError(StatusCode::INVALID_ARG)
+ << "area IDs to subscribe must not be empty";
}
}
- size_t intervalIndex = 0;
ClientIdType clientId = callback->asBinder().get();
+
for (const auto& option : options) {
int32_t propId = option.propId;
const std::vector<int32_t>& areaIds = option.areaIds;
- int64_t interval = 0;
- if (isContinuousProperty) {
- interval = intervals[intervalIndex];
- intervalIndex++;
- }
for (int32_t areaId : areaIds) {
PropIdAreaId propIdAreaId = {
.propId = propId,
.areaId = areaId,
};
if (isContinuousProperty) {
- VehiclePropValue propValueRequest{
- .prop = propId,
- .areaId = areaId,
- };
- mSubscriptionsByClient[clientId][propIdAreaId] =
- std::make_unique<RecurrentSubscription>(
- mTimer,
- [this, callback, propValueRequest] {
- mGetValue(callback, propValueRequest);
- },
- interval);
- } else {
- mSubscriptionsByClient[clientId][propIdAreaId] =
- std::make_unique<OnChangeSubscription>();
+ if (auto result = updateSampleRateLocked(clientId, propIdAreaId, option.sampleRate);
+ !result.ok()) {
+ return result;
+ }
}
+
+ mSubscribedPropsByClient[clientId].insert(propIdAreaId);
mClientsByPropIdArea[propIdAreaId][clientId] = callback;
}
}
return {};
}
-Result<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId,
- const std::vector<int32_t>& propIds) {
+VhalResult<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId,
+ const std::vector<int32_t>& propIds) {
std::scoped_lock<std::mutex> lockGuard(mLock);
- if (mSubscriptionsByClient.find(clientId) == mSubscriptionsByClient.end()) {
- return Error() << "No property was subscribed for the callback";
+ if (mSubscribedPropsByClient.find(clientId) == mSubscribedPropsByClient.end()) {
+ return StatusError(StatusCode::INVALID_ARG)
+ << "No property was subscribed for the callback";
}
std::unordered_set<int32_t> subscribedPropIds;
- for (auto const& [propIdAreaId, _] : mSubscriptionsByClient[clientId]) {
+ for (auto const& propIdAreaId : mSubscribedPropsByClient[clientId]) {
subscribedPropIds.insert(propIdAreaId.propId);
}
for (int32_t propId : propIds) {
if (subscribedPropIds.find(propId) == subscribedPropIds.end()) {
- return Error() << "property ID: " << propId << " is not subscribed";
+ return StatusError(StatusCode::INVALID_ARG)
+ << "property ID: " << propId << " is not subscribed";
}
}
- auto& subscriptions = mSubscriptionsByClient[clientId];
- auto it = subscriptions.begin();
- while (it != subscriptions.end()) {
- int32_t propId = it->first.propId;
+ auto& propIdAreaIds = mSubscribedPropsByClient[clientId];
+ auto it = propIdAreaIds.begin();
+ while (it != propIdAreaIds.end()) {
+ int32_t propId = it->propId;
if (std::find(propIds.begin(), propIds.end(), propId) != propIds.end()) {
- auto& clients = mClientsByPropIdArea[it->first];
+ if (auto result = removeSampleRateLocked(clientId, *it); !result.ok()) {
+ return result;
+ }
+
+ auto& clients = mClientsByPropIdArea[*it];
clients.erase(clientId);
if (clients.empty()) {
- mClientsByPropIdArea.erase(it->first);
+ mClientsByPropIdArea.erase(*it);
+ mContSubConfigsByPropIdArea.erase(*it);
}
- it = subscriptions.erase(it);
+ it = propIdAreaIds.erase(it);
} else {
it++;
}
}
- if (subscriptions.empty()) {
- mSubscriptionsByClient.erase(clientId);
+ if (propIdAreaIds.empty()) {
+ mSubscribedPropsByClient.erase(clientId);
}
return {};
}
-Result<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId) {
+VhalResult<void> SubscriptionManager::unsubscribe(SubscriptionManager::ClientIdType clientId) {
std::scoped_lock<std::mutex> lockGuard(mLock);
- if (mSubscriptionsByClient.find(clientId) == mSubscriptionsByClient.end()) {
- return Error() << "No property was subscribed for this client";
+ if (mSubscribedPropsByClient.find(clientId) == mSubscribedPropsByClient.end()) {
+ return StatusError(StatusCode::INVALID_ARG) << "No property was subscribed for this client";
}
- auto& subscriptions = mSubscriptionsByClient[clientId];
- for (auto const& [propIdAreaId, _] : subscriptions) {
+ auto& subscriptions = mSubscribedPropsByClient[clientId];
+ for (auto const& propIdAreaId : subscriptions) {
+ if (auto result = removeSampleRateLocked(clientId, propIdAreaId); !result.ok()) {
+ return result;
+ }
+
auto& clients = mClientsByPropIdArea[propIdAreaId];
clients.erase(clientId);
if (clients.empty()) {
mClientsByPropIdArea.erase(propIdAreaId);
+ mContSubConfigsByPropIdArea.erase(propIdAreaId);
}
}
- mSubscriptionsByClient.erase(clientId);
+ mSubscribedPropsByClient.erase(clientId);
return {};
}
@@ -208,10 +273,8 @@
if (mClientsByPropIdArea.find(propIdAreaId) == mClientsByPropIdArea.end()) {
continue;
}
- for (const auto& [clientId, client] : mClientsByPropIdArea[propIdAreaId]) {
- if (!mSubscriptionsByClient[clientId][propIdAreaId]->isOnChange()) {
- continue;
- }
+
+ for (const auto& [_, client] : mClientsByPropIdArea[propIdAreaId]) {
clients[client].push_back(&value);
}
}
@@ -220,25 +283,7 @@
bool SubscriptionManager::isEmpty() {
std::scoped_lock<std::mutex> lockGuard(mLock);
- return mSubscriptionsByClient.empty() && mClientsByPropIdArea.empty();
-}
-
-SubscriptionManager::RecurrentSubscription::RecurrentSubscription(
- std::shared_ptr<RecurrentTimer> timer, std::function<void()>&& action, int64_t interval)
- : mAction(std::make_shared<std::function<void()>>(action)), mTimer(timer) {
- mTimer->registerTimerCallback(interval, mAction);
-}
-
-SubscriptionManager::RecurrentSubscription::~RecurrentSubscription() {
- mTimer->unregisterTimerCallback(mAction);
-}
-
-bool SubscriptionManager::RecurrentSubscription::isOnChange() {
- return false;
-}
-
-bool SubscriptionManager::OnChangeSubscription::isOnChange() {
- return true;
+ return mSubscribedPropsByClient.empty() && mClientsByPropIdArea.empty();
}
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
index 49f5b7e..f48b906 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/DefaultVehicleHalTest.cpp
@@ -1308,25 +1308,7 @@
TEST_F(DefaultVehicleHalTest, testSubscribeGlobalContinuous) {
VehiclePropValue testValue{
.prop = GLOBAL_CONTINUOUS_PROP,
- .value.int32Values = {0},
};
- // Set responses for all the hardware getValues requests.
- getHardware()->setGetValueResponder(
- [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
- const std::vector<GetValueRequest>& requests) {
- std::vector<GetValueResult> results;
- for (auto& request : requests) {
- VehiclePropValue prop = request.prop;
- prop.value.int32Values = {0};
- results.push_back({
- .requestId = request.requestId,
- .status = StatusCode::OK,
- .prop = prop,
- });
- }
- (*callback)(results);
- return StatusCode::OK;
- });
std::vector<SubscribeOptions> options = {
{
@@ -1353,28 +1335,6 @@
}
TEST_F(DefaultVehicleHalTest, testSubscribeGlobalContinuousRateOutOfRange) {
- VehiclePropValue testValue{
- .prop = GLOBAL_CONTINUOUS_PROP,
- .value.int32Values = {0},
- };
- // Set responses for all the hardware getValues requests.
- getHardware()->setGetValueResponder(
- [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
- const std::vector<GetValueRequest>& requests) {
- std::vector<GetValueResult> results;
- for (auto& request : requests) {
- VehiclePropValue prop = request.prop;
- prop.value.int32Values = {0};
- results.push_back({
- .requestId = request.requestId,
- .status = StatusCode::OK,
- .prop = prop,
- });
- }
- (*callback)(results);
- return StatusCode::OK;
- });
-
// The maxSampleRate is 100, so the sample rate should be the default max 100.
std::vector<SubscribeOptions> options = {
{
@@ -1398,24 +1358,6 @@
}
TEST_F(DefaultVehicleHalTest, testSubscribeAreaContinuous) {
- // Set responses for all the hardware getValues requests.
- getHardware()->setGetValueResponder(
- [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
- const std::vector<GetValueRequest>& requests) {
- std::vector<GetValueResult> results;
- for (auto& request : requests) {
- VehiclePropValue prop = request.prop;
- prop.value.int32Values = {0};
- results.push_back({
- .requestId = request.requestId,
- .status = StatusCode::OK,
- .prop = prop,
- });
- }
- (*callback)(results);
- return StatusCode::OK;
- });
-
std::vector<SubscribeOptions> options = {
{
.propId = AREA_CONTINUOUS_PROP,
@@ -1436,6 +1378,8 @@
// Sleep for 1s, which should generate ~20 events.
std::this_thread::sleep_for(std::chrono::seconds(1));
+ getClient()->unsubscribe(getCallbackClient(), std::vector<int32_t>({AREA_CONTINUOUS_PROP}));
+
std::vector<VehiclePropValue> events;
while (true) {
auto maybeResults = getCallback()->nextOnPropertyEventResults();
@@ -1509,28 +1453,6 @@
}
TEST_F(DefaultVehicleHalTest, testUnsubscribeContinuous) {
- VehiclePropValue testValue{
- .prop = GLOBAL_CONTINUOUS_PROP,
- .value.int32Values = {0},
- };
- // Set responses for all the hardware getValues requests.
- getHardware()->setGetValueResponder(
- [](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
- const std::vector<GetValueRequest>& requests) {
- std::vector<GetValueResult> results;
- for (auto& request : requests) {
- VehiclePropValue prop = request.prop;
- prop.value.int32Values = {0};
- results.push_back({
- .requestId = request.requestId,
- .status = StatusCode::OK,
- .prop = prop,
- });
- }
- (*callback)(results);
- return StatusCode::OK;
- });
-
std::vector<SubscribeOptions> options = {
{
.propId = GLOBAL_CONTINUOUS_PROP,
@@ -1621,12 +1543,6 @@
}
TEST_F(DefaultVehicleHalTest, testOnBinderDiedUnlinked) {
- // First subscribe to a continuous property so that we register a death recipient for our
- // client.
- VehiclePropValue testValue{
- .prop = GLOBAL_CONTINUOUS_PROP,
- .value.int32Values = {0},
- };
// Set responses for all the hardware getValues requests.
getHardware()->setGetValueResponder(
[](std::shared_ptr<const IVehicleHardware::GetValuesCallback> callback,
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
index 66aef7c..4df4e1a 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.cpp
@@ -32,9 +32,14 @@
using ::aidl::android::hardware::automotive::vehicle::VehiclePropConfig;
using ::aidl::android::hardware::automotive::vehicle::VehiclePropValue;
+MockVehicleHardware::MockVehicleHardware() {
+ mRecurrentTimer = std::make_unique<RecurrentTimer>();
+}
+
MockVehicleHardware::~MockVehicleHardware() {
std::unique_lock<std::mutex> lk(mLock);
mCv.wait(lk, [this] { return mThreadCount == 0; });
+ mRecurrentTimer.reset();
}
std::vector<VehiclePropConfig> MockVehicleHardware::getAllPropertyConfigs() const {
@@ -83,6 +88,42 @@
return StatusCode::OK;
}
+StatusCode MockVehicleHardware::updateSampleRate(int32_t propId, int32_t areaId, float sampleRate) {
+ std::shared_ptr<std::function<void()>> action;
+
+ {
+ std::scoped_lock<std::mutex> lockGuard(mLock);
+ if (mRecurrentActions[propId][areaId] != nullptr) {
+ // Remove the previous action register for this [propId, areaId].
+ mRecurrentTimer->unregisterTimerCallback(mRecurrentActions[propId][areaId]);
+ }
+ if (sampleRate == 0) {
+ return StatusCode::OK;
+ }
+
+ // We are sure 'propertyChangeCallback' would be alive because we would unregister timer
+ // before destroying 'this' which owns mPropertyChangeCallback.
+ const PropertyChangeCallback* propertyChangeCallback = mPropertyChangeCallback.get();
+ action = std::make_shared<std::function<void()>>([propertyChangeCallback, propId, areaId] {
+ std::vector<VehiclePropValue> values = {
+ {
+ .prop = propId,
+ .areaId = areaId,
+ },
+ };
+ (*propertyChangeCallback)(values);
+ });
+ // Store the action in a map so that we could remove the action later.
+ mRecurrentActions[propId][areaId] = action;
+ }
+
+ // In mock implementation, we generate a new property change event for this property at sample
+ // rate.
+ int64_t interval = static_cast<int64_t>(1'000'000'000. / sampleRate);
+ mRecurrentTimer->registerTimerCallback(interval, action);
+ return StatusCode::OK;
+}
+
void MockVehicleHardware::registerOnPropertyChangeEvent(
std::unique_ptr<const PropertyChangeCallback> callback) {
std::scoped_lock<std::mutex> lockGuard(mLock);
diff --git a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
index cb8b6a0..743841c 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
+++ b/automotive/vehicle/aidl/impl/vhal/test/MockVehicleHardware.h
@@ -18,6 +18,7 @@
#define android_hardware_automotive_vehicle_aidl_impl_vhal_test_MockVehicleHardware_H_
#include <IVehicleHardware.h>
+#include <RecurrentTimer.h>
#include <VehicleHalTypes.h>
#include <android-base/thread_annotations.h>
@@ -38,6 +39,8 @@
class MockVehicleHardware final : public IVehicleHardware {
public:
+ MockVehicleHardware();
+
~MockVehicleHardware();
std::vector<aidl::android::hardware::automotive::vehicle::VehiclePropConfig>
@@ -55,6 +58,8 @@
void registerOnPropertyChangeEvent(
std::unique_ptr<const PropertyChangeCallback> callback) override;
void registerOnPropertySetErrorEvent(std::unique_ptr<const PropertySetErrorCallback>) override;
+ aidl::android::hardware::automotive::vehicle::StatusCode updateSampleRate(
+ int32_t propId, int32_t areaId, float sampleRate) override;
// Test functions.
void setPropertyConfigs(
@@ -117,6 +122,11 @@
std::list<std::vector<ResultType>>* storedResponses) const REQUIRES(mLock);
DumpResult mDumpResult;
+
+ // RecurrentTimer is thread-safe.
+ std::shared_ptr<RecurrentTimer> mRecurrentTimer;
+ std::unordered_map<int32_t, std::unordered_map<int32_t, std::shared_ptr<std::function<void()>>>>
+ mRecurrentActions GUARDED_BY(mLock);
};
} // namespace vehicle
diff --git a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
index 2a468f6..3f59363 100644
--- a/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
+++ b/automotive/vehicle/aidl/impl/vhal/test/SubscriptionManagerTest.cpp
@@ -16,6 +16,7 @@
#include "SubscriptionManager.h"
+#include <MockVehicleHardware.h>
#include <VehicleHalTypes.h>
#include <aidl/android/hardware/automotive/vehicle/BnVehicleCallback.h>
@@ -86,19 +87,21 @@
class SubscriptionManagerTest : public testing::Test {
public:
void SetUp() override {
- mManager = std::make_unique<SubscriptionManager>(
- [](const std::shared_ptr<IVehicleCallback>& callback,
- const VehiclePropValue& value) {
- callback->onPropertyEvent(
- VehiclePropValues{
- .payloads = {value},
- },
- 0);
- });
+ mHardware = std::make_shared<MockVehicleHardware>();
+ mManager = std::make_unique<SubscriptionManager>(mHardware.get());
mCallback = ndk::SharedRefBase::make<PropertyCallback>();
// Keep the local binder alive.
mBinder = mCallback->asBinder();
mCallbackClient = IVehicleCallback::fromBinder(mBinder);
+ std::shared_ptr<IVehicleCallback> callbackClient = mCallbackClient;
+ mHardware->registerOnPropertyChangeEvent(
+ std::make_unique<IVehicleHardware::PropertyChangeCallback>(
+ [callbackClient](std::vector<VehiclePropValue> updatedValues) {
+ VehiclePropValues values = {
+ .payloads = std::move(updatedValues),
+ };
+ callbackClient->onPropertyEvent(values, 0);
+ }));
}
SubscriptionManager* getManager() { return mManager.get(); }
@@ -115,6 +118,7 @@
std::unique_ptr<SubscriptionManager> mManager;
std::shared_ptr<PropertyCallback> mCallback;
std::shared_ptr<IVehicleCallback> mCallbackClient;
+ std::shared_ptr<MockVehicleHardware> mHardware;
SpAIBinder mBinder;
};