From 5be8c17ea1da15cb9e882f4b19a1c3a58ebbe1f9 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Thu, 27 Sep 2018 10:36:00 -0700 Subject: [PATCH] Mares bluetooth support tweaks So it turns out that we really shouldn't send the command and arguments as one single packet in all situations, because at least the Mares Matrix really seems to want that "wait for ACK before sending arguments". See commit 59bfb0f3189b ("Add support for the Mares Matrix") for details. Also, because bluetooth is slow (and it looks like it's particularly slow for us with the Qt bluetooth code for some reason), we don't want to ask for big packets of data, because it seems to cause a buffer overflow on the BlueLink Pro when the serial data from the dive computer arrives faster than the bluetooth data can be sent to the downloading side. So when using the BLE transport, limit the packet size to 128 bytes and disable the command splitting. With this, I can download hundreds of kB of data from the Mares Quad Air successfully over BLE. It's *slow*, but it works. Signed-off-by: Linus Torvalds --- src/mares_iconhd.c | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/mares_iconhd.c b/src/mares_iconhd.c index daeeef7..6708ca6 100644 --- a/src/mares_iconhd.c +++ b/src/mares_iconhd.c @@ -72,6 +72,7 @@ typedef struct mares_iconhd_device_t { unsigned char version[140]; unsigned int model; unsigned int packetsize; + unsigned int splitcommand; } mares_iconhd_device_t; static dc_status_t mares_iconhd_device_set_fingerprint (dc_device_t *abstract, const unsigned char data[], unsigned int size); @@ -152,14 +153,17 @@ mares_iconhd_transfer (mares_iconhd_device_t *device, { dc_status_t status = DC_STATUS_SUCCESS; dc_device_t *abstract = (dc_device_t *) device; + unsigned int split_csize; assert (csize >= 2); if (device_is_cancelled (abstract)) return DC_STATUS_CANCELLED; + split_csize = device->splitcommand ? 2 : csize; + // Send the command header to the dive computer. - status = dc_iostream_write (device->iostream, command, csize, NULL); + status = dc_iostream_write (device->iostream, command, split_csize, NULL); if (status != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to send the command."); return status; @@ -179,6 +183,15 @@ mares_iconhd_transfer (mares_iconhd_device_t *device, return DC_STATUS_PROTOCOL; } + // Send any remaining command payload to the dive computer. + if (csize > split_csize) { + status = dc_iostream_write (device->iostream, command + split_csize, csize - split_csize, NULL); + if (status != DC_STATUS_SUCCESS) { + ERROR (abstract->context, "Failed to send the command."); + return status; + } + } + // Read the packet. status = dc_iostream_read (device->iostream, answer, asize, NULL); if (status != DC_STATUS_SUCCESS) { @@ -228,6 +241,15 @@ mares_iconhd_device_open (dc_device_t **out, dc_context_t *context, dc_iostream_ device->model = 0; device->packetsize = 0; + /* + * At least the Mares Matrix needs the command to be split into + * base and argument, with a wait for the ACK byte in between. + * + * See commit 59bfb0f3189b ("Add support for the Mares Matrix") + * for details. + */ + device->splitcommand = 1; + // Set the serial communication protocol (115200 8E1). status = dc_iostream_configure (device->iostream, 115200, 8, DC_PARITY_EVEN, DC_STOPBITS_ONE, DC_FLOWCONTROL_NONE); if (status != DC_STATUS_SUCCESS) { @@ -301,6 +323,26 @@ mares_iconhd_device_open (dc_device_t **out, dc_context_t *context, dc_iostream_ break; } + if (dc_iostream_get_transport(device->iostream) == DC_TRANSPORT_BLE) { + /* + * Don't ask for larger amounts of data with the BLE + * transport - it will fail. I suspect there is a buffer + * overflow in the BlueLink Pro dongle when bluetooth is + * slower than the serial protocol that the dongle talks to + * the dive computer. + */ + if (device->packetsize > 128) + device->packetsize = 128; + /* + * With BLE, don't wait for ACK before sending the arguments + * to a command. + * + * There is some timing issue that makes that take too long + * and causes the command to be aborted. + */ + device->splitcommand = 0; + } + *out = (dc_device_t *) device; return DC_STATUS_SUCCESS;