Patches to add basic support for the Suunto EON Steel

[Jef Driesen]

On 23-10-14 00:34, Linus Torvalds wrote:
> Ok, I re-organized my patches a bit, and here are four patches to
> support the basic dive log download of the new Suunto EON Steel dive
> computer. I combined some of my basic support patches into one, since
> I didn't feel like I needed to show the odd history of having
> implemented some things in stages (ie the second attachment was
> originally four patches that didn't do anything useful unless
> combined)..
>
> Three of the patches are for libdivecomputer:
>
>   - patch 1/3: the preparatory parser support patch
>
>   - patch 2/3: the basic core dive data downloader for the EON Steel
>
>   - patch 3/3: the small addition to parse more than the basic data
>
> Patch 2/3 works on its own, but means that you *just* get the core
> dive data (with full dive profiles and cylinder pressures). Patch 3/3
> gives you the serial number and firmware version etc, but that relies
> on 1/3.
>
> The fourth patch (1/1) is the patch to subsurface to actually use the
> new parser support in libdivecomputer, so that you can get the EON
> Steel serial numbers etc.
>
> This makes your Suunto EON Steel quite usable with subsurface. There
> is more we could do, but it's all fairly secondary stuff. I'd like to
> get this all accepted before I even bother starting to look at the
> other things I can do with the dive computer.
>
> Of course, since apparently the EON Steel won't really be *available*
> until Nov 15th or so, right now these patches are useful mainly to
> people with preproduction models like me. But wouldn't it be nice if
> we could just say that we support the thing before it's even available
> for sale?

I did a first review of your Eon Steel backend. Here are my comments:

Both the windows and non-libusb builds are broken. The msvc build it's a bit 
tricky. Because msvc doesn't support C99, I compile the code as C++. All the C99 
features I use in libdivecomputer are supported in C++, so this mostly works. 
But there are also some extras necessary, like explicit casts from void pointer 
to other pointers. But even the mingw (gcc) build fails. It fails on the ERROR 
macros with "error: called object '0' is not a function". Not sure why.

The directory_entry structure with it's zero length array is most likely also 
not very portable. C99 has flexible arrays, but I'm not sure what msvc supports.

Better integration with the existing libdivecomputer infrastructure. We already 
have logging functions, array_uint{16,24,32}_{be,le} helper functions for 
dealing with endianness, dynamic memory buffers (dc_buffer_t), etc. There is no 
need to re-implement any of those.

Out of memory error handling. I know the arguments for calling exit very well, 
but for libdivecomputer I made the choice to handle out of memory differently. 
Please stick to this pattern.

Many functions return -1 in the case of an error. Can you rework this as 
dc_status_t values? BTW, if you want to use goto style error handling, that's fine.

The remaining ones are mainly style issues:

For the name of the backend, I prefer a single word like "eonsteel" or just 
"steel" instead of eon_steel. This is just to follow the existing practice in 
libdivecomputer were backends are named DC_FAMILY_VENDOR_PRODUCT. So no 
underscores in the product part.

Main structures (struct eon_steel, eon_parser) and all entry point functions 
(public or through the vtable) get the same backend prefix.

I also prefer to see all structures and enums typedef'ed, so you can use them 
without the struct and enum keywords.

That's it for now.

The parser logic is a bit hard to follow without having some example data to 
look at. Can you share some data files?

Jef