esp32-ble2mqtt icon indicating copy to clipboard operation
esp32-ble2mqtt copied to clipboard

Using binary payloads for characteristics

Open DurandA opened this issue 4 years ago • 5 comments

Hi @shmuelzon!

Do you have any plan to support binary payloads (e.g. for complex variable length characteristics)?

It seems that this feature was planned with CHAR_TYPE_VARIABLE but never completed. I thought that maybe CHAR_TYPE_UTF8S would do the job but since it is handled as a C null-terminated string this will fail as soon as the payload contains 0x00.

DurandA avatar Nov 19 '19 10:11 DurandA

Hey,

What do you mean by binary payloads? Send them as binary over MQTT? The variable characteristic type was taken from the GATT definitions, like the rest of the types. Currently, all unhandled types are converted to byte arrays, in string form, i.e. a payload of: [0x01, 0x02, 0x03] would be published as "1,2,3". Whatever application listens on the bus can do whatever it wants with this data and parse it however it feels fit. Though it's not based on anything real, I've always felt MQTT payload should be human-readable. That being said, we could always add a CHAR_TYPE_RAW which would keep the data as is and send it on the bus.

shmuelzon avatar Nov 20 '19 06:11 shmuelzon

Thanks for your feedback.

What do you mean by binary payloads? Send them as binary over MQTT?

Exactly

That being said, we could always add a CHAR_TYPE_RAW which would keep the data as is and send it on the bus.

That would be awesome! In my experience, this is easier to keep it in binary for some payloads when integrated with other services (e.g. Node-RED).

DurandA avatar Nov 21 '19 08:11 DurandA

So, I was looking at the code to add the CHAR_TYPE_RAW but then noticed that UTF8 is supposed to do exactly what you want. As far as BLE is concerned it's not null-terminated. When converting the BLE payload to an MQTT message, I use the received data length to determine when the string is complete. MQTT payloads also aren't null-terminated so I should also use the payload length and simply copy it as-is as the BLE payload. Anyways, I think the current implementation of writing a UTF8 value to BLE would probably fail if anyone tries to use it so I rewrote it and it should now also work as raw data as well.

If you have a device you can use this on, would you be willing to test the following patch and try to use utf8 for raw data? I'm worried there might be an off-by-one error in there...

diff --git a/main/ble_utils.c b/main/ble_utils.c
index 321cf8c..98fa712 100644
--- a/main/ble_utils.c
+++ b/main/ble_utils.c
@@ -593,11 +593,11 @@ uint8_t *atochar(ble_uuid_t uuid, const char *data, size_t len, size_t *ret_len)
         /* String values consume the rest of the buffer */
         case CHAR_TYPE_UTF8S:
         {
-            size_t len = strlen(val);
+            size_t l = len - (val - str);
 
-            strcpy((char *)p, val);
+            memcpy(p, val, l);
 
-            p += len + 1;
+            p += l;
             break;
         }
         /* IEEE-754 floating point format */

Thanks!

shmuelzon avatar Nov 22 '19 14:11 shmuelzon

Thanks for your time. I gave your patch a try. It does not have an off-by-one error but unfortunately this is not enough as there are further string operations in both atochar() and chartoa().

Even with your patch, the end of the payload is skipped in atochar() if there is a 0x00 somewhere in the payload. I think this is due to the use of strndup() or strtok(): https://github.com/shmuelzon/esp32-ble2mqtt/blob/156ce8b3f819c2bf008a2ad3a5371bb7996af31b/main/ble_utils.c#L458-L459 For chartoa() similar stuff happens as the payload is cut to the first 0x00. This is due to the the use of strlen(): https://github.com/shmuelzon/esp32-ble2mqtt/blob/156ce8b3f819c2bf008a2ad3a5371bb7996af31b/main/ble2mqtt.c#L424-L425

DurandA avatar Nov 25 '19 13:11 DurandA

Hey,

Yes, you're right. I guess I was speeding through this one. The thing is, I might need to refactor both chartoa() and atochar() in order to drop the use of strtok() and maybe look for the commas manually. I can already think of other use-cases where the use of strtok() might be a problem even with strings. Until I have the time to do so, the below (untested) hack might get you running. It adds the 'raw' type and it has to be the first type in the characteristic's definition, but it should get you started. In the future, I still plan to use UTF8 also as a raw value...

diff --git a/get_gatt_assigned_numbers.py b/get_gatt_assigned_numbers.py
index 79d2388..599629b 100644
--- a/get_gatt_assigned_numbers.py
+++ b/get_gatt_assigned_numbers.py
@@ -97,6 +97,7 @@ def write_h(filename):
       '\n' \
       'typedef enum {\n' \
       '    CHAR_TYPE_UNKNOWN,\n'
+      '    CHAR_TYPE_RAW,\n'
       '    %s\n'
       '} characteristic_type_t;\n' \
       '\n' \
