Fix signet-client compiler warnings

[lundacode]

With this change all the client compiler warnings are fixed. It gives me a warm and fuzzy feeling.

[nthdimtech]

It would be nice to have a warning free build and lock it in with "-Werror". Two main problems I found are

1) I think most of the functions return values that are being casted to (unsigned int) should have just been "unsigned int" to begin with.

2) Printing errors instead of ignoring them is good for diagnostics but these failures should be reported to the caller instead. Handling the errors properly can be deferred for awhile by just assigning them to a value declared with the attribute((unused)) and adding a "//TODO" comment so I can add the error handling in the callers later.

[nthdimtech]

Sorry about the change of mind but after looking at how many areas of the code would need to be changed to properly handle all the errors I think the best course of action is to not add any new error handling but instead store the unused return values in unused variables declared with __attribute((unused)) and include a "//TODO" comment in each case indicating the error should be handled. This will keep this commit more focused and I can work on improving the error handling gradually later

[lundacode]

1. I think most of the functions return values that are being casted to (unsigned int) should have just been
   "unsigned int" to begin with.

That sounds OK at first glance, but there are two problems with it. a) the client code often compares these to signed int (a Qt peculiarity, using int for size types) then we end up casting there. b) it'd somewhat go against the convention of returning -1 on error.

I'd rather return int, -1 error, and make the caller check before using it.

2. Printing errors instead of ignoring them is good for diagnostics but these failures should be reported to the caller instead. Handling the errors properly can be deferred for awhile by just assigning them to a value declared with the **attribute**((unused))  and adding a "//TODO" comment so I can add the error handling in the callers later.

Yes, I agree. I'll try to do something like that.

A completely different approach would be to get rid if these functions (returning sizes depending on device_type) and refactor the code to have something like:

struct signet_device {
    const int type;
    const int blk_size;
    const int max_entry_data_size;
    ...
}

initialize once, check errors once, and then confidently use device.blk_size etc, without any need for error checking and propagation. What do you think?

[lundacode]

Sorry about the change of mind but after looking at how many areas of the code would need to be changed to properly handle all the errors I think the best course of action is to not add any new error handling but instead store the unused return values in unused variables declared with __attribute((unused)) and include a "//TODO" comment in each case indicating the error should be handled. This will keep this commit more focused and I can work on improving the error handling gradually later

Yes, I agree. I'll try to make it minimal, getting rid of warnings.

[nthdimtech]

I like your idea of a "struct signet_device" structure to resolve some of these casting/error propagation problems. I would make it an opaque handle though and keep the access functions but have them take the device handle. This would clear the way for the client to talk to multiple devices at once which I would like to make possible.

[lundacode]

I think this is a reasonably focused commit for fixing the remaining compile warnings.

[nthdimtech]

Looks good. The only issue I can see remaining is that "enter_progressing_state" can simply be removed. It's a static function with no references. That file is only built in the client to emulate the device from a file. The code started as a modified version of the firmware code but was never part of the firmware.

[lundacode]

Yes, indeed. Now removed. I got confused by the presence of other enter_progressing_state functions in the code.