If the first command output in a new connection exceeds 20 bytes,
mesh_gatt_write sets the SAR to FIRST as the write_mtu is initially 0
and the default is GATT_MTU-3 (20).
When pipe_write gets called, a new larger write_mtu has been set, but
the SAR is still set to FIRST. It's assumed that data->gatt_len >
max_len. However, it's not which causes lots of bogus output.
---
Here's an attempt to fix it following Luiz' comments.
1. The WriteValue code has been removed
2. The SAR logic has been moved to pipe_write
It was tested against the zephyr mesh_shell and a small mtu to test the
segmentation logic.
diff --git a/mesh/gatt.c b/mesh/gatt.c
index 001eb17a8..29273ba60 100644
--- a/mesh/gatt.c
+++ b/mesh/gatt.c
@@ -104,27 +104,6 @@ static void write_data_free(void *user_data)
free(data);
}
-static void write_setup(DBusMessageIter *iter, void *user_data)
-{
- struct write_data *data = user_data;
- struct iovec *iov = &data->iov;
- DBusMessageIter array, dict;
-
- dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, "y", &array);
- dbus_message_iter_append_fixed_array(&array, DBUS_TYPE_BYTE,
- &iov->iov_base, iov->iov_len);
- dbus_message_iter_close_container(iter, &array);
-
- dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY,
- DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING
- DBUS_TYPE_STRING_AS_STRING
- DBUS_TYPE_VARIANT_AS_STRING
- DBUS_DICT_ENTRY_END_CHAR_AS_STRING,
- &dict);
-
- dbus_message_iter_close_container(iter, &dict);
-}
-
uint16_t mesh_gatt_sar(uint8_t **pkt, uint16_t size)
{
const uint8_t *data = *pkt;
@@ -194,18 +173,26 @@ static bool pipe_write(struct io *io, void *user_data)
struct write_data *data = user_data;
struct iovec iov[2];
uint8_t sar;
- uint8_t max_len = write_mtu - 4;
+ uint8_t max_len;
if (data == NULL)
return true;
- print_byte_array("GATT-TX:\t", data->gatt_data, data->gatt_len);
+ max_len = write_mtu ? write_mtu - 3 - 1 : GATT_MTU - 3 - 1;
- sar = data->gatt_data[0];
+ print_byte_array("GATT-TX:\t", data->gatt_data, data->gatt_len);
data->iov.iov_base = data->gatt_data + 1;
data->iov.iov_len--;
+ sar = data->gatt_data[0] & GATT_TYPE_MASK;
+ data->gatt_len--;
+
+ if (data->gatt_len > max_len) {
+ sar |= GATT_SAR_FIRST;
+ data->iov.iov_len = max_len;
+ }
+
iov[0].iov_base = &sar;
iov[0].iov_len = sizeof(sar);
@@ -247,73 +234,6 @@ static bool pipe_write(struct io *io, void *user_data)
}
}
-static void write_reply(DBusMessage *message, void *user_data)
-{
- struct write_data *data = user_data;
- struct write_data *tmp;
- uint8_t *dptr = data->gatt_data;
- uint8_t max_len = GATT_MTU - 3;
- uint8_t max_seg = GATT_MTU - 4;
- DBusError error;
-
- dbus_error_init(&error);
-
- if (dbus_set_error_from_message(&error, message) == TRUE) {
- rl_printf("Failed to write: %s\n", error.name);
- dbus_error_free(&error);
- return;
- }
-
- if (data == NULL)
- return;
-
- switch (data->gatt_data[0] & GATT_SAR_MASK) {
- case GATT_SAR_FIRST:
- case GATT_SAR_CONTINUE:
- tmp = g_new0(struct write_data, 1);
- if (!data)
- return;
-
- *tmp = *data;
- tmp->gatt_data = g_malloc(data->gatt_len);
-
- if (!tmp->gatt_data) {
- g_free(tmp);
- return;
- }
-
- tmp->gatt_data[0] = dptr[0];
- data = tmp;
- memcpy(data->gatt_data + 1, dptr + max_len,
- data->gatt_len - max_seg);
- data->gatt_len -= max_seg;
- data->gatt_data[0] &= GATT_TYPE_MASK;
- data->iov.iov_base = data->gatt_data;
- if (max_len < data->gatt_len) {
- data->iov.iov_len = max_len;
- data->gatt_data[0] |= GATT_SAR_CONTINUE;
- } else {
- data->iov.iov_len = data->gatt_len;
- data->gatt_data[0] |= GATT_SAR_LAST;
- }
-
- break;
-
- default:
- if(data->cb)
- data->cb(message, data->user_data);
- return;
- }
-
- if (g_dbus_proxy_method_call(data->proxy, "WriteValue", write_setup,
- write_reply, data, write_data_free) == FALSE) {
- rl_printf("Failed to write\n");
- write_data_free(data);
- return;
- }
-
-}
-
static void write_io_destroy(void)
{
io_destroy(write_io);
@@ -363,12 +283,8 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
if (dbus_set_error_from_message(&error, message) == TRUE) {
dbus_error_free(&error);
- if (g_dbus_proxy_method_call(data->proxy, "WriteValue",
- write_setup, write_reply, data,
- write_data_free) == FALSE) {
- rl_printf("Failed to write\n");
- write_data_free(data);
- }
+ rl_printf("Failed to write\n");
+ write_data_free(data);
return;
}
@@ -404,8 +320,6 @@ bool mesh_gatt_write(GDBusProxy *proxy, uint8_t *buf, uint16_t len,
GDBusReturnFunction cb, void *user_data)
{
struct write_data *data;
- DBusMessageIter iter;
- uint8_t max_len;
if (!buf || !len)
return false;
@@ -417,17 +331,10 @@ bool mesh_gatt_write(GDBusProxy *proxy, uint8_t *buf, uint16_t len,
if (!data)
return false;
- max_len = write_mtu ? write_mtu - 3 : GATT_MTU - 3;
-
/* TODO: should keep in queue in case we need to cancel write? */
data->gatt_len = len;
data->gatt_data = g_memdup(buf, len);
- data->gatt_data[0] &= GATT_TYPE_MASK;
- if (max_len < len) {
- len = max_len;
- data->gatt_data[0] |= GATT_SAR_FIRST;
- }
data->iov.iov_base = data->gatt_data;
data->iov.iov_len = len;
data->proxy = proxy;
@@ -437,25 +344,12 @@ bool mesh_gatt_write(GDBusProxy *proxy, uint8_t *buf, uint16_t len,
if (write_io)
return pipe_write(write_io, data);
- if (g_dbus_proxy_get_property(proxy, "WriteAcquired", &iter)) {
- if (g_dbus_proxy_method_call(proxy, "AcquireWrite",
- acquire_setup, acquire_write_reply,
- data, NULL) == FALSE) {
- rl_printf("Failed to AcquireWrite\n");
- write_data_free(data);
- return false;
- }
- } else {
- if (g_dbus_proxy_method_call(data->proxy, "WriteValue",
- write_setup, write_reply, data,
- write_data_free) == FALSE) {
- rl_printf("Failed to write\n");
- write_data_free(data);
- return false;
- }
- print_byte_array("GATT-TX: ", buf, len);
- rl_printf("Attempting to write %s\n",
- g_dbus_proxy_get_path(proxy));
+ if (g_dbus_proxy_method_call(proxy, "AcquireWrite",
+ acquire_setup, acquire_write_reply,
+ data, NULL) == FALSE) {
+ rl_printf("Failed to AcquireWrite\n");
+ write_data_free(data);
+ return false;
}
return true;