Re: Modem status callback function patch

[Dmitry Lysenko <dvl36.ripe.nick@xxxxxxxxx>]

Hi Thomas

2012/9/17 Thomas Jarosch <thomas.jarosch@xxxxxxxxxxxxx>

Here are some small things we should discuss:

- It changes the transfer handling (_CANCEL stuff,
  replaces libusb_handle_events() with libusb_handle_events_completed)

As I understand libusb_cancel_transfer() call in my code doesn't need at all, as the usblib transfer already completed. So setting tc->completed = 1 would be enough. I added  libusb_cancel_transfer() to be extra sure. I  changed libusb_handle_events() to  libusb_handle_events_completed() because of libusb people recommend : "This function is kept primarily for backwards compatibility. All new code should call libusb_handle_events_completed() or libusb_handle_events_timeout_completed() to avoid race conditions."(source) This function was introduced in libusb 1.0.9. So additional dependency may occur.  

> This could have side effects.

Of course. I newby in libftdi and libusb so don't saw  many of such side effects.

 

- Change the "invalid modem status" magic value of 0xffff into a constant
  since we already use it in two places

 

OK.

 

- Does the "cancel" logic have to be tied to
  the modem status handling / callback?

Maybe it would be better have possibility to "unblock" waiting state of ftdi_transfer_data_done(). In my case hardware signalling about events via modem status lines (think I not alone in this). If the exception occured and device does not send data any more, my application is blocked at ftdi_transfer_data_done(). So I didn't find least intrusive method to unblock this waiting state then "cancel" transfer via setting of tc->completed = 1. Course this is extra feature. It give the possibility do not use ftdi_context and ftdi_transfer_control directly from library users code.

 

- Should we update ftdi->last_modem_status before
  calling the registered callback or afterwards?

 If we update last_modem_status afterwards, we lost possibility to check what exactly is changed in modem status bits. Course this can be done in users code, but this require addition of context. Sure understand that in callback function "last_modem_status" is not "last", but "prev".

- Does "register_callback" really need to poll the current status?

No. But if the user of library forgeting to call poll_modem_status() before the reading, then callback function will be called without actually modem status changes(only once).

Think this is not a big problem. So I can remove call to poll_modem_status() from register_callback()

 

  I would call this a "side effect", so we should either document it
  or do nothing USB related at all.

Agreed. I will remove usb related stuff and  will check this deeper.

WBR,

 Dmitry.

---

libftdi - see http://www.intra2net.com/en/developer/libftdi for details.
To unsubscribe send a mail to libftdi+unsubscribe@xxxxxxxxxxxxxxxxxxxxxxx