[PATCH] Added support for Cochran EMC and Commander Air import

[John Van Ostrand]

On Wed, Oct 22, 2014 at 3:23 AM, Jef Driesen <jef at libdivecomputer.org>
wrote:

> On 2014-10-07 18:09, John Van Ostrand wrote:
>
>> Specifically tested with EMC-20H and Commander Air_Nitrox this
>> commit should support the families for the most part however
>> there are may be differences in the DC signature, memory size,
>> and other factors that will prevent opening the device and
>> obtaining a clean import. Try it with other EMC and Commander
>> devices and check library ERRORs for the signature information
>> needed to add support.
>>
>
> First of all, sorry for the very late response. Please don't take this as
> a lack of interest! On the contrary. You're actually the first to
> contribute a complete new backend. Much appreciated!
>

It's nice to be appreciated, thanks.


> Now, there are a couple of issues that needs to be addressed before this
> can be included in libdivecomputer:
>
> You used a few Linux specific calls such as nanosleep() in your code. This
> obviously breaks the windows builds. The rule of thumb is that the dive
> computer backends should contain no platform specific code at all. All
> platform specific code should be moved to the platform specific modules.
> For example we already have a serial_sleep() function.
>

Those I should be able to change to serial_sleep.


> A related issue is that you used packed structures, which are a gcc
> extension. Although, I personally use mingw (gcc) for my Windows builds,
> the msvc compiler is supported too. Casting the raw data to a structure for
> easier parsing is non-portable anyway (e.g. little vs big endian).
> Therefore, in libdivecomputer we always de-serialize the data in a portable
> way using the array_uint{16,24,32}_{le,be} functions. During development
> the structures are indeed very convenient (I occasionally do that as well),
> but for the final version they should be replaced with something more
> portable.
>

Those packed structures are the dive logs and I realize they have a lot
more information than libdivecomputer needs but I wanted to save that
research because it's been useful in debugging and decoding other model's
logs and it may become useful in the future. You'll see that they are all
char or char arrays that I either de-serialize in-line (sometimes for good
reason) or use the array_uint* functions. I'd prefer to keep a structure in
place rather than directly access select bytes. Would a union between a
char array and the structure be a portable way of packing it? Would that be
an option?

Your dc_device_dump implementation uses a custom cochran_data_dump
> structure instead of the dc_buffer_t structure. You either need to get rid
> of this custom structure, or not implement this function. Right now if
> anyone calls this function with dc_buffer_t structure, bad things will
> happen. I guess this is also the reason why you added your own example
> application instead of the generic example application. If really
> necessary, you can implement and expose backend specific functions to
> support features outside the generic api.
>

I should be able to refactor that function to use dc_buffer properly. You
mentioned that the dump function was more for debugging or bypassing
libdivecomputer's foreach functions so I was liberal with it. I still want
to export all the data so how about I use dc_buffer directly, put pointers
to the data sections in first and stack the data in after, one big blob.
Maybe use a union to a structure to easily decode it.

I wrote the cochran_download program because I wanted a way for a user to
be able to download their data and send it to me easily. I assume there
will be lots more work needed to support various DCs. The data produced can
be used with a simulator I wrote.h

You implemented two backends. One for the EMC and one for the Commander.
> But the communication protocol is roughly identical, and you already handle
> the differences in the common code. That means you only need a single
> backend, which supports multiple models. This is very common in other
> backends too. Just name your backend after one of them (for example
> DC_FAMILY_COCHRAN_EMC).
>

I thought that being very specific about model was important in not leading
users to believe their computer was supported. There are significant
differences between models and possibly within a model that has different
features enabled that would cause the communication or decoding to fail. I
have access for four different models with 3 purchased recently which means
I don't have any duplicates to compare, duplicate models with different
features enable to compare, and I can't tell if things change within a
model over time.

A little background may help in this point and the next. I chatted with
Mike Cochran who refused his company's help in decoding because he was
concerned that it would leak hints about the decompression algorithm. He
also warned that others have bricked their DC trying to communicate with it
and even admitted their programmers have bricked DCs while developing their
Windows Analyst dive log software. I chose to take that quite seriously.

The code is extremely conservative in identifying DCs. Right now it uses a
6 digit model string (e.g. AM2315) which I suspect not only varies between
models but varies within a model based on what features are enabled (e.g.
more memory, additional gases) and possible based on which microcontroller
is used (perhaps it changes over the life of a model.)

For example the following seem to be specific between models and depending
on which features are enabled.
    Data format (ie log, samples)
    Baud rate
    Start and End memory addresses (needed when the log "wraps" around.

I've determined that byte 3 and 4 of that model string indicate the data
format but I have no way of determining the baud rate or memory range.

I'm also working on a third model except all I have is data.

Does this change your view on this?

During the communication you close and re-open the serial port. Are you
> sure this is really necessary? What kind of interface does the Cochran use?
> A usb-serial chip (prolific, ftdi, etc)? Maybe you just need the right
> trick like toggling some serial lines to restart the communication? The
> reason why I'm asking is that in future versions, opening the serial port
> will likely be moved to the application side, and in that case re-opening
> the port will no longer be possible.
>

You may recall me posting about communication in the past. The cable uses
an end-of-life FTDI chip to communicate with the three contacts on the DC
(3-wire serial?) The cochran DCs seem tricky. They initially take commands
at 9600 baud and they deliver results for small data transfers at the same
rate, but for logbook and sample data they operate at what I suspect is
their microcontroller's full speed, so I have unusual baud rates like
825,000 baud.

Their Analyst software always downloads the log data as a whole, something
I suspect isn't necessary and sample data can be quite big (3 bytes every
second) but I decided, based on Mr. Cochran's warning, that I should mimic
their windows software closely. That means I open and close the connection.

I could revisit that code and see if I can remove the close/open to see if
it works but I'd still need to do the baud change and there is a need for
several flushes to wake the DC up. The DC also logs computer connections
and if those logs looked unusual users might find Cochran contesting
warranty claims. I don't know that close/open is logged, I was being
careful.

Will your plans to move the open() to the application side allow for baud
rate changes that are needed by the Cochran?

There are a few other issues as well, but I think it makes no sense to dig
> into the details before addressing the more critical issues first.
>
> Jef
>

I've complete some work on a Subsurface log file import for Cochran .CAN
and .WAN files. That data is in the same format as the data streams from
the DC, except it's re-arranged slightly. I've refined my understanding of
the data format and some of the code used. I have the Gemeni (air
integrated) data format decoded. I also have samples of older DCs that I'm
dabbling with but might discard because they are too old.

That said there is a lot of duplicate function between the two programs and
if I revisit the libdivecomputer code I may find additional refinements.
I'm open to other ideas.

-- 
John Van Ostrand
At large on sabbatical
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://libdivecomputer.org/pipermail/devel/attachments/20141022/c4240596/attachment.html>