devel

devel@libdivecomputer.org

3 participants
290 discussions

Re: Tusa IQ-750 Element II import weirdness
by Linus Torvalds 09 May '16

09 May '16

On Sun, May 8, 2016 at 6:23 AM, Dan Bowkley <danbowkley(a)gmail.com> wrote: > > I'm using a Tusa IQ-750 and for the most part importing my dives works > flawlessly, just plug it in and hit go. The only issue is it seems to have > N2 and O2 backwards: my computer is set to air, and when the dives are > imported into SS it warns me that I have a very weird Nitrox mix of 79% O2. > I'm betting a pair of variables got swapped somewhere. This sounds more like a libdivecomputer issue than a subsurface one. Adding Jef and the libdivecomputer mailing list to the participants, Linus

3 2

SAC Rate for Sidemount
by John Van Ostrand 05 Apr '16

05 Apr '16

Subsurface calculates SAC rate based on tanks that have been used on the dive and will ignore other tanks listed in equipment. Until I realized this I thought my consumption which I had worked hard to reduce on that dive had improved. This could have been dangerous had I not questioned it. As a sidemount diver I switch tanks regularly during a dive and don't record the run time and pressure when the switch happens. It's not typical to do so. I also don't have a gas pressure integrated dive computer. I can work around this either by declaring my tank to be 200 cu ft, by declaring a working pressure of 474 bar, or by inserting at least one tank switch. I understand that the current method is a clean solution but I'm not sure it's practical. A user could get into trouble if they trusted the SAC rate without knowing that the second tank wasn't included. Solutions might be: 1. List all tanks with start/end pressure on the Info page and list their individual SAC rates. Display a total (i.e. effective) SAC rate. This gives the user the most conservative SAC rate but also allows them to mentally remove a tank (like Argon) that isn't used for breathing. Show effective SAC on the graph. 2. List all tanks with pressures on the info page and note tanks that were not explicitly used. SAC rate on graph would remain as is. 3. Warn the user somehow that a tank in the equipment list appears to be used but isn't explicitly used. Thoughts? -- John Van Ostrand At large on sabbatical

2 3

[PATCH] Cochran: Fixed buffer overflow in _parser_foreach().
by John Van Ostrand 16 Mar '16

16 Mar '16

Break out of loop if less than a sample size remains. --- src/cochran_commander_parser.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index f3c3769..9271ac6 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -588,6 +588,10 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callb continue; } + // Make sure we have a full sample + if (offset > size - layout->samplesize) + break; + // Depth is logged as change in feet, bit 0x40 means negative depth if (s[0] & 0x40) depth_qfeet -= (s[0] & 0x3f); @@ -617,46 +621,42 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callb } // Cochran EMC models store NDL and deco stop time - // int the 20th to 23rd sample + // in the 20th to 23rd sample if (layout->format == SAMPLE_EMC) { // Find the next sample by skipping over any event bytes. // This is only temporary so we can get NDL and deco stop // times which span two samples. - const unsigned char *n = s + layout->samplesize; cochran_events_t event; - - while ((*n & 0x80) && n < samples + size) { - cochran_commander_get_event_info(*n, &event); - n += event.data_bytes; - } + static const unsigned char *last_sample = NULL; // Tissue load is recorded across 20 samples, we ignore them // NDL and deco stop time is recorded across the next 4 samples // The first 2 are either NDL or stop time at deepest stop (if in deco) // The next 2 are total deco stop time. switch (time % 24) { - case 20: + case 21: if (deco_obligation) { /* Deco time for deepest stop, unused */ - int deco_time = (s[2] + n[2] * 256 + 1) * 60; + int deco_time = (last_sample[2] + s[2] * 256 + 1) * 60; } else { /* Send deco NDL sample */ sample.deco.type = DC_DECO_NDL; - sample.deco.time = (s[2] + n[2] * 256 + 1) * 60; // seconds + sample.deco.time = (last_sample[2] + s[2] * 256 + 1) * 60; // seconds sample.deco.depth = 0; if (callback) callback (DC_SAMPLE_DECO, sample, userdata); } break; - case 22: + case 23: /* Deco time, total obligation */ if (deco_obligation) { sample.deco.type = DC_DECO_DECOSTOP; sample.deco.depth = deco_ceiling * FEET; - sample.deco.time = (s[2] + n[2] * 256 + 1) * 60; // minutes + sample.deco.time = (last_sample[2] + s[2] * 256 + 1) * 60; // minutes if (callback) callback (DC_SAMPLE_DECO, sample, userdata); } break; } + last_sample = s; } time++; -- 2.4.3

1 0

[PATCH 1/2] Cochran: Merged logic to get for valid profiles in _foreach().
by John Van Ostrand 08 Mar '16

08 Mar '16

