Fwd: I/O layer refactoring

[Dirk Hohndel]

I'm not sure if everyone who is likely to be interested in this is on the libdivecomputer mailing list... Linus, Anton, Lubomir, Robert... if you have comments on the below, please respond over on that mailing list (not hear, let's help keep this discussion in one place). Thanks /D Begin forwarded message: From: Jef Driesen <j...@libdivecomputer.org> Subject: I/O layer refactoring Date: March 16, 2017 at 9:45:27 AM PDT To: libdivecomputer mailinglist <de...@libdivecomputer.org> Hi, I'm planning some major non-backwards compatible changes for the next release. The first one is a refactoring of the I/O layer to support bluetooth communication (and more). The remainder of this email contains a description of the problem and a proposal for how I would like to address this. Libdivecomputer's low-level I/O layer was pretty much designed with only serial communication in mind. This is very obvious if you look at the signature of the dc_device_open function: dc_status_t dc_device_open (dc_device_t **device, dc_context_t *context, dc_descriptor_t *descriptor, const char *name); That last parameter is the name of the serial port to open. But nowadays libdivecomputer does not only support serial communication, but also IrDA, USB and USB HID. So far we have been able to work around this api limitation. Because for these communication protocols, we can autodetect the device based on the IrDA device name, or the USB VID/PID numbers. Simply pass NULL as name, and libdivecomputer will take care of doing the device enumeration internally. Unfortunately, that workaround isn't entirely foolproof. For example if you have multiple devices connected, libdivecomputer will always pick the first device it recognizes as a dive computer. Thus you won't be able to connect to the other one(s). And if our heuristic to recognize a dive computer is wrong, it won't be able to pick a device at all. In practice, this won't cause any trouble because this is pretty rare corner case. Indeed, very few users will have two dive computers connected at the same time, and the heuristics are reasonably solid (e.g. the IrDA device name and the USB VID/PID numbers of the dive computer never change). But if we're going to add support for bluetooth communication, I wouldn't be surprised if the above assumption breaks. Since bluetooth is a very popular technology, it's no longer very unlikely to have multiple bluetooth devices connected at the same time (e.g. mouse/keyboard, phone, speakers, etc). On top of that, the bluetooth device name is often configurable by the user, and thus autodetection based on simple heuristics won't work anymore. The only way to fix that, is to move the device discovery to the application, and let the end-user select the correct device. That will require to expose the low-level I/O layer in the public api. But that alone isn't enough. We also need some way to pass the result of the discovery back to the dc_device_open function. Thus the "name" parameter needs to replaced with something more generic. The easiest solution would be to just pass a void pointer: dc_status_t dc_device_open (dc_device_t **device, dc_context_t *context, dc_descriptor_t *descriptor, void *iostream); And then the actual data type can depend on the communication mechanism: a string for serial, a 32bit address with lsap number or service name for IrDA, a 48bit address and port number for bluetooth, and so on. But if we're going to modify the api, we can also take it one step further. Why not move the opening and closing of the underlying I/O channel to the application, and pass the open connection as the parameter? If we make sure that each such I/O channel implements a common interface, then the dive computer backends are no longer tied to a specific I/O implementation. This has several advantages: * For bluetooth enabled devices, the application can offer the choice of using native bluetooth communication, or the legacy serial communication (e.g. the bluetooth serial port emulation mode of the operating system we are relying on today). * We can easily implement new I/O layers. For example a user-space driver for usb-serial chipsets (ftdi, pl2303, cp210x, cdc-acm) for use on mobile platforms (android, ios), where the kernel drivers are usually not available. Or a custom I/O layer, where the actual communication is implemented by the application. For the simulator we could a tcp/ip (or pipe) based implementation. There are a few disadvantages as well. First of all, this will of course require some extra code on the application side. The bare minimum would be something like this: dc_iostream_t *iostream = NULL; dc_device_t *device = NULL /* Open the communication channel. */ switch (type) case SERIAL:   dc_serial_enumerate(...);   dc_serial_open(&iostream, context, name);   break; case IRDA:   dc_irda_open(&iostream, context);   dc_irda_discover(iostream, ...);   dc_irda_connect_lsap(iostream, address, lsap);   break; case BLUETOOTH:   dc_bluetooth_open(&iostream, context);   dc_bluetooth_discover(iostream, ...);   dc_bluetooth_connect(iostream, address, port);   break; case CUSTOM:   dc_custom_open(&iostream, context, ...);   break; } /* Download dives as usual. */ dc_device_open(&device, context, descriptor, iostream); dc_device_foreach(device, ...); dc_device_close(device); /* Close the communication channel. */ dc_iostream_close(iostream); As you can see in the above pseudo code, it will certainly add some extra complexity, because suddenly the application will need some knowledge about internal details like the IrDA lsap number and the bluetooth port number. The interface of this new common iostream api would be modeled after the serial communication api (see the attached header file). This may seem a bit awkward, considered that most of the serial api is meaningless for the other implementations. But I don't see any alternative (*). Internally, in the dive computer backends, I just want to be able to call the function unconditionally, without having to check the type of the underlying I/O stream. Other implementations can just leave those functions unimplemented (causing the call to fail with DC_STATUS_UNSUPPORTED), or implement it as a no-op (always return DC_STATUS_SUCCESS). For IrDA and USB the first option will be the obvious choice. But for bluetooth the second option will be required in order to support dual serial/bluetooth devices. (*) I considered moving the serial communication specific functions (baudrate, dtr, rts, etc) to an intermediate interface. That would remove those functions from implementations where they don't make sense (IrDA and USB). But if we want dual serial/bluetooth support, then bluetooth will still need to implement that serial interface. So if we need it there anyway, then I think it's not worth the extra complexity. The only I/O implementation that doesn't really fit into this model is USB communication. USB support three different types of transfers (control, bulk and interrupt), while the iostream interface supports only a single set of read/write functions. Currently the cobalt is the only backend using USB, and it uses a combination of control and bulk transfers. We don't even have an abstraction layer and use libusb directly there. I'm not sure what would be the best way to deal with that, but maybe we can just leave this as-is for now? This will also have an impact on the list of supported devices. At the moment, libdivecomputer will exclude devices for which the underlying I/O layer isn't available. For example on Mac OS X, the Uwatec Smart dive computers are excluded because IrDA isn't supported. But with custom I/O layers, that's no longer possible and libdivecomputer will always have to report all models. Thus it will be up to the application to restrict the list if necessary. The only thing we can do is provide some new api that lets the application query whether a built-in I/O layer is available or not. Comments and feedback on the above proposal are welcome! Jef

