UEMIS Patch fixing alternating dive details

[Guido Lerch]

2015-09-17 17:06 GMT+02:00 Dirk Hohndel <dirk at hohndel.org>:

> 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 :-)
>

hä ?

>
> > 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 .
>

THANKS, I was waiting for this tip without actually ever asking directly
:-)

>
> 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.
>

I know, I was rushing though as I wanted this out, with proper testing,
before I leave on Saturday.

>
> 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
>



-- 
Best regards,
Guido
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20150917/05524c99/attachment.html>