From: Sophia Li <Sophia.Li@Sun.COM> - 2007-03-29 08:05:30
|
Thanks. I've been looking forward to this for long. But there seems to be few more issues to address. 1. need to add libusb_fini() void libusb_fini(void); 2. > 212 #define LIBUSB_LINUX_MAX_READWRITE (16 * 1024) > 218 #define LIBUSB_LINUX_MAX_CTRL_SIZE (4096 - 8) This is OS specific and not so good to be placed in the API file. Do we prefer to have this information exported by libusb than to have users figure out the values themselves? If so, shall we make a new function for each backend to report these values? E.g., int libusb_get_max_xfer_size(libusb_bus_id_t bus, libusb_transfer_type_t type, size_t *bytes); If an OS doesn't have this limit, just set the value to 0. Then the application itself needs to decide the right value to use. Or any other idea? 3. Do we really need these functions? What for? > 516 int libusb_get_devnum(libusb_device_id_t devid, unsigned char *devnum); > 555 int libusb_get_device_designator(libusb_device_id_t devid, > 556 unsigned char *buffer, size_t buflen); > 557 int libusb_get_bus_designator(libusb_device_id_t devid, > 558 unsigned char *buffer, size_t buflen); 4. The following descriptor functions are there since we can pre-cache them during libusb_init(). * libusb_get_device_desc() ......... Get cooked device desc. * libusb_get_config_desc() ......... Get cooked configuration desc. * libusb_get_interface_desc() ...... Get cooked interface desc. * libusb_get_endpoint_desc() ....... Get cooked endpoint desc. * libusb_get_raw_device_desc() ..... Get raw device desc. * libusb_get_raw_config_desc() ..... Get raw config desc. Do we need to add string descriptor function as well? It is impossible to pre-cache all the string descriptors. At most we can cache some commonly used str descrs, like manufacturer str, product str, serialnumber. Is that enough? 5. get_interface/endpoint_desc() needs to add alternate interface in the argument int libusb_get_interface_desc(libusb_device_id_t devid, int cfgidx, int ifcidx, int alt, struct usb_interface_desc *ifcdesc); int libusb_get_endpoint_desc(libusb_device_id_t devid, int cfgidx, int ifcidx, int alt, int eptidx, struct usb_endpoint_desc *eptdesc); 6. Though there were arguments that we shouldn't include bLength and bDescriptorType in the descriptor structures, I prefer to add them to the descriptor structures. It makes the API consistent to the USB spec. Every USB developer should know what bLength means and not misuse it. BTW: this function would be not necessary in that case. 617 int libusb_get_raw_device_desc(libusb_device_id_t devid, 618 unsigned char *buffer, size_t buflen); > 7. Isn't interval decided by endpoint descriptor? How can we set it in the intr request? > 821 /* Interrupt */ > 822 struct libusb_intr_request { > 823 libusb_dev_handle_t dev; > 824 unsigned char endpoint; > > 825 unsigned int interval; ... > 831 }; 8. As we discussed recently, the use of tags could be eliminated. So the tag can be removed from libusb_xxx_request structures, and the following functions can be changed to sth. like: int libusb_abort(uintptr_t req_addr); int libusb_wait(unsigned int num_reqs, uintptr_t *req_addrs, uintptr_t *req_addr); int libusb_poll(unsigned int num_reqs, uintptr_t *req_addrs, uintptr_t *req_addr); Shall we make an opaque typedef for the request address or just use uintptr_t? 9. As we discussed recently, poll/wait is not to used with callbacks. The transfer results should be placed in the libusb_xxx_request structures and be removed from the arguments of I/O callback functions. E.g., /* Bulk */ struct libusb_bulk_request { libusb_dev_handle_t dev; unsigned char endpoint; void *buf; size_t buflen; long timeout; unsigned long flags; int status; size_t transferred_bytes; }; typedef void (*libusb_bulk_callback_t)( struct libusb_bulk_request *ctrl, void *arg); int libusb_bulk(struct libusb_bulk_request *bulk); int libusb_bulk_submit(struct libusb_bulk_request *bulk, libusb_bulk_callback_t callback, void *arg); /* Isochronous */ struct libusb_isoc_result { int status; size_t transferred_bytes; }; struct libusb_isoc_request { libusb_dev_handle_t dev; unsigned char endpoint; unsigned int start_frame; unsigned long flags; unsigned int num_packets; struct libusb_isoc_packet { void *buf; size_t buflen; } *packets; struct libusb_isoc_result *results; }; typedef void (*libusb_isoc_callback_t)( struct libusb_isoc_request *iso, void *arg); int libusb_isoc_submit(struct libusb_isoc_request *iso, libusb_isoc_callback_t callback, void *arg); 10. It was once proposed to change devid into dev handle for these functions. Any objection? int libusb_get_configuration(libusb_dev_handle_t dev, int *cfg); int libusb_set_configuration(libusb_dev_handle_t dev, int cfg); There are two more advanced topics remained. One is the one request/multiple response model support. Another is the per-thread model support for embedded OS environment. I think these would both be desired features for openusb to have, but take more efforts to design. So let's begin with the basic API first and we can add those advanced features later. Sophia |