/*
 * libdivecomputer
 *
 * Copyright (C) 2016 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 DC_IOSTREAM_H
#define DC_IOSTREAM_H

#include <libdivecomputer/common.h>
#include <libdivecomputer/context.h>

#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */

/**
 * Opaque object representing a I/O stream.
 */
typedef struct dc_iostream_t dc_iostream_t;

/**
 * The parity checking scheme.
 */
typedef enum dc_parity_t {
	DC_PARITY_NONE, /**< No parity */
	DC_PARITY_ODD,  /**< Odd parity */
	DC_PARITY_EVEN, /**< Even parity */
	DC_PARITY_MARK, /**< Mark parity (always 1) */
	DC_PARITY_SPACE /**< Space parity (alwasy 0) */
} dc_parity_t;

/**
 * The number of stop bits.
 */
typedef enum dc_stopbits_t {
	DC_STOPBITS_ONE,          /**< 1 stop bit */
	DC_STOPBITS_ONEPOINTFIVE, /**< 1.5 stop bits*/
	DC_STOPBITS_TWO           /**< 2 stop bits */
} dc_stopbits_t;

/**
 * The flow control.
 */
typedef enum dc_flowcontrol_t {
	DC_FLOWCONTROL_NONE,     /**< No flow control */
	DC_FLOWCONTROL_HARDWARE, /**< Hardware (RTS/CTS) flow control */
	DC_FLOWCONTROL_SOFTWARE  /**< Software (XON/XOFF) flow control */
} dc_flowcontrol_t;

/**
 * The direction of the data transmission.
 */
typedef enum dc_direction_t {
	DC_DIRECTION_INPUT = 0x01,  /**< Input direction */
	DC_DIRECTION_OUTPUT = 0x02, /**< Output direction */
	DC_DIRECTION_ALL = DC_DIRECTION_INPUT | DC_DIRECTION_OUTPUT /**< All directions */
} dc_direction_t;

/**
 * The line signals.
 */
typedef enum dc_line_t {
	DC_LINE_DCD = 0x01, /**< Data carrier detect */
	DC_LINE_CTS = 0x02, /**< Clear to send */
	DC_LINE_DSR = 0x04, /**< Data set ready */
	DC_LINE_RNG = 0x08, /**< Ring indicator */
} dc_line_t;