diff --git a/main/ble2mqtt.c b/main/ble2mqtt.c
index 8cf3592..0f693c3 100644
--- a/main/ble2mqtt.c
+++ b/main/ble2mqtt.c
@@ -421,10 +421,10 @@ static void ble_on_device_characteristic_value(mac_addr_t mac,
     size_t value_len)
 {
     char *topic = ble_topic(mac, service, characteristic);
-    char *payload = chartoa(characteristic, value, value_len);
-    size_t payload_len = strlen(payload);
+    size_t payload_len;
+    char *payload = chartoa(characteristic, value, value_len, &payload_len);
 
-    ESP_LOGI(TAG, "Publishing: %s = %s", topic, payload);
+    ESP_LOGI(TAG, "Publishing: %s = %.*s", topic, payload_len, payload);
     mqtt_publish(topic, (uint8_t *)payload, payload_len, config_mqtt_qos_get(),
         config_mqtt_retained_get());
 }
diff --git a/main/ble_utils.c b/main/ble_utils.c
index 321cf8c..8b4c5ab 100644
--- a/main/ble_utils.c
+++ b/main/ble_utils.c
@@ -195,6 +195,7 @@ static characteristic_type_t ble_atotype(const char *type)
         { "reg-cert-data-list", CHAR_TYPE_REG_CERT_DATA_LIST },
         { "variable", CHAR_TYPE_VARIABLE },
         { "gatt-uuid", CHAR_TYPE_GATT_UUID },
+        { "raw", CHAR_TYPE_RAW },
         { NULL, CHAR_TYPE_UNKNOWN },
     };
 
@@ -263,6 +264,7 @@ static size_t ble_type_size(characteristic_type_t type)
     case CHAR_TYPE_REG_CERT_DATA_LIST:
     case CHAR_TYPE_VARIABLE:
     case CHAR_TYPE_GATT_UUID:
+    case CHAR_TYPE_RAW:
     case CHAR_TYPE_UNKNOWN:
         return 0;
     }
@@ -270,13 +272,19 @@ static size_t ble_type_size(characteristic_type_t type)
     return 0;
 }
 
-char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len)
+char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len, size_t *ret_len)
 {
     characteristic_type_t *types = ble_get_characteristic_types(uuid);
     static char buf[1024];
     char *p = buf;
     int i = 0;
 
+    if (types && types[0] == CHAR_TYPE_RAW)
+    {
+        *ret_len = len;
+        return (char *)data;
+    }
+
     /* A note from the Bluetooth specification:
      * If a format is not a whole number of octets, then the data shall be
      * contained within the least significant bits of the value, and all other
@@ -432,6 +440,9 @@ char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len)
             p += sprintf(p, "%f,", mantissa * pow(10.0f, exponent));
             break;
         }
+        case CHAR_TYPE_RAW:
+            /* Here for completeness, handled above */
+            break;
         case CHAR_TYPE_UINT128:
         case CHAR_TYPE_REG_CERT_DATA_LIST:
         case CHAR_TYPE_VARIABLE:
@@ -447,6 +458,7 @@ char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len)
         p += sprintf(p, "%u,", data[i]);
 
     *(p - 1) = '\0';
+    *ret_len = p - buf - 1;
     return buf;
 }
 
@@ -455,8 +467,16 @@ uint8_t *atochar(ble_uuid_t uuid, const char *data, size_t len, size_t *ret_len)
     characteristic_type_t *types = ble_get_characteristic_types(uuid);
     static uint8_t buf[512];
     uint8_t *p = buf;
-    char *str = strndup(data, len);
-    char *val = strtok(str, ",");
+    char *str, *val;
+
+    if (types && types[0] == CHAR_TYPE_RAW)
+    {
+        *ret_len = len;
+        return (uint8_t *)data;
+    }
+
+    str = strndup(data, len);
+    val = strtok(str, ",");
 
     for (; types && *types != -1 && val; types++)
     {
@@ -615,6 +635,9 @@ uint8_t *atochar(ble_uuid_t uuid, const char *data, size_t len, size_t *ret_len)
             p += 8;
             break;
         }
+        case CHAR_TYPE_RAW:
+            /* Here for completeness, handled above */
+            break;
         case CHAR_TYPE_UINT128:
         /* IEEE-11073 floating point format */
         case CHAR_TYPE_SFLOAT:
diff --git a/main/ble_utils.h b/main/ble_utils.h
index 6c6c181..e68fde8 100644
--- a/main/ble_utils.h
+++ b/main/ble_utils.h
@@ -55,7 +55,8 @@ char *mactoa(mac_addr_t mac);
 int atomac(const char *str, mac_addr_t mac);
 char *uuidtoa(ble_uuid_t uuid);
 int atouuid(const char *str, ble_uuid_t uuid);
-char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len);
+char *chartoa(ble_uuid_t uuid, const uint8_t *data, size_t len,
+    size_t *ret_len);
 uint8_t *atochar(ble_uuid_t uuid, const char *data, size_t len,
     size_t *ret_len);
 
diff --git a/main/gatt.h b/main/gatt.h
index e879fac..502a886 100644
--- a/main/gatt.h
+++ b/main/gatt.h
@@ -5,6 +5,7 @@
 
 typedef enum {
     CHAR_TYPE_UNKNOWN,
+    CHAR_TYPE_RAW,
     CHAR_TYPE_16BIT,
     CHAR_TYPE_24BIT,
     CHAR_TYPE_2BIT,

shmuelzon avatar Nov 26 '19 07:11 shmuelzon