Linus, Anton, everyone else...

Looking at the code we added to filter the dive computer list I realized
that this is just a silly hack and that this information needs to be
collected in libdivecomputer, which already has a weird attempt to
encapsulate the transport used. Right now that type is

typedef enum dc_transport_t {
        DC_TRANSPORT_NONE,
        DC_TRANSPORT_SERIAL,
        DC_TRANSPORT_USB,
        DC_TRANSPORT_IRDA
} dc_transport_t;

And we then define in desktop-widgets/downloadfromdivecomputer.cpp

// Workaround abuse of old libdc types
#define DC_TRANSPORT_BLUETOOTH 1024

And the way it is populated is pretty much this function:

dc_transport_t
dc_descriptor_get_transport (dc_descriptor_t *descriptor)
{
        if (descriptor == NULL)
                return DC_TRANSPORT_NONE;

        if (descriptor->type == DC_FAMILY_ATOMICS_COBALT)
                return DC_TRANSPORT_USB;
        else if (descriptor->type == DC_FAMILY_SUUNTO_EONSTEEL)
                return DC_TRANSPORT_USB;
        else if (descriptor->type == DC_FAMILY_SCUBAPRO_G2)
                return DC_TRANSPORT_USB;
        else if (descriptor->type == DC_FAMILY_UWATEC_SMART)
                return DC_TRANSPORT_IRDA;
        else
                return DC_TRANSPORT_SERIAL;
}

Which is of course completely useless.

I'm proposing to fix this to make this actually useful.

typedef enum dc_transport_t {
        DC_TRANSPORT_NONE = 0,
        DC_TRANSPORT_SERIAL,
        DC_TRANSPORT_USB_FTDI,
        DC_TRANSPORT_USB_HID,
        DC_TRANSPORT_USB_OTHER,
        DC_TRANSPORT_IRDA,
        DC_TRANSPORT_BT,
        DC_TRANSPORT_BLE
} dc_transport_t;


First, let's include that thing in the dc_descriptor_t:

struct dc_descriptor_t {
        const char *vendor;
        const char *product;
        dc_family_t type;
        unsigned int model;
        dc_transport_t transport[3]; // 3 assuming that someone builds a dive 
computer that does USB, BT, and BLE
        unsigned int serial;
};

Then we extend the g_descriptors to become useful:

dc_descriptor_t g_descriptors[] = {
//...
        {"Suunto", "Vyper Novo", DC_FAMILY_SUUNTO_D9, 0x1D, { 
DC_TRANSPORT_USB_FTDI , 0, 0 }},
        {"Suunto", "Zoop Novo",  DC_FAMILY_SUUNTO_D9, 0x1E, { 
DC_TRANSPORT_USB_FTDI , 0, 0 }},
        /* Suunto EON Steel */
#ifdef USBHID
        {"Suunto", "EON Steel", DC_FAMILY_SUUNTO_EONSTEEL, 0, { 
DC_TRANSPORT_USB_HID, DC_TRANSPORT_BLE, 0 }},
        {"Scubapro", "G2", DC_FAMILY_SCUBAPRO_G2, 0x11, { DC_TRANSPORT_USB_HID, 
DC_TRANSPORT_BLE, 0 }},
#endif
//...
        {"Shearwater", "Petrel 2", DC_FAMILY_SHEARWATER_PETREL, 3, { 
DC_TRANSPORT_BT, DC_TRANSPORT_BLE, 0 }},
//...
        {"Uwatec", "Smart Pro",     DC_FAMILY_UWATEC_SMART, 0x10, { 
DC_TRANSPORT_IRDA, 0, 0 }},
//...
}

Of course for those dive computers that change capabilities without
changing names, we need to list all the ones they might have (and explain
the problem in the FAQ / user manual) - as seen with the Petrel 2 in the
example (for the EON Steel it's at least just a firmware update that makes
it work).

And now with this information it becomes MUCH easier to write the
fill_computer_list() - we simply skip those descriptors that don't include
on of the supported transports: (soon) only DC_TRANSPORT_BLE on IOS,
DC_TRANSPORT_BT, DC_TRANSPORT_BLE, and DC_TRANSPORT_USB_FTDI on Android
(Anton seems to believe that DC_TRANSPORT_USB_HID might work as well),
etc.

Does this seem reasonable? What am I missing?

/D
_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to