[PATCH] Cochran: Code review changes
John Van Ostrand
john at vanostrand.com
Mon Jun 12 12:58:25 PDT 2017
Changed loops from do..while to for.
Changed time-related variable names to more customary names.
Changed datetime handling to streamline a clock-tick datetime decoding.
Create cochran_commander_read_retry() function to perform reads.
Correct device model strings.
Non-code review related:
Added new EMC device model string.
Created shorter variables for commonly used structure elements.
---
src/cochran_commander.c | 128 ++++++++++++++++++++++-------------------
src/cochran_commander_parser.c | 72 ++++++++++++-----------
2 files changed, 108 insertions(+), 92 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c
index 1c1009f..27ed67d 100644
--- a/src/cochran_commander.c
+++ b/src/cochran_commander.c
@@ -244,7 +244,10 @@ cochran_commander_get_model (cochran_commander_device_t *device)
{"\x11""21", COCHRAN_MODEL_COMMANDER_PRE21000},
{"\x11""22", COCHRAN_MODEL_COMMANDER_AIR_NITROX},
{"730", COCHRAN_MODEL_EMC_14},
+ {"731", COCHRAN_MODEL_EMC_14},
+ {"\x40""30", COCHRAN_MODEL_EMC_20},
{"A30", COCHRAN_MODEL_EMC_16},
+ {"A31", COCHRAN_MODEL_EMC_16},
{"230", COCHRAN_MODEL_EMC_20},
{"231", COCHRAN_MODEL_EMC_20},
};
@@ -473,6 +476,22 @@ cochran_commander_read (cochran_commander_device_t *device, dc_event_progress_t
return DC_STATUS_SUCCESS;
}
+static unsigned int
+cochran_commander_read_retry (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned int address, unsigned char data[], unsigned int size)
+{
+ unsigned int saved_current = progress->current;
+ dc_status_t rc;
+
+ for (unsigned int tries = 0; tries < 3; tries++) {
+ rc = cochran_commander_read (device, progress, address, data, size);
+ if (rc != DC_STATUS_SUCCESS)
+ progress->current = saved_current;
+ else
+ break;
+ }
+ return rc;
+}
+
/*
* For corrupt dives the end-of-samples pointer is 0xFFFFFFFF
@@ -574,9 +593,8 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d
// Loop through dives to find FP, Accumulate profile data size,
// and find the last dive with invalid profile
- unsigned int i = head_dive;
- do {
- i = ringbuffer_decrement(i, 1, 0, device->layout->rb_logbook_entry_count);
+ for (int x = dive_count; x >= 0; x--) {
+ unsigned int i = (device->layout->rb_logbook_entry_count + head_dive - (dive_count - x + 1)) % device->layout->rb_logbook_entry_count;
unsigned char *log_entry = data->logbook + i * device->layout->rb_logbook_entry_size;
@@ -600,7 +618,7 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d
// Accumulate read size for progress bar
sample_read_size += read_size;
}
- } while (i != tail_dive);
+ }
return sample_read_size;
}
@@ -623,7 +641,7 @@ cochran_commander_get_sample_parms(cochran_commander_device_t *device, cochran_d
unsigned int low_offset = 0xFFFFFFFF;
unsigned int high_offset = 0;
- for (int i = data->fp_dive_num + 1; i < dive_count; i++) {
+ for (unsigned int i = data->fp_dive_num + 1; i < dive_count; i++) {
pre_dive_offset = array_uint32_le (data->logbook + i * device->layout->rb_logbook_entry_size
+ device->layout->pt_profile_pre);
end_dive_offset = array_uint32_le (data->logbook + i * device->layout->rb_logbook_entry_size
@@ -833,15 +851,19 @@ static dc_status_t
cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t callback, void *userdata)
{
cochran_commander_device_t *device = (cochran_commander_device_t *) abstract;
+ const cochran_device_layout_t *layout = device->layout;
dc_status_t status = DC_STATUS_SUCCESS;
cochran_data_t data;
data.logbook = NULL;
+ // Often used variables
+ unsigned int rb_logbook_entry_count = layout->rb_logbook_entry_count;
+
// Calculate max data sizes
unsigned int max_config = sizeof(data.config);
- unsigned int max_logbook = device->layout->rb_logbook_end - device->layout->rb_logbook_begin;
- unsigned int max_sample = device->layout->rb_profile_end - device->layout->rb_profile_begin;
+ unsigned int max_logbook = layout->rb_logbook_end - layout->rb_logbook_begin;
+ unsigned int max_sample = layout->rb_profile_end - layout->rb_profile_begin;
// setup progress indication
dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER;
@@ -861,19 +883,19 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
return rc;
// Determine size of dive list to read.
- if (device->layout->endian == ENDIAN_LE)
- data.dive_count = array_uint16_le (data.config + device->layout->cf_dive_count);
+ if (layout->endian == ENDIAN_LE)
+ data.dive_count = array_uint16_le (data.config + layout->cf_dive_count);
else
- data.dive_count = array_uint16_be (data.config + device->layout->cf_dive_count);
+ data.dive_count = array_uint16_be (data.config + layout->cf_dive_count);
if (data.dive_count == 0)
// No dives to read
return DC_STATUS_SUCCESS;
- if (data.dive_count > device->layout->rb_logbook_entry_count) {
- data.logbook_size = device->layout->rb_logbook_entry_count * device->layout->rb_logbook_entry_size;
+ if (data.dive_count > rb_logbook_entry_count) {
+ data.logbook_size = rb_logbook_entry_count * layout->rb_logbook_entry_size;
} else {
- data.logbook_size = data.dive_count * device->layout->rb_logbook_entry_size;
+ data.logbook_size = data.dive_count * layout->rb_logbook_entry_size;
}
// Update progress indicator with new maximum
@@ -904,53 +926,57 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
// Emit a device info event.
dc_event_devinfo_t devinfo;
- devinfo.model = device->layout->model;
+ devinfo.model = layout->model;
devinfo.firmware = 0; // unknown
- if (device->layout->endian == ENDIAN_WORD_BE)
- devinfo.serial = array_uint32_word_be(data.config + device->layout->cf_serial_number);
+ if (layout->endian == ENDIAN_WORD_BE)
+ devinfo.serial = array_uint32_word_be(data.config + layout->cf_serial_number);
else
- devinfo.serial = array_uint32_le(data.config + device->layout->cf_serial_number);
+ devinfo.serial = array_uint32_le(data.config + layout->cf_serial_number);
device_event_emit (abstract, DC_EVENT_DEVINFO, &devinfo);
// Calculate profile RB effective head pointer
// Cochran seems to erase 8K chunks so round up.
unsigned int last_start_address;
- if (device->layout->endian == ENDIAN_WORD_BE)
- last_start_address = (array_uint32_word_be(data.config + device->layout->cf_last_interdive) & 0xfffff000) + 0x2000;
+ if (layout->endian == ENDIAN_WORD_BE)
+ last_start_address = (array_uint32_word_be(data.config + layout->cf_last_interdive) & 0xfffff000) + 0x2000;
else
- last_start_address = (array_uint32_le(data.config + device->layout->cf_last_interdive) & 0xfffff000) + 0x2000;
+ last_start_address = (array_uint32_le(data.config + layout->cf_last_interdive) & 0xfffff000) + 0x2000;
- if (last_start_address < device->layout->rb_profile_begin || last_start_address > device->layout->rb_profile_end) {
+ if (last_start_address < layout->rb_profile_begin || last_start_address > layout->rb_profile_end) {
ERROR(abstract->context, "Invalid profile ringbuffer head pointer in Cochran config block.");
status = DC_STATUS_DATAFORMAT;
goto error;
}
- unsigned int head_dive = 0, tail_dive = 0;
+ unsigned int head_dive = 0, tail_dive = 0, dive_count = 0;
- if (data.dive_count <= device->layout->rb_logbook_entry_count) {
+ if (data.dive_count <= rb_logbook_entry_count) {
head_dive = data.dive_count;
tail_dive = 0;
} else {
// Log wrapped
- tail_dive = data.dive_count % device->layout->rb_logbook_entry_count;
+ tail_dive = data.dive_count % rb_logbook_entry_count;
head_dive = tail_dive;
}
+
+ // Change tail to dive following the fingerprint dive.
if (data.fp_dive_num > -1)
- tail_dive = data.fp_dive_num + 1;
+ tail_dive = (data.fp_dive_num + 1) % rb_logbook_entry_count;
+
+ // Number of dives to read
+ dive_count = (rb_logbook_entry_count + head_dive - tail_dive) % rb_logbook_entry_count;
int invalid_profile_flag = 0;
// Loop through each dive
- unsigned int i = head_dive;
- do {
- i = ringbuffer_decrement(i, 1, 0, device->layout->rb_logbook_entry_count);
+ for (unsigned int x = dive_count; x > 0; x--) {
+ unsigned int i = (rb_logbook_entry_count + head_dive - (dive_count - x + 1)) % rb_logbook_entry_count;
- unsigned char *log_entry = data.logbook + i * device->layout->rb_logbook_entry_size;
+ unsigned char *log_entry = data.logbook + i * layout->rb_logbook_entry_size;
- unsigned int sample_start_address = array_uint32_le (log_entry + device->layout->pt_profile_begin);
- unsigned int sample_end_address = array_uint32_le (log_entry + device->layout->pt_profile_end);
+ unsigned int sample_start_address = array_uint32_le (log_entry + layout->pt_profile_begin);
+ unsigned int sample_end_address = array_uint32_le (log_entry + layout->pt_profile_end);
int sample_size = 0;
@@ -962,16 +988,15 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
sample_size = cochran_commander_profile_size(device, &data, i, PROFILE_SIZE_READ);
// Build dive blob
- unsigned int dive_size = device->layout->rb_logbook_entry_size + sample_size;
+ unsigned int dive_size = layout->rb_logbook_entry_size + sample_size;
unsigned char *dive = (unsigned char *) malloc(dive_size);
if (dive == NULL) {
status = DC_STATUS_NOMEMORY;
goto error;
}
- memcpy(dive, log_entry, device->layout->rb_logbook_entry_size); // log
+ memcpy(dive, log_entry, layout->rb_logbook_entry_size); // log
- int tries = 0;
// Read profile data
if (sample_size) {
if (sample_end_address == 0xFFFFFFFF)
@@ -979,55 +1004,42 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
sample_end_address = cochran_commander_guess_sample_end_address(device, &data, i);
if (sample_start_address <= sample_end_address) {
- do {
- int saved_progress = progress.current;
- rc = cochran_commander_read (device, &progress, sample_start_address, dive + device->layout->rb_logbook_entry_size, sample_size);
- if (rc != DC_STATUS_SUCCESS)
- progress.current = saved_progress;
- } while (rc != DC_STATUS_SUCCESS && tries++ < 3);
+ rc = cochran_commander_read_retry (device, &progress, sample_start_address, dive + layout->rb_logbook_entry_size, sample_size);
if (rc != DC_STATUS_SUCCESS) {
ERROR (abstract->context, "Failed to read the sample data.");
status = rc;
+ free(dive);
goto error;
}
} else {
// It wrapped the buffer, copy two sections
- unsigned int size = device->layout->rb_profile_end - sample_start_address;
-
- tries = 0;
- do {
- int saved_progress = progress.current;
- rc = cochran_commander_read (device, &progress, sample_start_address, dive + device->layout->rb_logbook_entry_size, size);
- if (rc != DC_STATUS_SUCCESS)
- progress.current = saved_progress;
- } while (rc != DC_STATUS_SUCCESS && tries++ < 3);
+ unsigned int size = layout->rb_profile_end - sample_start_address;
+
+ rc = cochran_commander_read_retry (device, &progress, sample_start_address, dive + layout->rb_logbook_entry_size, size);
if (rc != DC_STATUS_SUCCESS) {
ERROR (abstract->context, "Failed to read the sample data.");
status = rc;
+ free(dive);
goto error;
}
- tries = 0;
- do {
- int saved_progress = progress.current;
- rc = cochran_commander_read (device, &progress, device->layout->rb_profile_begin, dive + device->layout->rb_logbook_entry_size + size, sample_end_address - device->layout->rb_profile_begin);
- if (rc != DC_STATUS_SUCCESS)
- progress.current = saved_progress;
- } while (rc != DC_STATUS_SUCCESS && tries++ < 3);
+
+ rc = cochran_commander_read_retry (device, &progress, layout->rb_profile_begin, dive + layout->rb_logbook_entry_size + size, sample_end_address - layout->rb_profile_begin);
if (rc != DC_STATUS_SUCCESS) {
ERROR (abstract->context, "Failed to read the sample data.");
status = rc;
+ free(dive);
goto error;
}
}
}
- if (callback && !callback (dive, dive_size, dive + device->layout->pt_fingerprint, device->layout->fingerprint_size, userdata)) {
+ if (callback && !callback (dive, dive_size, dive + layout->pt_fingerprint, layout->fingerprint_size, userdata)) {
free(dive);
break;
}
free(dive);
- } while (i != tail_dive);
+ }
error:
free(data.logbook);
diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c
index d9521c0..484b195 100644
--- a/src/cochran_commander_parser.c
+++ b/src/cochran_commander_parser.c
@@ -39,7 +39,7 @@
#define COCHRAN_MODEL_EMC_20 4
// Cochran time stamps start at Jan 1, 1992
-#define COCHRAN_TIME_SHIFT 694242000
+#define COCHRAN_EPOCH 694242000
#define UNSUPPORTED 0xFFFFFFFF
@@ -50,8 +50,9 @@ typedef enum cochran_sample_format_t {
typedef enum cochran_date_encoding_t {
- DATE_ENCODING_ELEMENTAL,
- DATE_ENCODING_SECONDS_SINCE,
+ DATE_ENCODING_MSDHYM,
+ DATE_ENCODING_SMHDMY,
+ DATE_ENCODING_TICKS,
} cochran_date_encoding_t;
typedef struct cochran_parser_layout_t {
@@ -59,8 +60,7 @@ typedef struct cochran_parser_layout_t {
unsigned int headersize;
unsigned int samplesize;
cochran_date_encoding_t date_encoding;
- unsigned int second, minute, hour, day, month, year;
- unsigned int timestamp;
+ unsigned int datetime;
unsigned int pt_profile_begin;
unsigned int water_conductivity;
unsigned int pt_profile_pre;
@@ -118,9 +118,8 @@ static const cochran_parser_layout_t cochran_cmdr_1_parser_layout = {
SAMPLE_CMDR, // type
256, // headersize
2, // samplesize
- DATE_ENCODING_SECONDS_SINCE, // date_encoding
- 1, 0, 3, 2, 5, 4, // second, minute, hour, day, month, year, 1 byte each
- 8, // timestamp, 4 bytes
+ DATE_ENCODING_TICKS, // date_encoding
+ 8, // datetime, 4 bytes
0, // pt_profile_begin, 4 bytes
24, // water_conductivity, 1 byte, 0=low(fresh), 2=high(sea)
28, // pt_profile_pre, 4 bytes
@@ -143,9 +142,8 @@ static const cochran_parser_layout_t cochran_cmdr_parser_layout = {
SAMPLE_CMDR, // type
256, // headersize
2, // samplesize
- DATE_ENCODING_ELEMENTAL, // date_encoding
- 1, 0, 3, 2, 5, 4, // second, minute, hour, day, month, year, 1 byte each
- 0, // timestamp, 4 bytes
+ DATE_ENCODING_MSDHYM, // date_encoding
+ 0, // datetime, 6 bytes
6, // pt_profile_begin, 4 bytes
24, // water_conductivity, 1 byte, 0=low(fresh), 2=high(sea)
30, // pt_profile_pre, 4 bytes
@@ -168,9 +166,8 @@ static const cochran_parser_layout_t cochran_emc_parser_layout = {
SAMPLE_EMC, // type
512, // headersize
3, // samplesize
- DATE_ENCODING_ELEMENTAL, // date_encoding
- 0, 1, 2, 3, 4, 5, // second, minute, hour, day, month, year, 1 byte each
- 0, // timestamp, 4 bytes
+ DATE_ENCODING_SMHDMY, // date_encoding
+ 0, // datetime, 6 bytes
6, // pt_profile_begin, 4 bytes
24, // water_conductivity, 1 byte 0=low(fresh), 2=high(sea)
30, // pt_profile_pre, 4 bytes
@@ -387,25 +384,31 @@ cochran_commander_parser_get_datetime (dc_parser_t *abstract, dc_datetime_t *dat
if (abstract->size < layout->headersize)
return DC_STATUS_DATAFORMAT;
+ time_t ts;
+
if (datetime) {
- if (layout->date_encoding == DATE_ENCODING_SECONDS_SINCE) {
- time_t ts;
- struct tm *t;
- ts = array_uint32_le(data + layout->timestamp) + COCHRAN_TIME_SHIFT;
- t = localtime(&ts);
- datetime->second = t->tm_sec;
- datetime->minute = t->tm_min;
- datetime->hour = t->tm_hour;
- datetime->day = t->tm_mday;
- datetime->month = t->tm_mon + 1;
- datetime->year = 1900 + t->tm_year;
- } else {
- datetime->second = data[layout->second];
- datetime->minute = data[layout->minute];
- datetime->hour = data[layout->hour];
- datetime->day = data[layout->day];
- datetime->month = data[layout->month];
- datetime->year = data[layout->year] + (data[layout->year] > 91 ? 1900 : 2000);
+ switch (layout->date_encoding)
+ {
+ case DATE_ENCODING_MSDHYM:
+ datetime->second = data[layout->datetime + 1];
+ datetime->minute = data[layout->datetime + 0];
+ datetime->hour = data[layout->datetime + 3];
+ datetime->day = data[layout->datetime + 2];
+ datetime->month = data[layout->datetime + 5];
+ datetime->year = data[layout->datetime + 4] + (data[layout->datetime + 4] > 91 ? 1900 : 2000);
+ break;
+ case DATE_ENCODING_SMHDMY:
+ datetime->second = data[layout->datetime + 0];
+ datetime->minute = data[layout->datetime + 1];
+ datetime->hour = data[layout->datetime + 2];
+ datetime->day = data[layout->datetime + 3];
+ datetime->month = data[layout->datetime + 4];
+ datetime->year = data[layout->datetime + 5] + (data[layout->datetime + 5] > 91 ? 1900 : 2000);
+ break;
+ case DATE_ENCODING_TICKS:
+ ts = array_uint32_le(data + layout->datetime) + COCHRAN_EPOCH;
+ dc_datetime_localtime(datetime, ts);
+ break;
}
}
@@ -531,10 +534,11 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callb
// know what the dive summary values are (i.e. max depth, min temp)
if (array_uint32_le(data + layout->pt_profile_end) == 0xFFFFFFFF) {
corrupt_dive = 1;
+ dc_datetime_t d;
+ cochran_commander_parser_get_datetime(abstract, &d);
WARNING(abstract->context, "Incomplete dive on %02d/%02d/%02d at %02d:%02d:%02d, trying to parse samples",
- data[layout->year], data[layout->month], data[layout->day],
- data[layout->hour], data[layout->minute], data[layout->second]);
+ d.year, d.month, d.day, d.hour, d.minute, d.second);
// Eliminate inter-dive events
size = cochran_commander_backparse(parser, samples, size);
--
2.4.11
More information about the devel
mailing list