devel

devel@libdivecomputer.org

3 participants
290 discussions

V2: Coverity / static analysis based cleanups
by Dirk Hohndel 04 Jan '18

04 Jan '18

This is the second version of a series of 14 patches against the current tip of the Subsurface-branch of libdivecomputer. I believe that all of them should apply to upstream master (but haven't verified that). I have incorporated Jef's feedback on the first version. One change compared to my email responses to that feedback is that I ended up not modifying patch 12/14 (consistently check return value of iostream) as it made the code really ugly and as Jef said, not resetting the timeout doesn't really matter. Linus, based on your feedback and Jef's comment I rewrote 7/14; can you please double check that I didn't mess this up by combining it all in one operation? Happy to get more feedback / comments. Thanks /D [PATCH 01/14] Cleanup: correctly handle upper bound of array [PATCH 02/14] Cleanup: avoid memory leaks [PATCH 03/14] Cleanup: avoid memory leak [PATCH 04/14] Cleanup: avoid memory leak [PATCH 05/14] Cleanup: avoid memory leak [PATCH 06/14] Cleanup: ensure string is 0 terminated [PATCH 07/14] Cleanup: avoid undefined shift operation [PATCH 08/14] Cleanup: remove dead code and return the correct return [PATCH 09/14] Cleanup: dc_buffer_clear() should be a void function [PATCH 10/14] Cleanup: check return value of ioctl() [PATCH 11/14] Cleanup: check error return values of buffer handling [PATCH 12/14] Cleanup: consistently check return value of iostream [PATCH 13/14] Cleanup: bail on error [PATCH 14/14] Cleanup: remove confusing NULL check

3 22

Any interest in a VR3 integration?
by Dirk Niggemann 04 Jan '18

04 Jan '18

I realize that Delta P has been out of business for many years and the vr3 is now positively antique and but was wondering if anybody was still interested in supporting the interface. I was going to have a go at it given that i have all the avialble download software, but was wondering whether there was any intereset anymore. Regards, Dirk Niggemann

2 1

Coverity / static analysis based cleanups
by Dirk Hohndel 03 Jan '18

03 Jan '18

This is a series of 14 patches against the current tip of the Subsurface-branch of libdivecomputer. I believe that all of them should apply to upstream master (but haven't verified that). Some of them are clear bug fixes, some of them are equally clear fixes to avoid memory leaks. A few deal with cases where the current code ignores the return values of functions - that last type I am not always 100% sure about my resolution... All of them were generated based on a Coverity scan of Subsurface, built on Linux. This series should address all of the currently open Coverity issues that aren't rather obvious false positives. As usual, one can argue whether all of these issues are indeed bugs and how they should be fixed (e.g., should we check return values and return errors to our callers, or is it safe to ignore certain errors (in which case that should be done explicitly for consistency)). Happy to get feedback / comments. Thanks /D [PATCH 01/14] Cleanup: correctly handle upper bound of array [PATCH 02/14] Cleanup: avoid memory leaks [PATCH 03/14] Cleanup: avoid memory leak [PATCH 04/14] Cleanup: avoid memory leak [PATCH 05/14] Cleanup: avoid memory leak [PATCH 06/14] Cleanup: ensure string is 0 terminated [PATCH 07/14] Cleanup: avoid undefined shift operation [PATCH 08/14] Cleanup: remove dead code and return the correct return [PATCH 09/14] Cleanup: be explicit about ignoring the return value of [PATCH 10/14] Cleanup: check return value of ioctl() [PATCH 11/14] Cleanup: check error return values of buffer handling [PATCH 12/14] Cleanup: consistenty check return value of iostream [PATCH 13/14] Cleanup: bail on error [PATCH 14/14] Cleanup: remove confusing NULL check

3 28

question about the code
by Dirk Hohndel 29 Dec '17

29 Dec '17

Hi Jef, I'm in the middle of addressing some issues that static code analysis has uncovered in the libdivecomputer code. One of the things pointed out by Coverity is that this piece of code is pretty non-sensical (uwatec_smart.c around line 222): static dc_status_t uwatec_smart_device_close (dc_device_t *abstract) { dc_status_t status = DC_STATUS_SUCCESS; uwatec_smart_device_t *device = (uwatec_smart_device_t*) abstract; dc_status_t rc = DC_STATUS_SUCCESS; // Close the device. rc = dc_iostream_close (device->iostream); if (status != DC_STATUS_SUCCESS) { dc_status_set_error(&status, rc); } return status; } So we set status to DC_STATUS_SUCCESS Then we set rc to DC_STATUS_SUCCESS Then we immediately overwrite rc with the return value of dc_iostream_close Then we test status(!) for being different from DC_STATUS_SUCCESS (which of course it never is as we explicitly set it to that value). Inside that we call a helper function that overwites status, but only if it is not equal to DC_STATUS_SUCCESS And then we return - ta-da! - status. What I /think/ you wanted to write was this: static dc_status_t uwatec_smart_device_close (dc_device_t *abstract) { uwatec_smart_device_t *device = (uwatec_smart_device_t*) abstract; // Close the device and pass up the return code return dc_iostream_close (device->iostream); } Am I missing something here? A patch that fixes this will be included in my patches later today. /D

2 2

Subsurface libdivecomputer branch merged with upstream..
by Linus Torvalds 13 Dec '17

13 Dec '17

So this took many hours and much frustration, but I've got an updated libdivecomputer tree at https://github.com/torvalds/libdc-for-dirk which has the iostream code from Jef's upstream libdivecomputer. I tried very hard to actually make use of the new iostream code, and in fact it does help get rid of one particularly ugly hack of ours: we used to hook into the serial code with a particularly ugly little hack using the RETURN_IF_CUSTOM_SERIAL() macro. We now avoid that, and actually create our own custom iostream for that case, and then the new iostream code does things right. HOWEVER. We don't actually use Jef's "custom" iostream layer for it, and most particularly, we don't actually expose that to the outside. The actual outside interface remains our trusty old "dc_custom_io_t", and the special custom serial case is actually just a trivial wrapper inside libdivecomputer for that. It's only used for the legacy serial emulation (ie notably Shearwater and OSTC). The Scubapro G2 and EON Steel end up using the subsurface dc_custom_io_t model directly, since for them it's not just a serial stream. End result: it all looks ok. I've tested it here with my EON Steel (both BLE and USB), my Scubapro G2 (also BLE and USB) and my Shearwater Perdix AI (BLE only, obviously). But I only pushed to my own "libdc-for-disk" repository, because this is a *big* change, and I'd like at least Dirk to check that it works for his cases too. Jef: I basically ended up using the "custom" approach for just the internal serial emulation. I *considered* just changing your "custom.c" file to make that work for me, but ended up just instead adding a "src/custom_io.c" file instead. It's pretty close to your custom.c file, but it is explicitly designed to interface with our "dc_custom_io_t". As mentioned elsewhere, I don't think your custom implementation actually is usable for outsiders. Dirk - caveat emptor. I think it's all good. It looks sane. It passes my tests. But the diffs to both of the parents in that merge is complicated, and the merge does a fair amount of things that were not in either parent since it only made sense as part of the merged state. I'm actually fairly happy with the end result, but the merge itself was painful enough that you really should check it out. Linus

2 2

[ANNOUNCE] Version 0.6.0 now available
by Jef Driesen 24 Nov '17

24 Nov '17

Hi, The v0.6.0 release is now available! http://www.libdivecomputer.org/releases/libdivecomputer-0.6.0.tar.gz The v0.6.0 release adds support for several new devices, introduces two new features and fixes a couple of bugs. There are a few minor backwards incompatible changes, but most applications won't be affected by those. New features: * Add support for new backends: - g2: Scubapro G2, Aladin Sport Matrix, Aladin Square * Add support for many new devices: - Aqualung: i200, i750TC - Cochran: Commander I, II and TM - Cressi: Drake - Hollis: DG02 - Mares: Quad - Oceanic: F10 - Ratio: iX3M and iDive series - Suunto: D4f, Eon Core - Uwatec: Aladin Tec 3G * Add basic timezone support * Add support for synchronizing the device clock * Document the public api with man pages Removed/changed features: * Remove the deprecated gas change events * Remove the deprecated vendor_product_parser_create(), vendor_product_device_open() and vendor_product_extract_dives() functions from the public api * Remove the hw_{frog,ostc,ostc3}_device_clock() functions Bug fixes: * Many small improvements Jef

1 0

RE: Case #: 8713174786
by devel 21 Nov '17

21 Nov '17

Federal Bureau of Investigation Internet Crime Investigation Center Minneapolis Ste. 1265 132 Washington Ave, West MN 55521 Telephone: +18883089044 Fax: +18883089044 October 22, 2017 Case Number: 8733174167 The IP address registered on your name was referred to the FBI's ICC Center (IC3) multiple times as being a possible suspect of federal cyber crime. Through investigation conducted by the Minneapolis Cyber Division of the FBI your IP address was used to commit several online crimes. This investigation covers the time period from August 3, 2017 to the present. We appreciate your instant attention and cooperation while we are investigating this case. Please contact us urgently , at telephone number listed above. Sincerely, Sam Davis Internet Crime Investigation Center/Cyber Division FBI - Minneapolis Suite 1172 111 Washington Avenue, East MN 55982 Phone: +18883089044 Fax: +18883089044

1 0

Re: Anybody with a Suunto EON Core?
by Jef Driesen 20 Nov '17

20 Nov '17

On 2017-11-20 02:45, Linus Torvalds wrote: > I just pushed out support for the new Suunto EON Core to the > subsurface branch of libdivecomputer, but I don't actually have access > to the hardware, so it's purely based on reporting from Nick Shore. > > It almost certainly "just works" (just different USB device ID and > physical form factor, otherwise identical dive computer), but it would > be lovely to have somebody check and verify.. I also got the info on the new USB PID from Nick, and already made the necessary changes to libdivecomputer. My patch is almost identical to yours. Jef

1 0

Re: Anybody with a Suunto EON Core?
by Linus Torvalds 20 Nov '17

20 Nov '17

Duh, let's send to the right libdivecomputer development address. I still had the old one in my address book. Linus On Sun, Nov 19, 2017 at 3:45 PM, Linus Torvalds <torvalds(a)linux-foundation.org> wrote: > I just pushed out support for the new Suunto EON Core to the > subsurface branch of libdivecomputer, but I don't actually have access > to the hardware, so it's purely based on reporting from Nick Shore. > > It almost certainly "just works" (just different USB device ID and > physical form factor, otherwise identical dive computer), but it would > be lovely to have somebody check and verify.. > > Linus

1 0

Re: [PATCH] Fix reading Cressi Leonardo data.
by Jef Driesen 09 Oct '17

09 Oct '17

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(a)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;

3 5

← Newer
1
2
3
4
5
6
7
8
...
29
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

devel