Since moving to per-dive download of profile data (and now rbstream download) the data->sample_data_offset and data->sample_size variables aren't used so calculating them doesn't make sense. --- src/cochran_commander.c | 68 ------------------------------------------------- 1 file changed, 68 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index e517027..3e125bb 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -59,9 +59,6 @@ typedef struct cochran_data_t { int invalid_profile_dive_num;
unsigned int logbook_size; - - unsigned int sample_data_offset; - unsigned int sample_size; } cochran_data_t;
typedef struct cochran_device_layout_t { @@ -608,69 +605,6 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d }
- -static void -cochran_commander_get_sample_parms(cochran_commander_device_t *device, cochran_data_t *data) -{ - dc_device_t *abstract = (dc_device_t *) device; - unsigned int pre_dive_offset = 0, end_dive_offset = 0; - - unsigned int dive_count = 0; - if (data->dive_count < device->layout->rb_logbook_entry_count) - dive_count = data->dive_count; - else - dive_count = device->layout->rb_logbook_entry_count; - - // Find lowest and highest offsets into sample data - unsigned int low_offset = 0xFFFFFFFF; - unsigned int high_offset = 0; - - 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 - + device->layout->pt_profile_end); - - // Validate offsets, allow 0xFFFFFFF for end_dive_offset - // because we handle that as a special case. - if (pre_dive_offset < device->layout->rb_profile_begin || - pre_dive_offset > device->layout->rb_profile_end) { - ERROR(abstract->context, "Invalid pre-dive offset (%08x) on dive %d.", pre_dive_offset, i); - continue; - } - - if (end_dive_offset < device->layout->rb_profile_begin || - (end_dive_offset > device->layout->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; - - if (pre_dive_offset < low_offset) - low_offset = pre_dive_offset; - if (end_dive_offset > high_offset && end_dive_offset != 0xFFFFFFFF ) - high_offset = end_dive_offset; - } - - if (pre_dive_offset > end_dive_offset) { - high_offset = device->layout->rb_profile_end; - low_offset = device->layout->rb_profile_begin; - data->sample_data_offset = low_offset; - data->sample_size = high_offset - low_offset; - } else if (low_offset < 0xFFFFFFFF && high_offset > 0) { - data->sample_data_offset = low_offset; - data->sample_size = high_offset - data->sample_data_offset; - } else { - data->sample_data_offset = 0; - data->sample_size = 0; - } -} - - dc_status_t cochran_commander_device_open (dc_device_t **out, dc_context_t *context, const char *name) { @@ -907,8 +841,6 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call progress.maximum -= (max_sample - profile_read_size); device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
- cochran_commander_get_sample_parms(device, &data); - // Emit a device info event. dc_event_devinfo_t devinfo; devinfo.model = layout->model;
No support but an initial patch to support Commander TM dive computers. --- src/cochran_commander.c | 38 +++++++++++++++++++++++++++++++++----- src/cochran_commander_parser.c | 1 + src/descriptor.c | 1 + 3 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index 3e125bb..bc10fc9 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -33,11 +33,12 @@
#define C_ARRAY_SIZE(array) (sizeof (array) / sizeof *(array))
-#define COCHRAN_MODEL_COMMANDER_PRE21000 0 -#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 1 -#define COCHRAN_MODEL_EMC_14 2 -#define COCHRAN_MODEL_EMC_16 3 -#define COCHRAN_MODEL_EMC_20 4 +#define COCHRAN_MODEL_COMMANDER_TM 0 +#define COCHRAN_MODEL_COMMANDER_PRE21000 1 +#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 2 +#define COCHRAN_MODEL_EMC_14 3 +#define COCHRAN_MODEL_EMC_16 4 +#define COCHRAN_MODEL_EMC_20 5
typedef enum cochran_endian_t { ENDIAN_LE, @@ -113,6 +114,29 @@ static const dc_device_vtable_t cochran_commander_device_vtable = { cochran_commander_device_close /* close */ };
+// Cochran Commander TM, pre-dates pre-21000 s/n +static const cochran_device_layout_t cochran_cmdr_tm_device_layout = { + COCHRAN_MODEL_COMMANDER_TM, // model + 24, // address_bits + ENDIAN_WORD_BE, // endian + 9600, // baudrate + 0x146, // cf_dive_count + 0x158, // cf_last_log + 0xffffff, // cf_last_interdive + 0x15c, // cf_serial_number + 0x000000, // rb_logbook_begin + 0x002328, // rb_logbook_end + 90, // rb_logbook_entry_size + 100, // rb_logbook_entry_count + 0x002328, // rb_profile_begin + 0x008000, // rb_profile_end + 15, // pt_fingerprint + 4, // fingerprint_size + 0, // pt_profile_pre + 0, // pt_profile_begin + 90, // pt_profile_end (Next begin pointer is the end) +}; + // Cochran Commander pre-21000 s/n static const cochran_device_layout_t cochran_cmdr_1_device_layout = { COCHRAN_MODEL_COMMANDER_PRE21000, // model @@ -240,6 +264,7 @@ static unsigned int cochran_commander_get_model (cochran_commander_device_t *device) { const cochran_commander_model_t models[] = { + {"\x01""12", COCHRAN_MODEL_COMMANDER_TM}, {"\x11""21", COCHRAN_MODEL_COMMANDER_PRE21000}, {"\x11""22", COCHRAN_MODEL_COMMANDER_AIR_NITROX}, {"730", COCHRAN_MODEL_EMC_14}, @@ -646,6 +671,9 @@ cochran_commander_device_open (dc_device_t **out, dc_context_t *context, const c
unsigned int model = cochran_commander_get_model(device); switch (model) { + case COCHRAN_MODEL_COMMANDER_TM: + device->layout = &cochran_cmdr_tm_device_layout; + break; case COCHRAN_MODEL_COMMANDER_PRE21000: device->layout = &cochran_cmdr_1_device_layout; break; diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index b58a6c4..b6ee17a 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -198,6 +198,7 @@ static const cochran_events_t cochran_events[] = { {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_NONE, SAMPLE_FLAGS_BEGIN}, // Entered decompression mode + {0xC7, 1, SAMPLE_EVENT_VIOLATION,SAMPLE_FLAGS_BEGIN}, // Entered Gauge mode (e.g. locked out) {0xC8, 1, SAMPLE_EVENT_PO2, SAMPLE_FLAGS_BEGIN}, // PO2 too high {0xCC, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN}, // Low Cylinder 1 pressure {0xCE, 1, SAMPLE_EVENT_NONE, SAMPLE_FLAGS_BEGIN}, // Non-decompression warning diff --git a/src/descriptor.c b/src/descriptor.c index 76157da..5093700 100644 --- a/src/descriptor.c +++ b/src/descriptor.c @@ -297,6 +297,7 @@ static const dc_descriptor_t g_descriptors[] = { {"Cochran", "EMC-14", DC_FAMILY_COCHRAN_COMMANDER, 2}, {"Cochran", "EMC-16", DC_FAMILY_COCHRAN_COMMANDER, 3}, {"Cochran", "EMC-20H", DC_FAMILY_COCHRAN_COMMANDER, 4}, + {"Cochran", "Commander TM", DC_FAMILY_COCHRAN_COMMANDER, 5}, };
typedef struct dc_descriptor_iterator_t {
On 2017-07-15 22:39, John Van Ostrand wrote:
-#define COCHRAN_MODEL_COMMANDER_PRE21000 0 -#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 1 -#define COCHRAN_MODEL_EMC_14 2 -#define COCHRAN_MODEL_EMC_16 3 -#define COCHRAN_MODEL_EMC_20 4 +#define COCHRAN_MODEL_COMMANDER_TM 0 +#define COCHRAN_MODEL_COMMANDER_PRE21000 1 +#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 2 +#define COCHRAN_MODEL_EMC_14 3 +#define COCHRAN_MODEL_EMC_16 4 +#define COCHRAN_MODEL_EMC_20 5
These values are now out of sync with the ones in src/descriptor.c:
--- a/src/descriptor.c +++ b/src/descriptor.c @@ -297,6 +297,7 @@ static const dc_descriptor_t g_descriptors[] = { {"Cochran", "EMC-14", DC_FAMILY_COCHRAN_COMMANDER, 2}, {"Cochran", "EMC-16", DC_FAMILY_COCHRAN_COMMANDER, 3}, {"Cochran", "EMC-20H", DC_FAMILY_COCHRAN_COMMANDER, 4},
- {"Cochran", "Commander TM", DC_FAMILY_COCHRAN_COMMANDER, 5},
};
That's not only confusing but also breaks the parser side when using the dc_parser_new2() function. That function obtains the model number from the descriptor, while the dc_parser_new() gets it from the devinfo event.
Jef
On Thu, Aug 10, 2017 at 10:55 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-07-15 22:39, John Van Ostrand wrote:
-#define COCHRAN_MODEL_COMMANDER_PRE21000 0 -#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 1 -#define COCHRAN_MODEL_EMC_14 2 -#define COCHRAN_MODEL_EMC_16 3 -#define COCHRAN_MODEL_EMC_20 4 +#define COCHRAN_MODEL_COMMANDER_TM 0 +#define COCHRAN_MODEL_COMMANDER_PRE21000 1 +#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 2 +#define COCHRAN_MODEL_EMC_14 3 +#define COCHRAN_MODEL_EMC_16 4 +#define COCHRAN_MODEL_EMC_20 5
These values are now out of sync with the ones in src/descriptor.c:
--- a/src/descriptor.c
+++ b/src/descriptor.c @@ -297,6 +297,7 @@ static const dc_descriptor_t g_descriptors[] = { {"Cochran", "EMC-14", DC_FAMILY_COCHRAN_COMMANDER, 2}, {"Cochran", "EMC-16", DC_FAMILY_COCHRAN_COMMANDER, 3}, {"Cochran", "EMC-20H", DC_FAMILY_COCHRAN_COMMANDER, 4},
{"Cochran", "Commander TM", DC_FAMILY_COCHRAN_COMMANDER, 5},};
That's not only confusing but also breaks the parser side when using the dc_parser_new2() function. That function obtains the model number from the descriptor, while the dc_parser_new() gets it from the devinfo event.
Okay, I did a re-arrangement, the wrong way again, based on the assumption that no Cochran users are storing dive blobs for later processing.
For previously supported Cochran computers high-speed read of log and profile data started at byte 0. Older models that lack the high-speed transfer function use the standard speed read commands and so the log and profile data are read at higher addresses. --- src/cochran_commander.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index bc10fc9..6b168ed 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -754,6 +754,7 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) cochran_commander_device_t *device = (cochran_commander_device_t *) abstract; dc_status_t rc = DC_STATUS_SUCCESS; unsigned char config[1024]; + unsigned int size = device->layout->rb_profile_end - device->layout->rb_logbook_begin;
// Make sure buffer is good. if (!dc_buffer_clear(buffer)) { @@ -762,14 +763,14 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) }
// Reserve space - if (!dc_buffer_resize(buffer, device->layout->rb_profile_end)) { + if (!dc_buffer_resize(buffer, size)) { ERROR(abstract->context, "Insufficient buffer space available."); return DC_STATUS_NOMEMORY; }
// Determine size for progress dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER; - progress.maximum = sizeof(config) + device->layout->rb_profile_end; + progress.maximum = sizeof(config) + size; device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
// Emit ID block @@ -783,7 +784,7 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) return rc;
// Read the sample data, from 0 to sample end will include logbook - rc = cochran_commander_read (device, &progress, 0, dc_buffer_get_data(buffer), device->layout->rb_profile_end); + rc = cochran_commander_read (device, &progress, device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
On 2017-07-15 22:39, John Van Ostrand wrote:
For previously supported Cochran computers high-speed read of log and profile data started at byte 0. Older models that lack the high-speed transfer function use the standard speed read commands and so the log and profile data are read at higher addresses.
I don't really understand the reason for this change. With this change you are only downloading the memory area containing the logbook and profile ringbuffers, and not a full memory dump. Can you explain why you changed this?
Jef
On Thu, Aug 10, 2017 at 10:57 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-07-15 22:39, John Van Ostrand wrote:
For previously supported Cochran computers high-speed read of log and profile data started at byte 0. Older models that lack the high-speed transfer function use the standard speed read commands and so the log and profile data are read at higher addresses.
I don't really understand the reason for this change. With this change you are only downloading the memory area containing the logbook and profile ringbuffers, and not a full memory dump. Can you explain why you changed this?
I figured I should start staging changes into patches that are more easily reviewed and doing it in a way that still results in a good build in case a build occurs at the commit. This patch is a staging patch intended to set a path for support for Commander TM computers. This patch also fixes an assumption I made in _device_dump.
Prior to this patch the read command that read the log and profile data always had addresses that started at 0. Log data always started at 0 and profile data followed after. So layout->rb_logbook_begin for every computer was always 0x0. In _device_dump I should have used this variable but I used a hard coded 0. Using the variable makes more sense.
In effect, this patch doesn't change what the dump contains for computers supported at the time of this patch. A dump done after this patch matches a dump done prior to it.
As to downloading *all* memory I presumed it wasn't supposed to dump all memory. I presume it was only supposed to dump log and profile data.
There may be two reasons why I thought the device_dump was only intended to dump the user data, i.e. logbook and profile data. Prior to my patches currently under review, the code uses what I now realize is a high-speed read function which seems intended to only download log and profile data probably because of the large size. I was confused when working with the older Commander TM models because the high-speed download commands didn't work and I had no reference to how to download it (i.e. no vendor program to observe.) Then I decided to brute force it recently, take the DC apart, obtain the IC specs, and try a wide range of commands. I realized that the read commands I've been using to access some data on new models were generic low-speed read commands and I could use them to access all memory areas on the older computer. Because it had randomized SRAM logbook and profile data it took a while to find the user data. It starts at 0x10000.
I could read all data on the old computer and dump all that data but there are also 32K of RAM and ROM which I figure only confuses the data and doesn't help the user much. RAM, being RAM, changes a lot.
That said, I've started experimenting with a simulator that uses a full memory dump so there might be some benefit to changing this. The new simulator would be simplified and might be more robust when working with a less predictable program like the vendor's software.
On 2017-08-10 18:01, John Van Ostrand wrote:
On Thu, Aug 10, 2017 at 10:57 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-07-15 22:39, John Van Ostrand wrote:
For previously supported Cochran computers high-speed read of log and profile data started at byte 0. Older models that lack the high-speed transfer function use the standard speed read commands and so the log and profile data are read at higher addresses.
I don't really understand the reason for this change. With this change you are only downloading the memory area containing the logbook and profile ringbuffers, and not a full memory dump. Can you explain why you changed this?
I figured I should start staging changes into patches that are more easily reviewed and doing it in a way that still results in a good build in case a build occurs at the commit. This patch is a staging patch intended to set a path for support for Commander TM computers. This patch also fixes an assumption I made in _device_dump.
Prior to this patch the read command that read the log and profile data always had addresses that started at 0. Log data always started at 0 and profile data followed after. So layout->rb_logbook_begin for every computer was always 0x0. In _device_dump I should have used this variable but I used a hard coded 0. Using the variable makes more sense.
In effect, this patch doesn't change what the dump contains for computers supported at the time of this patch. A dump done after this patch matches a dump done prior to it.
I know the behavior remains the same for the existing devices. That's quite obvious when reading the code. But that's not what my question was about. It's about this part of your answer:
As to downloading *all* memory I presumed it wasn't supposed to dump all memory. I presume it was only supposed to dump log and profile data.
There may be two reasons why I thought the device_dump was only intended to dump the user data, i.e. logbook and profile data. Prior to my patches currently under review, the code uses what I now realize is a high-speed read function which seems intended to only download log and profile data probably because of the large size. I was confused when working with the older Commander TM models because the high-speed download commands didn't work and I had no reference to how to download it (i.e. no vendor program to observe.) Then I decided to brute force it recently, take the DC apart, obtain the IC specs, and try a wide range of commands. I realized that the read commands I've been using to access some data on new models were generic low-speed read commands and I could use them to access all memory areas on the older computer. Because it had randomized SRAM logbook and profile data it took a while to find the user data. It starts at 0x10000.
I could read all data on the old computer and dump all that data but there are also 32K of RAM and ROM which I figure only confuses the data and doesn't help the user much. RAM, being RAM, changes a lot.
That said, I've started experimenting with a simulator that uses a full memory dump so there might be some benefit to changing this. The new simulator would be simplified and might be more robust when working with a less predictable program like the vendor's software.
Memory dumps are not only useful for debugging today's problems. They can also help future development (developing new features, regression testing against old data, etc). But if you only download part of the memory, and it turns out the ringbuffer starts at some other address then your initial assumption, or your new feature needs some info from another area, then the old memory dump is suddenly useless. That's why a memory dump should (ideally) contain everything, and also require as little logic as possible (e.g. no knowledge of a certain address range).
A memory dump typically contains other sections besides the logbook and profile ringbuffer(s) too. For example:
* Ringbuffer metadata (pointers where the next item should be written) * Configuration data (user configurable settings) * Dive computer state (decompression tissue loadings, etc) * ... (lots of things we don't know about)
There is probably lots of stuff we aren't interested in, but that's difficult to tell in advance if you don't even know what's in there. So making sure to include everything is the most future proof :-)
Having said that, if the download protocol uses addresses where certain ranges are mapped to the ROM (firmware code) or even RAM, then that's indeed another story. We're not really interested in that kind of stuff. Just the flash (or some other non-volatile storage technology) where the dives are being stored. But so far I've never seen any dive computer where the download protocol gives access to that kind of data (except for the ostc firmware updater).
Have you checked (on the newer devices), whether a read operation at the same address with the slow and fast commands, yields the same data?
Jef
On Fri, Aug 11, 2017 at 10:33 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-08-10 18:01, John Van Ostrand wrote:
On Thu, Aug 10, 2017 at 10:57 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-07-15 22:39, John Van Ostrand wrote:
For previously supported Cochran computers high-speed read of
log and profile data started at byte 0. Older models that lack the high-speed transfer function use the standard speed read commands and so the log and profile data are read at higher addresses.
I don't really understand the reason for this change. With this change you are only downloading the memory area containing the logbook and profile ringbuffers, and not a full memory dump. Can you explain why you changed this?
I figured I should start staging changes into patches that are more easily reviewed and doing it in a way that still results in a good build in case a build occurs at the commit. This patch is a staging patch intended to set a path for support for Commander TM computers. This patch also fixes an assumption I made in _device_dump.
Prior to this patch the read command that read the log and profile data always had addresses that started at 0. Log data always started at 0 and profile data followed after. So layout->rb_logbook_begin for every computer was always 0x0. In _device_dump I should have used this variable but I used a hard coded 0. Using the variable makes more sense.
In effect, this patch doesn't change what the dump contains for computers supported at the time of this patch. A dump done after this patch matches a dump done prior to it.
I know the behavior remains the same for the existing devices. That's quite obvious when reading the code. But that's not what my question was about. It's about this part of your answer:
As to downloading *all* memory I presumed it wasn't supposed to dump all
memory. I presume it was only supposed to dump log and profile data.
There may be two reasons why I thought the device_dump was only intended to dump the user data, i.e. logbook and profile data. Prior to my patches currently under review, the code uses what I now realize is a high-speed read function which seems intended to only download log and profile data probably because of the large size. I was confused when working with the older Commander TM models because the high-speed download commands didn't work and I had no reference to how to download it (i.e. no vendor program to observe.) Then I decided to brute force it recently, take the DC apart, obtain the IC specs, and try a wide range of commands. I realized that the read commands I've been using to access some data on new models were generic low-speed read commands and I could use them to access all memory areas on the older computer. Because it had randomized SRAM logbook and profile data it took a while to find the user data. It starts at 0x10000.
I could read all data on the old computer and dump all that data but there are also 32K of RAM and ROM which I figure only confuses the data and doesn't help the user much. RAM, being RAM, changes a lot.
That said, I've started experimenting with a simulator that uses a full memory dump so there might be some benefit to changing this. The new simulator would be simplified and might be more robust when working with a less predictable program like the vendor's software.
Memory dumps are not only useful for debugging today's problems. They can also help future development (developing new features, regression testing against old data, etc). But if you only download part of the memory, and it turns out the ringbuffer starts at some other address then your initial assumption, or your new feature needs some info from another area, then the old memory dump is suddenly useless. That's why a memory dump should (ideally) contain everything, and also require as little logic as possible (e.g. no knowledge of a certain address range).
A memory dump typically contains other sections besides the logbook and profile ringbuffer(s) too. For example:
- Ringbuffer metadata (pointers where the next item should be written)
- Configuration data (user configurable settings)
- Dive computer state (decompression tissue loadings, etc)
- ... (lots of things we don't know about)
There is probably lots of stuff we aren't interested in, but that's difficult to tell in advance if you don't even know what's in there. So making sure to include everything is the most future proof :-)
The ID, conf, and misc blocks should give us everything we want. ID tells us the model and f/w version (I think), conf pages give us the configurable settings and I think misc gives us tissue loading. The original code was dumping all this. I'm not sure why it was removed.
Having said that, if the download protocol uses addresses where certain
ranges are mapped to the ROM (firmware code) or even RAM, then that's indeed another story. We're not really interested in that kind of stuff. Just the flash (or some other non-volatile storage technology) where the dives are being stored. But so far I've never seen any dive computer where the download protocol gives access to that kind of data (except for the ostc firmware updater).
I disassembled the Commander TM to identify the MCU. In the downloads I can see memory structures that match what the specs suggest should be there, the interrupt table is visible at the beginning of ROM. I've done captures minutes apart and compared the results. The parts that change correspond strongly to the RAM, LCD RAM, and internal IO registers outlined in the MCU spec, things expected to be dynamic. So I'm pretty sure we're looking at the entire 32K address space of the MCU. I've not gathered the courage to disassemble my expensive Cochran computers (or borrowed ones) so I'm guessing a little more with those.
There are two write commands that I'm aware of. One appears to be able to address all mapped space. It's how the time is changed. The format is:
0xa0 0xXX 0xYY 0xZZ 0xVV 0xWW (write 0xWWVV bytes (following command) to address 0xZZYYXX)
Like the slow read command this one is supported across all DCs I've tested.
Have you checked (on the newer devices), whether a read operation at the same address with the slow and fast commands, yields the same data?
I have. Slow reads seem to read the MCU addressing space. The EMC-20H looks very similar to the TM (same MCU?) The Commander II appears slightly different (different MCU?) but it also appears to be the full address space.
Typo on ID string. Addresses for old commander set to use actual read addresses instead of starting at 0. --- src/cochran_commander.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index 6b168ed..9655522 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -124,12 +124,12 @@ static const cochran_device_layout_t cochran_cmdr_tm_device_layout = { 0x158, // cf_last_log 0xffffff, // cf_last_interdive 0x15c, // cf_serial_number - 0x000000, // rb_logbook_begin - 0x002328, // rb_logbook_end + 0x010000, // rb_logbook_begin + 0x012328, // rb_logbook_end 90, // rb_logbook_entry_size 100, // rb_logbook_entry_count - 0x002328, // rb_profile_begin - 0x008000, // rb_profile_end + 0x012328, // rb_profile_begin + 0x020000, // rb_profile_end 15, // pt_fingerprint 4, // fingerprint_size 0, // pt_profile_pre @@ -264,7 +264,7 @@ static unsigned int cochran_commander_get_model (cochran_commander_device_t *device) { const cochran_commander_model_t models[] = { - {"\x01""12", COCHRAN_MODEL_COMMANDER_TM}, + {"\x0a""12", COCHRAN_MODEL_COMMANDER_TM}, {"\x11""21", COCHRAN_MODEL_COMMANDER_PRE21000}, {"\x11""22", COCHRAN_MODEL_COMMANDER_AIR_NITROX}, {"730", COCHRAN_MODEL_EMC_14},
- TM doesn't support high-speed transfer so - Use 0x05 read command - Don't change to higher baud rate - Still reset to 9600 so we wait for heartbeat - TM has a different config command (one byte) - TM has only one config page - Alter cochran_commander_read_config function - Assume 512 config pages, because it seems standard - Remove size parameter to function since we assume 512 - Alter progress maximum calculation to reflect single page --- src/cochran_commander.c | 69 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 48 insertions(+), 21 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index 9655522..a77eabb 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -357,7 +357,7 @@ cochran_commander_packet (cochran_commander_device_t *device, dc_event_progress_ } }
- if (high_speed) { + if (high_speed && device->layout->baudrate != 9600) { // Give the DC time to process the command. dc_serial_sleep(device->port, 45);
@@ -421,23 +421,31 @@ cochran_commander_read_id (cochran_commander_device_t *device, unsigned char id[
static dc_status_t -cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[], unsigned int size) +cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[]) { dc_device_t *abstract = (dc_device_t *) device; dc_status_t rc = DC_STATUS_SUCCESS;
+ unsigned int pages = 2; + + if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) + pages = 1; + // Read two 512 byte blocks into one 1024 byte buffer - for (unsigned int i = 0; i < 2; i++) { - const unsigned int len = size / 2; + for (unsigned int i = 0; i < pages; i++) {
unsigned char command[2] = {0x96, i}; - rc = cochran_commander_packet(device, progress, command, sizeof(command), data + i * len, len, 0); + unsigned int command_size = 2; + if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) + command_size = 1; + + rc = cochran_commander_packet(device, progress, command, command_size, data + i * 512, 512, 0); if (rc != DC_STATUS_SUCCESS) return rc;
dc_event_vendor_t vendor; - vendor.data = data + i * len; - vendor.size = len; + vendor.data = data + i * 512; + vendor.size = 512; device_event_emit (abstract, DC_EVENT_VENDOR, &vendor); }
@@ -471,15 +479,27 @@ cochran_commander_read (cochran_commander_device_t *device, dc_event_progress_t break; case 24: // Commander uses 24 byte addressing - command[0] = 0x15; - command[1] = (address ) & 0xff; - command[2] = (address >> 8) & 0xff; - command[3] = (address >> 16) & 0xff; - command[4] = (size ) & 0xff; - command[5] = (size >> 8 ) & 0xff; - command[6] = (size >> 16 ) & 0xff; - command[7] = 0x04; - command_size = 8; + if (device->layout->baudrate == 9600) { + // Older commander, use low-speed read command + command[0] = 0x05; + command[1] = (address ) & 0xff; + command[2] = (address >> 8) & 0xff; + command[3] = (address >> 16) & 0xff; + command[4] = (size ) & 0xff; + command[5] = (size >> 8 ) & 0xff; + command_size = 6; + } else { + // Newer commander with high-speed read command + command[0] = 0x15; + command[1] = (address ) & 0xff; + command[2] = (address >> 8) & 0xff; + command[3] = (address >> 16) & 0xff; + command[4] = (size ) & 0xff; + command[5] = (size >> 8 ) & 0xff; + command[6] = (size >> 16 ) & 0xff; + command[7] = 0x04; + command_size = 8; + } break; default: return DC_STATUS_UNSUPPORTED; @@ -754,6 +774,7 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) cochran_commander_device_t *device = (cochran_commander_device_t *) abstract; dc_status_t rc = DC_STATUS_SUCCESS; unsigned char config[1024]; + unsigned int config_size = 1024; unsigned int size = device->layout->rb_profile_end - device->layout->rb_logbook_begin;
// Make sure buffer is good. @@ -768,9 +789,12 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) return DC_STATUS_NOMEMORY; }
+ if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) + config_size = 512; + // Determine size for progress dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER; - progress.maximum = sizeof(config) + size; + progress.maximum = config_size + size; device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
// Emit ID block @@ -779,12 +803,12 @@ cochran_commander_device_dump (dc_device_t *abstract, dc_buffer_t *buffer) vendor.size = sizeof (device->id); device_event_emit (abstract, DC_EVENT_VENDOR, &vendor);
- rc = cochran_commander_read_config (device, &progress, config, sizeof(config)); + rc = cochran_commander_read_config (device, &progress, config); if (rc != DC_STATUS_SUCCESS) return rc;
- // Read the sample data, from 0 to sample end will include logbook - rc = cochran_commander_read (device, &progress, device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end); + // Read the sample data, logbook and sample data are contiguous + rc = cochran_commander_read (device, &progress, device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc; @@ -813,6 +837,9 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call unsigned int max_logbook = layout->rb_logbook_end - layout->rb_logbook_begin; unsigned int max_sample = layout->rb_profile_end - layout->rb_profile_begin;
+ if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) + max_config = 512; + // setup progress indication dc_event_progress_t progress = EVENT_PROGRESS_INITIALIZER; progress.maximum = max_config + max_logbook + max_sample; @@ -826,7 +853,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
// Read config dc_status_t rc = DC_STATUS_SUCCESS; - rc = cochran_commander_read_config(device, &progress, data.config, sizeof(data.config)); + rc = cochran_commander_read_config(device, &progress, data.config); if (rc != DC_STATUS_SUCCESS) return rc;
On 2017-07-15 22:39, John Van Ostrand wrote:
static dc_status_t -cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[], unsigned int size) +cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[])
Removing the size parameter is a bad idea. Any function that takes some buffer as argument should also have a corresponding size parameter. Even if it's only used to verify whether the caller passed the right amount:
if (size % 512 != 0) return some_error;
If you simply trust the caller to always pass a buffer of the right size, you'll run into trouble someday!
dc_device_t *abstract = (dc_device_t *) device; dc_status_t rc = DC_STATUS_SUCCESS;
- unsigned int pages = 2;
- if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)
pages = 1;- // Read two 512 byte blocks into one 1024 byte buffer
- for (unsigned int i = 0; i < 2; i++) {
const unsigned int len = size / 2;
for (unsigned int i = 0; i < pages; i++) {
unsigned char command[2] = {0x96, i};
rc = cochran_commander_packet(device, progress, command,sizeof(command), data + i * len, len, 0);
unsigned int command_size = 2;if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)command_size = 1;rc = cochran_commander_packet(device, progress, command,command_size, data + i * 512, 512, 0); if (rc != DC_STATUS_SUCCESS) return rc;
How about using the size argument to calculate the number of pages? That way the function becomes even more generic. Only the caller needs to know how many pages there are. The function just needs to handle the difference between the 1 or 2 byte command variant (and even that can be based on the number of pages).
if (device->layout->baudrate == 9600) {// Older commander, use low-speed read commandcommand[0] = 0x05;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command_size = 6;} else {// Newer commander with high-speed read commandcommand[0] = 0x15;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command[6] = (size >> 16 ) & 0xff;command[7] = 0x04;command_size = 8;}
So this means that the byte sequences:
05 9D FF 00 43 00 05 BD 7F 00 43 00
that are send in the cochran_commander_read_id() function are actually read operations at the addresses 0xFF9D and 0x7FBD (and a size of 0x0043 bytes). Interesting. I didn't realize that before.
- // Read the sample data, from 0 to sample end will include logbook
- rc = cochran_commander_read (device, &progress,
device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end);
- // Read the sample data, logbook and sample data are contiguous
- rc = cochran_commander_read (device, &progress,
device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
This can't work. The size variable is calculated as:
layout->rb_profile_end - layout->rb_logbook_begin
For the Commander TM that is 0x010000 bytes (0x020000 - 0x010000). But the size field in the command is only 16 bits wide. So you'll try to read 0 bytes. Oops. I think the only solution here is to split the large read operation into multiple smaller blocks.
Jef
On Thu, Aug 10, 2017 at 11:00 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-07-15 22:39, John Van Ostrand wrote:
static dc_status_t -cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[], unsigned int size) +cochran_commander_read_config (cochran_commander_device_t *device, dc_event_progress_t *progress, unsigned char data[])
Removing the size parameter is a bad idea. Any function that takes some buffer as argument should also have a corresponding size parameter. Even if it's only used to verify whether the caller passed the right amount:
if (size % 512 != 0) return some_error;
If you simply trust the caller to always pass a buffer of the right size, you'll run into trouble someday!
Read on.
dc_device_t *abstract = (dc_device_t *) device;dc_status_t rc = DC_STATUS_SUCCESS;
unsigned int pages = 2;if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)pages = 1;// Read two 512 byte blocks into one 1024 byte buffer
for (unsigned int i = 0; i < 2; i++) {const unsigned int len = size / 2;
for (unsigned int i = 0; i < pages; i++) { unsigned char command[2] = {0x96, i};
rc = cochran_commander_packet(device, progress, command,sizeof(command), data + i * len, len, 0);
unsigned int command_size = 2;if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM)command_size = 1;rc = cochran_commander_packet(device, progress, command,command_size, data + i * 512, 512, 0); if (rc != DC_STATUS_SUCCESS) return rc;
How about using the size argument to calculate the number of pages? That way the function becomes even more generic. Only the caller needs to know how many pages there are. The function just needs to handle the difference between the 1 or 2 byte command variant (and even that can be based on the number of pages).
This was the resolution I chose.
if (device->layout->baudrate == 9600) {
// Older commander, use low-speed read commandcommand[0] = 0x05;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command_size = 6;} else {// Newer commander with high-speed read commandcommand[0] = 0x15;command[1] = (address ) & 0xff;command[2] = (address >> 8) & 0xff;command[3] = (address >> 16) & 0xff;command[4] = (size ) & 0xff;command[5] = (size >> 8 ) & 0xff;command[6] = (size >> 16 ) & 0xff;command[7] = 0x04;command_size = 8;}So this means that the byte sequences:
05 9D FF 00 43 00 05 BD 7F 00 43 00
that are send in the cochran_commander_read_id() function are actually read operations at the addresses 0xFF9D and 0x7FBD (and a size of 0x0043 bytes). Interesting. I didn't realize that before.
It seems so obvious, but yes, that's exactly what it means. It didn't dawn on me until I had to get creative reading the Commander TM.
// Read the sample data, from 0 to sample end will include logbook
rc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end);
// Read the sample data, logbook and sample data are contiguousrc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
This can't work. The size variable is calculated as:
layout->rb_profile_end - layout->rb_logbook_begin
For the Commander TM that is 0x010000 bytes (0x020000 - 0x010000). But the size field in the command is only 16 bits wide. So you'll try to read 0 bytes. Oops. I think the only solution here is to split the large read operation into multiple smaller blocks.
I thought so too but when I ask for ask for 0 bytes it returns 32K.
On 2017-08-10 19:32, John Van Ostrand wrote:
On Thu, Aug 10, 2017 at 11:00 AM, Jef Driesen jef@libdivecomputer.org
So this means that the byte sequences:
05 9D FF 00 43 00 05 BD 7F 00 43 00
that are send in the cochran_commander_read_id() function are actually read operations at the addresses 0xFF9D and 0x7FBD (and a size of 0x0043 bytes). Interesting. I didn't realize that before.
It seems so obvious, but yes, that's exactly what it means. It didn't dawn on me until I had to get creative reading the Commander TM.
Having less "magic values" is always nice!
// Read the sample data, from 0 to sample end will includelogbook
rc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end);
// Read the sample data, logbook and sample data arecontiguous
rc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
This can't work. The size variable is calculated as:
layout->rb_profile_end - layout->rb_logbook_begin
For the Commander TM that is 0x010000 bytes (0x020000 - 0x010000). But the size field in the command is only 16 bits wide. So you'll try to read 0 bytes. Oops. I think the only solution here is to split the large read operation into multiple smaller blocks.
I thought so too but when I ask for ask for 0 bytes it returns 32K.
That's a bit weird, but if it works, fine. The rationale behind it is probably that doing a read operation for 0 bytes makes little sense, so they can re-use that value to mean 0x10000. In that case we should probably add some extra checks:
if (size > 0x10000) return DC_STATUS_INVALIDARGS;
if (size == 0) return DC_STATUS_SUCCESS;
and some comment saying that 0 is interpreted by the device as 0x10000. Otherwise someone will ask the same question in the future. And by that time we probably forgot about that detail :-)
Jef
On Fri, Aug 11, 2017 at 10:18 AM, Jef Driesen jef@libdivecomputer.org wrote:
On 2017-08-10 19:32, John Van Ostrand wrote:
On Thu, Aug 10, 2017 at 11:00 AM, Jef Driesen jef@libdivecomputer.org
So this means that the byte sequences:
05 9D FF 00 43 00 05 BD 7F 00 43 00
that are send in the cochran_commander_read_id() function are actually read operations at the addresses 0xFF9D and 0x7FBD (and a size of 0x0043 bytes). Interesting. I didn't realize that before.
It seems so obvious, but yes, that's exactly what it means. It didn't dawn on me until I had to get creative reading the Commander TM.
Having less "magic values" is always nice!
// Read the sample data, from 0 to sample end will include logbook
rc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), device->layout->rb_profile_end);
// Read the sample data, logbook and sample data are contiguousrc = cochran_commander_read (device, &progress,device->layout->rb_logbook_begin, dc_buffer_get_data(buffer), size); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to read the sample data."); return rc;
This can't work. The size variable is calculated as:
layout->rb_profile_end - layout->rb_logbook_begin
For the Commander TM that is 0x010000 bytes (0x020000 - 0x010000). But the size field in the command is only 16 bits wide. So you'll try to read 0 bytes. Oops. I think the only solution here is to split the large read operation into multiple smaller blocks.
I thought so too but when I ask for ask for 0 bytes it returns 32K.
That's a bit weird, but if it works, fine. The rationale behind it is probably that doing a read operation for 0 bytes makes little sense, so they can re-use that value to mean 0x10000. In that case we should probably add some extra checks:
if (size > 0x10000) return DC_STATUS_INVALIDARGS;
if (size == 0) return DC_STATUS_SUCCESS;
and some comment saying that 0 is interpreted by the device as 0x10000. Otherwise someone will ask the same question in the future. And by that time we probably forgot about that detail :-)
Good idea.
The Cochran Commander TM appears to be a first generation Commander with limited storage and function compared to later models. This commit provides full support for downloading Commander TM logs and profiles. --- src/cochran_commander.c | 62 +++++++++++--- src/cochran_commander_parser.c | 178 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 219 insertions(+), 21 deletions(-)
diff --git a/src/cochran_commander.c b/src/cochran_commander.c index a77eabb..39bfe8e 100644 --- a/src/cochran_commander.c +++ b/src/cochran_commander.c @@ -87,6 +87,7 @@ typedef struct cochran_device_layout_t { unsigned int pt_profile_pre; unsigned int pt_profile_begin; unsigned int pt_profile_end; + unsigned int pt_dive_number; } cochran_device_layout_t;
typedef struct cochran_commander_device_t { @@ -120,6 +121,7 @@ static const cochran_device_layout_t cochran_cmdr_tm_device_layout = { 24, // address_bits ENDIAN_WORD_BE, // endian 9600, // baudrate + 4096, // rbstream_size 0x146, // cf_dive_count 0x158, // cf_last_log 0xffffff, // cf_last_interdive @@ -135,6 +137,7 @@ static const cochran_device_layout_t cochran_cmdr_tm_device_layout = { 0, // pt_profile_pre 0, // pt_profile_begin 90, // pt_profile_end (Next begin pointer is the end) + 20, // pt_dive_number };
// Cochran Commander pre-21000 s/n @@ -159,6 +162,7 @@ static const cochran_device_layout_t cochran_cmdr_1_device_layout = { 28, // pt_profile_pre 0, // pt_profile_begin 128, // pt_profile_end + 68, // pt_dive_number };
@@ -184,6 +188,7 @@ static const cochran_device_layout_t cochran_cmdr_device_layout = { 30, // pt_profile_pre 6, // pt_profile_begin 128, // pt_profile_end + 70, // pt_dive_number };
// Cochran EMC-14 @@ -208,6 +213,7 @@ static const cochran_device_layout_t cochran_emc14_device_layout = { 30, // pt_profile_pre 6, // pt_profile_begin 256, // pt_profile_end + 86, // pt_dive_number };
// Cochran EMC-16 @@ -232,6 +238,7 @@ static const cochran_device_layout_t cochran_emc16_device_layout = { 30, // pt_profile_pre 6, // pt_profile_begin 256, // pt_profile_end + 86, // pt_dive_number };
// Cochran EMC-20 @@ -256,6 +263,7 @@ static const cochran_device_layout_t cochran_emc20_device_layout = { 30, // pt_profile_pre 6, // pt_profile_begin 256, // pt_profile_end + 86, // pt_dive_number };
@@ -569,6 +577,8 @@ cochran_commander_profile_size(cochran_commander_device_t *device, cochran_data_ static unsigned int cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_data_t *data) { + unsigned int base = device->layout->rb_logbook_begin; + // We track profile ringbuffer usage to determine which dives have profile data int profile_capacity_remaining = device->layout->rb_profile_end - device->layout->rb_profile_begin;
@@ -587,10 +597,13 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d
// Remove the pre-dive events that occur after the last dive unsigned int rb_head_ptr; - if (device->layout->endian == ENDIAN_WORD_BE) - rb_head_ptr = (array_uint32_word_be(data->config + device->layout->cf_last_log) & 0xfffff000) + 0x2000; + if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) + // TM uses SRAM and does not need to erase pages + rb_head_ptr = base + array_uint32_word_be(data->config + device->layout->cf_last_log); + else if (device->layout->endian == ENDIAN_WORD_BE) + rb_head_ptr = base + (array_uint32_word_be(data->config + device->layout->cf_last_log) & 0xfffff000) + 0x2000; else - rb_head_ptr = (array_uint32_le(data->config + device->layout->cf_last_log) & 0xfffff000) + 0x2000; + rb_head_ptr = base + (array_uint32_le(data->config + device->layout->cf_last_log) & 0xfffff000) + 0x2000;
unsigned int head_dive = 0, tail_dive = 0;
@@ -604,13 +617,13 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d }
unsigned int last_profile_idx = (device->layout->rb_logbook_entry_count + head_dive - 1) % device->layout->rb_logbook_entry_count; - unsigned int last_profile_end = array_uint32_le(data->logbook + last_profile_idx * device->layout->rb_logbook_entry_size + device->layout->pt_profile_end); + unsigned int last_profile_end = base + array_uint32_le(data->logbook + last_profile_idx * device->layout->rb_logbook_entry_size + device->layout->pt_profile_end); unsigned int last_profile_pre = 0xFFFFFFFF;
if (device->layout->endian == ENDIAN_WORD_BE) - last_profile_pre = array_uint32_word_be(data->config + device->layout->cf_last_log); + last_profile_pre = base + array_uint32_word_be(data->config + device->layout->cf_last_log); else - last_profile_pre = array_uint32_le(data->config + device->layout->cf_last_log); + last_profile_pre = base + array_uint32_le(data->config + device->layout->cf_last_log);
if (rb_head_ptr > last_profile_end) profile_capacity_remaining -= rb_head_ptr - last_profile_end; @@ -628,7 +641,7 @@ cochran_commander_find_fingerprint(cochran_commander_device_t *device, cochran_d break; }
- unsigned int profile_pre = array_uint32_le(log_entry + device->layout->pt_profile_pre); + unsigned int profile_pre = base + array_uint32_le(log_entry + device->layout->pt_profile_pre);
unsigned int sample_size = cochran_commander_profile_size(device, data, i, profile_pre, last_profile_pre); last_profile_pre = profile_pre; @@ -836,6 +849,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call unsigned int max_config = sizeof(data.config); unsigned int max_logbook = layout->rb_logbook_end - layout->rb_logbook_begin; unsigned int max_sample = layout->rb_profile_end - layout->rb_profile_begin; + unsigned int base = device->layout->rb_logbook_begin;
if (device->layout->model == COCHRAN_MODEL_COMMANDER_TM) max_config = 512; @@ -863,9 +877,11 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call else data.dive_count = array_uint16_be (data.config + layout->cf_dive_count);
- if (data.dive_count == 0) + if (data.dive_count == 0) { // No dives to read + WARNING(abstract->context, "This dive computer has no recorded dives."); return DC_STATUS_SUCCESS; + }
if (data.dive_count > rb_logbook_entry_count) { data.logbook_size = rb_logbook_entry_count * layout->rb_logbook_entry_size; @@ -885,7 +901,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call }
// Request log book - rc = cochran_commander_read(device, &progress, 0, data.logbook, data.logbook_size); + rc = cochran_commander_read(device, &progress, layout->rb_logbook_begin, data.logbook, data.logbook_size); if (rc != DC_STATUS_SUCCESS) { status = rc; goto error; @@ -893,6 +909,7 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
// Locate fingerprint, recent dive with invalid profile and calc read size unsigned int profile_read_size = cochran_commander_find_fingerprint(device, &data); + // Update progress indicator with new maximum progress.maximum -= (max_sample - profile_read_size); device_event_emit (abstract, DC_EVENT_PROGRESS, &progress); @@ -927,7 +944,12 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call dive_count = (rb_logbook_entry_count + head_dive - tail_dive) % rb_logbook_entry_count;
// Create rbstream - unsigned int last_start_address = array_uint32_le( data.logbook + ((head_dive - 1) * layout->rb_logbook_entry_size) + layout->pt_profile_end ); + unsigned int last_start_address = 0; + if (layout->endian == ENDIAN_WORD_BE) + last_start_address = base + array_uint32_word_be( data.config + layout->cf_last_log ); + else + last_start_address = base + array_uint32_le( data.config + layout->cf_last_log ); + rc = dc_rbstream_new (&rbstream, abstract, 1, layout->rbstream_size, layout->rb_profile_begin, layout->rb_profile_end, last_start_address );
int invalid_profile_flag = 0; @@ -938,8 +960,24 @@ cochran_commander_device_foreach (dc_device_t *abstract, dc_dive_callback_t call
unsigned char *log_entry = data.logbook + i * layout->rb_logbook_entry_size;
- 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); + unsigned int sample_start_address = 0; + unsigned int sample_end_address = 0; + + if (layout->model == COCHRAN_MODEL_COMMANDER_TM) { + sample_start_address = base + array_uint24_le (log_entry + layout->pt_profile_begin); + sample_end_address = last_start_address; + // Commander TM has SRAM which seems to randomize when they lose power for too long + // Check for bad entries. + if (sample_start_address < layout->rb_profile_begin || sample_start_address > layout->rb_profile_end || + sample_end_address < layout->rb_profile_begin || sample_end_address > layout->rb_profile_end || + array_uint16_le(log_entry + layout->pt_dive_number) % layout->rb_logbook_entry_count != i) { + ERROR(abstract->context, "Corrupt dive (%d).", i); + continue; + } + } else { + sample_start_address = base + array_uint32_le (log_entry + layout->pt_profile_begin); + sample_end_address = base + array_uint32_le (log_entry + layout->pt_profile_end); + }
int sample_size = 0, pre_size = 0;
diff --git a/src/cochran_commander_parser.c b/src/cochran_commander_parser.c index b6ee17a..60a1b31 100644 --- a/src/cochran_commander_parser.c +++ b/src/cochran_commander_parser.c @@ -32,11 +32,12 @@
#define C_ARRAY_SIZE(array) (sizeof (array) / sizeof *(array))
-#define COCHRAN_MODEL_COMMANDER_PRE21000 0 -#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 1 -#define COCHRAN_MODEL_EMC_14 2 -#define COCHRAN_MODEL_EMC_16 3 -#define COCHRAN_MODEL_EMC_20 4 +#define COCHRAN_MODEL_COMMANDER_TM 0 +#define COCHRAN_MODEL_COMMANDER_PRE21000 1 +#define COCHRAN_MODEL_COMMANDER_AIR_NITROX 2 +#define COCHRAN_MODEL_EMC_14 3 +#define COCHRAN_MODEL_EMC_16 4 +#define COCHRAN_MODEL_EMC_20 5
// Cochran time stamps start at Jan 1, 1992 #define COCHRAN_EPOCH 694242000 @@ -44,6 +45,7 @@ #define UNSUPPORTED 0xFFFFFFFF
typedef enum cochran_sample_format_t { + SAMPLE_TM, SAMPLE_CMDR, SAMPLE_EMC, } cochran_sample_format_t; @@ -59,6 +61,7 @@ typedef struct cochran_parser_layout_t { cochran_sample_format_t format; unsigned int headersize; unsigned int samplesize; + unsigned int pt_sample_interval; cochran_date_encoding_t date_encoding; unsigned int datetime; unsigned int pt_profile_begin; @@ -114,10 +117,36 @@ static const dc_parser_vtable_t cochran_commander_parser_vtable = { NULL /* destroy */ };
+static const cochran_parser_layout_t cochran_cmdr_tm_parser_layout = { + SAMPLE_TM, // format + 90, // headersize + 1, // samplesize + 72, // pt_sample_interval + DATE_ENCODING_TICKS, // date_encoding + 15, // datetime, 4 bytes + 0, // pt_profile_begin, 4 bytes + UNSUPPORTED, // water_conductivity, 1 byte, 0=low(fresh), 2=high(sea) + 0, // pt_profile_pre, 4 bytes + 83, // start_temp, 1 byte, F + UNSUPPORTED, // start_depth, 2 bytes, /4=ft + 20, // dive_number, 2 bytes + UNSUPPORTED, // altitude, 1 byte, /4=kilofeet + UNSUPPORTED, // pt_profile_end, 4 bytes + UNSUPPORTED, // end_temp, 1 byte F + 57, // divetime, 2 bytes, minutes + 49, // max_depth, 2 bytes, /4=ft + 51, // avg_depth, 2 bytes, /4=ft + 74, // oxygen, 4 bytes (2 of) 2 bytes, /256=% + UNSUPPORTED, // helium, 4 bytes (2 of) 2 bytes, /256=% + 82, // min_temp, 1 byte, /2+20=F + UNSUPPORTED, // max_temp, 1 byte, /2+20=F +}; + static const cochran_parser_layout_t cochran_cmdr_1_parser_layout = { - SAMPLE_CMDR, // type + SAMPLE_CMDR, // format 256, // headersize 2, // samplesize + UNSUPPORTED, // pt_sample_interval DATE_ENCODING_TICKS, // date_encoding 8, // datetime, 4 bytes 0, // pt_profile_begin, 4 bytes @@ -139,9 +168,10 @@ static const cochran_parser_layout_t cochran_cmdr_1_parser_layout = { };
static const cochran_parser_layout_t cochran_cmdr_parser_layout = { - SAMPLE_CMDR, // type + SAMPLE_CMDR, // format 256, // headersize 2, // samplesize + UNSUPPORTED, // pt_sample_interval DATE_ENCODING_MSDHYM, // date_encoding 0, // datetime, 6 bytes 6, // pt_profile_begin, 4 bytes @@ -163,9 +193,10 @@ static const cochran_parser_layout_t cochran_cmdr_parser_layout = { };
static const cochran_parser_layout_t cochran_emc_parser_layout = { - SAMPLE_EMC, // type + SAMPLE_EMC, // format 512, // headersize 3, // samplesize + UNSUPPORTED, // pt_sample_interval DATE_ENCODING_SMHDMY, // date_encoding 0, // datetime, 6 bytes 6, // pt_profile_begin, 4 bytes @@ -336,6 +367,11 @@ cochran_commander_parser_create (dc_parser_t **out, dc_context_t *context, unsig parser->model = model;
switch (model) { + case COCHRAN_MODEL_COMMANDER_TM: + parser->layout = &cochran_cmdr_tm_parser_layout; + parser->events = NULL; // No inter-dive events on this model + parser->nevents = 0; + break; case COCHRAN_MODEL_COMMANDER_PRE21000: parser->layout = &cochran_cmdr_1_parser_layout; parser->events = cochran_cmdr_event_bytes; @@ -442,6 +478,8 @@ cochran_commander_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, *((unsigned int*) value) = (data[layout->min_temp] / 2.0 + 20 - 32) / 1.8; break; case DC_FIELD_TEMPERATURE_MAXIMUM: + if (layout->max_temp == UNSUPPORTED) + return DC_STATUS_UNSUPPORTED; if (data[layout->max_temp] == 0xFF) return DC_STATUS_UNSUPPORTED; *((unsigned int*) value) = (data[layout->max_temp] / 2.0 + 20 - 32) / 1.8; @@ -486,6 +524,8 @@ cochran_commander_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, // for density assume // 0 = 1000kg/m³, 2 = 1025kg/m³ // and other values are linear + if (layout->water_conductivity == UNSUPPORTED) + return DC_STATUS_UNSUPPORTED; if ((data[layout->water_conductivity] & 0x3) == 0) water->type = DC_WATER_FRESH; else @@ -495,6 +535,8 @@ cochran_commander_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, case DC_FIELD_ATMOSPHERIC: // Cochran measures air pressure and stores it as altitude. // Convert altitude (measured in 1/4 kilofeet) back to pressure. + if (layout->altitude == UNSUPPORTED) + return DC_STATUS_UNSUPPORTED; *(double *) value = ATM / BAR * pow(1 - 0.0000225577 * data[layout->altitude] * 250.0 * FEET, 5.25588); break; default: @@ -506,8 +548,114 @@ cochran_commander_parser_get_field (dc_parser_t *abstract, dc_field_type_t type, }
+/* + * Parse early Commander computers + */ static dc_status_t -cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) +cochran_commander_parser_samples_foreach_tm (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) +{ + cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; + const cochran_parser_layout_t *layout = parser->layout; + const unsigned char *data = abstract->data; + const unsigned char *samples = data + layout->headersize; + + if (abstract->size < layout->headersize) + return DC_STATUS_DATAFORMAT; + + unsigned int size = abstract->size - layout->headersize; + unsigned int sample_interval = data[layout->pt_sample_interval]; + + dc_sample_value_t sample = {0}; + unsigned int time = 0, last_sample_time = 0; + unsigned int offset = 2; + unsigned int deco_ceiling = 0; + + unsigned int temp = samples[0]; // Half degrees F + unsigned int depth = samples[1]; // Half feet + + last_sample_time = sample.time = time; + if (callback) callback (DC_SAMPLE_TIME, sample, userdata); + + sample.depth = (depth / 2.0) * FEET; + if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); + + sample.temperature = (temp / 2.0 - 32.0) / 1.8; + if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); + + sample.gasmix = 0; + if (callback) callback(DC_SAMPLE_GASMIX, sample, userdata); + + while (offset < size) { + const unsigned char *s = samples + offset; + + sample.time = time; + if (last_sample_time != sample.time) { + // We haven't issued this time yet. + last_sample_time = sample.time; + if (callback) callback (DC_SAMPLE_TIME, sample, userdata); + } + + if (*s & 0x80) { + // Event or temperate change byte + if (*s & 0x60) { + // Event byte + switch (*s) { + case 0xC5: // Deco obligation begins + break; + case 0xD8: // Deco obligation ends + break; + case 0xAB: // Decrement ceiling (deeper) + deco_ceiling += 10; // feet + + sample.deco.type = DC_DECO_DECOSTOP; + sample.deco.time = 60; // We don't know the duration + sample.deco.depth = deco_ceiling * FEET; + if (callback) callback(DC_SAMPLE_DECO, sample, userdata); + break; + case 0xAD: // Increment ceiling (shallower) + deco_ceiling -= 10; // feet + + sample.deco.type = DC_DECO_DECOSTOP; + sample.deco.depth = deco_ceiling * FEET; + sample.deco.time = 60; // We don't know the duration + if (callback) callback(DC_SAMPLE_DECO, sample, userdata); + break; + } + } else { + // Temp change + if (*s & 0x10) + temp -= (*s & 0x0f); + else + temp += (*s & 0x0f); + sample.temperature = (temp / 2.0 - 32.0) / 1.8; + if (callback) callback (DC_SAMPLE_TEMPERATURE, sample, userdata); + } + + offset++; + continue; + } + + // Depth sample + if (s[0] & 0x40) + depth -= s[0] & 0x3f; + else + depth += s[0] & 0x3f; + + sample.depth = (depth / 2.0) * FEET; + if (callback) callback (DC_SAMPLE_DEPTH, sample, userdata); + + offset++; + time += sample_interval; + } + return DC_STATUS_SUCCESS; +} + + +/* + * Parse Commander I (Pre-21000 s/n), II and EMC computers + */ +static dc_status_t +cochran_commander_parser_samples_foreach_emc (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) { cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; const cochran_parser_layout_t *layout = parser->layout; @@ -722,3 +870,15 @@ cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callb
return DC_STATUS_SUCCESS; } + + +static dc_status_t +cochran_commander_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t callback, void *userdata) +{ + cochran_commander_parser_t *parser = (cochran_commander_parser_t *) abstract; + + if (parser->model == COCHRAN_MODEL_COMMANDER_TM) + return cochran_commander_parser_samples_foreach_tm (abstract, callback, userdata); + else + return cochran_commander_parser_samples_foreach_emc (abstract, callback, userdata); +}
-
Jef Driesen -
John Van Ostrand