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
_______________________________________________
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
