Hi All,

I've appended a more proper patch to fix this behaviour. It extends
the argument list of get_strings_and_reopen() and get_strings(). I
introduced default arguments to make this API-break backwards
compatible.  These could, maybe at a later stage, be removed.  Whats
not clear to me is how to deal with open(struct libusb_device *dev).
Should in here get_strings_and_reopen() not completely removed?

While this patch tries to minimize the api breakage. I'd suggest to
join get_strings_and_reopen()  and get_strings() and to make them
private. as they are only useful for vendor(), product() and serial()
while we should save the used stings in the member variables after
successfully opening a device based on their usage.

Related to this is there a way in libusb to detect if the string
descriptor, which is optional, is empty or not available?

Cheers,
Matthias   

Am Thu, 25 May 2017 00:53:12 +0200
schrieb Matthias Janke <[email protected]>:

> Hi Thomas,
> 
> Am Tue, 23 May 2017 21:26:23 +0200
> schrieb Thomas Jarosch <[email protected]>:
> 
> > Hi Matthias,
> > 
> > Am 12.05.2017 um 13:32 schrieb Matthias Janke:  
> > >   I'm using the c++ wrapper of libftdi (git co of yesterday) to
> > >   communicate with a FT2232HL on a Lattice XO2 Eval board
> > >   (LCMXO2-7000HE-B-EVN) this board has no programmed in iSerial.
> > >   
> > >   A simple c++ program which just opens and closes the device
> > > fails with "libusb_get_string_descriptor_ascii() failed" this is
> > > also true for the find_all_pp example (find_all, works). This
> > > failure originates in the use of the get_strings() function
> > > inside all c++ open() functions. This function wants all three
> > > iManufractuer, iProduct and iSerial to be present. The C-API
> > > allows one to pass NULL to ignore these descriptors. This is
> > > currently not possible with the c++ api. To properly fix this I
> > > suspect an api break is needed. As temporary ugly workaround I
> > > use the appended patch. Are there other ideas to fix this?     
> > 
> > what about this open() function from ftdi.cpp:
> > 
> > int Context::open(int vendor, int product, const std::string&
> > description, const std::string& serial, unsigned int index)
> > {
> >     // translate empty strings to NULL
> >     // -> do not use them to find the device (vs. require an empty
> > string to be set in the EEPROM)
> >     const char* c_description=NULL;
> >     const char* c_serial=NULL;
> >     if (!description.empty())
> >         c_description=description.c_str();
> >     if (!serial.empty())
> >         c_serial=serial.c_str();
> > 
> >     int ret = ftdi_usb_open_desc_index(d->ftdi, vendor, product,
> > c_description, c_serial, index);
> > 
> >     if (ret < 0)
> >        return ret;
> >  
> up to this point everything looks ok. but
> this: 
> >     return get_strings_and_reopen();  
> 
> innternaly calls get_strings() which unconditioaly tries to get all!
> strings:
> 
> int Context::get_strings()
>  323 {
>  324     // Prepare buffers
>  325     char vendor[512], desc[512], serial[512];
>  326 
>  327     int ret = ftdi_usb_get_strings(d->ftdi, d->dev, vendor, 512,
> desc, 512, serial, 512); 328 
>  329     if (ret < 0)
>  330         return -1;
>  331 
>  332     d->vendor = vendor;
>  333     d->description = desc;
>  334     d->serial = serial;
>  335 
>  336     return 1;
>  337 }
> 
> cheers,
> Matthias
> 
> > }
> > 
> > 
> >   
> > -> if you pass an empty string for "std::string serial",    
> > it should pass down NULL for the serial parameter of the C API.
> > 
> > What happens if you use this specific open() function?
> > 
> > Cheers,
> > Thomas
> > 
> > --
> > libftdi - see http://www.intra2net.com/en/developer/libftdi for
> > details. To unsubscribe send a mail to
> > [email protected]     
> 
> 
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for
> details. To unsubscribe send a mail to
> [email protected]   



--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [email protected]   
>From 46adbc0ca89491f92b2ccbd46adb4cdbdd0bb53e Mon Sep 17 00:00:00 2001
From: Matthias Janke <[email protected]>
Date: Thu, 13 Jul 2017 18:10:44 +0200
Subject: [PATCH] The C++ API and C API differ in how they open a device. The C
 API only considers the user supplied strings iVendor, iProduct and iSerial to
 open a device. The C++ API internaly tries after opening the device with the
 user supplied strings, to read all strings via get_strings_and_reopen(). This
 fails if one string in example iSerial is not defined in the devices'
 descriptor. As such the whole open operation failes and the C++ API could
 only open devices with complete descriptors.

