Mesh: meshctl configuration output fails in gatt.c:pipe_write
RFC v2 1/1 Mesh: meshctl configuration output fails in gatt.c:pipe_write
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.
mesh/gatt.c +19 -125
@@ -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;