From 29fd19f40dffae9baa1939831602d5d017c7cbb6 Mon Sep 17 00:00:00 2001
From: Gilles Boccon-Gibod <bok@bok.net>
Date: Fri, 12 Aug 2022 14:22:31 -0700
Subject: [PATCH] gbg/fix subscribe lambda (#23)

* don't use a lambda as a subscriber

* update tests to look at side effects instead of internals
---
 bumble/gatt_client.py | 14 +++++++--
 tests/gatt_test.py    | 67 +++++++++++++++++++++++++++----------------
 2 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/bumble/gatt_client.py b/bumble/gatt_client.py
index 65a3c84..94799a3 100644
--- a/bumble/gatt_client.py
+++ b/bumble/gatt_client.py
@@ -547,7 +547,9 @@ class Client:
         for subscriber_set in subscriber_sets:
             if subscriber is not None:
                 subscriber_set.add(subscriber)
-            subscriber_set.add(lambda value: characteristic.emit('update', self.connection, value))
+            # Add the characteristic as a subscriber, which will result in the characteristic
+            # emitting an 'update' event when a notification or indication is received
+            subscriber_set.add(characteristic)
 
         await self.write_value(cccd, struct.pack('<H', bits), with_response=True)
 
@@ -741,7 +743,10 @@ class Client:
         if not subscribers:
             logger.warning('!!! received notification with no subscriber')
         for subscriber in subscribers:
-            subscriber(notification.attribute_value)
+            if callable(subscriber):
+                subscriber(notification.attribute_value)
+            else:
+                subscriber.emit('update', notification.attribute_value)
 
     def on_att_handle_value_indication(self, indication):
         # Call all subscribers
@@ -749,7 +754,10 @@ class Client:
         if not subscribers:
             logger.warning('!!! received indication with no subscriber')
         for subscriber in subscribers:
-            subscriber(indication.attribute_value)
+            if callable(subscriber):
+                subscriber(indication.attribute_value)
+            else:
+                subscriber.emit('update', indication.attribute_value)
 
         # Confirm that we received the indication
         self.send_confirmation(ATT_Handle_Value_Confirmation())
diff --git a/tests/gatt_test.py b/tests/gatt_test.py
index 2c7ab9a..4bfcce8 100644
--- a/tests/gatt_test.py
+++ b/tests/gatt_test.py
@@ -419,10 +419,12 @@ async def test_subscribe_notify():
     assert(len(c) == 1)
     c3 = c[0]
 
+    c1._called = False
     c1._last_update = None
 
-    def on_c1_update(connection, value):
-        c1._last_update = (connection, value)
+    def on_c1_update(value):
+        c1._called = True
+        c1._last_update = value
 
     c1.on('update', on_c1_update)
     await peer.subscribe(c1)
@@ -434,58 +436,73 @@ async def test_subscribe_notify():
     assert(not characteristic1._last_subscription[2])
     await server.indicate_subscribers(characteristic1)
     await async_barrier()
-    assert(c1._last_update is None)
+    assert(not c1._called)
     await server.notify_subscribers(characteristic1)
     await async_barrier()
-    assert(c1._last_update is not None)
-    assert(c1._last_update[1] == characteristic1.value)
+    assert(c1._called)
+    assert(c1._last_update == characteristic1.value)
 
-    assert(peer.gatt_client.notification_subscribers[c1.handle])
+    c1._called = False
     await peer.unsubscribe(c1)
-    assert(c1.handle not in peer.gatt_client.notification_subscribers)
+    await server.notify_subscribers(characteristic1)
+    assert(not c1._called)
 
+    c2._called = False
     c2._last_update = None
 
     def on_c2_update(value):
-        c2._last_update = (connection, value)
+        c2._called = True
+        c2._last_update = value
 
     await peer.subscribe(c2, on_c2_update)
     await async_barrier()
     await server.notify_subscriber(characteristic2._last_subscription[0], characteristic2)
     await async_barrier()
-    assert(c2._last_update is None)
+    assert(not c2._called)
     await server.indicate_subscriber(characteristic2._last_subscription[0], characteristic2)
     await async_barrier()
-    assert(c2._last_update is not None)
-    assert(c2._last_update[1] == characteristic2.value)
+    assert(c2._called)
+    assert(c2._last_update == characteristic2.value)
 
-    assert(on_c2_update in peer.gatt_client.indication_subscribers[c2.handle])
+    c2._called = False
     await peer.unsubscribe(c2, on_c2_update)
-    assert(on_c2_update not in peer.gatt_client.indication_subscribers[c2.handle])
+    await server.indicate_subscriber(characteristic2._last_subscription[0], characteristic2)
+    await async_barrier()
+    assert(not c2._called)
 
-    c3._last_update = None
+    def on_c3_update(value):
+        c3._called = True
+        c3._last_update = value
 
-    def on_c3_update(connection, value):
-        c3._last_update = (connection, value)
+    def on_c3_update_2(value):
+        c3._called_2 = True
+        c3._last_update_2 = value
 
     c3.on('update', on_c3_update)
-    await peer.subscribe(c3)
+    await peer.subscribe(c3, on_c3_update_2)
     await async_barrier()
     await server.notify_subscriber(characteristic3._last_subscription[0], characteristic3)
     await async_barrier()
-    assert(c3._last_update is not None)
-    assert(c3._last_update[1] == characteristic3.value)
+    assert(c3._called)
+    assert(c3._last_update == characteristic3.value)
+    assert(c3._called_2)
+    assert(c3._last_update_2 == characteristic3.value)
     characteristic3.value = bytes([1, 2, 3])
     await server.indicate_subscriber(characteristic3._last_subscription[0], characteristic3)
     await async_barrier()
-    assert(c3._last_update is not None)
-    assert(c3._last_update[1] == characteristic3.value)
+    assert(c3._called)
+    assert(c3._last_update == characteristic3.value)
+    assert(c3._called_2)
+    assert(c3._last_update_2 == characteristic3.value)
 
-    assert(peer.gatt_client.notification_subscribers[c3.handle])
-    assert(peer.gatt_client.indication_subscribers[c3.handle])
+    c3._called = False
+    c3._called_2 = False
     await peer.unsubscribe(c3)
-    assert(c3.handle not in peer.gatt_client.notification_subscribers)
-    assert(c3.handle not in peer.gatt_client.indication_subscribers)
+    await server.notify_subscriber(characteristic3._last_subscription[0], characteristic3)
+    await server.indicate_subscriber(characteristic3._last_subscription[0], characteristic3)
+    await async_barrier()
+    assert(not c3._called)
+    assert(not c3._called_2)
 
 
 # -----------------------------------------------------------------------------