This commit fixes this behaviour, by extending the get_strings_and_reopen() and get_strings() argument lists, to configure the strings to read.
---
 ftdipp/ftdi.cpp | 57 ++++++++++++++++++++++++++++++++++-----------------------
 ftdipp/ftdi.hpp |  4 ++--
 2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/ftdipp/ftdi.cpp b/ftdipp/ftdi.cpp
index 3d59d9d..3301044 100644
--- a/ftdipp/ftdi.cpp
+++ b/ftdipp/ftdi.cpp
@@ -87,7 +87,7 @@ int Context::open(int vendor, int product)
     if (ret < 0)
        return ret;
 
-    return get_strings_and_reopen();
+    return get_strings_and_reopen(false,false,false);
 }
 
 int Context::open(int vendor, int product, const std::string& description, const std::string& serial, unsigned int index)
@@ -106,7 +106,7 @@ int Context::open(int vendor, int product, const std::string& description, const
     if (ret < 0)
        return ret;
 
-    return get_strings_and_reopen();
+    return get_strings_and_reopen(false,description.empty(),serial.empty());
 }
 
 int Context::open(const std::string& description)
@@ -116,7 +116,7 @@ int Context::open(const std::string& description)
     if (ret < 0)
        return ret;
 
-    return get_strings_and_reopen();
+    return get_strings_and_reopen(false,true,false);
 }
 
 int Context::open(struct libusb_device *dev)
@@ -319,41 +319,46 @@ const char* Context::error_string()
     return ftdi_get_error_string(d->ftdi);
 }
 
-int Context::get_strings()
+int Context::get_strings(bool vendor, bool description, bool serial)
 {
     // Prepare buffers
-    char vendor[512], desc[512], serial[512];
+    char ivendor[512], idesc[512], iserial[512];
 
-    int ret = ftdi_usb_get_strings(d->ftdi, d->dev, vendor, 512, desc, 512, serial, 512);
+    int ret = ftdi_usb_get_strings(d->ftdi, d->dev, vendor?ivendor:NULL, 512, description?idesc:NULL, 512, serial?iserial:NULL, 512);
 
     if (ret < 0)
         return -1;
 
-    d->vendor = vendor;
-    d->description = desc;
-    d->serial = serial;
+    d->vendor = ivendor;
+    d->description = idesc;
+    d->serial = iserial;
 
     return 1;
 }
 
-int Context::get_strings_and_reopen()
+int Context::get_strings_and_reopen(bool vendor, bool description, bool serial)
 {
-    if ( d->dev == 0 )
-    {
-        d->dev = libusb_get_device(d->ftdi->usb_dev);
-    }
+    int ret = 0;
 
-    // Get device strings (closes device)
-    int ret=get_strings();
-    if (ret < 0)
+    if(vendor || description || serial)
     {
-        d->open = 0;
-        return ret;
-    }
+        if ( d->dev == 0 )
+        {
+            d->dev = libusb_get_device(d->ftdi->usb_dev);
+        }
+
+        // Get device strings (closes device)
+        ret=get_strings(vendor, description, serial);
+        if (ret < 0)
+        {
+            d->open = 0;
+            return ret;
+        }
 
-    // Reattach device
-    ret = ftdi_usb_open_dev(d->ftdi, d->dev);
-    d->open = (ret >= 0);
+        // Reattach device
+        ret = ftdi_usb_open_dev(d->ftdi, d->dev);
+        d->open = (ret >= 0);
+    }
 
     return ret;
 }
@@ -362,6 +367,8 @@ int Context::get_strings_and_reopen()
  */
 const std::string& Context::vendor()
 {
+    if(d->vendor.empty())
+    	get_strings_and_reopen(true,false,false);
     return d->vendor;
 }
 
@@ -369,6 +376,8 @@ const std::string& Context::vendor()
  */
 const std::string& Context::description()
 {
+    if(d->description.empty())
+    	get_strings_and_reopen(false,true,false);
     return d->description;
 }
 
@@ -376,6 +385,8 @@ const std::string& Context::description()
  */
 const std::string& Context::serial()
 {
+    if(d->serial.empty())
+    	get_strings_and_reopen(false,false,true);
     return d->serial;
 }
 
diff --git a/ftdipp/ftdi.hpp b/ftdipp/ftdi.hpp
index 6a7f893..dc035cc 100644
--- a/ftdipp/ftdi.hpp
+++ b/ftdipp/ftdi.hpp
@@ -134,8 +134,8 @@ public:
     const char* error_string();
 
 protected:
-    int get_strings();
-    int get_strings_and_reopen();
+    int get_strings(bool vendor=true, bool description=true, bool serial=true);
+    int get_strings_and_reopen(bool vendor=true, bool description=true, bool serial=true);
 
     /* Properties */
     struct ftdi_context* context();
-- 
2.13.2

Reply via email to