Previously cochran_check_profiles() was called to establish which dive is the earliest one with a profile. Now logic in foreach() counts profile data to detect when dives no longer have valid profile data. --- src/cochran_commander.c | 146 +++++++++++++++--------------------------------- src/cochran_commander.h | 4 +- 2 files changed, 46 insertions(+), 104 deletions(-) diff --git a/src/cochran_commander.c b/src/cochran_commander.c index 127518d..47ecebb 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -67,7 +67,7 @@ const cochran_device_info_t cochran_cmdr_device_info = { const cochran_conf_offsets_t cochran_cmdr_conf_offsets = { 0x046, // dive_count 0x06e, // last_log - 0x071, // last_interdive + 0x200, // last_interdive 0x0aa, // serial_number }; @@ -202,8 +202,6 @@ cochran_read_id(dc_device_t *device, unsigned char *id, int size); static dc_status_t cochran_commander_read (dc_device_t *abstract, dc_event_progress_t *progress, unsigned int address, unsigned char data[], unsigned int size); -static void -cochran_check_profiles(dc_device_t *abstract, cochran_data_t *data); static dc_status_t @@ -599,13 +597,17 @@ cochran_read_config (dc_device_t *abstract, cochran_data_t *data, dc_event_progr dc_status_t rc; unsigned char command[2] = { 0x96, 0x00 }; - rc = cochran_packet(device, progress, command, sizeof(command), data->config, sizeof(data->config), 0); - if (rc != DC_STATUS_SUCCESS) - return rc; + // Read two 512 byte blocks into one 1024 byte buffer + for (int i = 0; i < 2; i++) { + command[1] = i; + rc = cochran_packet(device, progress, command, sizeof(command), data->config + i * 512, sizeof(data->config) / 2, 0); + if (rc != DC_STATUS_SUCCESS) + return rc; - vendor.data = data->config; - vendor.size = sizeof(data->config); - device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); + vendor.data = data->config + i * 512; + vendor.size = sizeof(data->config) / 2; + device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); + } return DC_STATUS_SUCCESS; } @@ -806,9 +808,6 @@ cochran_commander_device_read_all (dc_device_t *abstract, cochran_data_t *data) if (rc != DC_STATUS_SUCCESS) return rc; - // Determine logs that haven't had profile data overwritten - cochran_check_profiles(abstract, data); - return DC_STATUS_SUCCESS; } @@ -919,6 +918,17 @@ cochran_commander_device_foreach (dc_device_t *abstract, if (rc != DC_STATUS_SUCCESS) return rc; + // Calculate profile RB effective head pointer + // Cochran seems to erase 8K chunks so round up. + int last_start_address = (array_uint32_le(data.config + device->conf_ptr->last_interdive) & 0xfffff000) + 0x2000; + if (last_start_address < device->info->rb_profile_begin || last_start_address > device->info->rb_profile_end) { + ERROR(abstract->context, "Invalid profile ringbuffer head pointer in Cochra config block."); + return DC_STATUS_DATAFORMAT; + } + + // We track profile ringbuffer usage to determine which dives have profile data + int profile_capacity_remaining = device->info->rb_profile_end - device->info->rb_profile_begin; + // Loop through each dive int i; for (i = data.dive_count - 1; i > data.fp_dive_num; i--) { @@ -940,19 +950,32 @@ cochran_commander_device_foreach (dc_device_t *abstract, // Corrupt dive, guess the end address sample_end_address = cochran_guess_sample_end_address(device, &data, i); - sample = data.sample + sample_start_address - data.sample_data_offset; + // Determine if sample exists + if (profile_capacity_remaining > 0) { + // Subtract this dive's profile size including post-dive events + profile_capacity_remaining -= (last_start_address - sample_start_address); + // Adjust for a dive that wraps the buffer + if (sample_start_address > last_start_address) + profile_capacity_remaining -= device->info->rb_profile_end - device->info->rb_profile_begin; + } + last_start_address = sample_start_address; - // Determine size of sample - unsigned int dive_num =array_uint16_le(log_entry + device->log_ptr->dive_number); - if (dive_num >= data.profile_tail) - sample_size = sample_end_address - sample_start_address; - else + if (profile_capacity_remaining < 0) { + // There is no profile for this dive + sample = NULL; sample_size = 0; + WARNING(abstract->context, "Profile data for dive %d was overwritten by newer dives.", + array_uint16_le(log_entry + device->log_ptr->dive_number)); + } else { + // Calculate the size of the profile only + sample = data.sample + sample_start_address - data.sample_data_offset; + sample_size = sample_end_address - sample_start_address; - if (sample_size < 0) - // Adjust for ring buffer wrap-around - sample_size += device->info->rb_profile_end - - device->info->rb_profile_begin; + if (sample_size < 0) + // Adjust for ring buffer wrap-around + sample_size += device->info->rb_profile_end + - device->info->rb_profile_begin; + } fingerprint = log_entry; // Date bytes @@ -990,82 +1013,3 @@ cochran_commander_device_foreach (dc_device_t *abstract, return DC_STATUS_SUCCESS; } - - -/* -* cochran_check_profiles(dc_device_t *abstract) -* -* Scan logs to identify which have valid profile data, i.e. the profiles -* have not been overwritten in the ringbuffer. -* -*/ - -static void -cochran_check_profiles(dc_device_t *abstract, cochran_data_t *data) { - cochran_device_t *device = (cochran_device_t *) abstract; - - // Find head log entry - // Set handy point to first dive's dive number - unsigned char *l = data->logbook + device->log_ptr->dive_number; - int head; - for (head = 0; head < device->info->logbook_entry_count - 1; head ++) { - int this_dive = array_uint16_le(l); - l += device->info->logbook_entry_size; - int next_dive = array_uint16_le(l); - - if (next_dive < this_dive || next_dive == 0xFFFF) - break; - } - - // This is the last piece of data - unsigned int head_ptr; - if (device->info->endian == ENDIAN_LE) - head_ptr = array_uint32_le(data->config + device->conf_ptr->last_log); - else - head_ptr = array_uint32_be(data->config + device->conf_ptr->last_log); - - // Cochran (commander at least) erases 8k blocks, so round up - head_ptr = (head_ptr & 0xfffff000) + 0x2000; - - // Now move backwards until we find a profile that overlaps this one. - // Wrap around the ring buffer if needed. - - int tail; - int profile_wrap = 0; - int n = head - 1; - if (n < 0) - n = MIN(device->info->logbook_entry_count - 1, head); - - while (n != head) { - unsigned int start_dive_ptr = array_uint32_le(data->logbook + device->info->logbook_entry_size * n + device->log_ptr->profile_begin_offset); - unsigned int end_dive_ptr = array_uint32_le(data->logbook + device->info->logbook_entry_size * n + device->log_ptr->profile_end_offset); - - if (start_dive_ptr == 0xFFFFFFFF || start_dive_ptr == 0) { - n--; - if (n < 0) - n = MIN(device->info->logbook_entry_count - 1, head); - continue; - } - - tail = n; - - if (profile_wrap) { - if (start_dive_ptr < head_ptr) { - // We've found the last valid dive - break; - } - } else { - if (start_dive_ptr > end_dive_ptr) - profile_wrap = 1; - } - - n--; - if (n < 0) - n = MIN(device->info->logbook_entry_count - 1, head); - } - - data->log_head = head; - data->profile_head = head; - - data->profile_tail = tail; -} diff --git a/src/cochran_commander.h b/src/cochran_commander.h index f6e3921..14cfedf 100644 --- a/src/cochran_commander.h +++ b/src/cochran_commander.h @@ -96,14 +96,12 @@ typedef struct cochran_log_offsets_t { } cochran_log_offsets_t; typedef struct cochran_data_t { - unsigned char config[512]; + unsigned char config[1024]; unsigned char *logbook; unsigned char *sample; unsigned short int dive_count; int fp_dive_num; - int log_head, log_tail; // head and tail for log ringbuffer - int profile_head, profile_tail; // Entries that have valid profiles unsigned int logbook_size; -- 2.4.3

1 1

[PATCH] Cochran: Made numerous changes after review
by John Van Ostrand 07 Mar '16

07 Mar '16

- More localization of variables - Variable name changes to be more consistent with other backends - Removed special PTY serial_setup case - Avoided potential double-free situations - Consolidated serial queue flushing - Verified heartbeat bytes - Used goto-style error handling on _open - Bounds checking on pointers - Removed rounding reads to nearest 8k. - Issue DEVINFO event in foreach - Separated dive data from device_t - switched to date-based fingerprint - Many other small bugs and style fixes. --- src/cochran_commander.c | 553 +++++++++++++++++++---------------------- src/cochran_commander.h | 52 ++-- src/cochran_commander_parser.c | 14 +- 3 files changed, 283 insertions(+), 336 deletions(-) diff --git a/src/cochran_commander.c b/src/cochran_commander.c index c457b3a..127518d 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -49,28 +49,29 @@ */ // Cochran Commander Nitrox -cochran_device_info_t cochran_cmdr_device_info = { +const cochran_device_info_t cochran_cmdr_device_info = { COCHRAN_MODEL_COMMANDER_AIR_NITROX, // model - ADDRESS_24_BIT, // address_size + 24, // address_bits ENDIAN_BE, // endian 115200, // high_baud 0x00000000, // rb_logbook_begin 0x00020000, // rb_lobook_end 0x00020000, // rb_profile_begin 0x00100000, // rb_profile_end - 256, // log_size - 512, // max_log + 256, // logbook_entry_size + 512, // logbook_entry_count 2, // max_gas SAMPLE_CMDR, // sample_format }; -cochran_conf_offsets_t cochran_cmdr_conf_offsets = { +const cochran_conf_offsets_t cochran_cmdr_conf_offsets = { 0x046, // dive_count 0x06e, // last_log 0x071, // last_interdive + 0x0aa, // serial_number }; -cochran_log_offsets_t cochran_cmdr_log_offsets = { +const cochran_log_offsets_t cochran_cmdr_log_offsets = { 1, 0, 3, 2, 5, 4, // sec, min, hour, day, mon, year, 1 byte each 6, // profile_begin_offset, 4 bytes 24, // water_conductivity, 1 byte, 0=low(fresh), 2=high(sea) @@ -91,7 +92,7 @@ cochran_log_offsets_t cochran_cmdr_log_offsets = { }; // inter-dive event lengths -struct event_size cmdr_event_bytes[15] = { +const struct event_size cmdr_event_bytes[15] = { {0x00, 17}, {0x01, 21}, {0x02, 18}, {0x03, 17}, {0x06, 19}, {0x07, 19}, {0x08, 19}, {0x09, 19}, {0x0a, 19}, @@ -100,63 +101,64 @@ struct event_size cmdr_event_bytes[15] = { { -1, 1} }; // Cochran EMC-14 -cochran_device_info_t cochran_emc14_device_info = { +const cochran_device_info_t cochran_emc14_device_info = { COCHRAN_MODEL_EMC_14, // model - ADDRESS_32_BIT, // address_size + 32, // address_bits ENDIAN_LE, // endian 806400, // high_baud 0x00000000, // rb_logbook_begin 0x00020000, // rb_lobook_end 0x00022000, // rb_profile_begin 0x00200000, // rb_profile_end - 512, // log_size - 256, // max_log + 512, // logbook_entry_size + 256, // logbook_entry_count 2, // max_gas SAMPLE_EMC, // sample_format }; // Cochran EMC-16 -cochran_device_info_t cochran_emc16_device_info = { +const cochran_device_info_t cochran_emc16_device_info = { COCHRAN_MODEL_EMC_16, // model - ADDRESS_32_BIT, // address_size + 32, // address_bits ENDIAN_LE, // endian 806400, // high_baud 0x00000000, // rb_logbook_begin 0x00080000, // rb_lobook_end 0x00094000, // rb_profile_begin 0x00800000, // rb_profile_end - 512, // log_size - 1024, // max_log + 512, // logbook_entry_size + 1024, // logbook_entry_count 2, // max_gas SAMPLE_EMC, // sample_format }; // Cochran EMC-20 -cochran_device_info_t cochran_emc20_device_info = { +const cochran_device_info_t cochran_emc20_device_info = { COCHRAN_MODEL_EMC_20, // model - ADDRESS_32_BIT, // address_size + 32, // address_bits ENDIAN_LE, // endian 806400, // high_baud 0x00000000, // rb_logbook_begin 0x00080000, // rb_lobook_end 0x00094000, // rb_profile_begin 0x01000000, // rb_profile_end - 512, // log_size - 1024, // max_log + 512, // logbook_entry_size + 1024, // logbook_entry_count 2, // max_gas SAMPLE_EMC, // sample_format }; // Common EMC offsets -cochran_conf_offsets_t cochran_emc_conf_offsets = { +const cochran_conf_offsets_t cochran_emc_conf_offsets = { 0x0d2, // dive_count 0x13e, // last_log 0x142, // last_interdive + 0x1e6, // serial_number }; -cochran_log_offsets_t cochran_emc_log_offsets = { +const cochran_log_offsets_t cochran_emc_log_offsets = { 0, 1, 2, 3, 4, 5, // sec, min, hour, day, mon, year, 1 byte each 6, // profile_begin_offset, 4 bytes 24, // water_conductivity, 1 byte 0=low(fresh), 2=high(sea) @@ -176,7 +178,7 @@ cochran_log_offsets_t cochran_emc_log_offsets = { 407, // max_temp, 1 byte, /2+20=F }; -struct event_size emc_event_bytes[15] = { +const struct event_size emc_event_bytes[15] = { {0x00, 19}, {0x01, 23}, {0x02, 20}, {0x03, 19}, {0x06, 21}, {0x07, 21}, {0x0a, 21}, {0x0b, 21}, {0x0f, 19}, @@ -196,12 +198,12 @@ static const dc_device_vtable_t cochran_commander_device_vtable = { static dc_status_t -cochran_read_id(dc_device_t *device); +cochran_read_id(dc_device_t *device, unsigned char *id, int size); static dc_status_t cochran_commander_read (dc_device_t *abstract, dc_event_progress_t *progress, unsigned int address, unsigned char data[], unsigned int size); static void -cochran_check_profiles(dc_device_t *abstract); +cochran_check_profiles(dc_device_t *abstract, cochran_data_t *data); static dc_status_t @@ -210,17 +212,18 @@ cochran_packet (cochran_device_t *device, dc_event_progress_t *progress, unsigned char answer[], unsigned int asize, int high_speed) { dc_device_t *abstract = (dc_device_t *) device; - unsigned int bytes_read = 0, n, read_size; - unsigned int ptr; - int rc; if (device_is_cancelled (abstract)) return DC_STATUS_CANCELLED; // Send the command to the device, one byte at a time - for (ptr = 0; ptr < csize; ptr++) { - if (ptr) serial_sleep(device->port, 16); // 16 ms - n = serial_write(device->port, command + ptr, 1); + // If sent all at once the command is ignored. It's like the DC + // has no buffering. + for (int i = 0; i < csize; i++) { + // Give the DC time to read the character. + if (i) serial_sleep(device->port, 16); // 16 ms + + unsigned int n = serial_write(device->port, command + i, 1); if (n != 1) { ERROR (abstract->context, "Failed to send the command."); return EXITCODE (n); @@ -228,38 +231,40 @@ cochran_packet (cochran_device_t *device, dc_event_progress_t *progress, } if (high_speed) { + int rc = 0; + + // Give the DC time to process the command. + // This is required. serial_sleep(device->port, 45); // Rates are odd, like 806400 for the EMC, 115200 for commander rc = serial_configure(device->port, device->info->high_baud, 8, SERIAL_PARITY_NONE, 2, SERIAL_FLOWCONTROL_NONE); if (rc == -1) { - // Assume we are talking to a pty, a simulator - rc = serial_configure(device->port, 115200, 8, SERIAL_PARITY_NONE, - 2, SERIAL_FLOWCONTROL_NONE); - if (rc == -1) { - ERROR (abstract->context, "Failed to set the high baud rate."); - return DC_STATUS_IO; - } + ERROR (abstract->context, "Failed to set the high baud rate."); + return DC_STATUS_IO; } } // Receive the answer from the device. // Use 1024 byte "packets" so we can display progress. - while (bytes_read < asize) { - if (asize - bytes_read > 1024) - read_size = 1024; + unsigned int nbytes = 0; + while (nbytes < asize) { + unsigned int len = 0; + + if (asize - nbytes > 1024) + len = 1024; else - read_size = asize - bytes_read; + len = asize - nbytes; - n = serial_read (device->port, answer + bytes_read, read_size); - if (n != read_size) { + int n = serial_read (device->port, answer + nbytes, len); + if (n != len) { ERROR (abstract->context, "Failed to receive data, expected %u," - "read %u.", read_size, n); + "read %u.",len, n); return EXITCODE (n); } - bytes_read += n; + nbytes += n; if (progress) { progress->current += n; @@ -281,51 +286,39 @@ cochran_commander_serial_setup (cochran_device_t *device) 2, SERIAL_FLOWCONTROL_NONE); if (rc == -1) { ERROR (device->base.context, "Failed to set the terminal attributes."); - serial_close (device->port); return DC_STATUS_IO; } - serial_set_queue_size(device->port, 4096, 4096); - - // Make sure everything is in a sane state. - serial_flush (device->port, SERIAL_QUEUE_OUTPUT); - serial_flush (device->port, SERIAL_QUEUE_INPUT); - - serial_set_break(device->port, 1); - serial_sleep(device->port, 16); - - serial_set_break(device->port, 0); - // Set the timeout for receiving data (5000 ms). if (serial_set_timeout (device->port, 5000) == -1) { ERROR (device->base.context, "Failed to set the timeout."); - serial_close (device->port); return DC_STATUS_IO; } + // Wake up DC and trigger heartbeat + serial_set_break(device->port, 1); + serial_sleep(device->port, 16); + serial_set_break(device->port, 0); + + // Clear old heartbeats + serial_flush (device->port, SERIAL_QUEUE_BOTH); + // Wait for heartbeat byte before send - char answer[1]; - int n = serial_read(device->port, answer, 1); + unsigned char answer = 0; + int n = serial_read(device->port, &answer, 1); if (n != 1) { + serial_close (device->port); ERROR (device->base.context, "Failed to receive device heartbeat."); return EXITCODE (n); } - return DC_STATUS_SUCCESS; -} - - -dc_status_t -cochran_commander_serial_open(cochran_device_t *device) -{ - // Open the device. - int rc = serial_open (&device->port, device->base.context, device->name); - if (rc == -1) { - ERROR (device->base.context, "Failed to open the serial port."); - return DC_STATUS_IO; + if (answer != 0xAA) { + serial_close (device->port); + ERROR (device->base.context, "Received bad hearbeat byte (%02x).", answer); + return DC_STATUS_PROTOCOL; } - return cochran_commander_serial_setup(device); + return DC_STATUS_SUCCESS; } @@ -334,7 +327,7 @@ cochran_commander_device_open (dc_device_t **out, dc_context_t *context, const char *name) { cochran_device_t *device = NULL; - dc_status_t rc; + dc_status_t status; if (out == NULL) return DC_STATUS_INVALIDARGS; @@ -348,29 +341,31 @@ cochran_commander_device_open (dc_device_t **out, dc_context_t *context, // Set the default values. device->port = NULL; - device->name = name; - device->data.logbook = NULL; - device->data.sample = NULL; cochran_commander_device_set_fingerprint((dc_device_t *) device, NULL, 0); - rc = cochran_commander_serial_open(device); - if (rc != DC_STATUS_SUCCESS) { - dc_device_deallocate((dc_device_t *) device); - return rc; + // Open the device. + int rc = serial_open (&device->port, device->base.context, name); + if (rc == -1) { + ERROR (device->base.context, "Failed to open the serial port."); + status = DC_STATUS_IO; + goto error_free; } + rc = cochran_commander_serial_setup(device); + if (rc != DC_STATUS_SUCCESS) + return rc; + // Read ID from the device - rc = cochran_read_id((dc_device_t *) device); + unsigned char id[67]; + status = cochran_read_id((dc_device_t *) device, id, sizeof(id)); - if (rc != DC_STATUS_SUCCESS) { + if (status != DC_STATUS_SUCCESS) { ERROR (context, "Device not responding."); - serial_close (device->port); - dc_device_deallocate((dc_device_t *) device); - return rc; + goto error_close; } - memcpy(device->model_string, device->data.id + 0x3b, 8); + memcpy(device->model_string, id + 0x3b, 8); device->model_string[8] = 0; device->base.devinfo.model = cochran_commander_model_id(device->model_string); @@ -383,9 +378,8 @@ cochran_commander_device_open (dc_device_t **out, dc_context_t *context, device->model_string[2], device->model_string[3], device->model_string[4], device->model_string[5], device->model_string[6], device->model_string[7]); - serial_close (device->port); - dc_device_deallocate((dc_device_t *) device); - return DC_STATUS_UNSUPPORTED; + status = DC_STATUS_UNSUPPORTED; + goto error_close; } // Get model capability, protocol and data format @@ -395,24 +389,27 @@ cochran_commander_device_open (dc_device_t **out, dc_context_t *context, *out = (dc_device_t *) device; return DC_STATUS_SUCCESS; + +error_close: + serial_close (device->port); +error_free: + dc_device_deallocate ((dc_device_t *) device); + return status; } dc_status_t cochran_commander_device_close (dc_device_t *abstract) { + dc_status_t status = DC_STATUS_SUCCESS; cochran_device_t *device = (cochran_device_t*) abstract; // Close the device. if (serial_close (device->port) == -1) { - return DC_STATUS_IO; + dc_status_set_error( &status, DC_STATUS_IO ); } - // Free memory. - free (device->data.logbook); - free (device->data.sample); - - return DC_STATUS_SUCCESS; + return status; } @@ -421,15 +418,14 @@ cochran_commander_device_set_fingerprint (dc_device_t *abstract, const unsigned char data[], unsigned int size) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = &device->data; - if (size && size != sizeof (d->fingerprint)) + if (size && size != sizeof (device->fingerprint)) return DC_STATUS_INVALIDARGS; if (size) - memcpy (&d->fingerprint, data, sizeof (d->fingerprint)); + memcpy (device->fingerprint, data, sizeof (device->fingerprint)); else - memset (&d->fingerprint, 0xFF, sizeof (d->fingerprint)); + memset (device->fingerprint, 0xFF, sizeof (device->fingerprint)); return DC_STATUS_SUCCESS; } @@ -453,9 +449,9 @@ cochran_commander_read (dc_device_t *abstract, dc_event_progress_t *progress, unsigned char command[10]; unsigned char command_size; - switch (device->info->address_size) + switch (device->info->address_bits) { - case ADDRESS_32_BIT: + case 32: // EMC uses 32 bit addressing command[0] = 0x15; command[1] = (address ) & 0xff; @@ -469,7 +465,7 @@ cochran_commander_read (dc_device_t *abstract, dc_event_progress_t *progress, command[9] = 0x05; command_size = 10; break; - case ADDRESS_24_BIT: + case 24: // Commander uses 24 byte addressing command[0] = 0x15; command[1] = (address ) & 0xff; @@ -523,7 +519,7 @@ cochran_commander_model_id(const char *model) // Get device capability, protocol info and data format for a device dc_status_t -cochran_commander_info (const int model, cochran_device_info_t **info, cochran_conf_offsets_t **conf_ptr, cochran_log_offsets_t **log_ptr, struct event_size (**event_bytes)[15]) +cochran_commander_info (const int model, const cochran_device_info_t **info, const cochran_conf_offsets_t **conf_ptr, const cochran_log_offsets_t **log_ptr, const struct event_size (**event_bytes)[15]) { // Determine model if (model == COCHRAN_MODEL_EMC_20) @@ -564,99 +560,51 @@ cochran_commander_info (const int model, cochran_device_info_t **info, cochran_c static dc_status_t -cochran_read_id (dc_device_t *abstract) +cochran_read_id (dc_device_t *abstract, unsigned char *id, int size) { cochran_device_t *device = (cochran_device_t *) abstract; dc_status_t rc; unsigned char command[6] = {0x05, 0x9D, 0xFF, 0x00, 0x43, 0x00}; - rc = cochran_packet(device, NULL, command, 6, device->data.id, 67, 0); + rc = cochran_packet(device, NULL, command, sizeof(command), id, size, 0); if (rc != DC_STATUS_SUCCESS) return rc; - if (strncmp((const char *)device->data.id, "(C)", 3) != 0) { - // It's a Commander, read again - memcpy(device->data.id0, device->data.id, 67); - device->data.extra_id_flag = 1; - + if (strncmp((const char *) id, "(C)", 3) != 0) { + // It's a Commander, read a different location command[1] = 0xBD; command[2] = 0x7F; - rc = cochran_packet(device, NULL, command, 6, device->data.id, 67, 0); + rc = cochran_packet(device, NULL, command, sizeof(command), id, size, 0); if (rc != DC_STATUS_SUCCESS) return rc; } - return DC_STATUS_SUCCESS; -} - - -static dc_status_t -cochran_read_config (dc_device_t *abstract, dc_event_progress_t *progress) -{ - cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *data = &device->data; + // Emit ID block dc_event_vendor_t vendor; - - dc_status_t rc; - unsigned char command[2] = { 0x96, 0x00 }; - - int n; - for (n = 0; n < 2; n++) { - command[1] = n; - rc = cochran_packet(device, progress, command, 2, data->config[n], 512, 0); - if (rc != DC_STATUS_SUCCESS) - return rc; - - vendor.data = data->config[n]; - vendor.size = 512; - device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); - } + vendor.data = id; + vendor.size = size; + device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); return DC_STATUS_SUCCESS; } static dc_status_t -cochran_read_misc (dc_device_t *abstract, dc_event_progress_t *progress) +cochran_read_config (dc_device_t *abstract, cochran_data_t *data, dc_event_progress_t *progress) { cochran_device_t *device = (cochran_device_t *) abstract; - dc_status_t rc; dc_event_vendor_t vendor; - unsigned char command[7] = { 0x89, 0x05, 0x00, 0x00, 0x00, 0xDC, 0x05 }; - - switch (device->info->model & 0xFF0000) - { - case COCHRAN_MODEL_COMMANDER_FAMILY: - command[2] = 0xCA; - command[3] = 0xFD; - break; - case COCHRAN_MODEL_EMC_FAMILY: - command[2] = 0xE0; - command[3] = 0x03; - break; - default: - return DC_STATUS_UNSUPPORTED; - } - - // Send first byte then wait for heartbeat before sending the rest - serial_write(device->port, command, 1); - - char answer[1]; - int n = serial_read(device->port, answer, 1); - if (n != 1) { - ERROR (abstract->context, "Failed to receive device heartbeat."); - return EXITCODE (n); - } - - rc = cochran_packet(device, progress, command + 1, 6, device->data.misc, 1500, 0); + dc_status_t rc; + unsigned char command[2] = { 0x96, 0x00 }; + rc = cochran_packet(device, progress, command, sizeof(command), data->config, sizeof(data->config), 0); if (rc != DC_STATUS_SUCCESS) return rc; - vendor.data = device->data.misc; - vendor.size = 1500; + vendor.data = data->config; + vendor.size = sizeof(data->config); device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); return DC_STATUS_SUCCESS; @@ -664,18 +612,17 @@ cochran_read_misc (dc_device_t *abstract, dc_event_progress_t *progress) static dc_status_t -cochran_read_logbook (dc_device_t *abstract, dc_event_progress_t *progress) +cochran_read_logbook (dc_device_t *abstract, cochran_data_t *data, dc_event_progress_t *progress) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = &device->data; dc_status_t rc; - if (d->logbook) - free(d->logbook); + if (data->logbook) + free(data->logbook); // Allocate space for log book. - d->logbook = (unsigned char *) malloc(d->logbook_size); - if (device == NULL) { + data->logbook = (unsigned char *) malloc(data->logbook_size); + if (data->logbook == NULL) { ERROR (abstract->context, "Failed to allocate memory."); return DC_STATUS_NOMEMORY; } @@ -683,38 +630,38 @@ cochran_read_logbook (dc_device_t *abstract, dc_event_progress_t *progress) serial_sleep(device->port, 800); // set back to 9600 baud - cochran_commander_serial_setup(device); + rc = cochran_commander_serial_setup(device); + if (rc != DC_STATUS_SUCCESS) + return rc; // Request log book - rc = cochran_commander_read(abstract, progress, 0, d->logbook, - d->logbook_size); + rc = cochran_commander_read(abstract, progress, 0, data->logbook, + data->logbook_size); return rc; } static void -cochran_find_fingerprint(dc_device_t *abstract) +cochran_find_fingerprint(dc_device_t *abstract, cochran_data_t *data) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = (cochran_data_t *) &device->data; // Skip to fingerprint to reduce time - d->fp_dive_num = d->dive_count - 1; + data->fp_dive_num = data->dive_count; + data->fp_dive_num--; - while (d->fp_dive_num >= 0 && memcmp(&d->fingerprint, - d->logbook + d->fp_dive_num * device->info->log_size - + device->log_ptr->dive_number, - sizeof(d->fingerprint))) - d->fp_dive_num--; + while (data->fp_dive_num >= 0 && memcmp(device->fingerprint, + data->logbook + data->fp_dive_num * device->info->logbook_entry_size, + sizeof(device->fingerprint))) + data->fp_dive_num--; } static void -cochran_get_sample_parms(dc_device_t *abstract) +cochran_get_sample_parms(dc_device_t *abstract, cochran_data_t *data) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = (cochran_data_t *) &device->data; unsigned int pre_dive_offset = 0, end_dive_offset = 0; unsigned int low_offset, high_offset; @@ -723,12 +670,27 @@ cochran_get_sample_parms(dc_device_t *abstract) high_offset = 0; int i; - for (i = d->fp_dive_num + 1; i < d->dive_count; i++) { - pre_dive_offset = array_uint32_le (&(d->logbook[i * device->info->log_size - + device->log_ptr->profile_pre_offset])); - end_dive_offset = array_uint32_le (&(d->logbook[i * device->info->log_size - + device->log_ptr->profile_end_offset])); + for (i = data->fp_dive_num + 1; i < data->dive_count; i++) { + pre_dive_offset = array_uint32_le (data->logbook + i * device->info->logbook_entry_size + + device->log_ptr->profile_pre_offset); + end_dive_offset = array_uint32_le (data->logbook + i * device->info->logbook_entry_size + + device->log_ptr->profile_end_offset); + + // Validate offsets, allow 0xFFFFFFF for end_dive_offset + // because we handle that as a special case. + if (pre_dive_offset < device->info->rb_profile_begin + || pre_dive_offset > device->info->rb_profile_end) { + ERROR(abstract->context, "Invalid pre-dive offset (%08x) on dive %d.", pre_dive_offset, i); + continue; + } + if (end_dive_offset < device->info->rb_profile_begin + || (end_dive_offset > device->info->rb_profile_end + && end_dive_offset != 0xFFFFFFFF)) { + ERROR(abstract->context, "Invalid end-dive offset (%08x) on dive %d.", end_dive_offset, i); + continue; + } + // Check for ring buffer wrap-around. if (pre_dive_offset > end_dive_offset) break; @@ -740,39 +702,33 @@ cochran_get_sample_parms(dc_device_t *abstract) } if (pre_dive_offset > end_dive_offset) { - // Since I can't tell how much memory it has, I'll round. - // I'll round to 128K, dives longer than 12 hrs aren't likely - // and memory in sizes not rounded to 128K might be odd. - high_offset = ((pre_dive_offset - 1) & 0xE0000) + 0x20000; + high_offset = device->info->rb_profile_end; low_offset = device->info->rb_profile_begin; - d->sample_data_offset = low_offset; - d->sample_size = high_offset - low_offset; + data->sample_data_offset = low_offset; + data->sample_size = high_offset - low_offset; } else if (low_offset < 0xFFFFFFFF && high_offset > 0) { - // Round offset and size to 16K boundary - d->sample_data_offset = low_offset & 0xFFFFC000; - high_offset = ((high_offset - 1) & 0xFFFFC000) + 0x4000; - d->sample_size = high_offset - d->sample_data_offset; + data->sample_data_offset = low_offset; + data->sample_size = high_offset - data->sample_data_offset; } else { - d->sample_data_offset = 0; - d->sample_size = 0; + data->sample_data_offset = 0; + data->sample_size = 0; } } static dc_status_t -cochran_read_samples(dc_device_t *abstract, dc_event_progress_t *progress) +cochran_read_samples(dc_device_t *abstract, cochran_data_t *data, dc_event_progress_t *progress) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = (cochran_data_t *) &device->data; dc_status_t rc; - if (d->sample_size > 0) { - if (d->sample) - free(d->sample); + if (data->sample_size > 0) { + if (data->sample) + free(data->sample); - d->sample = (unsigned char *) malloc(d->sample_size); - if (d->sample == NULL) { + data->sample = (unsigned char *) malloc(data->sample_size); + if (data->sample == NULL) { ERROR (abstract->context, "Failed to allocate memory."); return DC_STATUS_NOMEMORY; } @@ -780,11 +736,13 @@ cochran_read_samples(dc_device_t *abstract, dc_event_progress_t *progress) serial_sleep(device->port, 800); // set back to 9600 baud - cochran_commander_serial_setup(device); + rc = cochran_commander_serial_setup(device); + if (rc != DC_STATUS_SUCCESS) + return rc; // Read the sample data - rc = cochran_commander_read (abstract, progress, d->sample_data_offset, - d->sample, d->sample_size); + rc = cochran_commander_read (abstract, progress, data->sample_data_offset, + data->sample, data->sample_size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc; @@ -796,11 +754,10 @@ cochran_read_samples(dc_device_t *abstract, dc_event_progress_t *progress) static dc_status_t -cochran_commander_device_read_all (dc_device_t *abstract) +cochran_commander_device_read_all (dc_device_t *abstract, cochran_data_t *data) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = (cochran_data_t *) &device->data; - dc_event_progress_t progress = {0}; + dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER; dc_status_t rc; int max_config, max_logbook, max_sample; @@ -815,39 +772,42 @@ cochran_commander_device_read_all (dc_device_t *abstract) device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); // Read config - rc = cochran_read_config(abstract, &progress); + rc = cochran_read_config(abstract, data, &progress); if (rc != DC_STATUS_SUCCESS) return rc; // Determine size of dive list to read. Round up to nearest 16K if (device->info->endian == ENDIAN_LE) - d->dive_count = array_uint16_le (d->config[0] + device->conf_ptr->dive_count); + data->dive_count = array_uint16_le (data->config + device->conf_ptr->dive_count); else - d->dive_count = array_uint16_be (d->config[0] + device->conf_ptr->dive_count); + data->dive_count = array_uint16_be (data->config + device->conf_ptr->dive_count); - d->logbook_size = ((d->dive_count * device->info->log_size) & 0xFFFFC000) - + 0x4000; + if (data->dive_count > device->info->logbook_entry_count) { + data->logbook_size = device->info->logbook_entry_count * device->info->logbook_entry_size; + } else { + data->logbook_size = data->dive_count * device->info->logbook_entry_size; + } - progress.maximum -= max_logbook - d->logbook_size; + progress.maximum -= max_logbook - data->logbook_size; device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); - rc = cochran_read_logbook(abstract, &progress); + rc = cochran_read_logbook(abstract, data, &progress); if (rc != DC_STATUS_SUCCESS) return rc; // Determine sample memory to read - cochran_find_fingerprint(abstract); - cochran_get_sample_parms(abstract); + cochran_find_fingerprint(abstract, data); + cochran_get_sample_parms(abstract, data); - progress.maximum -= max_sample - d->sample_size; + progress.maximum -= max_sample - data->sample_size; device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); - rc = cochran_read_samples(abstract, &progress); + rc = cochran_read_samples(abstract, data, &progress); if (rc != DC_STATUS_SUCCESS) return rc; // Determine logs that haven't had profile data overwritten - cochran_check_profiles(abstract); + cochran_check_profiles(abstract, data); return DC_STATUS_SUCCESS; } @@ -859,64 +819,49 @@ cochran_commander_device_read_all (dc_device_t *abstract) (d)[3] = ((i) >> 24) & 0xff) dc_status_t -cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *data) +cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *dump) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *d = (cochran_data_t *) &device->data; - dc_event_progress_t progress = {0}; - dc_event_vendor_t vendor; - + dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER; dc_status_t rc; - int size; + cochran_data_t data; + data.logbook = NULL; + data.sample = NULL; // Make sure buffer is good. - if (!dc_buffer_clear(data)) { + if (!dc_buffer_clear(dump)) { ERROR (abstract->context, "Uninitialized buffer."); return DC_STATUS_INVALIDARGS; } - // Determine size for progress - size = 512 + 512 + 1500 + device->info->rb_profile_end; + // Reserve space + if (!dc_buffer_resize(dump, device->info->rb_profile_end)) { + ERROR(abstract->context, "Insufficient buffer space available."); + return DC_STATUS_NOMEMORY; + } + // Determine size for progress progress.current = 0; - progress.maximum = size; + progress.maximum = 512 + device->info->rb_profile_end; device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); - // Emit ID blocks as vendor data - if (d->extra_id_flag) { - vendor.data = d->id0; - vendor.size = 67; - device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); - } - - vendor.data = d->id; - vendor.size = 67; - device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); - - rc = cochran_read_config (abstract, &progress); - if (rc != DC_STATUS_SUCCESS) - return rc; - - rc = cochran_read_misc (abstract, &progress); + rc = cochran_read_config (abstract, &data, &progress); if (rc != DC_STATUS_SUCCESS) return rc; // Read logbook and sample memory - // Reserve space - if (!dc_buffer_resize(data, device->info->rb_profile_end)) { - ERROR(abstract->context, "Insufficient buffer space available."); - return DC_STATUS_NOMEMORY; - } - serial_sleep(device->port, 800); // set back to 9600 baud - cochran_commander_serial_setup(device); + rc = cochran_commander_serial_setup(device); + if (rc != DC_STATUS_SUCCESS) { + return rc; + } // Read the sample data, from 0 to sample end will include logbook rc = cochran_commander_read (abstract, &progress, 0, - dc_buffer_get_data(data), device->info->rb_profile_end); + dc_buffer_get_data(dump), device->info->rb_profile_end); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc; @@ -935,15 +880,15 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *data) static unsigned int cochran_guess_sample_end_address(cochran_device_t *device, cochran_data_t *data, unsigned int log_num) { - const unsigned char *log_entry = data->logbook + device->info->log_size * log_num; + const unsigned char *log_entry = data->logbook + device->info->logbook_entry_size * log_num; if (log_num == data->dive_count) - // Return next usable address from config0 page - return array_uint32_le(data->config[0] + // Return next usable address from config page + return array_uint32_le(data->config + device->info->rb_profile_end); // Next log's start address - return array_uint32_le(log_entry + device->info->log_size + 6); + return array_uint32_le(log_entry + device->info->logbook_entry_size + 6); } @@ -952,7 +897,6 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t callback, void *userdata) { cochran_device_t *device = (cochran_device_t *) abstract; - cochran_data_t *data = &device->data; unsigned int sample_start_address, sample_end_address; dc_status_t rc; @@ -960,28 +904,47 @@ cochran_commander_device_foreach (dc_device_t *abstract, unsigned char *log_entry, *fingerprint, *sample, *dive; int sample_size, dive_size; - rc = cochran_commander_device_read_all (abstract); + cochran_data_t data; + data.logbook = NULL; + data.sample = NULL; + rc = cochran_commander_device_read_all (abstract, &data); + + // Emit a device info event. + dc_event_devinfo_t devinfo; + devinfo.model = abstract->devinfo.model; + devinfo.firmware = 0; // unknown + devinfo.serial = array_uint32_le(data.config + device->conf_ptr->serial_number); + device_event_emit (abstract, DC_EVENT_DEVINFO, &devinfo); if (rc != DC_STATUS_SUCCESS) return rc; // Loop through each dive int i; - for (i = data->dive_count - 1; i > data->fp_dive_num; i--) { - log_entry = data->logbook + i * device->info->log_size; + for (i = data.dive_count - 1; i > data.fp_dive_num; i--) { + log_entry = data.logbook + i * device->info->logbook_entry_size; sample_start_address = array_uint32_le (log_entry + 6); - sample_end_address = array_uint32_le (log_entry + device->info->log_size / 2); + sample_end_address = array_uint32_le (log_entry + device->info->logbook_entry_size / 2); + + // Validate + if (sample_start_address < device->info->rb_profile_begin + || sample_start_address > device->info->rb_profile_end + || sample_end_address < device->info->rb_profile_begin + || (sample_end_address > device->info->rb_profile_end + && sample_end_address != 0xFFFFFFFF)) { + continue; + } if (sample_end_address == 0xFFFFFFFF) // Corrupt dive, guess the end address - sample_end_address = cochran_guess_sample_end_address(device, data, i); + sample_end_address = cochran_guess_sample_end_address(device, &data, i); - sample = data->sample + sample_start_address - data->sample_data_offset; + sample = data.sample + sample_start_address - data.sample_data_offset; // Determine size of sample unsigned int dive_num =array_uint16_le(log_entry + device->log_ptr->dive_number); - if (dive_num >= data->profile_tail) + if (dive_num >= data.profile_tail) sample_size = sample_end_address - sample_start_address; else sample_size = 0; @@ -991,28 +954,28 @@ cochran_commander_device_foreach (dc_device_t *abstract, sample_size += device->info->rb_profile_end - device->info->rb_profile_begin; - fingerprint = log_entry + device->log_ptr->dive_number; + fingerprint = log_entry; // Date bytes // Build dive blob - dive_size = device->info->log_size + sample_size; + dive_size = device->info->logbook_entry_size + sample_size; dive = (unsigned char *) malloc(dive_size); if (dive == NULL) return DC_STATUS_NOMEMORY; - memcpy(dive, log_entry, device->info->log_size); // log + memcpy(dive, log_entry, device->info->logbook_entry_size); // log // Copy profile data if (sample_size) { if (sample_start_address <= sample_end_address) { - memcpy(dive + device->info->log_size, sample, + memcpy(dive + device->info->logbook_entry_size, sample, sample_size); } else { // It wrapped the buffer, copy two sections unsigned int size = device->info->rb_profile_end - sample_start_address; - memcpy(dive + device->info->log_size, sample, size); - memcpy(dive + device->info->log_size + size, - data->sample, sample_end_address - device->info->rb_profile_begin); + memcpy(dive + device->info->logbook_entry_size, sample, size); + memcpy(dive + device->info->logbook_entry_size + size, + data.sample, sample_end_address - device->info->rb_profile_begin); } } @@ -1038,16 +1001,16 @@ cochran_commander_device_foreach (dc_device_t *abstract, */ static void -cochran_check_profiles(dc_device_t *abstract) { +cochran_check_profiles(dc_device_t *abstract, cochran_data_t *data) { cochran_device_t *device = (cochran_device_t *) abstract; // Find head log entry // Set handy point to first dive's dive number - unsigned char *l = device->data.logbook + device->log_ptr->dive_number; + unsigned char *l = data->logbook + device->log_ptr->dive_number; int head; - for (head = 0; head < device->info->max_log - 1; head ++) { + for (head = 0; head < device->info->logbook_entry_count - 1; head ++) { int this_dive = array_uint16_le(l); - l += device->info->log_size; + l += device->info->logbook_entry_size; int next_dive = array_uint16_le(l); if (next_dive < this_dive || next_dive == 0xFFFF) @@ -1057,9 +1020,9 @@ cochran_check_profiles(dc_device_t *abstract) { // This is the last piece of data unsigned int head_ptr; if (device->info->endian == ENDIAN_LE) - head_ptr = array_uint32_le(device->data.config[0] + device->conf_ptr->last_log); + head_ptr = array_uint32_le(data->config + device->conf_ptr->last_log); else - head_ptr = array_uint32_be(device->data.config[0] + device->conf_ptr->last_log); + head_ptr = array_uint32_be(data->config + device->conf_ptr->last_log); // Cochran (commander at least) erases 8k blocks, so round up head_ptr = (head_ptr & 0xfffff000) + 0x2000; @@ -1071,16 +1034,16 @@ cochran_check_profiles(dc_device_t *abstract) { int profile_wrap = 0; int n = head - 1; if (n < 0) - n = MIN(device->info->max_log - 1, head); + n = MIN(device->info->logbook_entry_count - 1, head); while (n != head) { - unsigned int start_dive_ptr = array_uint32_le(device->data.logbook + device->info->log_size * n + device->log_ptr->profile_begin_offset); - unsigned int end_dive_ptr = array_uint32_le(device->data.logbook + device->info->log_size * n + device->log_ptr->profile_end_offset); + unsigned int start_dive_ptr = array_uint32_le(data->logbook + device->info->logbook_entry_size * n + device->log_ptr->profile_begin_offset); + unsigned int end_dive_ptr = array_uint32_le(data->logbook + device->info->logbook_entry_size * n + device->log_ptr->profile_end_offset); if (start_dive_ptr == 0xFFFFFFFF || start_dive_ptr == 0) { n--; if (n < 0) - n = MIN(device->info->max_log - 1, head); + n = MIN(device->info->logbook_entry_count - 1, head); continue; } @@ -1098,11 +1061,11 @@ cochran_check_profiles(dc_device_t *abstract) { n--; if (n < 0) - n = MIN(device->info->max_log - 1, head); + n = MIN(device->info->logbook_entry_count - 1, head); } - device->data.log_head = head; - device->data.profile_head = head; + data->log_head = head; + data->profile_head = head; - device->data.profile_tail = tail; + data->profile_tail = tail; } diff --git a/src/cochran_commander.h b/src/cochran_commander.h index 22f17a3..f6e3921 100644 --- a/src/cochran_commander.h +++ b/src/cochran_commander.h @@ -24,30 +24,21 @@ #define COCHRAN_TIMESTAMP_OFFSET 694242000 #define COCHRAN_MODEL_SIZE 8 -#define COCHRAN_FINGERPRINT_SIZE 2 +#define COCHRAN_FINGERPRINT_SIZE 6 #define UNSUPPORTED 0xFFFFFFFF -typedef enum cochran_model_t { - COCHRAN_MODEL_UNKNOWN = 0, - COCHRAN_MODEL_EMC_FAMILY = 1 << 16, - COCHRAN_MODEL_EMC_14, - COCHRAN_MODEL_EMC_16, - COCHRAN_MODEL_EMC_20, - COCHRAN_MODEL_COMMANDER_FAMILY = 2 << 16, - COCHRAN_MODEL_COMMANDER_AIR_NITROX, -} cochran_model_t; +#define COCHRAN_MODEL_UNKNOWN = -1, +#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 0 +#define COCHRAN_MODEL_EMC_14 1 +#define COCHRAN_MODEL_EMC_16 2 +#define COCHRAN_MODEL_EMC_20 3 typedef enum cochran_endian_t { ENDIAN_LE, ENDIAN_BE, } cochran_endian_t; -typedef enum cochran_addresssize_t { - ADDRESS_24_BIT, - ADDRESS_32_BIT, -} cochran_addresssize_t; - typedef enum cochran_sample_format_t { SAMPLE_UNKNOWN, SAMPLE_CMDR, @@ -58,8 +49,8 @@ typedef enum cochran_sample_format_t { // Used in reading the information from the device // and (in the case of max_*) used when decoding. typedef struct cochran_device_info_t { - cochran_model_t model; - cochran_addresssize_t address_size; + int model; + int address_bits; cochran_endian_t endian; int high_baud; @@ -68,8 +59,8 @@ typedef struct cochran_device_info_t { int rb_profile_begin; int rb_profile_end; - int log_size; - int max_log; + int logbook_entry_size; + int logbook_entry_count; int max_gas; cochran_sample_format_t sample_format; @@ -80,6 +71,7 @@ typedef struct cochran_conf_offsets_t { int dive_count; int last_log; int last_interdive; + int serial_number; } cochran_conf_offsets_t; // Offsets into each log block @@ -104,18 +96,11 @@ typedef struct cochran_log_offsets_t { } cochran_log_offsets_t; typedef struct cochran_data_t { - unsigned char id0[67]; - unsigned char id[67]; - unsigned char config[2][512]; - unsigned char misc[1500]; + unsigned char config[512]; unsigned char *logbook; unsigned char *sample; - unsigned int extra_id_flag; - unsigned int config_count; - unsigned short int dive_count; - unsigned char fingerprint[COCHRAN_FINGERPRINT_SIZE]; int fp_dive_num; int log_head, log_tail; // head and tail for log ringbuffer int profile_head, profile_tail; // Entries that have valid profiles @@ -133,14 +118,13 @@ struct event_size { typedef struct cochran_device_t { dc_device_t base; - const char *name; // serial port name serial_t *port; unsigned char model_string[COCHRAN_MODEL_SIZE + 1]; - cochran_device_info_t *info; - cochran_conf_offsets_t *conf_ptr; - cochran_log_offsets_t *log_ptr; - struct event_size (*event_bytes)[15]; - cochran_data_t data; // dive data used in parsing + unsigned char fingerprint[COCHRAN_FINGERPRINT_SIZE]; + const cochran_device_info_t *info; + const cochran_conf_offsets_t *conf_ptr; + const cochran_log_offsets_t *log_ptr; + const struct event_size (*event_bytes)[15]; } cochran_device_t; @@ -157,4 +141,4 @@ dc_status_t cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *data); int cochran_commander_model_id(const char *model); dc_status_t cochran_commander_info(const int model, - cochran_device_info_t **info, cochran_conf_offsets_t **conf_ptr, cochran_log_offsets_t **log_ptr, struct event_size (**event_bytes)[15]); + const cochran_device_info_t **info, const cochran_conf_offsets_t **conf_ptr, const cochran_log_offsets_t **log_ptr, const struct event_size (**event_bytes)[15]); diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index 45184fe..0100a94 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -81,9 +81,9 @@ typedef struct cochran_commander_parser_t cochran_commander_parser_t; struct cochran_commander_parser_t { dc_parser_t base; unsigned int model; - cochran_device_info_t *info; - cochran_log_offsets_t *log_ptr; - struct event_size (*event_bytes)[15]; + const cochran_device_info_t *info; + const cochran_log_offsets_t *log_ptr; + const struct event_size (*event_bytes)[15]; }; static dc_parser_vtable_t cochran_commander_parser_vtable = { @@ -136,7 +136,7 @@ cochran_commander_parser_get_datetime (dc_parser_t *abstract, dc_datetime_t *datetime) { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; - cochran_log_offsets_t *l = parser->log_ptr; + const cochran_log_offsets_t *l = parser->log_ptr; const unsigned char *log_entry = abstract->data; datetime->second = log_entry[l->sec]; @@ -220,7 +220,7 @@ cochran_commander_handle_event (dc_parser_t *abstract, * Used to find the end of a dive that has an incomplete dive-end * block. It parses backwards past inter-dive events. */ -int cochran_backparse(dc_parser_t *abstract, unsigned const char *samples, int size, struct event_size (*event_bytes)[15]) { +int cochran_backparse(dc_parser_t *abstract, unsigned const char *samples, int size, const struct event_size (*event_bytes)[15]) { int result = size, best_result = size; for (int x = 0; (*event_bytes)[x].code != -1; x++) { @@ -351,9 +351,9 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, const cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; const cochran_log_offsets_t *l = parser->log_ptr; const unsigned char *log_entry = abstract->data; - const unsigned char *samples = log_entry + parser->info->log_size; + const unsigned char *samples = log_entry + parser->info->logbook_entry_size; - unsigned int size = abstract->size - parser->info->log_size; + unsigned int size = abstract->size - parser->info->logbook_entry_size; int sampling_size; switch (parser->info->sample_format) { -- 2.4.3

1 0

[PATCH 1/2] Cochran: Addressed simpler code review issues
by John Van Ostrand 27 Feb '16

27 Feb '16

Mostly style, best practice issues and a few bugs. --- src/cochran_cmdr_parser.c | 135 +++++++++++++++---------- src/cochran_commander.c | 18 ++-- src/cochran_commander_parser.c | 79 +++++++++------ src/cochran_commander_parser.h | 104 ++++++++----------- src/cochran_emc_parser.c | 222 +++++++++++++++-------------------------- 5 files changed, 259 insertions(+), 299 deletions(-) diff --git a/src/cochran_cmdr_parser.c b/src/cochran_cmdr_parser.c index 4e2915b..20d4fd2 100644 --- a/src/cochran_cmdr_parser.c +++ b/src/cochran_cmdr_parser.c @@ -40,7 +40,8 @@ cochran_cmdr_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, unsigned int flags, void *value) { const unsigned char *data = abstract->data; - const unsigned char *log = data + COCHRAN_MODEL_SIZE; + const unsigned char *log_entry = data + COCHRAN_MODEL_SIZE; + unsigned int minutes = 0, qfeet = 0; dc_gasmix_t *gasmix = (dc_gasmix_t *) value; dc_salinity_t *water = (dc_salinity_t *) value; @@ -48,60 +49,72 @@ cochran_cmdr_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, if (value) { switch (type) { case DC_FIELD_TEMPERATURE_SURFACE: - *((unsigned int*) value) = ((float) log[CMD_START_TEMP] - 32) / 1.8; + *((unsigned int*) value) = (log_entry[CMD_START_TEMP] - 32.0) / 1.8; break; case DC_FIELD_TEMPERATURE_MINIMUM: - if (array_uint16_le(log + CMD_MIN_TEMP) == 0xFFFF) - *((double *) value) = 0; + if (log_entry[CMD_MIN_TEMP] == 0xFF) + return DC_STATUS_UNSUPPORTED; else - *((unsigned int*) value) = ((float) log[CMD_MIN_TEMP] / 2 + *((unsigned int*) value) = (log_entry[CMD_MIN_TEMP] / 2.0 + 20 - 32) / 1.8; break; case DC_FIELD_TEMPERATURE_MAXIMUM: - if (array_uint16_le(log + CMD_MAX_TEMP) == 0xFFFF) - *((double *) value) = 0; + if (log_entry[CMD_MAX_TEMP] == 0xFF) + return DC_STATUS_UNSUPPORTED; else - *((unsigned int*) value) = ((float) log[CMD_MAX_TEMP] / 2 + *((unsigned int*) value) = (log_entry[CMD_MAX_TEMP] / 2.0 + 20 - 32) / 1.8; break; case DC_FIELD_DIVETIME: - if (array_uint16_le(log + CMD_BT) == 0xFFFF) - *((double *) value) = 0; + minutes = array_uint16_le(log_entry + CMD_BT); + + if (minutes == 0xFFFF) + return DC_STATUS_UNSUPPORTED; else - *((unsigned int *) value) = array_uint16_le (log + CMD_BT) * 60; + *((unsigned int *) value) = minutes * 60; break; case DC_FIELD_MAXDEPTH: - if (array_uint16_le(log + CMD_MAX_DEPTH) == 0xFFFF) - *((double *) value) = 0; + qfeet = array_uint16_le(log_entry + CMD_MAX_DEPTH); + if (qfeet == 0xFFFF) + return DC_STATUS_UNSUPPORTED; else - *((double *) value) = (float) array_uint16_le (log - + CMD_MAX_DEPTH) / 4 * FEET; + *((double *) value) = qfeet / 4.0 * FEET; break; case DC_FIELD_AVGDEPTH: - if (array_uint16_le(log + CMD_AVG_DEPTH) == 0xFFFF) - *((double *) value) = 0; + qfeet = array_uint16_le(log_entry + CMD_AVG_DEPTH); + if (qfeet == 0xFFFF) + return DC_STATUS_UNSUPPORTED; else - *((double *) value) = (float) array_uint16_le (log - + CMD_AVG_DEPTH) / 4 * FEET; + *((double *) value) = qfeet / 4.0 * FEET; break; case DC_FIELD_GASMIX_COUNT: *((unsigned int *) value) = 2; break; case DC_FIELD_GASMIX: - gasmix->oxygen = (double) array_uint16_le (log - + CMD_O2_PERCENT + 2 * flags) / 256 / 100; + // Gas percentages are decimal and encoded as + // highbyte = integer portion + // lowbyte = decimal portion, divide by 256 to get decimal value + gasmix->oxygen = array_uint16_le (log_entry + + CMD_O2_PERCENT + 2 * flags) / 256.0 / 100; gasmix->helium = 0; gasmix->nitrogen = 1.0 - gasmix->oxygen - gasmix->helium; break; case DC_FIELD_SALINITY: - // 0 = low conductivity, 1 = high, maybe there's a 2? - water->type = ( (log[CMD_WATER_CONDUCTIVITY] & 0x3) == 0 + // 0x00 = low conductivity, 0x10 = high, maybe there's a 0x01 and 0x11? + // Assume Cochran's conductivity ranges from 0 to 3 + // 0 is fresh water, anything else is sea water + // for density assume + // 0 = 1000kg/m³, 2 = 1025kg/m³ + // and other values are linear + water->type = ( (log_entry[CMD_WATER_CONDUCTIVITY] & 0x3) == 0 ? DC_WATER_FRESH : DC_WATER_SALT ); - water->density = 1000 + 12.5 * (log[CMD_WATER_CONDUCTIVITY] & 0x3); + water->density = 1000 + 12.5 * (log_entry[CMD_WATER_CONDUCTIVITY] & 0x3); break; case DC_FIELD_ATMOSPHERIC: + // Cochran measures air pressure and stores it as altitude. + // Convert altitude (measured in 1/4 kilofeet) back to pressure. *(double *) value = ATM / BAR * pow(1 - 0.0000225577 - * (double) log[CMD_ALTITUDE] * 250 * FEET, 5.25588); + * log_entry[CMD_ALTITUDE] * 250.0 * FEET, 5.25588); break; default: return DC_STATUS_UNSUPPORTED; @@ -116,31 +129,34 @@ dc_status_t cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) { - const unsigned char *log = abstract->data + COCHRAN_MODEL_SIZE; - const unsigned char *samples = log + COCHRAN_CMDR_LOG_SIZE; + const unsigned char *log_entry = abstract->data + COCHRAN_MODEL_SIZE; + const unsigned char *samples = log_entry + COCHRAN_CMDR_LOG_SIZE; unsigned int size = abstract->size - COCHRAN_MODEL_SIZE - COCHRAN_CMDR_LOG_SIZE; - const unsigned char *s; - dc_sample_value_t sample = {0}, empty_sample = {0}; - unsigned int time = 0, last_sample_time; + dc_sample_value_t sample = {0}; + unsigned int time = 0, last_sample_time = 0; unsigned int offset = 0; double temperature; - double depth; + int depth_qfeet; double ascent_rate; unsigned char corrupt_dive = 0; - int cmdr_event_bytes[15][2] = { {0x00, 17}, {0x01, 21}, {0x02, 18}, - {0x03, 17}, {0x06, 19}, {0x07, 19}, - {0x08, 19}, {0x09, 19}, {0x0a, 19}, - {0x0b, 21}, {0x0c, 19}, {0x0d, 19}, - {0x0e, 19}, {0x10, 21}, - { -1, 1} }; - - if (array_uint32_le(log + COCHRAN_CMDR_LOG_SIZE / 2) == 0xFFFFFFFF) { + struct event_size cmdr_event_bytes[15] = { {0x00, 17}, {0x01, 21}, {0x02, 18}, + {0x03, 17}, {0x06, 19}, {0x07, 19}, + {0x08, 19}, {0x09, 19}, {0x0a, 19}, + {0x0b, 21}, {0x0c, 19}, {0x0d, 19}, + {0x0e, 19}, {0x10, 21}, + { -1, 1} }; + + // In rare circumstances Cochran computers won't record the end-of-dive + // log entry block. When the end-sample pointer is 0xFFFFFFFF it's corrupt. + // That means we don't really know where the dive samples end and we don't + // know what the dive summary values are (i.e. max depth, min temp) + if (array_uint32_le(log_entry + COCHRAN_CMDR_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; - WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples", log[CMD_YEAR], log[CMD_MON], log[CMD_DAY], log[CMD_HOUR], log[CMD_MIN], log[CMD_SEC]); + WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples", log_entry[CMD_YEAR], log_entry[CMD_MON], log_entry[CMD_DAY], log_entry[CMD_HOUR], log_entry[CMD_MIN], log_entry[CMD_SEC]); // Eliminate inter-dive events size = cochran_backparse(abstract, samples, size, &cmdr_event_bytes); @@ -150,19 +166,19 @@ cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, // and temp ever other second. // Prime values from the dive log section - depth = array_uint16_le (log + CMD_START_DEPTH) / 4; + depth_qfeet = array_uint16_le (log_entry + CMD_START_DEPTH); last_sample_time = sample.time = time; if (callback) callback (DC_SAMPLE_TIME, sample, userdata); - sample.depth = depth * FEET; + sample.depth = (double) depth_qfeet * FEET; if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); - sample.temperature = (float) *(log + CMD_START_TEMP) - 32 / 1.8; + sample.temperature = (log_entry[CMD_START_TEMP] - 32.0) / 1.8; if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); while (offset < size) { - sample = empty_sample; + const unsigned char *s = NULL; sample.time = time; if (callback && last_sample_time != sample.time) { @@ -175,10 +191,15 @@ cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, // If corrupt_dive end before offset if (corrupt_dive) { - if ( s[0] == 0x10 - || s[0] == 0xFF - || s[0] == 0xA8) { - break; + // When we aren't sure where the sample data ends we can + // look for events that shouldn't be in the sample data. + // 0xFF is unwritten memory + // 0xA8 indicates start of post-dive interval + // 0xE3 (switch to FO2 mode) and 0xF3 (switch to blend 1) occur + // at dive start so when we see them after the first second we + // found the beginning of the next dive. + if (s[0] == 0xFF || s[0] == 0xA8) { + break; } if (time > 1 && (s[0] == 0xE3 || s[0] == 0xF3)) break; @@ -187,25 +208,33 @@ cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, // Check for event if (s[0] & 0x80) { offset += cochran_commander_handle_event(abstract, callback, - userdata, s[0], offset, time); + userdata, s[0], time); continue; } // Depth is logged as change in feet, bit 0x40 means negative depth - depth += (float) (s[0] & 0x3F) / 4 * (s[0] & 0x40 ? -1 : 1); - sample.depth = depth * FEET; + if (s[0] & 0x40) + depth_qfeet -= (s[0] & 0x3f); + else + depth_qfeet += (s[0] & 0x3f); + + sample.depth = depth_qfeet / 4.0 * FEET; if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); // Ascent rate is logged in the 0th sample, temp in the 1st, repeat. if (time % 2 == 0) { // Ascent rate - ascent_rate = (float) (s[1] & 0x7f) / 4 * (s[1] & 0x80 ? 1 : -1); + if (s[1] & 0x80) + ascent_rate = (s[1] & 0x7f) / 4.0; + else + ascent_rate = - (s[1] & 0x7f) / 4.0; + sample.ascent_rate = ascent_rate * FEET; if (callback) callback (DC_SAMPLE_ASCENT_RATE, sample, userdata); } else { // Temperature logged in half degrees F above 20 - temperature = (float) s[1] / 2 + 20; + temperature = s[1] / 2.0 + 20; sample.temperature = (temperature - 32.0) / 1.8; if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); diff --git a/src/cochran_commander.c b/src/cochran_commander.c index ae2dc45..d11df92 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -855,7 +855,7 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *data) static unsigned int cochran_guess_sample_end_address(cochran_layout_t *layout, cochran_data_t *data, unsigned int log_num) { - const unsigned char *log = data->logbook + layout->rb_log_size * log_num; + const unsigned char *log_entry = data->logbook + layout->rb_log_size * log_num; if (log_num == data->dive_count) // Return next usable address from config0 page @@ -863,7 +863,7 @@ cochran_guess_sample_end_address(cochran_layout_t *layout, cochran_data_t *data, + layout->rb_profile_end); // Next log's start address - return array_uint32_le(log + layout->rb_log_size + 6); + return array_uint32_le(log_entry + layout->rb_log_size + 6); } @@ -878,7 +878,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, unsigned int sample_start_address, sample_end_address; dc_status_t rc; - unsigned char *log, *fingerprint, *sample, *dive; + unsigned char *log_entry, *fingerprint, *sample, *dive; int sample_size, dive_size; rc = cochran_commander_device_read_all (abstract); @@ -889,10 +889,10 @@ cochran_commander_device_foreach (dc_device_t *abstract, // Loop through each dive int i; for (i = data->dive_count - 1; i > data->fp_dive_num; i--) { - log = data->logbook + i * layout->rb_log_size; + log_entry = data->logbook + i * layout->rb_log_size; - sample_start_address = array_uint32_le (log + 6); - sample_end_address = array_uint32_le (log + layout->rb_log_size / 2); + sample_start_address = array_uint32_le (log_entry + 6); + sample_end_address = array_uint32_le (log_entry + layout->rb_log_size / 2); if (sample_end_address == 0xFFFFFFFF) // Corrupt dive, guess the end address @@ -901,7 +901,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, sample = data->sample + sample_start_address - data->sample_data_offset; // Determine size of sample - unsigned int dive_num =array_uint16_le(log + layout->pt_log_dive_number); + unsigned int dive_num =array_uint16_le(log_entry + layout->pt_log_dive_number); if (dive_num >= data->profile_tail) sample_size = sample_end_address - sample_start_address; else @@ -912,7 +912,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, sample_size += layout->rb_profile_end - layout->rb_profile_begin; - fingerprint = log + layout->pt_log_dive_number; + fingerprint = log_entry + layout->pt_log_dive_number; // Build dive blob dive_size = COCHRAN_MODEL_SIZE + layout->rb_log_size + sample_size; @@ -921,7 +921,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, return DC_STATUS_NOMEMORY; memcpy(dive, data->id + 0x3B, 8); // model string - memcpy(dive + COCHRAN_MODEL_SIZE, log, layout->rb_log_size); // log + memcpy(dive + COCHRAN_MODEL_SIZE, log_entry, layout->rb_log_size); // log // Copy profile data if (sample_size) { diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index e2b854c..a6380e4 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -43,9 +43,6 @@ static dc_status_t cochran_commander_parser_get_field (dc_parser_t *abstract, static dc_status_t cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata); -int cochran_commander_handle_event (dc_parser_t *abstract, - dc_sample_callback_t callback, void *userdata, unsigned char code, - unsigned int offset, unsigned int time); typedef struct cochran_commander_parser_t cochran_commander_parser_t; @@ -81,6 +78,8 @@ cochran_commander_parser_create (dc_parser_t **out, dc_context_t *context) return DC_STATUS_NOMEMORY; } + parser->layout = NULL; + *out = (dc_parser_t *) parser; return DC_STATUS_SUCCESS; @@ -92,12 +91,14 @@ cochran_commander_parser_set_data (dc_parser_t *abstract, const unsigned char *data, unsigned int size) { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; - abstract->data = data; - abstract->size = size; // Determine cochran data format // abstract->data is prefixed by the model string - parser->layout = cochran_commander_get_layout(data); + // FIXME: Instead of prefixing the model string, pass the model + // number to the cochran_commander_parser_create() function. That's + // how other backends do this. + if (size >= COCHRAN_MODEL_SIZE) + parser->layout = cochran_commander_get_layout(data); return DC_STATUS_SUCCESS; } @@ -110,22 +111,24 @@ cochran_commander_parser_get_datetime (dc_parser_t *abstract, { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; const unsigned char *data = abstract->data; - const unsigned char *log = data + COCHRAN_MODEL_SIZE; + const unsigned char *log_entry = data + COCHRAN_MODEL_SIZE; + + // FIXME: See my comment regarding a layout structure below. if (parser->layout->date_format == DATE_SMHDMY) { - datetime->second = log[0]; - datetime->minute = log[1]; - datetime->hour = log[2]; - datetime->day = log[3]; - datetime->month = log[4]; - datetime->year = log[5] + (log[5] > 91 ? 1900 : 2000); + datetime->second = log_entry[0]; + datetime->minute = log_entry[1]; + datetime->hour = log_entry[2]; + datetime->day = log_entry[3]; + datetime->month = log_entry[4]; + datetime->year = log_entry[5] + (log_entry[5] > 91 ? 1900 : 2000); } else { - datetime->second = log[1]; - datetime->minute = log[0]; - datetime->hour = log[3]; - datetime->day = log[2]; - datetime->month = log[5]; - datetime->year = log[4] + (log[5] > 91 ? 1900 : 2000); + datetime->second = log_entry[1]; + datetime->minute = log_entry[0]; + datetime->hour = log_entry[3]; + datetime->day = log_entry[2]; + datetime->month = log_entry[5]; + datetime->year = log_entry[4] + (log_entry[4] > 91 ? 1900 : 2000); } return DC_STATUS_SUCCESS; @@ -137,14 +140,20 @@ cochran_commander_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; + // FIXME: These two get_field functions appear very similar. I think + // you can easily merge them if you introduce a layout structure + // where you have the offset of each field. Check the uwatec parser + // with the uwatec_smart_header_info_t struct for an example. You + // already have the EMC_xxx and CMD_xxx macros. + + // FIXME: I suggest using the same model numbers as in descriptor.c + // and just list all models explictly with extra case statements. switch (parser->layout->model & 0xFF0000) { case COCHRAN_MODEL_COMMANDER_FAMILY: return cochran_cmdr_parser_get_field(abstract, type, flags, value); - break; case COCHRAN_MODEL_EMC_FAMILY: return cochran_emc_parser_get_field(abstract, type, flags, value); - break; } return DC_STATUS_UNSUPPORTED; @@ -156,6 +165,10 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; + // FIXME: Same remarks here. At first sight these two functions look + // again very similar. I wonder if we can merge them and deal with + // the difference by means of the info in a layout structure? + switch (parser->layout->model & 0xFF0000) { case COCHRAN_MODEL_COMMANDER_FAMILY: @@ -187,7 +200,7 @@ cochran_commander_get_event_info(const unsigned char code, int cochran_commander_handle_event (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata, unsigned char code, - unsigned int offset, unsigned int time) + unsigned int time) { dc_sample_value_t sample = {0}; cochran_events_t event; @@ -220,18 +233,15 @@ cochran_commander_handle_event (dc_parser_t *abstract, break; default: // Don't send known events of type NONE - if (! event.type == SAMPLE_EVENT_NONE) { + if (event.type != SAMPLE_EVENT_NONE) { sample.event.type = event.type; sample.event.flags = event.flag; if (callback) callback (DC_SAMPLE_EVENT, sample, userdata); } } } else { - // Unknown event, send it so we know we missed something - sample.event.type = SAMPLE_EVENT_NONE; - sample.event.flags = SAMPLE_FLAGS_NONE; - sample.event.value = code; - if (callback) callback (DC_SAMPLE_EVENT, sample, userdata); + // Unknown event, send warning so we know we missed something + WARNING(abstract->context, "Unkown event 0x%02x", code); } return event.data_bytes; @@ -243,13 +253,16 @@ cochran_commander_handle_event (dc_parser_t *abstract, * Used to find the end of a dive that has an incomplete dive-end * block. It parses backwards past inter-dive events. */ - -int cochran_backparse(dc_parser_t *abstract, const char *samples, int size, int (*event_bytes)[15][2]) { +int cochran_backparse(dc_parser_t *abstract, unsigned const char *samples, int size, struct event_size (*event_bytes)[15]) { int result = size, best_result = size; - for (int x = 0; (*event_bytes)[x][0] != -1; x++) { - int ptr = size - (*event_bytes)[x][1]; - if (ptr > 0 && samples[ptr] == (*event_bytes)[x][0]) { + for (int x = 0; (*event_bytes)[x].code != -1; x++) { + int ptr = size - (*event_bytes)[x].size; + if (ptr > 0 && samples[ptr] == (*event_bytes)[x].code) { + // Recurse to find the largest match. Because we are parsing backwards + // and the events vary in size we can't be sure the byte that matches + // the event code is an event code or data from inside a longer or shorter + // event. result = cochran_backparse(abstract, samples, ptr, event_bytes); } diff --git a/src/cochran_commander_parser.h b/src/cochran_commander_parser.h index 730edf7..0a125e5 100644 --- a/src/cochran_commander_parser.h +++ b/src/cochran_commander_parser.h @@ -22,70 +22,52 @@ typedef struct cochran_events_t { unsigned char code; unsigned char data_bytes; - const char *name; parser_sample_event_t type; parser_sample_flags_t flag; } cochran_events_t; +struct event_size { + int code; + int size; +}; + +// FIXME: Mark this variable as const! +// FIXME: This does not belong in a header file! Now you end up with +// multiple instances, one in each source file. +// FIXME: I actually prefer to see the emc and cmdr files merged into +// the commander file anyway. If you read my comments there, I think we +// can share most of the code there, so using separate files becomes +// pointless. And then the previous remark is no longer an issue. static cochran_events_t cochran_events[] = { - { 0xA8, 1, "Entered PDI mode", - SAMPLE_EVENT_SURFACE, SAMPLE_FLAGS_BEGIN }, - { 0xA9, 1, "Exited PDI mode", - SAMPLE_EVENT_SURFACE, SAMPLE_FLAGS_END }, - { 0xAB, 5, "Ceiling decrease", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xAD, 5, "Ceiling increase", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xBD, 1, "Switched to nomal PO2 setting", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xC0, 1, "Switched to FO2 21% mode", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xC1, 1, "Ascent rate greater than limit", - SAMPLE_EVENT_ASCENT, SAMPLE_FLAGS_BEGIN }, - { 0xC2, 1, "Low battery warning", - SAMPLE_EVENT_BATTERY, SAMPLE_FLAGS_NONE }, - { 0xC3, 1, "CNS Oxygen toxicity warning", - SAMPLE_EVENT_OLF, SAMPLE_FLAGS_NONE }, - { 0xC4, 1, "Depth exceeds user set point", - SAMPLE_EVENT_MAXDEPTH, SAMPLE_FLAGS_NONE }, - { 0xC5, 1, "Entered decompression mode", - SAMPLE_EVENT_DEEPSTOP, SAMPLE_FLAGS_BEGIN }, - { 0xC8, 1, "PO2 too high", - SAMPLE_EVENT_FLOOR, SAMPLE_FLAGS_BEGIN }, - { 0xCC, 1, "Low Cylinder 1 pressure", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN }, - { 0xCE, 1, "Non-decompression warning", - SAMPLE_EVENT_RBT, SAMPLE_FLAGS_BEGIN }, - { 0xCD, 1, "Switched to deco blend", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xD0, 1, "Breathing rate alarm", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN }, - { 0xD3, 1, "Low gas 1 flow rate", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xD6, 1, "Depth is less than ceiling", - SAMPLE_EVENT_CEILING, SAMPLE_FLAGS_BEGIN }, - { 0xD8, 1, "End decompression mode", - SAMPLE_EVENT_DEEPSTOP, SAMPLE_FLAGS_END }, - { 0xE1, 1, "End ascent rate warning", - SAMPLE_EVENT_ASCENT, SAMPLE_FLAGS_END }, - { 0xE2, 1, "Low SBAT battery warning", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xE3, 1, "Switched to FO2 mode", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xE5, 1, "Switched to PO2 mode", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xEE, 1, "End non-decompresison warning", - SAMPLE_EVENT_RBT, SAMPLE_FLAGS_END }, - { 0xEF, 1, "Switch to blend 2", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xF0, 1, "Breathing rate alarm", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_END }, - { 0xF3, 1, "Switch to blend 1", - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, - { 0xF6, 1, "End Depth is less than ceiling", - SAMPLE_EVENT_CEILING, SAMPLE_FLAGS_END }, - { 0x00, 1, NULL, - SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE } + { 0xA8, 1, SAMPLE_EVENT_SURFACE, SAMPLE_FLAGS_BEGIN }, // Entered PDI mode + { 0xA9, 1, SAMPLE_EVENT_SURFACE, SAMPLE_FLAGS_END }, // Exited PDI mode + { 0xAB, 5, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Ceiling decrease + { 0xAD, 5, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Ceiling increase + { 0xBD, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switched to nomal PO2 setting + { 0xC0, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switched to FO2 21% mode + { 0xC1, 1, SAMPLE_EVENT_ASCENT, SAMPLE_FLAGS_BEGIN }, // Ascent rate greater than limit + { 0xC2, 1, SAMPLE_EVENT_BATTERY, SAMPLE_FLAGS_NONE }, // Low battery warning + { 0xC3, 1, SAMPLE_EVENT_OLF, SAMPLE_FLAGS_NONE }, // CNS Oxygen toxicity warning + { 0xC4, 1, SAMPLE_EVENT_MAXDEPTH, SAMPLE_FLAGS_NONE }, // Depth exceeds user set point + { 0xC5, 1, SAMPLE_EVENT_DEEPSTOP, SAMPLE_FLAGS_BEGIN }, // Entered decompression mode + { 0xC8, 1, SAMPLE_EVENT_FLOOR, SAMPLE_FLAGS_BEGIN }, // PO2 too high + { 0xCC, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN }, // Low Cylinder 1 pressure + { 0xCE, 1, SAMPLE_EVENT_RBT, SAMPLE_FLAGS_BEGIN }, // Non-decompression warning + { 0xCD, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switched to deco blend + { 0xD0, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN }, // Breathing rate alarm + { 0xD3, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Low gas 1 flow rate + { 0xD6, 1, SAMPLE_EVENT_CEILING, SAMPLE_FLAGS_BEGIN }, // Depth is less than ceiling + { 0xD8, 1, SAMPLE_EVENT_DEEPSTOP, SAMPLE_FLAGS_END }, // End decompression mode + { 0xE1, 1, SAMPLE_EVENT_ASCENT, SAMPLE_FLAGS_END }, // End ascent rate warning + { 0xE2, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Low SBAT battery warning + { 0xE3, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switched to FO2 mode + { 0xE5, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switched to PO2 mode + { 0xEE, 1, SAMPLE_EVENT_RBT, SAMPLE_FLAGS_END }, // End non-decompresison warning + { 0xEF, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switch to blend 2 + { 0xF0, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_END }, // Breathing rate alarm + { 0xF3, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE }, // Switch to blend 1 + { 0xF6, 1, SAMPLE_EVENT_CEILING, SAMPLE_FLAGS_END }, // End Depth is less than ceiling + { 0x00, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_NONE } }; @@ -101,8 +83,8 @@ void cochran_commander_get_event_info(const unsigned char code, cochran_events_t *event); int cochran_commander_handle_event (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata, unsigned char code, - unsigned int offset, unsigned int time); -int cochran_backparse(dc_parser_t *abstract, const char *samples, int size, int (*event_bytes)[15][2]); + unsigned int time); +int cochran_backparse(dc_parser_t *abstract, unsigned const char *samples, int size, struct event_size (*event_bytes)[15]); // EMC FAMILY diff --git a/src/cochran_emc_parser.c b/src/cochran_emc_parser.c index c4a4d56..aae484f 100644 --- a/src/cochran_emc_parser.c +++ b/src/cochran_emc_parser.c @@ -35,103 +35,101 @@ #include "cochran_commander_parser.h" -struct dive_stats { - unsigned int dive_time; - float max_depth; - float avg_depth; - float min_temp; - float max_temp; -}; - // Size of inter-dive events -int emc_event_bytes[15][2] = { {0x00, 19}, {0x01, 23}, {0x02, 20}, +struct event_size emc_event_bytes[15] = { {0x00, 19}, {0x01, 23}, {0x02, 20}, {0x03, 19}, {0x06, 21}, {0x07, 21}, {0x0a, 21}, {0x0b, 21}, {0x0f, 19}, {0x10, 21}, { -1, 1} }; -static void cochran_emc_parse_dive_stats (dc_parser_t *abstract, - struct dive_stats *stats); - dc_status_t cochran_emc_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, unsigned int flags, void *value) { - const unsigned char *log = abstract->data + COCHRAN_MODEL_SIZE; + const unsigned char *log_entry = abstract->data + COCHRAN_MODEL_SIZE; unsigned char corrupt_dive = 0; - if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) + // FIXME: Is there a reason why you have this global check instead + // of checking the actual value for 0xFFFF, like you do in the + // commander function? + if (array_uint32_le(log_entry + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) corrupt_dive = 1; dc_gasmix_t *gasmix = (dc_gasmix_t *) value; dc_salinity_t *water = (dc_salinity_t *) value; - struct dive_stats stats; - if (value) { + + // Parse data to recreate dive summary information + sample_statistics_t parsed_stats = {0}; + if (corrupt_dive) + cochran_emc_parser_samples_foreach(abstract, sample_statistics_cb, (void *) &parsed_stats); + switch (type) { case DC_FIELD_TEMPERATURE_SURFACE: - *((unsigned int*) value) = ((float) log[EMC_START_TEMP] - 32) / 1.8; + *((unsigned int*) value) = (log_entry[EMC_START_TEMP] - 32.0) / 1.8; break; case DC_FIELD_TEMPERATURE_MINIMUM: if (corrupt_dive) { - cochran_emc_parse_dive_stats(abstract, &stats); - *((unsigned int*) value) = stats.min_temp; + return DC_STATUS_UNSUPPORTED; } else - *((unsigned int*) value) = ((float) log[EMC_MIN_TEMP] / 2 + *((unsigned int*) value) = (log_entry[EMC_MIN_TEMP] / 2.0 + 20 - 32) / 1.8; break; case DC_FIELD_TEMPERATURE_MAXIMUM: if (corrupt_dive) { - cochran_emc_parse_dive_stats(abstract, &stats); - *((unsigned int*) value) = stats.max_temp; + return DC_STATUS_UNSUPPORTED; } else - *((unsigned int*) value) = ((float) log[EMC_MAX_TEMP] / 2 + *((unsigned int*) value) = (log_entry[EMC_MAX_TEMP] / 2.0 + 20 - 32) / 1.8; break; case DC_FIELD_DIVETIME: if (corrupt_dive) { - cochran_emc_parse_dive_stats(abstract, &stats); - *((unsigned int*) value) = stats.dive_time; + *((unsigned int*) value) = parsed_stats.divetime; } else - *((unsigned int *) value) = array_uint16_le (log + EMC_BT) * 60; + *((unsigned int *) value) = array_uint16_le (log_entry + EMC_BT) * 60; break; case DC_FIELD_MAXDEPTH: if (corrupt_dive) { - cochran_emc_parse_dive_stats(abstract, &stats); - *((unsigned int*) value) = stats.max_depth; + *((unsigned int*) value) = parsed_stats.maxdepth; } else - *((double *) value) = array_uint16_le (log + EMC_MAX_DEPTH) / 4 + *((double *) value) = array_uint16_le (log_entry + EMC_MAX_DEPTH) / 4.0 * FEET; break; case DC_FIELD_AVGDEPTH: if (corrupt_dive) { - cochran_emc_parse_dive_stats(abstract, &stats); - *((unsigned int*) value) = stats.avg_depth; + return DC_STATUS_UNSUPPORTED; } else - *((double *) value) = array_uint16_le (log + EMC_AVG_DEPTH) / 4 + *((double *) value) = array_uint16_le (log_entry + EMC_AVG_DEPTH) / 4.0 * FEET; break; case DC_FIELD_GASMIX_COUNT: *((unsigned int *) value) = 10; break; case DC_FIELD_GASMIX: - gasmix->oxygen = (double) array_uint16_le (log - + EMC_O2_PERCENT + 2 * flags) / 256 / 100; - gasmix->helium = (double) array_uint16_le (log - + EMC_HE_PERCENT + 2 * flags) / 256 / 100; + // Gas percentages are decimal and encoded as + // highbyte = integer value + // lowbyte = decimal portion, divide by 256 to get decimal value + gasmix->oxygen = array_uint16_le (log_entry + + EMC_O2_PERCENT + 2 * flags) / 256.0 / 100; + gasmix->helium = array_uint16_le (log_entry + + EMC_HE_PERCENT + 2 * flags) / 256.0 / 100; gasmix->nitrogen = 1.0 - gasmix->oxygen - gasmix->helium; break; case DC_FIELD_SALINITY: - // 0 = low conductivity, 2 = high, maybe there's a 1? - water->type = ( (log[EMC_WATER_CONDUCTIVITY] & 0x3) == 0 + // 0x00 = low conductivity, 0x10 = high, maybe there's a 0x01 and 0x11? + water->type = ( (log_entry[EMC_WATER_CONDUCTIVITY] & 0x3) == 0 ? DC_WATER_FRESH : DC_WATER_SALT ); - water->density = 1000 + 12.5 * (log[EMC_WATER_CONDUCTIVITY] & 0x3); + // Assume Cochran's conductivity ranges from 0 to 3 + // so 0 = 1000kg/m³, 2 = 1025kg/m³ + water->density = 1000 + 12.5 * (log_entry[EMC_WATER_CONDUCTIVITY] & 0x3); break; case DC_FIELD_ATMOSPHERIC: + // Cochran measures atm pressure and stores it as altitude. + // Convert altitude (measured in 1/4 kilofeet) back to pressure. *(double *) value = ATM / BAR * pow(1 - 0.0000225577 - * (double) log[EMC_ALTITUDE] * 250 * FEET, 5.25588); + * log_entry[EMC_ALTITUDE] * 250.0 * FEET, 5.25588); break; default: return DC_STATUS_UNSUPPORTED; @@ -141,33 +139,37 @@ cochran_emc_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, return DC_STATUS_SUCCESS; } - +// TODO: Not reviewed in detail yet dc_status_t cochran_emc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) { const unsigned char *data = abstract->data; - const unsigned char *log = data + COCHRAN_MODEL_SIZE; - const unsigned char *samples = log + COCHRAN_EMC_LOG_SIZE; + const unsigned char *log_entry = data + COCHRAN_MODEL_SIZE; + const unsigned char *samples = log_entry + COCHRAN_EMC_LOG_SIZE; unsigned int size = abstract->size - COCHRAN_MODEL_SIZE - COCHRAN_EMC_LOG_SIZE; - const unsigned char *s; dc_sample_value_t sample = {0}, empty_sample = {0}; - unsigned int time = 0, last_sample_time; + unsigned int time = 0, last_sample_time = 0; unsigned int offset = 0; + int depth_qfeet = 0; double temperature; - double depth; + double start_depth = 0; double ascent_rate; unsigned char deco_obligation = 0; unsigned int deco_ceiling = 0; - unsigned int deco_time; + unsigned int deco_time = 0; unsigned char corrupt_dive = 0; - if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { + // In rare circumstances Cochran computers won't record the end-of-dive + // log entry block. When the end-sample pointer is 0xFFFFFFFF it's corrupt. + // That means we don't really know where the dive samples end and we don't + // know what the dive summary values are (i.e. max depth, min temp) + if (array_uint32_le(log_entry + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; - WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples.", log[EMC_YEAR], log[EMC_MON], log[EMC_DAY], log[EMC_HOUR], log[EMC_MIN], log[EMC_SEC]); + WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples.", log_entry[EMC_YEAR], log_entry[EMC_MON], log_entry[EMC_DAY], log_entry[EMC_HOUR], log_entry[EMC_MIN], log_entry[EMC_SEC]); // Eliminate inter-dive events size = cochran_backparse(abstract, samples, size, &emc_event_bytes); @@ -181,18 +183,20 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, */ // Prime with values from the dive log section - depth = array_uint16_le (log + EMC_START_DEPTH) / 256; + start_depth = array_uint16_le (log_entry + EMC_START_DEPTH) / 256; last_sample_time = sample.time = time; if (callback) callback (DC_SAMPLE_TIME, sample, userdata); - sample.depth = depth * FEET; + sample.depth = start_depth * FEET; if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); - sample.temperature = (log[EMC_START_TEMP] - 32) / 1.8; + sample.temperature = (log_entry[EMC_START_TEMP] - 32) / 1.8; if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); while (offset < size) { + const unsigned char *s = NULL; + sample = empty_sample; sample.time = time; @@ -206,9 +210,14 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, // If corrupt_dive end before offset if (corrupt_dive) { - if ( s[0] == 0x10 - || s[0] == 0xFF - || s[0] == 0xA8) { + // When we aren't sure where the sample data ends we can + // look for events that shouldn't be in the sample data. + // 0xFF is unwritten memory + // 0xA8 indicates start of post-dive interval + // 0xE3 (switch to FO2 mode) and 0xF3 (switch to blend 1) occur + // at dive start so when we see them after the first second we + // found the beginning of the next dive. + if (s[0] == 0xFF || s[0] == 0xA8) { break; } if (time > 1 && (s[0] == 0xE3 || s[0] == 0xF3)) @@ -218,7 +227,7 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, // Check for event if (s[0] & 0x80) { offset += cochran_commander_handle_event(abstract, callback, - userdata, s[0], offset, time); + userdata, s[0], time); switch (s[0]) { case 0xC5: // Deco obligation begins @@ -231,7 +240,7 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, deco_ceiling += 10; // feet sample.deco.type = DC_DECO_DECOSTOP; - sample.deco.time = (array_uint16_le(s + 3) + 1) * 60; + sample.deco.time = (array_uint16_le(s + 3) + 1) * 60; sample.deco.depth = deco_ceiling * FEET; if (callback) callback(DC_SAMPLE_DECO, sample, userdata); break; @@ -262,25 +271,33 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, // Depth is logged as change in feet, bit 0x40 means negative depth - depth += (float) (s[0] & 0x3F) / 4 * (s[0] & 0x40 ? -1 : 1); - sample.depth = depth * FEET; + if (s[0] & 0x40) + depth_qfeet -= (s[0] & 0x3f); + else + depth_qfeet += (s[0] & 0x3f); + + sample.depth = (start_depth + depth_qfeet / 4.0) * FEET; if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); // Ascent rate is logged in the 0th sample, temp in the 1st, repeat. if (time % 2 == 0) { // Ascent rate - ascent_rate = (float) (s[1] & 0x7f) / 4 * (s[1] & 0x80 ? 1 : -1); + if (s[1] & 0x80) + ascent_rate = (s[1] & 0x7f) / 4.0; + else + ascent_rate = - (s[1] & 0x7f) / 4.0; + sample.ascent_rate = ascent_rate * FEET; if (callback) callback (DC_SAMPLE_ASCENT_RATE, sample, userdata); } else { // Temperature logged in half degrees F above 20 - temperature = (float) s[1] / 2 + 20; + temperature = s[1] / 2.0 + 20; sample.temperature = (temperature - 32.0) / 1.8; if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); } - + // Some samples are split over two seconds // Skip over event bytes to find the next sample const unsigned char *n = s + COCHRAN_EMC_SAMPLE_SIZE; @@ -326,84 +343,3 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, return DC_STATUS_SUCCESS; } - -void cochran_emc_parse_dive_stats (dc_parser_t *abstract, - struct dive_stats *stats) -{ - const unsigned char *log = abstract->data + COCHRAN_MODEL_SIZE; - const unsigned char *samples = log + COCHRAN_EMC_LOG_SIZE; - unsigned int size = abstract->size - COCHRAN_MODEL_SIZE - - COCHRAN_EMC_LOG_SIZE; - const unsigned char *s; - - unsigned int offset = 0; - float depth_m = 0, depth, temp; - unsigned int time = 0; - unsigned char corrupt_dive = 0; - - if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { - corrupt_dive = 1; - - // Eliminate inter-dive events - size = cochran_backparse(abstract, samples, size, &emc_event_bytes); - } - - // Prime with values from the dive log section - depth = array_uint16_le (log + EMC_START_DEPTH) / 256; - - stats->max_depth = depth; - stats->min_temp = 999; - stats->avg_depth = 0; - stats->max_temp = 0; - - while (offset < size) { - s = samples + offset; - - if (corrupt_dive) { - if (s[0] == 0x10 - || s[0] == 0xFF - || s[0] == 0xA8) { - // End corrupted dive - break; - } - if (time > 1 && (s[0] == 0xE3 || s[0] == 0xF3)) - break; - } - - // Check for event - if (s[0] & 0x80) { - cochran_events_t event; - cochran_commander_get_event_info(s[0], &event); - - // Advance some bytes - if (event.code) - offset += event.data_bytes; - else - offset ++; - - continue; - } - - // Depth is logged as change in feet, bit 0x40 means negative depth - depth += (float) (s[0] & 0x3F) / 4 * (s[0] & 0x40 ? -1 : 1); - depth_m = depth * FEET; - - if (depth_m > stats->max_depth) stats->max_depth = depth_m; - - stats->avg_depth = (stats->avg_depth * (time - 1) + depth_m) / time; - - if (time % 2 != 0) { - // Temperature logged in half degrees F above 20 - temp = (float) s[1] / 2 + 20; - temp = (temp - 32.0) / 1.8; - - if (temp < stats->min_temp) stats->min_temp = temp; - if (temp > stats->max_temp) stats->max_temp = temp; - } - - time ++; - offset += COCHRAN_EMC_SAMPLE_SIZE; - } - - stats->dive_time = time - 1; -} -- 2.4.3

1 1

Re: Cochran Changes
by Jef Driesen 06 Feb '16

06 Feb '16

On 2016-01-29 10:55, john(a)vanostrand.com wrote: > On Wed, Jan 27, 2016 at 10:21 AM, Jef Driesen > <jef(a)libdivecomputer.org> wrote: >> The dc_device_foreach() function must return the dives in reverse >> chronological order. This is a requirement for the download only new >> dives feature. When dives are returned in reverse chronological >> order, an application (or libdivecomputer itself) can simply stop >> downloading dives as soon as a previously downloaded dive is >> recognized. Very simple and efficient. > > This is my loop in dc_device_foreach. > > int i; > for (i = data->dive_count - 1; i > data->fp_dive_num; i--) { Sorry, I missed that. >> Thus if we need to return the dives in reverse chronological order, >> it makes sense to process (and also download) the data in this >> order. Otherwise you'll end up with a rather inefficient >> implementation. >> >> I have to read through your documentation again, but processing the >> dives in reverse order might also make the recovery of corrupt dives >> easier. If the tail of a dive is missing, then it can run at most >> until the start of the next dive. And due the reverse order we >> already have that one. >> >>> 3. Corrupt dive handling. In some cases (like a low battery >>> especially in >>> cold water) the computer resets during a dive. This results in a >>> "start-dive" block written but no valid "end-dive" block written. >>> We know >>> information from the start of a dive (like date/time, gasses, >>> profile start >>> pointer, etc.) but we don't know information accumulated during or >>> at the >>> end of a dive (like end-profile pointer, max depth, min temp, >>> etc.) I've >>> taken to guessing the end of a dive by starting with the next >>> dive's >>> pre-dive-profile-pointer and backing up until we think we have the >>> previous >>> dive's end. We haven't resolved our differences on this. It seems >>> to down >>> to the question: Do we present a partial or broken profile in the >>> interest >>> of giving the diver something or do we give nothing in the >>> interest of >>> being accurate? >> >> That's a difficult question. In general, I prefer to be very strict >> and simply fail on unexpected data. Usually this is the correct >> thing to do, because such unexpected data often turns out to be an >> wrong assumption in the code. So being strict helps finding bugs. >> But sometimes the data is really wrong (due to a firmware bug, >> running out of battery during the dive, etc), and if it happens >> frequently, then a workaround might indeed be necessary. >> >> It also depends on where the data "corruption" is located. If the >> information needed to move from one dive to another is good, but we >> are unsure about the contents of the dive, then we can return the >> bogus dive to the application and let the parser deal with it. You >> might get incorrect data for that particular dive, or even a failure >> to parse the dive. This would ensure that we can still download the >> other dives. But if the primary structure is damaged and we can no >> longer safely move to the next dive, then I think we should fail. > > I've been working on code to retrieve corrupt dives and it's becoming > unwieldy. In working backwards from a future dive's profile I have to > remove inter-dive events which vary in length. it's possible that two > events might match the data and the code would have to decide which > event to use. The code also needs to do range checking, like ensuring > it doesn't exceed the malloc'ed memory and things like ring-buffer > wrapping. I should also remove any surface time (the time the DC still > stores samples in case you re-descend.) I think the problem of recovering corrupt samples becomes a lot easier if we can defer it to the parser. At the device layer we simply use the begin/end/pre-dive pointers. If the tail of the dive is missing (e.g. end pointer equal to 0xFFFFFFFF), we assume it runs to the start of the next dive. And then it's up to the parser to deal with this. In the parser we no longer have to worry about ringbuffer wrapping, only the end of the buffer. That should makes things already easier. > There are ways to make this better but I think I'm going to have to > think about it for a bit. I'm considering pulling that code. Any ideas > on what I should do with the code so it isn't lost. An #ifdef maybe? > > I've done some work and I may have a decent algorithm to backtrack > over the inter-dive events to get to good data. It goes as far as > removing good data in an effort to eliminate giving bad data. > > To refresh the issue: it has never been finding the beginning of data. > The issue has been finding the end. ‎We do have the start of the > next dive or, if no next dive, we have the ring buffer head pointer. > What happens if we simply use those pointers is that interdive data, > the data recorded between dives like power on events, is interpreted > as profile data. The profile then looks erratic at the end. So the > trick is to backtrack to find the real end of good data. Ideally wed > be able to do that accurately but if we have to remove good data to be > sure it would result in only loosing a few seconds of dive samples. > > Now that I've thought about the problem more and identified many of > the issues I have a standalone test program that's doing a good job of > backtracking to get to good data. I've used recursion to simplify the > code and derive at an optimal solution. In the dozen examples I'm > using it's working on 11 of them well enough to remove all non profile > data. The 12th leaves several seconds of non profile data. Don't we have the pre-dive pointer to indicate where the interdive data starts? Thus if the tail of a dive is missing, then the end is not really at the start of the next dive, but at the start of the interdive data. So the risk of trying to interpret interdive data as sample data disappears. Or am I missing something here? Jef

2 2

[PATCH 1/2] Cochran: Added warning when we process an incomplete dive.
by John Van Ostrand 06 Feb '16

06 Feb '16

--- src/cochran_cmdr_parser.c | 2 ++ src/cochran_emc_parser.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/cochran_cmdr_parser.c b/src/cochran_cmdr_parser.c index a7db1cd..4e2915b 100644 --- a/src/cochran_cmdr_parser.c +++ b/src/cochran_cmdr_parser.c @@ -140,6 +140,8 @@ cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, if (array_uint32_le(log + COCHRAN_CMDR_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; + WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples", log[CMD_YEAR], log[CMD_MON], log[CMD_DAY], log[CMD_HOUR], log[CMD_MIN], log[CMD_SEC]); + // Eliminate inter-dive events size = cochran_backparse(abstract, samples, size, &cmdr_event_bytes); } diff --git a/src/cochran_emc_parser.c b/src/cochran_emc_parser.c index 322f3d4..c891a2a 100644 --- a/src/cochran_emc_parser.c +++ b/src/cochran_emc_parser.c @@ -167,6 +167,8 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; + WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples\n", log[EMC_YEAR], log[EMC_MON], log[EMC_DAY], log[EMC_HOUR], log[EMC_MIN], log[EMC_SEC]); + // Eliminate inter-dive events size = cochran_backparse(abstract, samples, size, &emc_event_bytes); } -- 2.4.3

1 1

Cochran Simulator, tools and data
by John Van Ostrand 06 Feb '16

06 Feb '16

Jef, You may be interested in these files if you want to work with the Cochran code. simcochran.c is a simulator that now uses ptys so I can avoid having to use some serial port emulator. It's as easy as running: simcochran -e emc emc.tar.bz and cmdr.tar.bz are data files for use with simcochran and list. These include some incomplete dives. list.c is a program to list the dives from a memory file. This is handy for finding corrupt dives. list -c cmdr/memory -- John Van Ostrand At large on sabbatical

1 0

[PATCH] Cochran: Added back-parser to eliminate inter-dive events on incomplete dives.
by John Van Ostrand 06 Feb '16

06 Feb '16

Incomplete dives may have profile data but since we don't have an end-pointer, we have to use the next dive's pointer or ringbuffer head point in config to get close to the end of the dive. This often includes inter-dive events. We can eliminate those by parsing backwards. --- src/cochran_cmdr_parser.c | 14 ++++++++++++-- src/cochran_commander.c | 9 +++++++-- src/cochran_commander_parser.c | 26 +++++++++++++++++++++++++- src/cochran_commander_parser.h | 2 ++ src/cochran_emc_parser.c | 19 ++++++++++++++++--- 5 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/cochran_cmdr_parser.c b/src/cochran_cmdr_parser.c index 73b344f..a7db1cd 100644 --- a/src/cochran_cmdr_parser.c +++ b/src/cochran_cmdr_parser.c @@ -130,10 +130,20 @@ cochran_cmdr_parser_samples_foreach (dc_parser_t *abstract, double depth; double ascent_rate; unsigned char corrupt_dive = 0; - - if (array_uint32_le(log + COCHRAN_CMDR_LOG_SIZE / 2) == 0xFFFFFFFF) + int cmdr_event_bytes[15][2] = { {0x00, 17}, {0x01, 21}, {0x02, 18}, + {0x03, 17}, {0x06, 19}, {0x07, 19}, + {0x08, 19}, {0x09, 19}, {0x0a, 19}, + {0x0b, 21}, {0x0c, 19}, {0x0d, 19}, + {0x0e, 19}, {0x10, 21}, + { -1, 1} }; + + if (array_uint32_le(log + COCHRAN_CMDR_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; + // Eliminate inter-dive events + size = cochran_backparse(abstract, samples, size, &cmdr_event_bytes); + } + // Cochran samples depth every second and varies between ascent rate // and temp ever other second. diff --git a/src/cochran_commander.c b/src/cochran_commander.c index a8b2f4a..ae2dc45 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -195,8 +195,13 @@ cochran_packet (cochran_device_t *device, dc_event_progress_t *progress, rc = serial_configure(device->port, layout->high_baud_rate, 8, SERIAL_PARITY_NONE, 2, SERIAL_FLOWCONTROL_NONE); if (rc == -1) { - ERROR (abstract->context, "Failed to set the high baud rate."); - return DC_STATUS_IO; + // Assume we are talking to a pty, a simulator + rc = serial_configure(device->port, 115200, 8, SERIAL_PARITY_NONE, + 2, SERIAL_FLOWCONTROL_NONE); + if (rc == -1) { + ERROR (abstract->context, "Failed to set the high baud rate."); + return DC_STATUS_IO; + } } } diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index 955dba0..e2b854c 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -100,7 +100,7 @@ cochran_commander_parser_set_data (dc_parser_t *abstract, parser->layout = cochran_commander_get_layout(data); return DC_STATUS_SUCCESS; -} +} // There are two date formats used by Cochran @@ -236,3 +236,27 @@ cochran_commander_handle_event (dc_parser_t *abstract, return event.data_bytes; } + +/* +* cochran_backparse +* +* Used to find the end of a dive that has an incomplete dive-end +* block. It parses backwards past inter-dive events. +*/ + +int cochran_backparse(dc_parser_t *abstract, const char *samples, int size, int (*event_bytes)[15][2]) { + int result = size, best_result = size; + + for (int x = 0; (*event_bytes)[x][0] != -1; x++) { + int ptr = size - (*event_bytes)[x][1]; + if (ptr > 0 && samples[ptr] == (*event_bytes)[x][0]) { + result = cochran_backparse(abstract, samples, ptr, event_bytes); + } + + if (result < best_result) { + best_result = result; + } + } + + return best_result; +} diff --git a/src/cochran_commander_parser.h b/src/cochran_commander_parser.h index 4e9fc01..730edf7 100644 --- a/src/cochran_commander_parser.h +++ b/src/cochran_commander_parser.h @@ -102,6 +102,8 @@ void cochran_commander_get_event_info(const unsigned char code, int cochran_commander_handle_event (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata, unsigned char code, unsigned int offset, unsigned int time); +int cochran_backparse(dc_parser_t *abstract, const char *samples, int size, int (*event_bytes)[15][2]); + // EMC FAMILY dc_status_t cochran_emc_parser_get_field (dc_parser_t *abstract, diff --git a/src/cochran_emc_parser.c b/src/cochran_emc_parser.c index 884f8aa..322f3d4 100644 --- a/src/cochran_emc_parser.c +++ b/src/cochran_emc_parser.c @@ -43,6 +43,12 @@ struct dive_stats { float max_temp; }; +// Size of inter-dive events +int emc_event_bytes[15][2] = { {0x00, 19}, {0x01, 23}, {0x02, 20}, + {0x03, 19}, {0x06, 21}, {0x07, 21}, + {0x0a, 21}, {0x0b, 21}, {0x0f, 19}, + {0x10, 21}, + { -1, 1} }; static void cochran_emc_parse_dive_stats (dc_parser_t *abstract, struct dive_stats *stats); @@ -158,10 +164,13 @@ cochran_emc_parser_samples_foreach (dc_parser_t *abstract, unsigned int deco_time; unsigned char corrupt_dive = 0; - - if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) + if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; + // Eliminate inter-dive events + size = cochran_backparse(abstract, samples, size, &emc_event_bytes); + } + /* * Cochran samples depth every second and varies between ascent rate * and temp ever other second. @@ -330,9 +339,13 @@ void cochran_emc_parse_dive_stats (dc_parser_t *abstract, unsigned int time = 0; unsigned char corrupt_dive = 0; - if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) + if (array_uint32_le(log + COCHRAN_EMC_LOG_SIZE / 2) == 0xFFFFFFFF) { corrupt_dive = 1; + // Eliminate inter-dive events + size = cochran_backparse(abstract, samples, size, &emc_event_bytes); + } + // Prime with values from the dive log section depth = array_uint16_le (log + EMC_START_DEPTH) / 256; -- 2.4.3

1 0

← Newer
1
...
8
9
10
11
12
13
14
...
29
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

devel