[PATCH] Fix reading Cressi Leonardo data.
Всеволод Величко
torkvemada at sorokdva.net
Mon Oct 2 04:32:34 PDT 2017
Jef,
Thanks for your feedback!
I've experimented with the transfer for some time and finally found
that without those "ping" packages it really never works for some
reason, regardless how hard do we try to purge incoming garbage data.
There's a working version of the script[1], which successfully dumps
the data on each call, but if I remove first ping() or call dtr-toggle
just once at startup, it failed to receive the dump. I'll try to do
double-toggle at startup later today, may be it'll help.
Regading the delays, here I attach the pcap file from the Windows
Cressi software. I was unable to find which packets correspond with
the set_dtr or clr_dtr, but if you can decode them, they come along
with the timings. There was two connections of the computer to the
adapter inside (hence also two full dumps of the data), first request
of dump comes at packet 2484, and the second one is at the packet
6069.
[1] http://paste.ubuntu.com/25660082/
--
Best wishes,
Vsevolod Velichko
2017-10-02 13:14 GMT+03:00 Jef Driesen <jef at libdivecomputer.org>:
> Vsevolod,
>
> Glad to see you found a way to fix the problem!
>
> However, I'm not really sure whether it's the right solution. You are
> calling the cressi_leonardo_transfer_prepare() function from inside the
> cressi_leonardo_packet() function. That means you're doing this for every
> single packet! But if the problem is only during the initial setup of the
> connection, then that's probably overkill. Note that due to the 300ms sleep
> in that function, there is also a serious impact on the performance. So you
> are probably correct that the setup phase needs some tweaking, but I doubt
> this is necessary for every single packet.
>
> I went back to have a look at some of the capture files I have, and the
> Cressi application doesn't seem to change DTR or RTS between packets. But it
> does this sequence at startup:
>
> CREATE Options: Open
> GET_BAUD_RATE
> GET_LINE_CONTROL
> GET_CHARS
> GET_HANDFLOW
> SET_BAUD_RATE Rate: 9600
> SET_RTS
> SET_DTR
> SET_LINE_CONTROL StopBits: 1 Parity: NONE WordLength: 8
> SET_CHAR EOF:0 ERR:0 BRK:0 EVT:0 XON:11 XOFF:13
> SET_HANDFLOW Shake:1 Replace:40 XonLimit:64 XoffLimit:64
> SET_QUEUE_SIZE InSize: 640 OutSize: 640
> SET_TIMEOUTS RI:-1 RM:0 RC:0 WM:0 WC:500
> SET_RTS
> SET_DTR
> GET_BAUD_RATE
> GET_LINE_CONTROL
> GET_CHARS
> GET_HANDFLOW
> SET_BAUD_RATE Rate: 115200
> SET_RTS
> SET_DTR
> SET_LINE_CONTROL StopBits: 1 Parity: NONE WordLength: 8
> SET_CHAR EOF:0 ERR:0 BRK:0 EVT:0 XON:11 XOFF:13
> SET_HANDFLOW Shake:1 Replace:40 XonLimit:64 XoffLimit:64
> GET_BAUD_RATE
> GET_LINE_CONTROL
> GET_CHARS
> GET_HANDFLOW
> SET_QUEUE_SIZE InSize: 640 OutSize: 640
> SET_TIMEOUTS RI:-1 RM:0 RC:0 WM:0 WC:500
> GET_BAUD_RATE
> GET_LINE_CONTROL
> GET_CHARS
> GET_HANDFLOW
> SET_BAUD_RATE Rate: 115200
> SET_RTS
> SET_DTR
> SET_LINE_CONTROL StopBits: 1 Parity: NONE WordLength: 8
> SET_CHAR EOF:0 ERR:0 BRK:0 EVT:0 XON:11 XOFF:13
> SET_HANDFLOW Shake:1 Replace:40 XonLimit:64 XoffLimit:64
> GET_BAUD_RATE
> GET_LINE_CONTROL
> GET_CHARS
> GET_HANDFLOW
> SET_QUEUE_SIZE InSize: 640 OutSize: 640
> SET_TIMEOUTS RI:-1 RM:0 RC:0 WM:0 WC:500
> CLR_DTR
> SET_DTR
> CLR_DTR
> WRITE 7B 31 30 30 30 30 30 31 33 39 33 30 36 7D
> FLUSH_BUFFERS
> SET_TIMEOUTS RI:-1 RM:0 RC:10000 WM:0 WC:500
> READ 7B 30 31 43 30 32 36 30 30 46 46 46 46 46 46 46 46 46 46 46 46 46 46
> 46 46 46 46 46 46 46 46 46 46 30 31 30 35 30 31 43 41 39 31 7D
>
> So it first sets DTR and RTS:
>
> SET_RTS
> SET_DTR
>
> (The fact that this happens a few times is probably due to use of some
> serial port package that doesn't change all settings at once. The
> libdivecomputer configure function does this with just one call.)
>
> And then before sending the first command it toggles DTR:
>
> CLR_DTR
> SET_DTR
> CLR_DTR
>
> And that might be the key to the solution, because libdivecomputer only does
> this:
>
> SET_RTS
> SET_DTR
>
> CLR_DTR
>
> So an extra SET_DTR and CLR_DTR at startup, with the right amount of delays
> in between, might already be sufficient.
>
> There is also a difference in behavior in this area between the windows and
> linux serial port code. Those initial SET_RTS and SET_DTR are there, because
> the windows api automatically sets or clears RTS and DTR when configuring
> the serial lines settings (based on the dcb.fDtrControl and dcb.fRtsControl
> settings). But on linux, the DTR/RTS lines are left alone, and you need to
> control them manually. That might explain why everything works on windows,
> but not on linux.
>
> So I think we need to do the following sequence:
>
> SET_RTS
> SET_DTR <- This can probably be omitted, but it makes sure windows and linux
> behave the same
> CLR_DTR
> SET_DTR
> CLR_DTR
>
> RTS only needs to be set once, but the dive computer probably reacts to the
> DTR toggling, so that's the part that matters I think. And we need some
> delays in between to make sure it doesn't happen too fast. When I reverse
> engineered this, I just looked at the final value for DTR (plus the initial
> set to get the same result on windows and linux), but now I think it's the
> toggling that does the trick.
>
> So in libdivecomputer calls that would translate to the following sequence.
> First initialize the serial port settings, making sure windows and linux are
> the same:
>
> dc_serial_configure (device->port, 115200, 8, DC_PARITY_NONE,
> DC_STOPBITS_ONE, DC_FLOWCONTROL_NONE)
> dc_serial_set_timeout (device->port, 1000);
> dc_serial_set_rts (device->port, 1);
> dc_serial_set_dtr (device->port, 1);
>
> Then discard garbage data as usual:
>
> dc_serial_sleep (device->port, 100);
> dc_serial_purge (device->port, DC_DIRECTION_ALL);
>
> And finally toggle the DTR line:
>
> dc_serial_set_dtr (device->port, 0);
> dc_serial_sleep (device->port, SOMEDELAY);
> dc_serial_set_dtr (device->port, 1);
> dc_serial_sleep (device->port, SOMEDELAY);
> dc_serial_set_dtr (device->port, 0);
>
> I only don't know how long the delays should be, because I have no timing
> info in the capture file.
>
> Does that make sense?
>
> Jef
>
>
> On 2017-10-01 13:38, Vsevolod Velichko wrote:
>>
>> Cressi Leonardo computers behave extremely unreliable if the dump is
>> requested right after the connection establishment.
>> The computer mostly sends nothing or sends garbage, so official Cressi
>> software uses request of small device memory part as a ping until
>> device become stable.
>> And DTR flag should be reset before each request, or device wouldn't
>> send any data in response.
>>
>> This patch fixes
>> https://github.com/Subsurface-divelog/subsurface/issues/391 and
>> probably all previous mentions of Leonardo problems.
>>
>> Signed-off-by: Vsevolod Velichko <torkvemada at sorokdva.net>
>> ---
>> src/cressi_leonardo.c | 133
>> +++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 89 insertions(+), 44 deletions(-)
>>
>> diff --git a/src/cressi_leonardo.c b/src/cressi_leonardo.c
>> index f21a223..14db35f 100644
>> --- a/src/cressi_leonardo.c
>> +++ b/src/cressi_leonardo.c
>> @@ -44,7 +44,10 @@
>> #define RB_PROFILE_END SZ_MEMORY
>> #define RB_PROFILE_DISTANCE(a,b) ringbuffer_distance (a, b, 0,
>> RB_PROFILE_BEGIN, RB_PROFILE_END)
>>
>> -#define MAXRETRIES 4
>> +#define PING_ADDRESS 0x1000
>> +#define PING_LENGTH 19
>> +
>> +#define MAXRETRIES 20
>> #define PACKETSIZE 32
>>
>> typedef struct cressi_leonardo_device_t {
>> @@ -74,6 +77,9 @@ static const dc_device_vtable_t
>> cressi_leonardo_device_vtable = {
>> static dc_status_t
>> cressi_leonardo_extract_dives (dc_device_t *abstract, const unsigned
>> char data[], unsigned int size, dc_dive_callback_t callback, void
>> *userdata);
>>
>> +static dc_status_t
>> +cressi_leonardo_transfer_prepare (cressi_leonardo_device_t *device,
>> dc_context_t *context);
>> +
>> static void
>> cressi_leonardo_make_ascii (const unsigned char raw[], unsigned int
>> rsize, unsigned char ascii[], unsigned int asize)
>> {
>> @@ -105,6 +111,13 @@ cressi_leonardo_packet (cressi_leonardo_device_t
>> *device, const unsigned char co
>> if (device_is_cancelled (abstract))
>> return DC_STATUS_CANCELLED;
>>
>> + // Prepare device for writing.
>> + status = cressi_leonardo_transfer_prepare (device,
>> abstract->context);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (abstract->context, "Failed to prepare device for
>> transfer.");
>> + return status;
>> + }
>> +
>> // Send the command to the device.
>> status = dc_serial_write (device->port, command, csize, NULL);
>> if (status != DC_STATUS_SUCCESS) {
>> @@ -141,6 +154,37 @@ cressi_leonardo_packet (cressi_leonardo_device_t
>> *device, const unsigned char co
>> }
>>
>> static dc_status_t
>> +cressi_leonardo_transfer_prepare (cressi_leonardo_device_t *device,
>> dc_context_t *context)
>> +{
>> + // Set the RTS line.
>> + dc_status_t status = dc_serial_set_rts (device->port, 1);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (context, "Failed to set the RTS line.");
>> + return status;
>> + }
>> +
>> + // Set the DTR line.
>> + status = dc_serial_set_dtr (device->port, 1);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (context, "Failed to set the DTR line.");
>> + return status;
>> + }
>> +
>> + dc_serial_sleep (device->port, 200);
>> +
>> + // Clear the DTR line.
>> + status = dc_serial_set_dtr (device->port, 0);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (context, "Failed to clear the DTR line.");
>> + return status;
>> + }
>> +
>> + dc_serial_sleep (device->port, 100);
>> + dc_serial_purge (device->port, DC_DIRECTION_ALL);
>> + return status;
>> +}
>> +
>> +static dc_status_t
>> cressi_leonardo_transfer (cressi_leonardo_device_t *device, const
>> unsigned char command[], unsigned int csize, unsigned char answer[],
>> unsigned int asize)
>> {
>> unsigned int nretries = 0;
>> @@ -197,39 +241,13 @@ cressi_leonardo_device_open (dc_device_t **out,
>> dc_context_t *context, const cha
>> goto error_close;
>> }
>>
>> - // Set the timeout for receiving data (1000 ms).
>> - status = dc_serial_set_timeout (device->port, 1000);
>> + // Set the timeout for receiving data (3000 ms).
>> + status = dc_serial_set_timeout (device->port, 3000);
>> if (status != DC_STATUS_SUCCESS) {
>> ERROR (context, "Failed to set the timeout.");
>> goto error_close;
>> }
>>
>> - // Set the RTS line.
>> - status = dc_serial_set_rts (device->port, 1);
>> - if (status != DC_STATUS_SUCCESS) {
>> - ERROR (context, "Failed to set the RTS line.");
>> - goto error_close;
>> - }
>> -
>> - // Set the DTR line.
>> - status = dc_serial_set_dtr (device->port, 1);
>> - if (status != DC_STATUS_SUCCESS) {
>> - ERROR (context, "Failed to set the DTR line.");
>> - goto error_close;
>> - }
>> -
>> - dc_serial_sleep (device->port, 200);
>> -
>> - // Clear the DTR line.
>> - status = dc_serial_set_dtr (device->port, 0);
>> - if (status != DC_STATUS_SUCCESS) {
>> - ERROR (context, "Failed to clear the DTR line.");
>> - goto error_close;
>> - }
>> -
>> - dc_serial_sleep (device->port, 100);
>> - dc_serial_purge (device->port, DC_DIRECTION_ALL);
>> -
>> *out = (dc_device_t *) device;
>>
>> return DC_STATUS_SUCCESS;
>> @@ -332,29 +350,56 @@ cressi_leonardo_device_dump (dc_device_t
>> *abstract, dc_buffer_t *buffer)
>> progress.maximum = SZ_MEMORY;
>> device_event_emit (abstract, DC_EVENT_PROGRESS, &progress);
>>
>> - // Send the command header to the dive computer.
>> - const unsigned char command[] = {0x7B, 0x31, 0x32, 0x33, 0x44,
>> 0x42,
>> 0x41, 0x7d};
>> - status = dc_serial_write (device->port, command, sizeof (command),
>> NULL);
>> - if (status != DC_STATUS_SUCCESS) {
>> - ERROR (abstract->context, "Failed to send the command.");
>> - return status;
>> + for (int attempt = 0; attempt < MAXRETRIES; ++attempt) {
>> + // Send the ping command to the dive computer.
>> + unsigned char tmp[PING_LENGTH] = {0};
>> + status = cressi_leonardo_device_read(abstract,
>> PING_ADDRESS, tmp,
>> sizeof(tmp));
>> + if (status == DC_STATUS_SUCCESS)
>> + break;
>> }
>>
>> - // Receive the header packet.
>> - unsigned char header[7] = {0};
>> - status = dc_serial_read (device->port, header, sizeof (header),
>> NULL);
>> if (status != DC_STATUS_SUCCESS) {
>> - ERROR (abstract->context, "Failed to receive the
>> answer.");
>> + ERROR (abstract->context, "Failed to ping device.");
>> return status;
>> }
>>
>> - // Verify the header packet.
>> - const unsigned char expected[] = {0x7B, 0x21, 0x44, 0x35, 0x42,
>> 0x33, 0x7d};
>> - if (memcmp (header, expected, sizeof (expected)) != 0) {
>> - ERROR (abstract->context, "Unexpected answer byte.");
>> - return DC_STATUS_PROTOCOL;
>> + for (int attempt = 0; attempt < MAXRETRIES; ++attempt) {
>> + status = cressi_leonardo_transfer_prepare (device,
>> abstract->context);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (abstract->context, "Failed to prepare
>> device for transfer.");
>> + continue;
>> + }
>> +
>> + // Send the command header to the dive computer.
>> + const unsigned char command[] = {0x7B, 0x31, 0x32, 0x33,
>> 0x44,
>> 0x42, 0x41, 0x7d};
>> + status = dc_serial_write (device->port, command, sizeof
>> (command), NULL);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (abstract->context, "Failed to send the
>> command.");
>> + continue;
>> + }
>> +
>> + // Receive the header packet.
>> + unsigned char header[7] = {0};
>> + status = dc_serial_read (device->port, header, sizeof
>> (header), NULL);
>> + if (status != DC_STATUS_SUCCESS) {
>> + ERROR (abstract->context, "Failed to receive the
>> answer.");
>> + continue;
>> + }
>> +
>> + // Verify the header packet.
>> + const unsigned char expected[] = {0x7B, 0x21, 0x44, 0x35,
>> 0x42, 0x33, 0x7d};
>> + if (memcmp (header, expected, sizeof (expected)) != 0) {
>> + ERROR (abstract->context, "Unexpected answer
>> byte.");
>> + status = DC_STATUS_PROTOCOL;
>> + continue;
>> + }
>> +
>> + break;
>> }
>>
>> + if (status != DC_STATUS_SUCCESS)
>> + return status;
>> +
>> unsigned char *data = dc_buffer_get_data (buffer);
>>
>> unsigned int nbytes = 0;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cressi.pcap.gz
Type: application/x-gzip
Size: 64791 bytes
Desc: not available
URL: <http://libdivecomputer.org/pipermail/devel/attachments/20171002/fb1b5a88/attachment-0001.bin>
More information about the devel
mailing list