manuelbl / JavaDoesUSB

USB library for Java
MIT License
136 stars 10 forks source link

Request: `transferOut` and `transferIn` overloads #14

Closed sirthias closed 4 months ago

sirthias commented 1 year ago

Currently JavaDoesUSB provides these two core methods on the USBDevice class:

void transferOut(int endpointNumber, byte[] data, int timeout);
byte[] transferIn(int endpointNumber, int timeout);

While they work fine and do their job I'd like to request the addition of these two overloads:

/**
 * Sends data to this device.
 * <p>
 * This method blocks until the data has been sent, the timeout period has expired
 * or an error has occurred. If the timeout expires, a {@link USBTimeoutException} is thrown.
 * </p>
 * <p>
 * This method can send data to bulk and interrupt endpoints.
 * </p>
 * <p>
 * If the sent data length is a multiple of the packet size, it is often
 * required to send an additional zero-length packet (ZLP) for the device
 * to actually process the data. This method will not do it automatically.
 * </p>
 *
 * @param endpointNumber the endpoint number (in the range between 1 and 127)
 * @param data           the byte array holding the data to send
 * @param offset         the offset into the data array to start sending from
 * @param length         the number of bytes to send
 * @param timeout        the timeout period, in milliseconds (0 for no timeout)
 */
void transferOut(int endpointNumber, byte[] data, int offset, int length, int timeout);

/**
 * Receives data from this device.
 * <p>
 * This method blocks until at least a packet has been received, the timeout period has expired
 * or an error has occurred. If the timeout expired, a {@link USBTimeoutException} is thrown.
 * </p>
 * <p>
 * The method returns the number of bytes that were received as the payload of a packet.
 * If the given buffer is large enough to hold this many bytes at/after the given offset
 * all the received bytes will have been copied into the given buffer array.
 * If the buffer was too small then the returned int will be larger than
 * `buf.length() - offset`.
 * Note that the returned number will be zero if the USB device sends zero-length packets to
 * indicate the end of a data unit.
 * </p>
 * <p>
 * This method can receive data from bulk and interrupt endpoints.
 * </p>
 *
 * @param endpointNumber the endpoint number (in the range between 1 and 127, i.e. without the direction bit)
 * @param buf            the byte array that received bytes are to be copied into
 * @param offset         the offset into `buf` that the received bytes should be copied to
 * @param timeout        the timeout period, in milliseconds (0 for no timeout)
 * @return the number of bytes received (which might be more than the number of bytes placed in `buf`)
 */
int transferIn(int endpointNumber, byte[] buf, int offset, int timeout);

Especially the first method would be great to have as it will reduce the amount of intermediate buffer copying that might otherwise be needed.

manuelbl commented 1 year ago

Can you describe how you are using this library? What kind of endpoints do you use? Do you optimize for high throughput or low latency? Do you send many small or rather big packets of data?

If you are optimising for high throughput for a bulk endpoint, I strongly recommend to use an output stream (see USBDevice.openOutputStream()). Since it is an output stream, it offers the kind of interface you are looking for (see OutputStream.write(). And the output stream offers considerably higher throughput as it uses multiple outstanding requests behind the scenes.

If you have a good use case, I'm happy to add the additional transferOut() method.

The proposal for transferIn() is more problematic. If the buffer is not big enough for an entire USB packet, which depends on the device and the selected USB speed, difficult to understand errors can occur. The kind of behavior you are proposing ("If the buffer was too small then the returned int will be larger...") can only be achieved by allocating a separate buffer in the implementation. So the buffer copy would still be needed.

Again, for bulk endpoints, I recommend using an input stream. It avoids the copying and will deliver the remaining data in the next call.

sirthias commented 1 year ago

In my case I'm talking to a device that can only deal with transfers in 60kb chunks, so larger payloads always needs to be sent in slices, which would entail no copying with the transferOut overload sketched out above. Without the overload every slice needs to be copied into a fresh array. I'm assuming that "spoon-feeding" external USB devices from a larger buffer isn't that rare, which is why I think this transferOut overload should be added.

The proposal for transferIn() is more problematic. ... can only be achieved by allocating a separate buffer in the implementation. So the buffer copy would still be needed.

Yes, I was afraid that this was the case. If there is no way to directly fill the target array without additional buffering in the JavaDoesUsb layer, then this second transferIn overload really doesn't make any sense. Still wanted to bring it up, because sometimes the intermediate layers can in fact avoid internal buffers if the target buffer is already available.

manuelbl commented 10 months ago

This has been incorporated into release 0.6.0 and later