Hi there, I did the modifications we discussed. You can find attached some new patches. I hope that now they are grouped correctly.
For the moment I updated only the HW_OSTC3 family to use the custom serial implementation. These patches can be applied on the origin/Subsurface-testing branch. Please let me know if the old native serial communication doesn't work after you apply them. I will send you a new patch for the Subsurface project to work with the new design of the *dc_serial_t* structure. Claudiu On Fri, Jun 26, 2015 at 11:29 PM, Claudiu Olteanu < [email protected]> wrote: > > >> One way to do this would be to copy Jef's copyright line and add yours >> below: >> >> * Copyright (C) 2008 Jef Driesen >> * Copyright (C) 2015 Claudiu Olteanu >> > > Thanks for the information. Now it is more clear for me. > > >> > I think that I should remove the fd, timeout and context fields and >> > to add a serial_t *port member >> >> See what I wrote above. I'm fine with other. I seem to remember that Jef >> in general prefers the structures with common parts in front that can >> simply be passed around and then work as expected... >> > > This is good to know. I believe that it would be easier to use a pointer > to > a *serial_t* structure since all the functions expect to receive a > pointer. > In the old implementation, all structures which are used to represent > a device contain a member which is a pointer to a *serial_t* structure. > Therefore I believe that this is more in line with the old design. > > >> > > This feels like it should have been squashed into the first patch... >> > >> > Yes, you are right. I forgot to add it in the beginning. >> >> As a general rule I do NOT enforce this. I am MUCH happier when people do >> small patches and commit often. But since you were reworking the patches >> anyway (and will be reworking them to address the data structure issues) >> it would be nice to turn commits that clearly are nothing but fixups into >> just that (git even has the cool "--fixup" feature in newer versions... > > > I didn't know about the --fixup feature. I will take it into consideration > on the > future, when I will create my final patches. Also I intend to submit new > ones, > which doesn't contain fixup commits. > > > >> Right now I can't merge this into the Subsurface-testing branch because >> most of our testers have USB (or more correctly, serial) based devices. >> But once this becomes transparent I can add this and we can get much more >> testing. > > > I totally understand that. I created the patches only to receive some > feedback > from you and to decide on the architecture. > > Claudiu > >
From 05d830371f2bce35820bebb7419ed27946b3a658 Mon Sep 17 00:00:00 2001 From: Claudiu Olteanu <[email protected]> Date: Sat, 27 Jun 2015 15:11:15 +0300 Subject: [PATCH 1/5] Extend the transport enum descriptor for serial communication Add a new transport type which can be used to identify Bluetooth serial communication. Signed-off-by Claudiu Oleanu <[email protected]> --- include/libdivecomputer/descriptor.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/libdivecomputer/descriptor.h b/include/libdivecomputer/descriptor.h index 6f9735d..f1c815d 100644 --- a/include/libdivecomputer/descriptor.h +++ b/include/libdivecomputer/descriptor.h @@ -33,7 +33,8 @@ typedef enum dc_transport_t { DC_TRANSPORT_NONE, DC_TRANSPORT_SERIAL, DC_TRANSPORT_USB, - DC_TRANSPORT_IRDA + DC_TRANSPORT_IRDA, + DC_TRANSPORT_BLUETOOTH } dc_transport_t; typedef struct dc_descriptor_t dc_descriptor_t; -- 2.1.4
From b4092f3b4b04f9b31097f6c79b2e1cdafcaf5a06 Mon Sep 17 00:00:00 2001 From: Claudiu Olteanu <[email protected]> Date: Sat, 27 Jun 2015 15:14:16 +0300 Subject: [PATCH 2/5] Create a generic way to represent any type of serial communication Add a structure which holds references to basic operations on a serial communication. This can be used to pass a set of function pointer callbacks in order to create a custom implementation for serial communication. Add a generic structure to represent the needed information for a serial communication. Implement the initialization method where the user can pass a set of function pointer callbacks and set some custom data for the serial device. Create open method for the native serial implementation. Signed-off-by: Claudiu Olteanu <[email protected]> --- include/libdivecomputer/Makefile.am | 3 +- include/libdivecomputer/custom_serial.h | 63 ++++++++++++++++++++++++++ src/Makefile.am | 3 +- src/custom_serial.c | 79 +++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 include/libdivecomputer/custom_serial.h create mode 100644 src/custom_serial.c diff --git a/include/libdivecomputer/Makefile.am b/include/libdivecomputer/Makefile.am index 71887d5..a0ed196 100644 --- a/include/libdivecomputer/Makefile.am +++ b/include/libdivecomputer/Makefile.am @@ -53,4 +53,5 @@ libdivecomputer_HEADERS = \ citizen.h \ citizen_aqualand.h \ divesystem.h \ - divesystem_idive.h + divesystem_idive.h \ + custom_serial.h diff --git a/include/libdivecomputer/custom_serial.h b/include/libdivecomputer/custom_serial.h new file mode 100644 index 0000000..a52d49b --- /dev/null +++ b/include/libdivecomputer/custom_serial.h @@ -0,0 +1,63 @@ +/* + * libdivecomputer + * + * Copyright (C) 2015 Claudiu Olteanu + * base on code that is Copyright (C) 2008 Jef Driesen + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + */ + +#ifndef CUSTOM_SERIAL_H +#define CUSTOM_SERIAL_H + +#include <string.h> +#include <stdlib.h> + +#include "context.h" +#include "descriptor.h" + +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + +typedef struct serial_t serial_t; + +typedef struct dc_serial_operations_t +{ + int (*open) (serial_t **device, dc_context_t *context, const char *name); + int (*close) (serial_t *device); + int (*read) (serial_t *device, void* data, unsigned int size); + int (*write) (serial_t *device, const void* data, unsigned int size); + int (*flush) (serial_t *device, int queue); + int (*get_received) (serial_t *device); + int (*get_transmitted) (serial_t *device); +} dc_serial_operations_t; + +typedef struct dc_serial_t { + serial_t *port; //serial device port + dc_transport_t type; //the type of the transport (USB, SERIAL, IRDA, BLUETOOTH) + void *data; //specific data for serial device + const dc_serial_operations_t *ops; //reference to a custom set of operations +} dc_serial_t; + +void dc_serial_init(dc_serial_t *device, void *data, const dc_serial_operations_t *ops); + +dc_status_t dc_serial_native_open(dc_serial_t **serial, dc_context_t *context, const char *devname); + +#ifdef __cplusplus +} +#endif /* __cplusplus */ +#endif /* CUSTOM_SERIAL_H */ diff --git a/src/Makefile.am b/src/Makefile.am index abbaa48..0c60d02 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -59,7 +59,8 @@ libdivecomputer_la_SOURCES = \ ringbuffer.h ringbuffer.c \ checksum.h checksum.c \ array.h array.c \ - buffer.c + buffer.c \ + custom_serial.c if OS_WIN32 libdivecomputer_la_SOURCES += serial.h serial_win32.c diff --git a/src/custom_serial.c b/src/custom_serial.c new file mode 100644 index 0000000..6e024b2 --- /dev/null +++ b/src/custom_serial.c @@ -0,0 +1,79 @@ +/* + * libdivecomputer + * + * Copyright (C) 2015 Claudiu Olteanu + * base on code that is Copyright (C) 2008 Jef Driesen + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301 USA + */ + +#include <libdivecomputer/custom_serial.h> +#include <serial.h> + +#include "context-private.h" + +const dc_serial_operations_t native_serial_ops = { + .open = serial_open, + .close = serial_close, + .read = serial_read, + .write = serial_write, + .flush = serial_flush, + .get_received = serial_get_received, + .get_transmitted = serial_get_transmitted +}; + + +void +dc_serial_init(dc_serial_t *device, void *data, const dc_serial_operations_t *ops) +{ + memset(device, 0, sizeof (*device)); + device->data = data; + device->ops = ops; +} + + +dc_status_t +dc_serial_native_open(dc_serial_t **out, dc_context_t *context, const char *devname) +{ + if (out == NULL) + return DC_STATUS_INVALIDARGS; + + // Allocate memory. + dc_serial_t *serial_device = (dc_serial_t *) malloc (sizeof (dc_serial_t)); + + if (serial_device == NULL) { + ERROR (context, "Failed to allocate memory."); + return DC_STATUS_NOMEMORY; + } + + // Initialize data and function pointers + dc_serial_init(serial_device, NULL, &native_serial_ops); + + // Open the serial device. + int rc = serial_open (&serial_device->port, context, devname); + if (rc == -1) { + ERROR (context, "Failed to open the serial port."); + free (serial_device); + return DC_STATUS_IO; + } + + // Set the type of the device + serial_device->type = DC_TRANSPORT_SERIAL; + + *out = serial_device; + + return DC_STATUS_SUCCESS; +} -- 2.1.4
From 803984baaf0f91020b4344bcdd5f801d1dad3e0f Mon Sep 17 00:00:00 2001 From: Claudiu Olteanu <[email protected]> Date: Sat, 27 Jun 2015 15:18:01 +0300 Subject: [PATCH 3/5] Use the dc_serial_t structure in HW OSTC family 3 Open a native serial device and use it in the HW OSTC3 implementation. This patch replaces the old serial structure with the new one, which can be used for custom serial implementations. Signed-off-by: Claudiu Olteanu <[email protected]> --- include/libdivecomputer/hw_ostc3.h | 2 ++ src/hw_ostc3.c | 41 +++++++++++++++++++------------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/include/libdivecomputer/hw_ostc3.h b/include/libdivecomputer/hw_ostc3.h index c60dc63..454d45c 100644 --- a/include/libdivecomputer/hw_ostc3.h +++ b/include/libdivecomputer/hw_ostc3.h @@ -2,6 +2,7 @@ * libdivecomputer * * Copyright (C) 2013 Jef Driesen + * Copyright (C) 2015 Claudiu Olteanu * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -26,6 +27,7 @@ #include "device.h" #include "parser.h" #include "buffer.h" +#include "custom_serial.h" #ifdef __cplusplus extern "C" { diff --git a/src/hw_ostc3.c b/src/hw_ostc3.c index a221fb2..a55c5cc 100644 --- a/src/hw_ostc3.c +++ b/src/hw_ostc3.c @@ -3,6 +3,7 @@ * * Copyright (C) 2013 Jef Driesen * Copyright (C) 2014 Anton Lundin + * Copyright (C) 2015 Claudiu Olteanu * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -88,7 +89,7 @@ typedef enum hw_ostc3_state_t { typedef struct hw_ostc3_device_t { dc_device_t base; - serial_t *port; + dc_serial_t *serial; unsigned char fingerprint[5]; hw_ostc3_state_t state; } hw_ostc3_device_t; @@ -163,7 +164,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, // Send the command. unsigned char command[1] = {cmd}; - int n = serial_write (device->port, command, sizeof (command)); + int n = device->serial->ops->write (device->serial->port, command, sizeof (command)); if (n != sizeof (command)) { ERROR (abstract->context, "Failed to send the command."); return EXITCODE (n); @@ -171,7 +172,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, // Read the echo. unsigned char echo[1] = {0}; - n = serial_read (device->port, echo, sizeof (echo)); + n = device->serial->ops->read (device->serial->port, echo, sizeof (echo)); if (n != sizeof (echo)) { ERROR (abstract->context, "Failed to receive the echo."); return EXITCODE (n); @@ -190,7 +191,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, if (input) { // Send the input data packet. - n = serial_write (device->port, input, isize); + n = device->serial->ops->write (device->serial->port, input, isize); if (n != isize) { ERROR (abstract->context, "Failed to send the data packet."); return EXITCODE (n); @@ -204,7 +205,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, unsigned int len = 1024; // Increase the packet size if more data is immediately available. - int available = serial_get_received (device->port); + int available = device->serial->ops->get_received (device->serial->port); if (available > len) len = available; @@ -213,7 +214,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, len = osize - nbytes; // Read the packet. - n = serial_read (device->port, output + nbytes, len); + n = device->serial->ops->read (device->serial->port, output + nbytes, len); if (n != len) { ERROR (abstract->context, "Failed to receive the answer."); return EXITCODE (n); @@ -232,7 +233,7 @@ hw_ostc3_transfer (hw_ostc3_device_t *device, if (cmd != EXIT) { // Read the ready byte. unsigned char answer[1] = {0}; - n = serial_read (device->port, answer, sizeof (answer)); + n = device->serial->ops->read (device->serial->port, answer, sizeof (answer)); if (n != sizeof (answer)) { ERROR (abstract->context, "Failed to receive the ready byte."); return EXITCODE (n); @@ -266,11 +267,11 @@ hw_ostc3_device_open (dc_device_t **out, dc_context_t *context, const char *name device_init (&device->base, context, &hw_ostc3_device_vtable); // Set the default values. - device->port = NULL; + device->serial = NULL; memset (device->fingerprint, 0, sizeof (device->fingerprint)); // Open the device. - int rc = serial_open (&device->port, context, name); + int rc = dc_serial_native_open (&device->serial, context, name); if (rc == -1) { ERROR (context, "Failed to open the serial port."); free (device); @@ -278,25 +279,25 @@ hw_ostc3_device_open (dc_device_t **out, dc_context_t *context, const char *name } // Set the serial communication protocol (115200 8N1). - rc = serial_configure (device->port, 115200, 8, SERIAL_PARITY_NONE, 1, SERIAL_FLOWCONTROL_NONE); + rc = serial_configure (device->serial->port, 115200, 8, SERIAL_PARITY_NONE, 1, SERIAL_FLOWCONTROL_NONE); if (rc == -1) { ERROR (context, "Failed to set the terminal attributes."); - serial_close (device->port); + device->serial->ops->close (device->serial->port); free (device); return DC_STATUS_IO; } // Set the timeout for receiving data (3000ms). - if (serial_set_timeout (device->port, 3000) == -1) { + if (serial_set_timeout (device->serial->port, 3000) == -1) { ERROR (context, "Failed to set the timeout."); - serial_close (device->port); + device->serial->ops->close (device->serial->port); free (device); return DC_STATUS_IO; } // Make sure everything is in a sane state. - serial_sleep (device->port, 300); - serial_flush (device->port, SERIAL_QUEUE_BOTH); + serial_sleep (device->serial->port, 300); + device->serial->ops->flush (device->serial->port, SERIAL_QUEUE_BOTH); device->state = OPEN; @@ -336,17 +337,17 @@ hw_ostc3_device_init_service (hw_ostc3_device_t *device) int n = 0; // We cant use hw_ostc3_transfer here, due to the different echos - n = serial_write (device->port, command, sizeof (command)); + n = device->serial->ops->write (device->serial->port, command, sizeof (command)); if (n != sizeof (command)) { ERROR (context, "Failed to send the command."); return EXITCODE (n); } // Give the device some time to enter service mode - serial_sleep (device->port, 100); + serial_sleep (device->serial->port, 100); // Read the response - n = serial_read (device->port, output, sizeof (output)); + n = device->serial->ops->read (device->serial->port, output, sizeof (output)); if (n != sizeof (output)) { ERROR (context, "Failed to receive the echo."); return EXITCODE (n); @@ -408,14 +409,14 @@ hw_ostc3_device_close (dc_device_t *abstract) rc = hw_ostc3_transfer (device, NULL, EXIT, NULL, 0, NULL, 0); if (rc != DC_STATUS_SUCCESS) { ERROR (abstract->context, "Failed to send the command."); - serial_close (device->port); + device->serial->ops->close (device->serial->port); free (device); return rc; } } // Close the device. - if (serial_close (device->port) == -1) { + if (device->serial->ops->close (device->serial->port) == -1) { free (device); return DC_STATUS_IO; } -- 2.1.4
From c9dfb26aca258672ceac788ceecb809f183db93c Mon Sep 17 00:00:00 2001 From: Claudiu Olteanu <[email protected]> Date: Sat, 27 Jun 2015 15:21:19 +0300 Subject: [PATCH 4/5] Implement custom open method for HW OSTC 3 family Create a custom open method for HW OSTC3 family. This method can be used to pass a reference to a dc_serial_t structure. In this way the applications can implement their own implementation for a serial communication and set their callbacks for the basic serial functions. Signed-off-by: Claudiu Olteanu <[email protected]> --- include/libdivecomputer/hw_ostc3.h | 3 ++ src/hw_ostc3.c | 57 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/include/libdivecomputer/hw_ostc3.h b/include/libdivecomputer/hw_ostc3.h index 454d45c..8463cb2 100644 --- a/include/libdivecomputer/hw_ostc3.h +++ b/include/libdivecomputer/hw_ostc3.h @@ -40,6 +40,9 @@ dc_status_t hw_ostc3_device_open (dc_device_t **device, dc_context_t *context, const char *name); dc_status_t +hw_ostc3_device_custom_open (dc_device_t **device, dc_context_t *context, dc_serial_t *serial); + +dc_status_t hw_ostc3_device_version (dc_device_t *device, unsigned char data[], unsigned int size); dc_status_t diff --git a/src/hw_ostc3.c b/src/hw_ostc3.c index a55c5cc..2d975b7 100644 --- a/src/hw_ostc3.c +++ b/src/hw_ostc3.c @@ -272,10 +272,10 @@ hw_ostc3_device_open (dc_device_t **out, dc_context_t *context, const char *name // Open the device. int rc = dc_serial_native_open (&device->serial, context, name); - if (rc == -1) { + if (rc != DC_STATUS_SUCCESS) { ERROR (context, "Failed to open the serial port."); free (device); - return DC_STATUS_IO; + return rc; } // Set the serial communication protocol (115200 8N1). @@ -307,6 +307,59 @@ hw_ostc3_device_open (dc_device_t **out, dc_context_t *context, const char *name } +dc_status_t +hw_ostc3_device_custom_open (dc_device_t **out, dc_context_t *context, dc_serial_t *serial) +{ + if (out == NULL || serial == NULL || serial->port == NULL) + return DC_STATUS_INVALIDARGS; + + // Allocate memory. + hw_ostc3_device_t *device = (hw_ostc3_device_t *) malloc (sizeof (hw_ostc3_device_t)); + if (device == NULL) { + ERROR (context, "Failed to allocate memory."); + return DC_STATUS_NOMEMORY; + } + + // Initialize the base class. + device_init (&device->base, context, &hw_ostc3_device_vtable); + + // Set the default values. + memset (device->fingerprint, 0, sizeof (device->fingerprint)); + + // Set the serial reference + device->serial = serial; + + if (serial->type == DC_TRANSPORT_SERIAL) { + // Set the serial communication protocol (115200 8N1). + int rc = serial_configure (device->serial->port, 115200, 8, SERIAL_PARITY_NONE, 1, SERIAL_FLOWCONTROL_NONE); + if (rc == -1) { + ERROR (context, "Failed to set the terminal attributes."); + device->serial->ops->close (device->serial->port); + free (device); + return DC_STATUS_IO; + } + } + + // Set the timeout for receiving data (3000ms). + if (serial_set_timeout (device->serial->port, 3000) == -1) { + ERROR (context, "Failed to set the timeout."); + device->serial->ops->close (device->serial->port); + free (device); + return DC_STATUS_IO; + } + + // Make sure everything is in a sane state. + serial_sleep (device->serial->port, 300); + device->serial->ops->flush (device->serial->port, SERIAL_QUEUE_BOTH); + + device->state = OPEN; + + *out = (dc_device_t *) device; + + return DC_STATUS_SUCCESS; +} + + static dc_status_t hw_ostc3_device_init_download (hw_ostc3_device_t *device) { -- 2.1.4
From e8a132a921f8aac846c9e21cd7032e174f441ee8 Mon Sep 17 00:00:00 2001 From: Claudiu Olteanu <[email protected]> Date: Sat, 27 Jun 2015 15:22:04 +0300 Subject: [PATCH 5/5] Implement custom device open method This method can be used by external applications to open a device and to pass their custom implementation for the serial communication. Signed-off-by: Claudiu Olteanu <[email protected]> --- include/libdivecomputer/device.h | 5 +++++ src/device.c | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/include/libdivecomputer/device.h b/include/libdivecomputer/device.h index 7ba4bd6..60590b5 100644 --- a/include/libdivecomputer/device.h +++ b/include/libdivecomputer/device.h @@ -2,6 +2,7 @@ * libdivecomputer * * Copyright (C) 2008 Jef Driesen + * Copyright (C) 2015 Claudiu Olteanu * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -27,6 +28,7 @@ #include "descriptor.h" #include "buffer.h" #include "datetime.h" +#include "custom_serial.h" #ifdef __cplusplus extern "C" { @@ -72,6 +74,9 @@ typedef int (*dc_dive_callback_t) (const unsigned char *data, unsigned int size, dc_status_t dc_device_open (dc_device_t **out, dc_context_t *context, dc_descriptor_t *descriptor, const char *name); +dc_status_t +dc_device_custom_open (dc_device_t **out, dc_context_t *context, dc_descriptor_t *descriptor, dc_serial_t *serial); + dc_family_t dc_device_get_type (dc_device_t *device); diff --git a/src/device.c b/src/device.c index d95585d..257dc75 100644 --- a/src/device.c +++ b/src/device.c @@ -2,6 +2,7 @@ * libdivecomputer * * Copyright (C) 2008 Jef Driesen + * Copyright (C) 2015 Claudiu Olteanu * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -174,6 +175,27 @@ dc_device_open (dc_device_t **out, dc_context_t *context, dc_descriptor_t *descr return rc; } +dc_status_t +dc_device_custom_open (dc_device_t **out, dc_context_t *context, dc_descriptor_t *descriptor, dc_serial_t *serial) +{ + dc_status_t rc = DC_STATUS_SUCCESS; + dc_device_t *device = NULL; + + if (out == NULL || descriptor == NULL || serial == NULL) + return DC_STATUS_INVALIDARGS; + + switch (dc_descriptor_get_type (descriptor)) { + case DC_FAMILY_HW_OSTC3: + rc = hw_ostc3_device_custom_open (&device, context, serial); + break; + default: + return DC_STATUS_INVALIDARGS; + } + + *out = device; + + return rc; +} int dc_device_isinstance (dc_device_t *device, const dc_device_vtable_t *vtable) -- 2.1.4
_______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
