necouchman commented on code in PR #610:
URL: https://github.com/apache/guacamole-server/pull/610#discussion_r2672810259
##########
src/libguac/guacamole/user-fntypes.h:
##########
@@ -499,5 +499,93 @@ typedef int guac_user_get_handler(guac_user* user,
guac_object* object,
typedef int guac_user_put_handler(guac_user* user, guac_object* object,
guac_stream* stream, char* mimetype, char* name);
-#endif
+/**
+ * Handler for Guacamole USB connect events, invoked when a "usbconnect"
+ * instruction has been received from a user. This indicates that the user
+ * has connected a USB device via WebUSB and it is available for redirection.
+ *
+ * @param user
+ * The user that connected the USB device.
+ *
+ * @param device_id
+ * The unique identifier for the USB device.
+ *
+ * @param vendor_id
+ * The vendor ID of the USB device.
+ *
+ * @param product_id
+ * The product ID of the USB device.
+ *
+ * @param device_name
+ * The human-readable name of the device.
+ *
+ * @param serial_number
+ * The serial number of the device, if available.
+ *
+ * @param device_class
+ * The USB device class.
+ *
+ * @param device_subclass
+ * The USB device subclass.
+ *
+ * @param device_protocol
+ * The USB device protocol.
+ *
+ * @param interface_data
+ * Encoded string containing interface and endpoint information in the
+ * format: "iface_num:class:subclass:protocol:ep_num:dir:type:size,..."
+ *
+ * @return
+ * Zero if the USB connect event was handled successfully, or non-zero if
+ * an error occurred.
+ */
+typedef int guac_user_usbconnect_handler(guac_user* user, const char*
device_id,
+ int vendor_id, int product_id, const char* device_name,
+ const char* serial_number, int device_class, int device_subclass,
+ int device_protocol, const char* interface_data);
Review Comment:
It might be good to indicate more explicitly which of these arguments is
required and which is optional?One of them - `serial_number` - says "if
available", which indicates it may be optional - is that the only one that
isn't required and is optional, and the rest are required?
##########
src/libguac/user-handlers.h:
##########
@@ -231,6 +231,27 @@ __guac_instruction_handler __guac_handshake_name_handler;
*/
__guac_instruction_handler __guac_handshake_timezone_handler;
+/**
+ * Internal initial handler for the usbconnect instruction. When a usbconnect
+ * instruction is received, this handler will be called. The client's
+ * usbconnect handler will be invoked if defined.
+ */
+__guac_instruction_handler __guac_handle_usbconnect;
+
+/**
+ * Internal initial handler for the usbdata instruction. When a usbdata
+ * instruction is received, this handler will be called. The client's
Review Comment:
`client's` -> `user's`
##########
src/libguac/guacamole/protocol.h:
##########
@@ -1155,5 +1155,42 @@ guac_protocol_version
guac_protocol_string_to_version(const char* version_string
*/
const char* guac_protocol_version_to_string(guac_protocol_version version);
-#endif
+/**
+ * Sends a usbdisconnect instruction over the given guac_socket connection,
+ * requesting that the client disconnect the specified USB device.
+ *
+ * @param socket
+ * The guac_socket connection to use.
+ *
+ * @param device_id
+ * The unique identifier of the USB device that should be disconnected.
+ *
+ * @return
+ * Zero on success, non-zero on error.
+ */
+int guac_protocol_send_usbdisconnect(guac_socket* socket,
+ const char* device_id);
+
+/**
+ * Sends a usbdata instruction over the given guac_socket connection,
+ * sending USB data to a specific endpoint on a client-side USB device.
+ *
+ * @param socket
+ * The guac_socket connection to use.
+ *
+ * @param device_id
+ * The unique identifier of the USB device.
+ *
+ * @param endpoint_number
+ * The endpoint number to send data to.
Review Comment:
I poked around in the code for anything else that refers to
`endpoint_number`, and I don't see this mentioned anywhere else. This may need
a little more documentation - the rest of the arguments, here, are pretty
self-explanatory, but `endpoint_number` doesn't really mean much to me?
##########
src/libguac/user-handlers.h:
##########
@@ -231,6 +231,27 @@ __guac_instruction_handler __guac_handshake_name_handler;
*/
__guac_instruction_handler __guac_handshake_timezone_handler;
+/**
+ * Internal initial handler for the usbconnect instruction. When a usbconnect
+ * instruction is received, this handler will be called. The client's
+ * usbconnect handler will be invoked if defined.
+ */
+__guac_instruction_handler __guac_handle_usbconnect;
+
+/**
+ * Internal initial handler for the usbdata instruction. When a usbdata
+ * instruction is received, this handler will be called. The client's
+ * usbdata handler will be invoked if defined.
+ */
+__guac_instruction_handler __guac_handle_usbdata;
+
+/**
+ * Internal initial handler for the usbdisconnect instruction. When a
+ * usbdisconnect instruction is received, this handler will be called.
+ * The client's usbdisconnect handler will be invoked if defined.
Review Comment:
`client's` -> `user's`
##########
src/libguac/guacamole/user.h:
##########
@@ -538,6 +538,64 @@ struct guac_user {
*/
guac_user_touch_handler* touch_handler;
+/**
+ * Handler for USB connect events sent by the Guacamole web-client.
+ *
+ * The handler takes the user that connected the device, the device ID,
+ * vendor ID, product ID, device name, serial number, device class,
+ * device subclass, device protocol, and interface data.
Review Comment:
As mentioned, above, it might be good to document which of these is required
and which may be optional.
##########
src/libguac/guacamole/protocol.h:
##########
@@ -1155,5 +1155,42 @@ guac_protocol_version
guac_protocol_string_to_version(const char* version_string
*/
const char* guac_protocol_version_to_string(guac_protocol_version version);
-#endif
+/**
+ * Sends a usbdisconnect instruction over the given guac_socket connection,
+ * requesting that the client disconnect the specified USB device.
+ *
+ * @param socket
+ * The guac_socket connection to use.
+ *
+ * @param device_id
+ * The unique identifier of the USB device that should be disconnected.
+ *
+ * @return
+ * Zero on success, non-zero on error.
+ */
+int guac_protocol_send_usbdisconnect(guac_socket* socket,
+ const char* device_id);
+
+/**
+ * Sends a usbdata instruction over the given guac_socket connection,
+ * sending USB data to a specific endpoint on a client-side USB device.
+ *
+ * @param socket
+ * The guac_socket connection to use.
+ *
+ * @param device_id
+ * The unique identifier of the USB device.
+ *
+ * @param endpoint_number
+ * The endpoint number to send data to.
+ *
+ * @param data
+ * The base64-encoded data to send to the USB device.
+ *
+ * @return
+ * Zero on success, non-zero on error.
+ */
+int guac_protocol_send_usbdata(guac_socket* socket, const char* device_id,
+ int endpoint_number, const char* data);
Review Comment:
Should there be a matching `connect` function implemented, here? I
understand that the `disconnect` function is provided so that a user could
disconnect a USB device while a connection is still active - I think it would
also be useful to be able to connect a USB device after a connection has
already started, which would likely require a `connect` function?
Or is there some other reason to implement a protocol-level `disconnect` but
not a `connect`?
##########
src/libguac/guacamole/user-fntypes.h:
##########
@@ -499,5 +499,93 @@ typedef int guac_user_get_handler(guac_user* user,
guac_object* object,
typedef int guac_user_put_handler(guac_user* user, guac_object* object,
guac_stream* stream, char* mimetype, char* name);
-#endif
+/**
+ * Handler for Guacamole USB connect events, invoked when a "usbconnect"
+ * instruction has been received from a user. This indicates that the user
+ * has connected a USB device via WebUSB and it is available for redirection.
+ *
+ * @param user
+ * The user that connected the USB device.
+ *
+ * @param device_id
+ * The unique identifier for the USB device.
+ *
+ * @param vendor_id
+ * The vendor ID of the USB device.
+ *
+ * @param product_id
+ * The product ID of the USB device.
+ *
+ * @param device_name
+ * The human-readable name of the device.
+ *
+ * @param serial_number
+ * The serial number of the device, if available.
+ *
+ * @param device_class
+ * The USB device class.
+ *
+ * @param device_subclass
+ * The USB device subclass.
+ *
+ * @param device_protocol
+ * The USB device protocol.
+ *
+ * @param interface_data
+ * Encoded string containing interface and endpoint information in the
+ * format: "iface_num:class:subclass:protocol:ep_num:dir:type:size,..."
+ *
+ * @return
+ * Zero if the USB connect event was handled successfully, or non-zero if
+ * an error occurred.
+ */
+typedef int guac_user_usbconnect_handler(guac_user* user, const char*
device_id,
+ int vendor_id, int product_id, const char* device_name,
+ const char* serial_number, int device_class, int device_subclass,
+ int device_protocol, const char* interface_data);
+/**
+* Handler for Guacamole USB data events, invoked when a "usbdata" instruction
+* has been received from a user. This carries data from a client-side USB
+* device to be processed on the server side.
+*
+* @param user
+* The user that sent the USB data.
+*
+* @param device_id
+* The unique identifier for the USB device.
+*
+* @param endpoint_number
+* The endpoint number the data originated from.
Review Comment:
As with the mention about in `protocol.h` - endpoint_number probably needs
some more thorough documentation as it appears to be a newly-introduced
concept, here.
##########
src/libguac/user-handlers.h:
##########
@@ -231,6 +231,27 @@ __guac_instruction_handler __guac_handshake_name_handler;
*/
__guac_instruction_handler __guac_handshake_timezone_handler;
+/**
+ * Internal initial handler for the usbconnect instruction. When a usbconnect
+ * instruction is received, this handler will be called. The client's
Review Comment:
`The client's` should probably be `The user's`, as this is a user handler,
not a client handler?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
