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 fd59802923..8e9089d600 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 @@ struct HalClientValues { std::list values; }; +using ClientId = uint64_t; + class SubscriptionManager { public: using OnPropertyUnsubscribed = std::function; @@ -100,7 +102,8 @@ public: * Updates subscription. Returns the vector of properties subscription that * needs to be updated in VehicleHAL. */ - StatusCode addOrUpdateSubscription(const sp& callback, + StatusCode addOrUpdateSubscription(ClientId clientId, + const sp& callback, const hidl_vec& optionList, std::list* outUpdatedOptions); @@ -119,7 +122,7 @@ public: * If there are no clients subscribed to given properties than callback function provided * in the constructor will be called. */ - void unsubscribe(const sp& callback, int32_t propId); + void unsubscribe(ClientId clientId, int32_t propId); private: std::list> getSubscribedClientsLocked(int32_t propId, int32_t area, @@ -131,7 +134,8 @@ private: sp getClientsForPropertyLocked(int32_t propId) const; - sp getOrCreateHalClientLocked(const sp &callback); + sp getOrCreateHalClientLocked(ClientId callingPid, + const sp& callback); void onCallbackDead(uint64_t cookie); @@ -160,7 +164,7 @@ private: mutable std::mutex mLock; - std::map, sp> mClients; + std::map> mClients; std::map> mPropToClients; std::map 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 1d45f4b6e0..c1e9e88609 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 @@ private: static bool isSampleRateFixed(VehiclePropertyChangeMode mode); static float checkSampleRate(const VehiclePropConfig& config, float sampleRate); + static ClientId getClientId(const sp& callback); private: VehicleHal* mHal; std::unique_ptr mConfigIndex; diff --git a/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp b/automotive/vehicle/2.0/default/common/src/SubscriptionManager.cpp index e0f3f31a3c..74f0a5f55e 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 @@ std::vector HalClient::getSubscribedProperties() const { } StatusCode SubscriptionManager::addOrUpdateSubscription( + ClientId clientId, const sp &callback, const hidl_vec &optionList, std::list* outUpdatedSubscriptions) { @@ -106,7 +107,7 @@ StatusCode SubscriptionManager::addOrUpdateSubscription( ALOGI("SubscriptionManager::addOrUpdateSubscription, callback: %p", callback.get()); - const sp& client = getOrCreateHalClientLocked(callback); + const sp& client = getOrCreateHalClientLocked(clientId, callback); if (client.get() == nullptr) { return StatusCode::INTERNAL_ERROR; } @@ -221,10 +222,11 @@ sp SubscriptionManager::getClientsForPropertyLocked( } sp SubscriptionManager::getOrCreateHalClientLocked( - const sp& callback) { - auto it = mClients.find(callback); + ClientId clientId, const sp& callback) { + auto it = mClients.find(clientId); + if (it == mClients.end()) { - uint64_t cookie = reinterpret_cast(callback.get()); + uint64_t cookie = reinterpret_cast(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 SubscriptionManager::getOrCreateHalClientLocked( } sp client = new HalClient(callback); - mClients.emplace(callback, client); + mClients.insert({clientId, client}); return client; } else { return it->second; } } -void SubscriptionManager::unsubscribe(const sp& 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::unsubscribe(const sp& callback, void SubscriptionManager::onCallbackDead(uint64_t cookie) { ALOGI("%s, cookie: 0x%" PRIx64, __func__, cookie); - IVehicleCallback* callback = reinterpret_cast(cookie); + ClientId clientId = cookie; std::vector 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 @@ void SubscriptionManager::onCallbackDead(uint64_t cookie) { } 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 f452be8978..ae543bb3fc 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 #include +#include #include "VehicleUtils.h" @@ -154,7 +155,8 @@ Return VehicleHalManager::subscribe(const sp &call } std::list 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 VehicleHalManager::subscribe(const sp &call Return VehicleHalManager::unsubscribe(const sp& callback, int32_t propId) { - mSubscriptionManager.unsubscribe(callback, propId); + mSubscriptionManager.unsubscribe(getClientId(callback), propId); return StatusCode::OK; } @@ -341,6 +343,18 @@ void VehicleHalManager::onAllClientsUnsubscribed(int32_t propertyId) { mHal->unsubscribe(propertyId); } +ClientId VehicleHalManager::getClientId(const sp& 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(callback.get()); + return static_cast(reinterpret_cast(hwCallback->onAsBinder())); + } else { + return static_cast(reinterpret_cast(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 7ec9b798a3..5688dd6da1 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 @@ private: TEST_F(SubscriptionManagerTest, multipleClients) { std::list 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, multipleClients) { TEST_F(SubscriptionManagerTest, negativeCases) { std::list 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, negativeCases) { TEST_F(SubscriptionManagerTest, mulipleSubscriptions) { std::list 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 @@ TEST_F(SubscriptionManagerTest, mulipleSubscriptions) { // 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, mulipleSubscriptions) { TEST_F(SubscriptionManagerTest, unsubscribe) { std::list 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 @@ TEST_F(SubscriptionManagerTest, unsubscribe) { 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); }