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