Multiple uuids

[dakhnod]

This branch adds the capability to handle multiple identical UUIDs within one service.
The first UUID is published unchanged, all following branches get "_1", "_2" etc. as suffix.

[dakhnod]

@shmuelzon In most cases we are using ble_device_characteristic_find_by_uuid to find out charcteristic, while I think we should be passing around the handle instead of Service UUID, characteristic UUID and index and use ble_device_characteristic_find_by_handle. That could eliminate the need to passing around the index. What do you think of that?

[dakhnod]

Also, I changed the Datatype for the AIO Digital characteristic. While it is true that each pin only occupies 2 bits, every additional pin occupies the next two bits. Four pins occupy the whole byte which we need to be able to write. Five pins occupy two bytes, so it's hard to predict how many bytes will be occupied.

[shmuelzon]

In most cases we are using ble_device_characteristic_find_by_uuid to find out charcteristic, while I think we should be passing around the handle instead of Service UUID, characteristic UUID and index and use ble_device_characteristic_find_by_handle. That could eliminate the need to passing around the index. What do you think of that?

IMHO, the handle is an internal BLE/GATT implementation detail, it's not something that someone less familiar with GATT will know about. The "real" information that's of interest is the service and characteristic IDs/names and that's why I chose to use those in the BLE wrapper APIs. While adding an index will make the API a bit more complex, I'd still prefer to keep it that way.

[shmuelzon]

Also, I changed the Datatype for the AIO Digital characteristic. While it is true that each pin only occupies 2 bits, every additional pin occupies the next two bits. Four pins occupy the whole byte which we need to be able to write. Five pins occupy two bytes, so it's hard to predict how many bytes will be occupied.

I understand. To be honest, the current values were scraped from the BLE SIG definitions so I never gave them much thought. In any case, can you please add this as a separate PR? It doesn't really belong as a part of supporting repeating UUIDs

[shmuelzon]

I've looked over you current code and I think it would be simpler to add an index field to ble_characteristic_t. Then, in ble_update_cache(), you can add something like (untested in any way):

diff --git a/main/ble.c b/main/ble.c
index b8e3de9..7ee2bc6 100644
--- a/main/ble.c
+++ b/main/ble.c
@@ -391,7 +391,7 @@ static void ble_update_cache(ble_device_t *dev)
     ble_service_t *service = NULL;
     ble_characteristic_t *characteristic = NULL;
     ble_uuid_t service_uuid, characteristic_uuid;
-    uint16_t count, i;
+    uint16_t count, i, index = 0;

     if (!dev)
         return;
@@ -423,8 +423,16 @@ static void ble_update_cache(ble_device_t *dev)
         else if (db[i].type == ESP_GATT_DB_CHARACTERISTIC)
         {
             esp_uuid_to_bt_uuid(db[i].uuid, characteristic_uuid);
+            if (characteristic &&
+                ble_uuid_equal(characteristic->uuid, characteristic_uuid))
+            {
+                index++;
+            }
+            else
+                index = 0;
             characteristic = ble_device_characteristic_add(service,
-                characteristic_uuid, db[i].attribute_handle, db[i].properties);
+                characteristic_uuid, index, db[i].attribute_handle,
+                db[i].properties);
         }
         else if (db[i].type == ESP_GATT_DB_DESCRIPTOR &&
             db[i].uuid.len == ESP_UUID_LEN_16 &&

With that, ble_foreach_characteristic() can just return the index that's part of ble_characteristic_t, ble_device_characteristic_find_by_uuid() will just compare the index along with the UUID and ble_device_info_get_by_conn_id_handle() will also be a lot simpler since it won't need to calculate the index again after finding the handle.

WDYT?

[dakhnod]

In most cases we are using ble_device_characteristic_find_by_uuid to find out charcteristic, while I think we should be passing around the handle instead of Service UUID, characteristic UUID and index and use ble_device_characteristic_find_by_handle. That could eliminate the need to passing around the index. What do you think of that?

IMHO, the handle is an internal BLE/GATT implementation detail, it's not something that someone less familiar with GATT will know about. The "real" information that's of interest is the service and characteristic IDs/names and that's why I chose to use those in the BLE wrapper APIs. While adding an index will make the API a bit more complex, I'd still prefer to keep it that way.

Nope, the handle is on GAP level, it is Bluetooth spec.
I do understand you argument though.

[shmuelzon]

Nope, the handle is on GAP level, it is Bluetooth spec. I do understand you argument though.

See? An implementation detail that I don't need to understand :)
Are you sure, though? The handle is basically the row index in the GATT table, no?

[dakhnod]

ble_foreach_characteristic

What parts of my code would that simplify? I am just trying to grasp your idea

[shmuelzon]

What parts of my code would that simplify? I am just trying to grasp your idea

You've already changed those functions so I hope you can agree they are now simpler.

[shmuelzon]

Hey, is this PR still a draft or is it ready?

[dakhnod]

@shmuelzon its ready

[shmuelzon]

Merged, thanks!