BLE fixes (#939)

* Ble update * Updated based on review * Updated based on review * Update lib/ble.toit Co-authored-by: Kasper Lund <[email protected]> * Update src/resources/ble_esp32.cc Co-authored-by: Kasper Lund <[email protected]> Co-authored-by: mikkel <[email protected]> Co-authored-by: mikkeldamsgaard <[email protected]>
toitlang · Aug 8, 2022 · 47472e8 · 47472e8
1 parent b99960c
commit 47472e8
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 22 deletions.
diff --git a/lib/ble.toit b/lib/ble.toit
@@ -59,6 +59,8 @@ BLE_CONNECT_MODE_NONE          ::= 0
 BLE_CONNECT_MODE_DIRECTIONAL   ::= 1
 BLE_CONNECT_MODE_UNDIRECTIONAL ::= 2
 
+BLE_DEFAULT_PREFERRED_MTU_ ::= 23
+
 /**
 Advertisement data as either sent by advertising or received through scanning.
 */
@@ -239,7 +241,8 @@ class RemoteCharacteristic:
   Writes the value of the characteristic on the remote service.
   */
   write_value value/ByteArray -> none:
-    ble_send_data_ service.client_.gatt_ handle value
+    ble_run_with_quota_backoff_:
+      ble_send_data_ service.client_.gatt_ handle value
 
 /**
 A client connected to a remote device.
@@ -316,8 +319,8 @@ class Service:
     characteristics_[uuid] = char
     return char
 
-  add_write_only_characteristic uuid -> WriteOnlyCharacteristic:
-    char := WriteOnlyCharacteristic this uuid
+  add_write_only_characteristic uuid --require_response/bool=true -> WriteOnlyCharacteristic:
+    char := WriteOnlyCharacteristic this uuid require_response
     characteristics_[uuid] = char
     return char
 
@@ -335,10 +338,11 @@ class Service:
     return characteristics_.get uuid
 
 
-BLE_CHR_TYPE_READ_ONLY_    ::= 1
-BLE_CHR_TYPE_WRITE_ONLY_   ::= 2
-BLE_CHR_TYPE_READ_WRITE_   ::= 3
-BLE_CHR_TYPE_NOTIFICATION_ ::= 4
+BLE_CHR_TYPE_READ_ONLY_         ::= 1
+BLE_CHR_TYPE_WRITE_ONLY_        ::= 2
+BLE_CHR_TYPE_READ_WRITE_        ::= 3
+BLE_CHR_TYPE_NOTIFICATION_      ::= 4
+BLE_CHR_TYPE_WRITE_ONLY_NO_RSP_ ::= 5
 
 BLE_WAIT_RECV_             ::= 1 << 0
 BLE_WAIT_ACCESSED_         ::= 1 << 1
@@ -358,6 +362,15 @@ abstract class Characteristic:
   constructor service/Service resource .uuid:
     state_ = ResourceState_ service.server_configuration_.resource_group_ resource
 
+  /**
+  The currently negotiated mtu of the characteristic. Only meaningful when a client is connected.
+  */
+  att_mtu -> int:
+    mtu := ble_get_mtu_ state_.resource
+    if mtu == 0: return BLE_DEFAULT_PREFERRED_MTU_
+    return mtu
+
+
 /**
 Base class of characteristics that clients can write to.
 */
@@ -366,9 +379,11 @@ abstract class WritableCharacteristic extends Characteristic:
     super service resource uuid
 
   value -> ByteArray?:
-    state_.wait_for_state BLE_WAIT_RECV_
-    data := ble_get_characteristics_value_ state_.resource
-    state_.clear_state BLE_WAIT_RECV_
+    data := null
+    while not data:
+      state_.wait_for_state BLE_WAIT_RECV_
+      data = ble_get_characteristics_value_ state_.resource
+      state_.clear_state BLE_WAIT_RECV_
     return data
 
 /**
@@ -378,7 +393,11 @@ class ReadOnlyCharacteristic extends Characteristic:
   value_/ByteArray := #[]
 
   constructor service/Service uuid/uuid_pkg.Uuid value/ByteArray:
-    resource := ble_add_server_characteristic_ service.resource_ uuid.to_byte_array BLE_CHR_TYPE_READ_ONLY_ value
+    resource := ble_add_server_characteristic_
+        service.resource_
+        uuid.to_byte_array
+        BLE_CHR_TYPE_READ_ONLY_
+        value
     super service resource uuid
     value_ = value
 
@@ -392,16 +411,24 @@ class ReadOnlyCharacteristic extends Characteristic:
 A characteristic that can only be written to by clients.
 */
 class WriteOnlyCharacteristic extends WritableCharacteristic:
-  constructor service/Service uuid/uuid_pkg.Uuid:
-    resource := ble_add_server_characteristic_ service.resource_ uuid.to_byte_array BLE_CHR_TYPE_WRITE_ONLY_ null
+  constructor service/Service uuid/uuid_pkg.Uuid require_response/bool:
+    resource := ble_add_server_characteristic_
+        service.resource_
+        uuid.to_byte_array
+        require_response ? BLE_CHR_TYPE_WRITE_ONLY_ : BLE_CHR_TYPE_WRITE_ONLY_NO_RSP_
+        null
     super service resource uuid
 
 /**
 A characteristic that allows both read and write by the client.
 */
 class ReadWriteCharacteristic extends WritableCharacteristic:
   constructor service/Service uuid/uuid_pkg.Uuid value/ByteArray:
-    resource := ble_add_server_characteristic_ service.resource_ uuid.to_byte_array BLE_CHR_TYPE_READ_WRITE_ value
+    resource := ble_add_server_characteristic_
+        service.resource_
+        uuid.to_byte_array
+        BLE_CHR_TYPE_READ_WRITE_
+        value
     super service resource uuid
 
   value= value/ByteArray -> none:
@@ -413,11 +440,16 @@ A characteristic that the client can subscribe to changes on.
 */
 class NotificationCharacteristic extends Characteristic:
   constructor service/Service uuid/uuid_pkg.Uuid:
-    resource := ble_add_server_characteristic_ service.resource_ uuid.to_byte_array BLE_CHR_TYPE_NOTIFICATION_ null
+    resource := ble_add_server_characteristic_
+        service.resource_
+        uuid.to_byte_array
+        BLE_CHR_TYPE_NOTIFICATION_
+        null
     super service resource uuid
 
   value= value/ByteArray -> none:
-    ble_notify_characteristics_value_ state_.resource value
+    ble_run_with_quota_backoff_:
+      ble_notify_characteristics_value_ state_.resource value
 
 /**
 The local BLE device.
@@ -428,17 +460,18 @@ If services is not empty, sets up the services for the advertiser.
 */
 class Device:
   resource_group_ := ?
-  resource_state_/monitor.ResourceState_? := null
+  resource_state_/ResourceState_? := null
 
-  constructor.default server_configuration/ServerConfiguration?=null:
+  constructor.default server_configuration/ServerConfiguration?=null --preferred_mtu/int=BLE_DEFAULT_PREFERRED_MTU_:
     server_configuration_resource_group := server_configuration != null
         ? server_configuration.resource_group_
         : null
     resource_group_ = ble_init_ server_configuration_resource_group
     add_finalizer this:: this.close
+    ble_set_preferred_mtu_ preferred_mtu
     try:
       gap := ble_gap_ resource_group_
-      resource_state := monitor.ResourceState_ resource_group_ gap
+      resource_state := ResourceState_ resource_group_ gap
       state := resource_state.wait_for_state STARTED_EVENT_
       resource_state_ = resource_state
 
@@ -527,6 +560,9 @@ CONNECTED_EVENT_            ::= 1 << 3
 CONNECT_FAILED_EVENT_       ::= 1 << 4
 DISCONNECTED_EVENT_         ::= 1 << 5
 
+ble_set_preferred_mtu_ mtu:
+  #primitive.ble.set_preferred_mtu
+
 ble_init_ config_resource_group:
   #primitive.ble.init
 
@@ -588,13 +624,32 @@ ble_add_server_service_ resource_group_ uuid:
   #primitive.ble.add_server_service
 
 ble_add_server_characteristic_ service_resource uuid type value:
+  return ble_run_with_quota_backoff_:
+    ble_add_server_characteristic_primitive_ service_resource uuid type value
+  unreachable
+
+ble_add_server_characteristic_primitive_ service_resource uuid type value:
   #primitive.ble.add_server_characteristic
 
-ble_set_characteristics_value_ gatt new_value:
+ble_set_characteristics_value_ gatt new_value -> none:
+  ble_run_with_quota_backoff_:
+    ble_set_characteristics_value_primitive_ gatt new_value
+
+ble_set_characteristics_value_primitive_ gatt new_value:
   #primitive.ble.set_characteristics_value
 
 ble_notify_characteristics_value_ gatt new_value:
   #primitive.ble.notify_characteristics_value
 
 ble_get_characteristics_value_ gatt:
   #primitive.ble.get_characteristics_value
+
+ble_get_mtu_ gatt:
+  #primitive.ble.get_att_mtu
+
+ble_run_with_quota_backoff_ [block]:
+  start := Time.monotonic_us
+  while true:
+    catch --unwind=(: it != "QUOTA_EXCEEDED"): return block.call
+    sleep --ms=10
+    if Time.monotonic_us - start > 2_000_000: throw DEADLINE_EXCEEDED_ERROR
diff --git a/src/primitive.h b/src/primitive.h
@@ -337,6 +337,8 @@ namespace toit {
   PRIMITIVE(set_characteristics_value, 2)    \
   PRIMITIVE(notify_characteristics_value, 2) \
   PRIMITIVE(get_characteristics_value, 1)    \
+  PRIMITIVE(get_att_mtu, 1)                  \
+  PRIMITIVE(set_preferred_mtu, 1)            \
 
 #define MODULE_DHCP(PRIMITIVE)               \
   PRIMITIVE(wait_for_lwip_dhcp_on_linux, 0)  \

diff --git a/src/resources/ble_esp32.cc b/src/resources/ble_esp32.cc
@@ -73,6 +73,7 @@ enum {
   kBLECharTypeWriteOnly = 2,
   kBLECharTypeReadWrite = 3,
   kBLECharTypeNotification = 4,
+  kBLECharTypeWriteOnlyNoRsp = 5,
 };
 
 const uint8 kBluetoothBaseUUID[16] = {
@@ -376,6 +377,9 @@ int BLEResourceGroup::init_server() {
           case kBLECharTypeWriteOnly:
             gatt_svr_chars[characteristic_idx].flags = BLE_GATT_CHR_F_WRITE;
             break;
+          case kBLECharTypeWriteOnlyNoRsp:
+            gatt_svr_chars[characteristic_idx].flags = BLE_GATT_CHR_F_WRITE_NO_RSP;
+            break;
           case kBLECharTypeReadWrite:
             gatt_svr_chars[characteristic_idx].flags = BLE_GATT_CHR_F_WRITE | BLE_GATT_CHR_F_READ;
             break;
@@ -477,7 +481,11 @@ static Object* object_to_mbuf(Process* process, Object* object, os_mbuf** result
     if (!object->byte_content(process->program(), &bytes, STRINGS_OR_BYTE_ARRAYS)) WRONG_TYPE;
     if (bytes.length() > 0) {
       os_mbuf* mbuf = ble_hs_mbuf_from_flat(bytes.address(), bytes.length());
-      if (!mbuf) MALLOC_FAILED;
+      // A null response is not an allocation error, as the mbufs are allocated on boot based on configuration settings.
+      // Therefore, a GC will do little to help the situation and will eventually result in the VM thinking it is out of memory.
+      // The mbuf will be freed eventually by the NimBLE stack. The client code will
+      // have to wait and then try again.
+      if (!mbuf) QUOTA_EXCEEDED;
       *result = mbuf;
     }
   }
@@ -1057,8 +1065,26 @@ PRIMITIVE(get_characteristics_value) {
   } else {
     ALLOCATION_FAILED;
   }
+}
+
+PRIMITIVE(set_preferred_mtu) {
+  ARGS(int, mtu);
+
+  int result = ble_att_set_preferred_mtu(mtu);
+
+  if (result) {
+    INVALID_ARGUMENT;
+  } else {
+    return process->program()->null_object();
+  }
+}
+
+PRIMITIVE(get_att_mtu) {
+  ARGS(BLEServerCharacteristicResource, resource);
+
+  uint16 mtu = ble_att_mtu(resource->conn_handle());
 
-  return ret_val;
+  return Smi::from(mtu);
 }
 
 } // namespace toit