Patches to add basic support for the Suunto EON Steel

[Jef Driesen]

On 01-11-14 19:29, Linus Torvalds wrote:
> On Fri, Oct 31, 2014 at 4:57 PM, Linus Torvalds
> <torvalds at linux-foundation.org> wrote:
>> On Fri, Oct 31, 2014 at 3:25 PM, Jef Driesen <jef at libdivecomputer.org> 
>> wrote:
>> 
>>> Proper integration with the libdivecomputer logging functions.
>> 
>> I'll take a look, today has been Linux work for me (and will be
>> halloween later).
> 
> Ok, here's a re-send of the libdivecomputer patches (no change to the
> subsurface part, so I'm not resending that patch). The two
> DC_FIELD_STRING patches haven't really changed, but they aren't
> identical either, due to changes around them.
> 
> This - once again - squashed all the changes into one "EON Steel
> support patch": your three patches and the change to use the
> libdivecomputer ERROR() function. It's still wrapped with the
> "report_error()" function due to return value issues (returning
> DC_STATUS_SUCCESS on error? That's just odd, but regardless, the
> internal EON steel functions return number of bytes transmitted etc,
> so the enums just wouldn't work even if they were actually error
> numbers),.

Just a quick comment, because I'm running out of time right now.

I still prefer to have proper integration of the logging functions. 
Calling ERROR from your report_error is an ugly hack that throws away 
several of the nice features of the ERROR macro. For example the line 
numbers and function names will always be those of your report_error() 
function, hiding the real source. And when building with all logging 
disabled, your report_error function is still being called. Your debug() 
function is still there too. Basically I want all printf's (directly or 
through a wrapper function) replaced.

Also, my patch with the casts was merely to indicate where the problems 
were, and not supposed to be applied as is. Ideally I would rather see 
the root problem "fixed" instead of fixing the symptoms.

For example by changing dive_directory to an array:

-static const char *dive_directory = "0:/dives";
+static const char dive_directory[] = "0:/dives";

and then calling memcpy instead of strcpy:

-       strcpy((char *)cmd+4, dive_directory);
-       cmdlen = 4+strlen(dive_directory)+1;
+       memcpy(cmd + 4, dive_directory, sizeof(dive_directory));
+       cmdlen = 4 + sizeof(dive_directory);

Saves a strlen call too :-)

This is just an example, but I think you'll understand what I mean.

> If you'd prefer an incremental diff instead, just holler. But I
> rebased on top of your tree, and you hadn't applied the patches, so I
> was assuming you want a clean resend.

A clean resend is what I prefer as well. These incremental changes don't 
add much value to the git history.

Jef