UEMIS Patch fixing alternating dive details

Dirk Hohndel dirk at hohndel.org
Thu Sep 17 08:06:18 PDT 2015


Ok, let's talk about learning :-)

On Thu, Sep 17, 2015 at 04:51:33PM +0200, Guido Lerch wrote:
> From cd5c20831b006fcb5da6397301026ac056fa949d Mon Sep 17 00:00:00 2001
> From: glerch <guido.lerch at gmail.com>
> Date: Thu, 17 Sep 2015 16:46:02 +0200
> Subject: [PATCH 7/7] UEMIS-fix alternating dive details

I am rewriting the subject of most of your commits to fit the pattern:

Uemis divelog: ..... (starting with lower case)

> Implemented suggestion from Dir on how to deal with dive details
                             ^^^^^
> not being found.

Proof-reading :-)

> I also cleaned up some unused variables, got rid of the last_log_file_nr
> because it's not needed anymore.

Should this have been two commits? Maybe not.

> I kept the UEMIS_DEBUG flags active on purpose.

Don't. Ideally they should just be passed in when calling cmake:

CFLAGS=-DUEMIS_DEBUG=7 cmake .

If you really want them in the C file for your tests then remove them
before you send patches to me.
> 
> One thing ins question I still need to check and this is whether we really
            ^^^
	    proof reading

> need to decrease the dive_to_read if we find deleted dive details, this is
> still in but I dont think it's really needed.
> 
> Signed-off-by: glerch <guido.lerch at gmail.com>
> ---
>  uemis-downloader.c | 51 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/uemis-downloader.c b/uemis-downloader.c
> index 0981f4b..43bda9b 100644
> --- a/uemis-downloader.c
> +++ b/uemis-downloader.c
> @@ -31,7 +31,7 @@
>  #define NUM_PARAM_BUFS 10
>  
>  // debugging setup
> -// #define UEMIS_DEBUG 1 + 2 + 4 + 8 + 16 + 32
> +#define UEMIS_DEBUG 1 + 2 + 4
>  
>  #define UEMIS_MAX_FILES 4000
>  #define UEMIS_MEM_FULL 1
> @@ -69,6 +69,7 @@ static int mbuf_size = 0;
>  
>  static int max_mem_used = -1;
>  static int next_table_index = 0;
> +static int dive_to_read = 0;
>  
>  /* helper function to parse the Uemis data structures */
>  static void uemis_ts(char *buffer, void *_when)
> @@ -367,7 +368,7 @@ static void buffer_add(char **buffer, int *buffer_size, char *buf)
>  		*buffer = realloc(*buffer, *buffer_size);
>  		strcat(*buffer, buf);
>  	}
> -#if UEMIS_DEBUG & 16
> +#if UEMIS_DEBUG & 8

Stuff like this mixed into a patch makes it much harder to see what you
are actually changing in the patch. That's why I encourage you to do
smaller commits that do just one thing.

If you look at everything above, the only real change is yet another
global variable that you added.

And it continues like this, the next two hunks are also just noise.

> -					/* Ugly, need something better than this
> -					 * essentially, if we start reading divelogs not from the start
> -					 * we have no idea on how many log entries are there that have no
> -					 * valid dive details */
> -					if (*dive_to_read >= dive->dc.diveid)
> -						*dive_to_read = (*dive_to_read - 2 >= 0 ? *dive_to_read - 2 : 0);
> +					uint32_t nr_found = 0;
> +					char *logfilenr = strstr(mbuf, "logfilenr");
> +					if (logfilenr) {
> +						sscanf(logfilenr, "logfilenr{int{%u", &nr_found);
> +						if (nr_found >= dive->dc.diveid)
> +							dive_to_read = dive_to_read - 2;
> +						if (dive_to_read < -1)
> +							dive_to_read = -1;
> +					}
>  				}
>  			}
> -			*dive_to_read = *dive_to_read + 1;
> +			dive_to_read++;

Yep, that should work with my Uemis. I'll test it next.

And I'll take your commit as it is (minus the part where you turn on
debugging by default). So no need to resubmit.

/D


More information about the subsurface mailing list