Uemis patches

[Dirk Hohndel]

Hi Guido

On Mon, Sep 07, 2015 at 03:45:44PM +0200, Guido Lerch wrote:
> 
> Attached two Uemis patches after the 10 Dirk applied for me.
> 
> Patch 0001
> Adds the first buddy to the dive
> 
> Patch 0002
> Fixes an issue where the original dive numbers coming from the Uemis where
> never parsed.
> 

You're patches are attached as binary objects and not as text - I believe
that's what the Mac does by default. That makes is a bit harder to quote
them for me. I think Robert has figured out a way to fix that for his
patches...

>     Adding buddy to dive
    
This patchs is missing the "Signed-off-by:" line

> diff --git a/uemis-downloader.c b/uemis-downloader.c
> index 0cb0f9fe49ac..8d81451022e1 100644
> --- a/uemis-downloader.c
> +++ b/uemis-downloader.c
> @@ -731,8 +731,8 @@ static void parse_tag(struct dive *dive, char *tag, char *val)
>  	} else if (!strcmp(tag, "u8SuitThickness")) {
>  		if (*suit_thickness[atoi(val)])
>  			uemis_add_string(translate("gettextFromC", suit_thickness[atoi(val)]), &dive->suit);
> -	} else if (!strcmp(tag, "dive_no")) {
> -		dive->number = atoi(val);
> +    } else if (!strcmp(tag, "nickname")) {
> +        uemis_add_string(val, &dive->buddy);

This is removing code that you added in the last patch (the "dive_no"
part). Is that intentional? If yes, could you mention in the commit
message why you are doing that?

And is adding the "nickname" parsing all that is needed to add the buddy?
Silly that I never did that :-)

Finally, as you can see here, you still seem to be indenting with spaces
rather than tabs.

> @@ -1215,13 +1215,6 @@ const char *do_uemis_import(device_data_t *data)
>  					break;
>  			}
>  
> -			/*
> -			for (int i = iStartCleanup; i < data->download_table->nr; i++)
> -			if (!data->download_table->dives[i]->downloaded) {
> -				uemis_delete_dive(data, data->download_table->dives[i]->dc.diveid);
> -				i = (i > iStartCleanup ? i-- : i = iStartCleanup);
> -			}
> -			 */

It would be nice to mention things like this in the commit message as
well. "Removed obsolete code that had been commented out" or something
like that.


Your second patch had the complete commit message run together in the
subject line of the patch.

>     Using the original dive number instead of the object_id in Uemis - added the capturing of the dive_no tag before the object_id is captured - reason why is that once the varible *dive is set to the object_id the dive_no isn't read during process_raw_buffer as it apprears before the object_id tag.
>     
>     Signed-off-by: glerch <guido.lerch at gmail.com>
> 
> diff --git a/uemis-downloader.c b/uemis-downloader.c
> index 8d81451022e1..c345b987a0b2 100644
> --- a/uemis-downloader.c
> +++ b/uemis-downloader.c
> @@ -792,6 +792,7 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, char *
>  	char *sections[10];
>  	int s, nr_sections = 0;
>  	struct dive *dive = NULL;
> +    char dive_no[10];

As above - whitespace.
 
>  #if UEMIS_DEBUG & 4
>  	fprintf(debugfile, "p_r_b %s\n", inbuf);
> @@ -835,6 +836,18 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, char *
>  			free(buf);
>  			return false;
>  		}
> +        /* quickhack and workaround to capture the original dive_no
> +         * i am doing this so I dont have to change the original design
> +         * but when parsing a dive we never parse the dive number because
> +         * at the time it's being read the *dive varible is not set because
> +         * the dive_no tag comes before the object_id in the uemis ans file
> +         */
> +        char *dive_no_buf = strdup(inbuf);
> +        char *dive_no_ptr = strstr(dive_no_buf, "dive_no{int{") + 12;
> +        char *dive_no_end = strstr(dive_no_ptr, "{");
> +        *dive_no_end = 0;
> +        strcpy(dive_no, dive_no_ptr);
> +        free(dive_no_buf);

This works (but the code is not indented with tabs which is why the diff
looks so odd.

>  	}
>  	while (!done) {
>  		/* the valid buffer ends with a series of delimiters */
> @@ -873,14 +886,11 @@ static bool process_raw_buffer(device_data_t *devdata, uint32_t deviceid, char *
>  #if UEMIS_DEBUG % 2
>  			fprintf(debugfile, "Adding new dive from log with object_id %d.\n", atoi(val));
>  #endif
> -			/* glerch Sep. 2015
> -			 * maybe I am missing something here but this seems wrong
> -			if (keep_number)
> -				dive->number = atoi(val);
> -			*/

Again, something to mention in the commit message

>  		} else if (is_dive && strcmp(tag, "logfilenr") == 0) {
>  			/* this one tells us which dive we are adding data to */
>  			dive = get_dive_by_uemis_diveid(devdata, atoi(val));
> +            if (strcmp(dive_no, "0"))
> +                dive->number = atoi(dive_no);
> 
>  			if (for_dive)
>  				*for_dive = atoi(val);
>  		} else if (!is_log && dive && !strcmp(tag, "divespot_id")) {

Yep, that should work.

Two nice patches. Perfect in their granularity.
You could have taken the two chunks that remove previously commented out
code and combined them into their own commit but I'm fine if you want to
leave them as is.

Would you mind trying to go back and fixing the whitespace / commit
messages?

I'll be happy to walk you through it interactively on IRC (or Google
hangout or something) if you want.

Or I'll explain it in email. Your call.

/D