c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Matthias Janke-2
  Hi All,
 
  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?
 
  Cheerio,
  Matthias

--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Matthias Janke-2
patch was not attached.

Am Fri, 12 May 2017 13:32:50 +0200
schrieb Matthias Janke <[hidden email]>:

>   Hi All,
>  
>   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?
>   Cheerio,
>   Matthias
>
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for
> details. To unsubscribe send a mail to
> [hidden email]  


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

0001-Temporary-workaround-to-make-c-open-funktions-work-w.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Thomas Jarosch
In reply to this post by Matthias Janke-2
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;

    return get_strings_and_reopen();
}



-> 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 [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Matthias Janke-2
Hi Thomas,

Am Tue, 23 May 2017 21:26:23 +0200
schrieb Thomas Jarosch <[hidden email]>:

> 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
> [hidden email]  


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

[PATCH]Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Matthias Janke-2
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 <[hidden email]>:

> Hi Thomas,
>
> Am Tue, 23 May 2017 21:26:23 +0200
> schrieb Thomas Jarosch <[hidden email]>:
>
> > 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
> > [hidden email]    
>
>
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for
> details. To unsubscribe send a mail to
> [hidden email]  


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

0001-The-C-API-and-C-API-differ-in-how-they-open-a-device.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH]Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Thomas Jarosch
Hi Matthias,

On Thursday, 13 July 2017 18:53:25 CEST Matthias Janke wrote:

> 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?

thanks for the updated patch! I've quickly browsed it and will take
a look in detail next week. The concept looked sound to me.

The patch will cause a small ABI breakage, so I'll increase
the .so version, too. Nothing to worry about though.
In fact we just need to increase the .so version of
the C++ wrapper, that should cause even less disruption.

Regarding your libusb question: I didn't look at the
libusb API for quite a while, so if it's possible,
it's probably documented somewhere.

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Thomas Jarosch
In reply to this post by Matthias Janke-2
Hi Matthias,

On Thursday, 13 July 2017 18:53:25 CEST Matthias Janke wrote:
> 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?

patch has been applied, thanks! I've massaged the changelog
a little and replaced tabs with spaces in two places.

> 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.

let's postpone any change like that for the next release,
I really like to get this release shipped.

If nobody complains, I'll do a v1.4rc2 next week
and hopefully that will be the final version 1.4, too.

Cheers,
Thomas


--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Re: c++ wraper: open(vid,pid) fails for devices with a missing iManufacturer, iProduct or iSerial

Matthias Janke-2
Hi Thomas,

Am Thu, 20 Jul 2017 18:00:54 +0200
schrieb Thomas Jarosch <[hidden email]>:

> Hi Matthias,
>
> On Thursday, 13 July 2017 18:53:25 CEST Matthias Janke wrote:
> > 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?  
>
> patch has been applied, thanks! I've massaged the changelog
> a little and replaced tabs with spaces in two places.
Thanks. I think I've messed up the logic in one open function. (Fixed
in the appended patch).


> > 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.  
>
> let's postpone any change like that for the next release,
> I really like to get this release shipped.

Yup. This was meant as longer term goal. I generaly think after c++11
the wrapper should get an overhaul to get rid of the return codes and
use exceptions, as they dont interfere with runtime anymore.

> If nobody complains, I'll do a v1.4rc2 next week
> and hopefully that will be the final version 1.4, too.
>
> Cheers,
> Thomas


Cheers,
Matthias
>
> --
> libftdi - see http://www.intra2net.com/en/developer/libftdi for
> details. To unsubscribe send a mail to
> [hidden email]  



--
libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to [hidden email]  

0001-follow-up-on-c-open-fix.-fixes-logic-in-open-with-lo.patch (856 bytes) Download Attachment