[PATCH] Fix reading Cressi Leonardo data.

[Jef Driesen]

On 2017-10-03 00:15, Vsevolod Velichko wrote:
> I just tested your idea, so here are the results:
> 1. Double toggle on-off of DTR flag appears to be changing nothing. It
> still works if it is done before each message, and it still fails if
> it's done only once at startup.
> 2. Removal of the ping is definitely leads to fail. In most cases it
> looks like hang-up in the middle of dump sending. I have no idea for
> what reason.

Can you send me some logs from those experiments? It will help me 
understand what's going on.

What values did you use for the delays?

> 3. Good news is that delays can be slightly decreased, so in modified
> patch below it's 120 ms instead of 300 ms.
> 
> In fact I suppose that this delay in each message shouldn't be
> considered something awful since in most cases it's done only 2-3
> times before full dump, and the multiple random access to device
> memory is hardly ever used.

You are right that libdivecomputer doesn't use random access internally, 
but there might be applications using it. It will for example also 
prevent us from optimizing the foreach implementation to download only 
the data we really need, instead of just downloading a full memory dump 
and extracting the parts we need. (I've no idea if this is actually 
worth doing or not.)

Anyway, I'm not really convinced that the dtr toggling and the delay is 
necessary on every packet. When I look through my capture files, then I 
have the impression that once the first packet is received correctly, 
the connection is stable. I see indeed lots of attempts for the first 
packet (and the cressi application even re-opens the serial port, which 
doesn't make any sense at all). But once the data starts flowing, the 
cressi application doesn't touch the serial line settings. That 
(combined with the fact that on Windows the current code works fine) is 
why I think this is unnecessary.

If the DTR toggling at startup alone isn't enough, then we probably need 
some sort of "pinging". But I still think the dtr toggling and delay is 
only needed there, and not for every packet. For example a more optimal 
solution might already be to integrate that code into the existing retry 
loop in the cressi_leonardo_transfer() function. That way, if the first 
attempt works immediately, then there will be no delay and thus also no 
performance problem. Only when transmitting a packet fails, we do the 
whole dtr toggling with delay before retrying again. In that case, the 
extra delay is fine, because there is a problem anyway.

I've attached a capture file. It's created with portmon (which 
unfortunately only works on 32-bit windows). This kind of capture file 
is a lot easier to interpret than the pcap capture because you don't 
have to deal with the overhead of the ftdi usb-serial chip.

The complete download session is from line 2192 to 31959. These are the 
commands that are being send:

WRITE   7B 31 30 30 30 30 30 31 33 39 33 30 36 7D -> {100000139306}
WRITE   7B 31 30 33 30 30 30 30 38 44 46 42 43 7D -> {10300008DFBC}
WRITE   7B 31 30 34 30 30 30 30 31 38 36 44 34 7D -> {1040000186D4}
WRITE   7B 31 30 35 30 30 30 30 31 43 33 37 34 7D -> {10500001C374}
WRITE   7B 31 30 36 30 30 30 30 46 30 33 45 34 7D -> {1060000F03E4}
WRITE   7B 31 30 38 30 30 30 30 31 38 44 33 37 7D -> {108000018D37}
WRITE   7B 31 30 38 32 30 30 30 41 42 37 32 33 7D -> {1082000AB723}
WRITE   7B 31 30 39 30 30 30 31 30 45 42 38 37 7D -> {10900010EB87}
WRITE   7B 31 32 33 44 42 41 7D                   -> {123DBA}

WRITE   7B 31 30 30 30 30 30 31 33 39 33 30 36 7D
WRITE   7B 31 30 33 30 30 30 30 38 44 46 42 43 7D
WRITE   7B 31 30 34 30 30 30 30 31 38 36 44 34 7D
WRITE   7B 31 30 35 30 30 30 30 31 43 33 37 34 7D
WRITE   7B 31 30 36 30 30 30 30 46 30 33 45 34 7D
WRITE   7B 31 30 38 30 30 30 30 31 38 44 33 37 7D
WRITE   7B 31 30 38 32 30 30 30 41 42 37 32 33 7D
WRITE   7B 31 30 39 30 30 30 31 30 45 42 38 37 7D
WRITE   7B 31 32 33 44 42 41 7D

WRITE   7B 31 30 30 30 30 30 31 33 39 33 30 36 7D -> {100000139306}
WRITE   7B 31 30 30 30 30 30 31 33 39 33 30 36 7D -> {100000139306}
WRITE   7B 32 32 36 38 45 39 7D                   -> {2268E9}

(Not sure why, but there are two full downloads in there. I also don't 
know what that last command does.)

If you look closely at this part, you can clearly see that the line 
settings are changed only at startup, and then again just before the 
{123DBA} dump command. But it looks like it's only increasing the buffer 
size (from 640 to 80960). All other settings (including DTR and RTS) 
remain at their current values.

Jef
-------------- next part --------------
A non-text attachment was scrubbed...
Name: leonardo.log.gz
Type: application/x-gzip
Size: 250754 bytes
Desc: not available
URL: <http://libdivecomputer.org/pipermail/devel/attachments/20171003/5aebc580/attachment-0001.bin>