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

[Jef Driesen]

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!

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.

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.

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.

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).

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.

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