Merge "Add VTS test case to cover multiple RANs per request" into tm-dev
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;
};
diff --git a/radio/aidl/vts/radio_config_test.cpp b/radio/aidl/vts/radio_config_test.cpp
index 5e1c811..81d87d2 100644
--- a/radio/aidl/vts/radio_config_test.cpp
+++ b/radio/aidl/vts/radio_config_test.cpp
@@ -59,6 +59,7 @@
serial = GetRandomSerialNumber();
ndk::ScopedAStatus res = radio_config->getHalDeviceCapabilities(serial);
ASSERT_OK(res);
+ EXPECT_EQ(std::cv_status::no_timeout, wait());
ALOGI("getHalDeviceCapabilities, rspInfo.error = %s\n",
toString(radioRsp_config->rspInfo.error).c_str());
}
@@ -70,6 +71,7 @@
serial = GetRandomSerialNumber();
ndk::ScopedAStatus res = radio_config->getSimSlotsStatus(serial);
ASSERT_OK(res);
+ EXPECT_EQ(std::cv_status::no_timeout, wait());
ALOGI("getSimSlotsStatus, rspInfo.error = %s\n",
toString(radioRsp_config->rspInfo.error).c_str());
}
@@ -166,31 +168,60 @@
* Test IRadioConfig.setSimSlotsMapping() for the response returned.
*/
TEST_P(RadioConfigTest, setSimSlotsMapping) {
- serial = GetRandomSerialNumber();
- SlotPortMapping slotPortMapping;
- slotPortMapping.physicalSlotId = 0;
- slotPortMapping.portId = 0;
- std::vector<SlotPortMapping> slotPortMappingList = {slotPortMapping};
- if (isDsDsEnabled()) {
- slotPortMapping.physicalSlotId = 1;
- slotPortMappingList.push_back(slotPortMapping);
- } else if (isTsTsEnabled()) {
- slotPortMapping.physicalSlotId = 1;
- slotPortMappingList.push_back(slotPortMapping);
- slotPortMapping.physicalSlotId = 2;
- slotPortMappingList.push_back(slotPortMapping);
- }
- ndk::ScopedAStatus res = radio_config->setSimSlotsMapping(serial, slotPortMappingList);
- ASSERT_OK(res);
- EXPECT_EQ(std::cv_status::no_timeout, wait());
- EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_config->rspInfo.type);
- EXPECT_EQ(serial, radioRsp_config->rspInfo.serial);
- ALOGI("setSimSlotsMapping, rspInfo.error = %s\n",
- toString(radioRsp_config->rspInfo.error).c_str());
- ASSERT_TRUE(CheckAnyOfErrors(radioRsp_config->rspInfo.error, {RadioError::NONE}));
+ // get slot status and set SIM slots mapping based on the result.
+ updateSimSlotStatus();
+ if (radioRsp_config->rspInfo.error == RadioError::NONE) {
+ SlotPortMapping slotPortMapping;
+ // put invalid value at first and adjust by slotStatusResponse.
+ slotPortMapping.physicalSlotId = -1;
+ slotPortMapping.portId = -1;
+ std::vector<SlotPortMapping> slotPortMappingList = {slotPortMapping};
+ if (isDsDsEnabled()) {
+ slotPortMappingList.push_back(slotPortMapping);
+ } else if (isTsTsEnabled()) {
+ slotPortMappingList.push_back(slotPortMapping);
+ slotPortMappingList.push_back(slotPortMapping);
+ }
+ for (size_t i = 0; i < radioRsp_config->simSlotStatus.size(); i++) {
+ ASSERT_TRUE(radioRsp_config->simSlotStatus[i].portInfo.size() > 0);
+ for (size_t j = 0; j < radioRsp_config->simSlotStatus[i].portInfo.size(); j++) {
+ if (radioRsp_config->simSlotStatus[i].portInfo[j].portActive) {
+ int32_t logicalSlotId =
+ radioRsp_config->simSlotStatus[i].portInfo[j].logicalSlotId;
+ // logicalSlotId should be 0 or positive numbers if the port
+ // is active.
+ EXPECT_GE(logicalSlotId, 0);
+ // logicalSlotId should be less than the maximum number of
+ // supported SIM slots.
+ EXPECT_LT(logicalSlotId, slotPortMappingList.size());
+ if (logicalSlotId >= 0 && logicalSlotId < slotPortMappingList.size()) {
+ slotPortMappingList[logicalSlotId].physicalSlotId = i;
+ slotPortMappingList[logicalSlotId].portId = j;
+ }
+ }
+ }
+ }
- // Give some time for modem to fully switch SIM configuration
- sleep(MODEM_SET_SIM_SLOT_MAPPING_DELAY_IN_SECONDS);
+ // set SIM slots mapping
+ for (size_t i = 0; i < slotPortMappingList.size(); i++) {
+ // physicalSlotId and portId should be 0 or positive numbers for the
+ // input of setSimSlotsMapping.
+ EXPECT_GE(slotPortMappingList[i].physicalSlotId, 0);
+ EXPECT_GE(slotPortMappingList[i].portId, 0);
+ }
+ serial = GetRandomSerialNumber();
+ ndk::ScopedAStatus res = radio_config->setSimSlotsMapping(serial, slotPortMappingList);
+ ASSERT_OK(res);
+ EXPECT_EQ(std::cv_status::no_timeout, wait());
+ EXPECT_EQ(RadioResponseType::SOLICITED, radioRsp_config->rspInfo.type);
+ EXPECT_EQ(serial, radioRsp_config->rspInfo.serial);
+ ALOGI("setSimSlotsMapping, rspInfo.error = %s\n",
+ toString(radioRsp_config->rspInfo.error).c_str());
+ ASSERT_TRUE(CheckAnyOfErrors(radioRsp_config->rspInfo.error, {RadioError::NONE}));
+
+ // Give some time for modem to fully switch SIM configuration
+ sleep(MODEM_SET_SIM_SLOT_MAPPING_DELAY_IN_SECONDS);
+ }
}
/*