From 31c68ba3386f4f3c2d35f68faf2589464ec68211 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Tue, 13 Jun 2017 16:31:16 +0200 Subject: [PATCH 1/5] Automatically re-transmit corrupt data packets Originally packets are only retried when a valid NAK packet with the busy error code is received. The retrying is now enabled for other types of errors also, such as data packets with checksum errors. --- src/divesystem_idive.c | 124 ++++++++++++++++++++++++----------------- 1 file changed, 73 insertions(+), 51 deletions(-) diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c index e2a5cda..3c53b8e 100644 --- a/src/divesystem_idive.c +++ b/src/divesystem_idive.c @@ -291,72 +291,94 @@ divesystem_idive_receive (divesystem_idive_device_t *device, unsigned char answe static dc_status_t -divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned char command[], unsigned int csize, unsigned char answer[], unsigned int asize) +divesystem_idive_packet (divesystem_idive_device_t *device, const unsigned char command[], unsigned int csize, unsigned char answer[], unsigned int asize, unsigned int *errorcode) { - dc_status_t rc = DC_STATUS_SUCCESS; + dc_status_t status = DC_STATUS_SUCCESS; dc_device_t *abstract = (dc_device_t *) device; unsigned char packet[MAXPACKET] = {0}; - unsigned int length = 0; - unsigned int nretries = 0; + unsigned int length = sizeof(packet); + unsigned int errcode = 0; - while (1) { - // Send the command. - rc = divesystem_idive_send (device, command, csize); - if (rc != DC_STATUS_SUCCESS) - return rc; + // Send the command. + status = divesystem_idive_send (device, command, csize); + if (status != DC_STATUS_SUCCESS) { + goto error; + } - // Receive the answer. - length = sizeof(packet); - rc = divesystem_idive_receive (device, packet, &length); - if (rc != DC_STATUS_SUCCESS) - return rc; + // Receive the answer. + status = divesystem_idive_receive (device, packet, &length); + if (status != DC_STATUS_SUCCESS) { + goto error; + } - // Verify the command byte. - if (packet[0] != command[0]) { - ERROR (abstract->context, "Unexpected packet header."); - return DC_STATUS_PROTOCOL; - } + // Verify the command byte. + if (packet[0] != command[0]) { + ERROR (abstract->context, "Unexpected packet header."); + status = DC_STATUS_PROTOCOL; + goto error; + } - // Check the ACK byte. - if (packet[length - 1] == ACK) - break; - - // Verify the NAK byte. - if (packet[length - 1] != NAK) { - ERROR (abstract->context, "Unexpected ACK/NAK byte."); - return DC_STATUS_PROTOCOL; - } - - // Verify the length of the packet. - if (length != 3) { - ERROR (abstract->context, "Unexpected packet length."); - return DC_STATUS_PROTOCOL; - } - - // Verify the error code. - unsigned int errcode = packet[1]; - if (errcode != BUSY) { - ERROR (abstract->context, "Received NAK packet with error code %02x.", errcode); - return DC_STATUS_PROTOCOL; - } - - // Abort if the maximum number of retries is reached. - if (nretries++ >= MAXRETRIES) - return DC_STATUS_PROTOCOL; - - // Delay the next attempt. - dc_serial_sleep(device->port, 100); + // Verify the ACK/NAK byte. + unsigned int type = packet[length - 1]; + if (type != ACK && type != NAK) { + ERROR (abstract->context, "Unexpected ACK/NAK byte."); + status = DC_STATUS_PROTOCOL; + goto error; } // Verify the length of the packet. - if (asize != length - 2) { + unsigned int expected = (type == ACK ? asize : 1) + 2; + if (length != expected) { ERROR (abstract->context, "Unexpected packet length."); - return DC_STATUS_PROTOCOL; + status = DC_STATUS_PROTOCOL; + goto error; + } + + // Get the error code from a NAK packet. + if (type == NAK) { + errcode = packet[1]; + ERROR (abstract->context, "Received NAK packet with error code %02x.", errcode); + status = DC_STATUS_PROTOCOL; + goto error; } memcpy(answer, packet + 1, length - 2); - return DC_STATUS_SUCCESS; +error: + if (errorcode) { + *errorcode = errcode; + } + + return status; +} + + +static dc_status_t +divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned char command[], unsigned int csize, unsigned char answer[], unsigned int asize) +{ + dc_status_t status = DC_STATUS_SUCCESS; + unsigned int errcode = 0; + + unsigned int nretries = 0; + while ((status = divesystem_idive_packet (device, command, csize, answer, asize, &errcode)) != DC_STATUS_SUCCESS) { + // Automatically discard a corrupted packet, + // and request a new one. + if (status != DC_STATUS_PROTOCOL && status != DC_STATUS_TIMEOUT) + break; + + // Abort if the device reports a fatal error. + if (errcode && errcode != BUSY) + break; + + // Abort if the maximum number of retries is reached. + if (nretries++ >= MAXRETRIES) + break; + + // Delay the next attempt. + dc_serial_sleep (device->port, 100); + } + + return status; } static dc_status_t From 35d1e6ff14308e85304a6952d1d50b35a14c027d Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sat, 3 Jun 2017 09:56:02 +0200 Subject: [PATCH 2/5] Propagate the error code to the caller --- src/divesystem_idive.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c index 3c53b8e..485c6d8 100644 --- a/src/divesystem_idive.c +++ b/src/divesystem_idive.c @@ -354,7 +354,7 @@ error: static dc_status_t -divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned char command[], unsigned int csize, unsigned char answer[], unsigned int asize) +divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned char command[], unsigned int csize, unsigned char answer[], unsigned int asize, unsigned int *errorcode) { dc_status_t status = DC_STATUS_SUCCESS; unsigned int errcode = 0; @@ -378,6 +378,10 @@ divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned cha dc_serial_sleep (device->port, 100); } + if (errorcode) { + *errorcode = errcode; + } + return status; } @@ -387,6 +391,7 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb dc_status_t rc = DC_STATUS_SUCCESS; divesystem_idive_device_t *device = (divesystem_idive_device_t *) abstract; unsigned char packet[MAXPACKET - 2]; + unsigned int errcode = 0; const divesystem_idive_commands_t *commands = &idive; if (device->model >= IX3M_EASY) { @@ -398,7 +403,7 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); unsigned char cmd_id[] = {commands->id.cmd, 0xED}; - rc = divesystem_idive_transfer (device, cmd_id, sizeof(cmd_id), packet, commands->id.size); + rc = divesystem_idive_transfer (device, cmd_id, sizeof(cmd_id), packet, commands->id.size, &errcode); if (rc != DC_STATUS_SUCCESS) return rc; @@ -424,7 +429,7 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb } unsigned char cmd_range[] = {commands->range.cmd, 0x8D}; - rc = divesystem_idive_transfer (device, cmd_range, sizeof(cmd_range), packet, commands->range.size); + rc = divesystem_idive_transfer (device, cmd_range, sizeof(cmd_range), packet, commands->range.size, &errcode); if (rc != DC_STATUS_SUCCESS) return rc; @@ -453,7 +458,7 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb unsigned char cmd_header[] = {commands->header.cmd, (number ) & 0xFF, (number >> 8) & 0xFF}; - rc = divesystem_idive_transfer (device, cmd_header, sizeof(cmd_header), packet, commands->header.size); + rc = divesystem_idive_transfer (device, cmd_header, sizeof(cmd_header), packet, commands->header.size, &errcode); if (rc != DC_STATUS_SUCCESS) return rc; @@ -475,7 +480,7 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb unsigned char cmd_sample[] = {commands->sample.cmd, (idx ) & 0xFF, (idx >> 8) & 0xFF}; - rc = divesystem_idive_transfer (device, cmd_sample, sizeof(cmd_sample), packet, commands->sample.size * commands->nsamples); + rc = divesystem_idive_transfer (device, cmd_sample, sizeof(cmd_sample), packet, commands->sample.size * commands->nsamples, &errcode); if (rc != DC_STATUS_SUCCESS) return rc; From 5178c0f755f9606a7c472a58d5172c89bb748d6a Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sat, 3 Jun 2017 09:59:17 +0200 Subject: [PATCH 3/5] Add extra NAK error codes --- src/divesystem_idive.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c index 485c6d8..1da536d 100644 --- a/src/divesystem_idive.c +++ b/src/divesystem_idive.c @@ -42,7 +42,14 @@ #define START 0x55 #define ACK 0x06 #define NAK 0x15 -#define BUSY 0x60 + +#define ERR_INVALID_CMD 0x10 +#define ERR_INVALID_LENGTH 0x20 +#define ERR_INVALID_DATA 0x30 +#define ERR_UNSUPPORTED 0x40 +#define ERR_UNAVAILABLE 0x58 +#define ERR_UNREADABLE 0x5F +#define ERR_BUSY 0x60 #define NSTEPS 1000 #define STEP(i,n) (NSTEPS * (i) / (n)) @@ -367,7 +374,7 @@ divesystem_idive_transfer (divesystem_idive_device_t *device, const unsigned cha break; // Abort if the device reports a fatal error. - if (errcode && errcode != BUSY) + if (errcode && errcode != ERR_BUSY) break; // Abort if the maximum number of retries is reached. From 3b967f9eb1296193ef154b663e59e9970bccc5ed Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sat, 3 Jun 2017 10:01:45 +0200 Subject: [PATCH 4/5] Improve the handling of devices without any dives Don't return an error if the dive computer reports there are no dives available! --- src/divesystem_idive.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c index 1da536d..f0c31dc 100644 --- a/src/divesystem_idive.c +++ b/src/divesystem_idive.c @@ -437,8 +437,13 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb unsigned char cmd_range[] = {commands->range.cmd, 0x8D}; rc = divesystem_idive_transfer (device, cmd_range, sizeof(cmd_range), packet, commands->range.size, &errcode); - if (rc != DC_STATUS_SUCCESS) - return rc; + if (rc != DC_STATUS_SUCCESS) { + if (errcode == ERR_UNAVAILABLE) { + return DC_STATUS_SUCCESS; // No dives found. + } else { + return rc; + } + } // Get the range of the available dive numbers. unsigned int first = array_uint16_le (packet + 0); From 24621ed5196c1b74206a41929c12be21c5ec7964 Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sat, 3 Jun 2017 10:07:14 +0200 Subject: [PATCH 5/5] Improve the handling of unreadable dives Dives that are reported by the dive computer as unreadable (for example due to a power loss during the dive) are now skipped instead of being reported as a fatal error. Those dives can't be retrieved, so there is no good reason to abort the download. --- src/divesystem_idive.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/divesystem_idive.c b/src/divesystem_idive.c index f0c31dc..d26e2b4 100644 --- a/src/divesystem_idive.c +++ b/src/divesystem_idive.c @@ -471,8 +471,14 @@ divesystem_idive_device_foreach (dc_device_t *abstract, dc_dive_callback_t callb (number ) & 0xFF, (number >> 8) & 0xFF}; rc = divesystem_idive_transfer (device, cmd_header, sizeof(cmd_header), packet, commands->header.size, &errcode); - if (rc != DC_STATUS_SUCCESS) - return rc; + if (rc != DC_STATUS_SUCCESS) { + if (errcode == ERR_UNREADABLE) { + WARNING(abstract->context, "Skipped unreadable dive!"); + continue; + } else { + return rc; + } + } if (memcmp(packet + 7, device->fingerprint, sizeof(device->fingerprint)) == 0) break;