/**
 * Configure the line settings.
 *
 * @param[in]  iostream     A valid I/O stream.
 * @param[in]  baudrate     The baud rate setting.
 * @param[in]  databits     The number of data bits.
 * @param[in]  parity       The parity setting.
 * @param[in]  stopbits     The number of stop bits.
 * @param[in]  flowcontrol  The flow control setting.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_configure (dc_iostream_t *iostream, unsigned int baudrate, unsigned int databits, dc_parity_t parity, dc_stopbits_t stopbits, dc_flowcontrol_t flowcontrol);

/**
 * Set the read timeout.
 *
 * There are three distinct modes available:
 *
 *  1. Blocking (timeout < 0):
 *
 *     The read operation is blocked until all the requested bytes have
 *     been received. If the requested number of bytes does not arrive,
 *     the operation will block forever.
 *
 *  2. Non-blocking (timeout == 0):
 *
 *     The read operation returns immediately with the bytes that have
 *     already been received, even if no bytes have been received.
 *
 *  3. Timeout (timeout > 0):
 *
 *     The read operation is blocked until all the requested bytes have
 *     been received. If the requested number of bytes does not arrive
 *     within the specified amount of time, the operation will return
 *     with the bytes that have already been received.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  timeout   The timeout in milliseconds.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_timeout (dc_iostream_t *iostream, int timeout);

/**
 * Set the state of the half duplex emulation.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  value     The half duplex state.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_halfduplex (dc_iostream_t *iostream, unsigned int value);

/**
 * Set the receive latency.
 *
 * The effect of this setting is highly platform and driver specific. On
 * Windows it does nothing at all, on Linux it controls the low latency
 * flag (e.g. only zero vs non-zero latency), and on Mac OS X it sets
 * the receive latency as requested.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  value     The latency in milliseconds.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_latency (dc_iostream_t *iostream, unsigned int value);

/**
 * Read data from the I/O stream.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[out] data      The memory buffer to read the data into.
 * @param[in]  size      The number of bytes to read.
 * @param[out] actual    An (optional) location to store the actual
 *                       number of bytes transferred.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_read (dc_iostream_t *iostream, void *data, size_t size, size_t *actual);

/**
 * Write data to the I/O stream.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  data      The memory buffer to write the data from.
 * @param[in]  size      The number of bytes to write.
 * @param[out] actual    An (optional) location to store the actual
 *                       number of bytes transferred.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_write (dc_iostream_t *iostream, const void *data, size_t size, size_t *actual);

/**
 * Flush the internal output buffer and wait until the data has been
 * transmitted.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_flush (dc_iostream_t *iostream);

/**
 * Discards all data from the internal buffers.
 *
 * @param[in]  iostream   A valid I/O stream.
 * @param[in]  direction  The direction of the buffer(s).
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_purge (dc_iostream_t *iostream, dc_direction_t direction);

/**
 * Set the state of the break condition.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  value     The break condition state.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_break (dc_iostream_t *iostream, unsigned int value);

/**
 * Set the state of the DTR line.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  value     The DTR line state.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_dtr (dc_iostream_t *iostream, unsigned int value);

/**
 * Set the state of the RTS line.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @param[in]  value     The RTS line state.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_set_rts (dc_iostream_t *iostream, unsigned int value);

/**
 * Query the number of available bytes in the input buffer.
 *
 * @param[in]   iostream  A valid I/O stream.
 * @param[out]  value     A location to store the number of bytes in the
 *                        input buffer.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_get_available (dc_iostream_t *iostream, size_t *value);

/**
 * Query the state of the line signals.
 *
 * @param[in]   iostream  A valid I/O stream.
 * @param[out]  value     A location to store the bitmap with the state
 *                        of the line signals.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_get_lines (dc_iostream_t *iostream, unsigned int *value);

/**
 * Suspend execution of the current thread for the specified amount of
 * time.
 *
 * @param[in]  iostream      A valid I/O stream.
 * @param[in]  milliseconds  The number of milliseconds to sleep.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_sleep (dc_iostream_t *iostream, unsigned int milliseconds);

/**
 * Close the I/O stream and free all resources.
 *
 * @param[in]  iostream  A valid I/O stream.
 * @returns #DC_STATUS_SUCCESS on success, or another #dc_status_t code
 * on failure.
 */
dc_status_t
dc_iostream_close (dc_iostream_t *iostream);


#ifdef __cplusplus
}
#endif /* __cplusplus */
#endif /* DC_IOSTREAM_H */

_______________________________________________ devel mailing list de...@libdivecomputer.org http://libdivecomputer.org/cgi-bin/mailman/listinfo/devel

_______________________________________________
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface