Can I get this as a patch with good commit message and SOB, please?

Thanks

/D

On Sat, Sep 19, 2015 at 05:15:30PM -0700, Linus Torvalds wrote:
> On Sat, Sep 19, 2015 at 4:47 PM, Dirk Hohndel <[email protected]> wrote:
> > Duh. Some times I hate it when you are obviously right. I'll fix that and 
> > make the Uemis download do the right thing.
> 
> Wait, I have a patch already.
> 
> This is *NOT*TESTED*. It's still downloading without this patch (and
> with the manual clearing), and I wanted to let that go on.
> 
> But I think this patch does the right thing. But right now I've just
> verified that it compiles.
> 
> This also contains the strstr NULL fix.
> 
> Anyway, the logic here is:
> 
>  - remove the hacky special uemis "uemis_set_max_diveid_from_dialog()"
> interface entirely
> 
>  - instead, export the "downloadTable" dive_table
> 
>  - the uemis "get the biggest dive ID" logic is now:
> 
>     (a) if there are dives in the downloadTable when we start, we will
> continue based on that table, and nothing but that table
> 
>        The logic is simple: if downloadTable.nr is non-NULL, then that
> means it's a "retry" event, and we obviously want to continue with
> what we had.
> 
>     (b) else, if "force download" was set, then we use the
> downloadTable even when nr was 0, because that just works out to the
> same thing as not looking at any old dives at all (which is what we
> want)
> 
>     (c) otherwise, we look through the existing normal divetable
> 
> This seems "obviously correct(tm)", but as mentioned, I have not in
> any way actually had time to *test* this yet. So it may have some
> stupid problem.
> 
>                        Linus

>  dive.h                             |  2 +-
>  qt-ui/downloadfromdivecomputer.cpp |  8 --------
>  uemis-downloader.c                 | 42 
> +++++++++++++++++++++-----------------
>  uemis.h                            |  1 -
>  4 files changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/dive.h b/dive.h
> index fa95f6721631..4eb44cfc9c8d 100644
> --- a/dive.h
> +++ b/dive.h
> @@ -498,7 +498,7 @@ struct dive_table {
>       struct dive **dives;
>  };
>  
> -extern struct dive_table dive_table;
> +extern struct dive_table dive_table, downloadTable;
>  extern struct dive displayed_dive;
>  extern struct dive_site displayed_dive_site;
>  extern int selected_dive;
> diff --git a/qt-ui/downloadfromdivecomputer.cpp 
> b/qt-ui/downloadfromdivecomputer.cpp
> index 7e18dae70baa..1fef9f6bff1e 100644
> --- a/qt-ui/downloadfromdivecomputer.cpp
> +++ b/qt-ui/downloadfromdivecomputer.cpp
> @@ -298,14 +298,6 @@ void 
> DownloadFromDCWidget::on_downloadCancelRetryButton_clicked()
>               diveImportedModel->clearTable();
>               clear_table(&downloadTable);
>       }
> -     if (ui.vendor->currentText() == "Uemis") {
> -             if (currentState == ERROR && downloadTable.nr > 0)
> -                     // let the uemis code know how far we've gotten
> -                     
> uemis_set_max_diveid_from_dialog(downloadTable.dives[downloadTable.nr - 
> 1]->dc.diveid);
> -             else
> -                     // fresh download, so only look at what's in the 
> dive_table
> -                     uemis_set_max_diveid_from_dialog(0);
> -     }
>       updateState(DOWNLOADING);
>  
>       // you cannot cancel the dialog, just the download
> diff --git a/uemis-downloader.c b/uemis-downloader.c
> index bc3528d72af4..3b8ddf900e89 100644
> --- a/uemis-downloader.c
> +++ b/uemis-downloader.c
> @@ -928,23 +928,32 @@ static bool process_raw_buffer(device_data_t *devdata, 
> uint32_t deviceid, char *
>       return true;
>  }
>  
> -static int max_diveid_from_dialog;
> -
> -void uemis_set_max_diveid_from_dialog(int diveid)
> -{
> -     max_diveid_from_dialog = diveid;
> -}
> -
> -static char *uemis_get_divenr(char *deviceidstr)
> +static char *uemis_get_divenr(char *deviceidstr, int force)
>  {
>       uint32_t deviceid, maxdiveid = 0;
>       int i;
>       char divenr[10];
> -
> +     struct dive_table *table;
>       deviceid = atoi(deviceidstr);
> -     struct dive *d;
> -     for_each_dive (i, d) {
> +
> +     /*
> +      * If we are are retrying after a disconnect/reconnect, we
> +      * will look up the highest dive number in the dives we
> +      * already have.
> +      *
> +      * Also, if "force_download" is true, do this even if we
> +      * don't have any dives (maxdiveid will remain zero)
> +      */
> +     if (force || downloadTable.nr)
> +             table = &downloadTable;
> +     else
> +             table = &dive_table;
> +
> +     for (i = 0; i < table->nr; i++) {
> +             struct dive *d = table->dives[i];
>               struct divecomputer *dc;
> +             if (!d)
> +                     continue;
>               for_each_dc (d, dc) {
>                       if (dc->model && !strcmp(dc->model, "Uemis Zurich") &&
>                           (dc->deviceid == 0 || dc->deviceid == 0x7fffffff || 
> dc->deviceid == deviceid) &&
> @@ -952,7 +961,7 @@ static char *uemis_get_divenr(char *deviceidstr)
>                               maxdiveid = dc->diveid;
>               }
>       }
> -     snprintf(divenr, 10, "%d", maxdiveid > max_diveid_from_dialog ? 
> maxdiveid : max_diveid_from_dialog);
> +     snprintf(divenr, 10, "%d", maxdiveid);
>       return strdup(divenr);
>  }
>  
> @@ -1059,7 +1068,7 @@ static bool load_uemis_divespot(const char *mountpath, 
> int divespot_id)
>  static void get_uemis_divespot(const char *mountpath, int divespot_id, 
> struct dive *dive)
>  {
>       struct dive_site *nds = get_dive_site_by_uuid(dive->dive_site_uuid);
> -     if (strstr(nds->name,"from Uemis")) {
> +     if (nds && nds->name && strstr(nds->name,"from Uemis")) {
>               if (load_uemis_divespot(mountpath, divespot_id)) {
>                       /* get the divesite based on the diveid, this should 
> give us
>                       * the newly created site
> @@ -1215,12 +1224,7 @@ const char *do_uemis_import(device_data_t *data)
>               goto bail;
>  
>       param_buff[1] = "notempty";
> -     /* if we force it we start downloading from the first dive on the Uemis;
> -      *  otherwise check which was the last dive downloaded */
> -     if (!force_download)
> -             newmax = uemis_get_divenr(deviceid);
> -     else
> -             newmax = strdup("0");
> +     newmax = uemis_get_divenr(deviceid, force_download);
>  
>       first = start = atoi(newmax);
>       dive_to_read = first;
> diff --git a/uemis.h b/uemis.h
> index 90ae99028799..5f32fe76c0af 100644
> --- a/uemis.h
> +++ b/uemis.h
> @@ -16,7 +16,6 @@ void uemis_parse_divelog_binary(char *base64, void *divep);
>  int uemis_get_weight_unit(int diveid);
>  void uemis_mark_divelocation(int diveid, int divespot, uint32_t 
> dive_site_uuid);
>  void uemis_set_divelocation(int divespot, char *text, double longitude, 
> double latitude);
> -void uemis_set_max_diveid_from_dialog(int diveid);
>  int uemis_get_divespot_id_by_diveid(uint32_t diveid);
>  
>  typedef struct

_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to