Merge "Use IBinder to identify callbacks in VHAL" into oc-dev
diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
index fd59802..8e9089d 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/SubscriptionManager.h
@@ -78,6 +78,8 @@
std::list<VehiclePropValue *> values;
};
+using ClientId = uint64_t;
+
class SubscriptionManager {
public:
using OnPropertyUnsubscribed = std::function<void(int32_t)>;
@@ -100,7 +102,8 @@
* Updates subscription. Returns the vector of properties subscription that
* needs to be updated in VehicleHAL.
*/
- StatusCode addOrUpdateSubscription(const sp<IVehicleCallback>& callback,
+ StatusCode addOrUpdateSubscription(ClientId clientId,
+ const sp<IVehicleCallback>& callback,
const hidl_vec<SubscribeOptions>& optionList,
std::list<SubscribeOptions>* outUpdatedOptions);
@@ -119,7 +122,7 @@
* If there are no clients subscribed to given properties than callback function provided
* in the constructor will be called.
*/
- void unsubscribe(const sp<IVehicleCallback>& callback, int32_t propId);
+ void unsubscribe(ClientId clientId, int32_t propId);
private:
std::list<sp<HalClient>> getSubscribedClientsLocked(int32_t propId,
int32_t area,
@@ -131,7 +134,8 @@
sp<HalClientVector> getClientsForPropertyLocked(int32_t propId) const;
- sp<HalClient> getOrCreateHalClientLocked(const sp<IVehicleCallback> &callback);
+ sp<HalClient> getOrCreateHalClientLocked(ClientId callingPid,
+ const sp<IVehicleCallback>& callback);
void onCallbackDead(uint64_t cookie);
@@ -160,7 +164,7 @@
mutable std::mutex mLock;
- std::map<sp<IVehicleCallback>, sp<HalClient>> mClients;
+ std::map<ClientId, sp<HalClient>> mClients;
std::map<int32_t, sp<HalClientVector>> mPropToClients;
std::map<int32_t, SubscribeOptions> mHalEventSubscribeOptions;
diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
index 1d45f4b..c1e9e88 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/VehicleHalManager.h
@@ -101,6 +101,7 @@
static bool isSampleRateFixed(VehiclePropertyChangeMode mode);
static float checkSampleRate(const VehiclePropConfig& config,
float sampleRate);
+ static ClientId getClientId(const sp<IVehicleCallback>& callback);
private:
VehicleHal* mHal;
std::unique_ptr<VehiclePropConfigIndex> mConfigIndex;
diff --git a/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp b/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
index e0f3f31..74f0a5f 100644
--- a/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
+++ b/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp
@@ -97,6 +97,7 @@
}
StatusCode SubscriptionManager::addOrUpdateSubscription(
+ ClientId clientId,
const sp<IVehicleCallback> &callback,
const hidl_vec<SubscribeOptions> &optionList,
std::list<SubscribeOptions>* outUpdatedSubscriptions) {
@@ -106,7 +107,7 @@
ALOGI("SubscriptionManager::addOrUpdateSubscription, callback: %p", callback.get());
- const sp<HalClient>& client = getOrCreateHalClientLocked(callback);
+ const sp<HalClient>& client = getOrCreateHalClientLocked(clientId, callback);
if (client.get() == nullptr) {
return StatusCode::INTERNAL_ERROR;
}
@@ -221,10 +222,11 @@
}
sp<HalClient> SubscriptionManager::getOrCreateHalClientLocked(
- const sp<IVehicleCallback>& callback) {
- auto it = mClients.find(callback);
+ ClientId clientId, const sp<IVehicleCallback>& callback) {
+ auto it = mClients.find(clientId);
+
if (it == mClients.end()) {
- uint64_t cookie = reinterpret_cast<uint64_t>(callback.get());
+ uint64_t cookie = reinterpret_cast<uint64_t>(clientId);
ALOGI("Creating new client and linking to death recipient, cookie: 0x%" PRIx64, cookie);
auto res = callback->linkToDeath(mCallbackDeathRecipient, cookie);
if (!res.isOk()) { // Client is already dead?
@@ -234,18 +236,18 @@
}
sp<HalClient> client = new HalClient(callback);
- mClients.emplace(callback, client);
+ mClients.insert({clientId, client});
return client;
} else {
return it->second;
}
}
-void SubscriptionManager::unsubscribe(const sp<IVehicleCallback>& callback,
+void SubscriptionManager::unsubscribe(ClientId clientId,
int32_t propId) {
MuxGuard g(mLock);
auto propertyClients = getClientsForPropertyLocked(propId);
- auto clientIter = mClients.find(callback);
+ auto clientIter = mClients.find(clientId);
if (clientIter == mClients.end()) {
ALOGW("Unable to unsubscribe: no callback found, propId: 0x%x", propId);
} else {
@@ -285,12 +287,12 @@
void SubscriptionManager::onCallbackDead(uint64_t cookie) {
ALOGI("%s, cookie: 0x%" PRIx64, __func__, cookie);
- IVehicleCallback* callback = reinterpret_cast<IVehicleCallback*>(cookie);
+ ClientId clientId = cookie;
std::vector<int32_t> props;
{
MuxGuard g(mLock);
- const auto& it = mClients.find(callback);
+ const auto& it = mClients.find(clientId);
if (it == mClients.end()) {
return; // Nothing to do here, client wasn't subscribed to any properties.
}
@@ -299,7 +301,7 @@
}
for (int32_t propId : props) {
- unsubscribe(callback, propId);
+ unsubscribe(clientId, propId);
}
}
diff --git a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
index f452be8..ae543bb 100644
--- a/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
+++ b/automotive/vehicle/2.0/default/common/src/VehicleHalManager.cpp
@@ -22,6 +22,7 @@
#include <fstream>
#include <android/log.h>
+#include <android/hardware/automotive/vehicle/2.0/BpHwVehicleCallback.h>
#include "VehicleUtils.h"
@@ -154,7 +155,8 @@
}
std::list<SubscribeOptions> updatedOptions;
- auto res = mSubscriptionManager.addOrUpdateSubscription(callback, verifiedOptions,
+ auto res = mSubscriptionManager.addOrUpdateSubscription(getClientId(callback),
+ callback, verifiedOptions,
&updatedOptions);
if (StatusCode::OK != res) {
ALOGW("%s failed to subscribe, error code: %d", __func__, res);
@@ -170,7 +172,7 @@
Return<StatusCode> VehicleHalManager::unsubscribe(const sp<IVehicleCallback>& callback,
int32_t propId) {
- mSubscriptionManager.unsubscribe(callback, propId);
+ mSubscriptionManager.unsubscribe(getClientId(callback), propId);
return StatusCode::OK;
}
@@ -341,6 +343,18 @@
mHal->unsubscribe(propertyId);
}
+ClientId VehicleHalManager::getClientId(const sp<IVehicleCallback>& callback) {
+ //TODO(b/32172906): rework this to get some kind of unique id for callback interface when this
+ // feature is ready in HIDL.
+
+ if (callback->isRemote()) {
+ BpHwVehicleCallback* hwCallback = static_cast<BpHwVehicleCallback*>(callback.get());
+ return static_cast<ClientId>(reinterpret_cast<intptr_t>(hwCallback->onAsBinder()));
+ } else {
+ return static_cast<ClientId>(reinterpret_cast<intptr_t>(callback.get()));
+ }
+}
+
} // namespace V2_0
} // namespace vehicle
} // namespace automotive
diff --git a/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp b/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
index 7ec9b79..5688dd6 100644
--- a/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
+++ b/automotive/vehicle/2.0/default/tests/SubscriptionManager_test.cpp
@@ -119,8 +119,10 @@
TEST_F(SubscriptionManagerTest, multipleClients) {
std::list<SubscribeOptions> updatedOptions;
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp1, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(2, cb2, subscrToProp1, &updatedOptions));
auto clients = manager.getSubscribedClients(
PROP1,
@@ -132,7 +134,8 @@
TEST_F(SubscriptionManagerTest, negativeCases) {
std::list<SubscribeOptions> updatedOptions;
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
// Wrong zone
auto clients = manager.getSubscribedClients(
@@ -158,7 +161,8 @@
TEST_F(SubscriptionManagerTest, mulipleSubscriptions) {
std::list<SubscribeOptions> updatedOptions;
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, subscrToProp1,
+ &updatedOptions));
auto clients = manager.getSubscribedClients(
PROP1,
@@ -169,7 +173,7 @@
// Same property, but different zone, to make sure we didn't unsubscribe
// from previous zone.
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, {
+ ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(1, cb1, {
SubscribeOptions {
.propId = PROP1,
.vehicleAreas = toInt(VehicleAreaZone::ROW_2),
@@ -190,15 +194,17 @@
TEST_F(SubscriptionManagerTest, unsubscribe) {
std::list<SubscribeOptions> updatedOptions;
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb1, subscrToProp1, &updatedOptions));
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb2, subscrToProp2, &updatedOptions));
- ASSERT_EQ(StatusCode::OK, manager.addOrUpdateSubscription(cb3, subscrToProp1and2,
- &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(1, cb1, subscrToProp1, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(2, cb2, subscrToProp2, &updatedOptions));
+ ASSERT_EQ(StatusCode::OK,
+ manager.addOrUpdateSubscription(3, cb3, subscrToProp1and2, &updatedOptions));
- ASSERT_ALL_EXISTS({cb1, cb3}, extractCallbacks(clientsToProp1()));
+ ASSERT_ALL_EXISTS({ cb1, cb3 }, extractCallbacks(clientsToProp1()));
ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
- manager.unsubscribe(cb1, PROP1);
+ manager.unsubscribe(1, PROP1);
assertOnPropertyUnsubscribedNotCalled();
ASSERT_ALL_EXISTS({cb3}, extractCallbacks(clientsToProp1()));
@@ -206,20 +212,20 @@
ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
// No one subscribed to PROP1, subscription for PROP2 is not affected.
- manager.unsubscribe(cb3, PROP1);
+ manager.unsubscribe(3, PROP1);
assertLastUnsubscribedProperty(PROP1);
ASSERT_ALL_EXISTS({cb2, cb3}, extractCallbacks(clientsToProp2()));
- manager.unsubscribe(cb3, PROP2);
+ manager.unsubscribe(3, PROP2);
assertOnPropertyUnsubscribedNotCalled();
ASSERT_ALL_EXISTS({cb2}, extractCallbacks(clientsToProp2()));
// The last client unsubscribed from this property.
- manager.unsubscribe(cb2, PROP2);
+ manager.unsubscribe(2, PROP2);
assertLastUnsubscribedProperty(PROP2);
// No one subscribed anymore
- manager.unsubscribe(cb1, PROP1);
+ manager.unsubscribe(1, PROP1);
assertLastUnsubscribedProperty(PROP1);
}