testing of download from divecomputer needed

Dirk Hohndel dirk at hohndel.org
Sun Jan 11 07:38:44 PST 2015


On Sun, Jan 11, 2015 at 12:48:09PM +0200, Miika Turkia wrote:
> On Sat, Jan 10, 2015 at 1:22 AM, Dirk Hohndel <dirk at hohndel.org> wrote:
> 
> > I redid the logic for downloading from the divecomputer.
> > The cool "select which dives you want" UI had some major flaws (as poor
> > Miika found out) which triggered me to actually read some of the ancient
> > code around downloading from dive computers which almost made me puke.
> >
> > So instead of patching around this and make the new UI kinda work I
> > decided to rip things out and redo them.
> >
> 
> A segmentation fault:
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000004a9036 in MainTab::updateDiveInfo (this=0xb20940, clear=false)
>     at ../qt-ui/maintab.cpp:517
> 517            ui.DiveType->setCurrentIndex(current_dc->divemode);

My apologies - I didn't test Robert's patches enough before pushing them
out.

2 issues here

a) do not use current_dc unless you KNOW that there is a valid
current_dive. This is the downside of hiding the access behind these
convenient little #defines - current_dc ends up dereferencing the
current_dive pointer. If that isn't valid things go kaboom.

b) regardless of that, when displaying information, you don't want
current_dc (and current_dive), anyway. You want displayed_dive and the a
lot less catchy get_dive_dc(&displayed_dive, dc_number); I guess I could
create a similar access macro for that one...

> First I tried to DL Vyper a couple of times with no success,

Why was that? Wet contacts? Bad connection?

> then Stinger succeeded on first go but got me this crash.

Yes, because of the sequence in which the download dialog does things,
Robert's patch would always cause a crash when clicking OK. I guess I'm
not the only one who didn't test a fresh download :-)

> When I get a DL error at the last of the new dives, I think discarding the
> current DL when I hit retry would be appropriate. (I have the first run and
> the retry on the DL list. Otherwise, highlight the newly downloaded dives.)

I'm a bit conflicted about this. One of the weird problems caused by the
design of the Uemis is that you cannot load all of the dives in one go if
you have more than about 50 dives to download. So in that case, the retry
will be incremental - it will add to the already downloaded dives. But I
guess I should special-case that and indeed clear out the list when you
click Retry - just makes more sense. Otherwise you always get these
confusing duplicate dives in there.

> Deselecting some of the downloaded dives enable a tick mark on one of the
> already de-selected ones. But this turns out to be that the list is not
> properly refreshed.. If I go to different desktop and back, the selection
> is done properly. (Initially I thought the first un-tick was not accepted
> and redid it due to display not being updated properly.) Anyway, I cannot
> really be sure of what would be imported due to the crashing after OK
> button.

Hmm. Odd. The code tries very hard to tell Qt whenever something changes
to make sure things get properly refreshed. I had very brief delays (a few
hundred miliseconds) to selecting / unselecting dives in that list, but
haven't seen a complete failure like you describe.

/D


More information about the subsurface mailing list