From f0fbe6b72558e3d32d592b3ae16c629f7eca6625 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 14 Oct 2015 10:23:14 +1000 Subject: [PATCH 01/10] hidpp10: drop hidpp10_list_devices - it's broken anyway Requires a ratbag_device argument (with the hidraw fd open) but then tries to list them - that doesn't quite work. Signed-off-by: Peter Hutterer --- src/hidpp10.c | 18 ------------------ src/hidpp10.h | 1 - 2 files changed, 19 deletions(-) diff --git a/src/hidpp10.c b/src/hidpp10.c index 612aed49..254ccb7d 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -1214,24 +1214,6 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, return 0; } -void hidpp10_list_devices(struct ratbag_device *device) { - struct hidpp10_device *dev; - int i; - - for (i = 0; i < 6; ++i) { - dev = hidpp10_device_new_from_idx(device, i); - if (!dev) - continue; - - log_info(device->ratbag, "[%d] %s %s (Wireless PID: %04x)\n", - i, - device_types[dev->device_type] ? device_types[dev->device_type] : "", - dev->name, dev->wpid); - hidpp10_device_destroy(dev); - } - -} - /* -------------------------------------------------------------------------- */ /* general device handling */ /* -------------------------------------------------------------------------- */ diff --git a/src/hidpp10.h b/src/hidpp10.h index 697189c4..03f5ad35 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -57,7 +57,6 @@ union hidpp10_message { void hidpp10_device_destroy(struct hidpp10_device *dev); int hidpp10_request_command(struct hidpp10_device *device, union hidpp10_message *msg); -void hidpp10_list_devices(struct ratbag_device *device); struct hidpp10_device *hidpp10_device_new_from_wpid(struct ratbag_device *device, uint16_t wpid); struct hidpp10_device *hidpp10_device_new_from_idx(struct ratbag_device *device, int idx); From 2314437e05d5467c4b0c7536142d790a6b843686 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 09:58:00 +1000 Subject: [PATCH 02/10] hidpp10: use direct read/write instead of the ratbag_hidraw functions HID++ 1.0 doesn't need hidraw other than the fd itself, and this is the first step to disentangling the hidpp10 code from libratbag. Signed-off-by: Peter Hutterer --- src/hidpp10.c | 49 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/src/hidpp10.c b/src/hidpp10.c index 254ccb7d..da2b8a5d 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include "hidpp10.h" @@ -90,15 +91,49 @@ static int hidpp10_write_command(struct hidpp10_device *dev, uint8_t *cmd, int size) { struct ratbag_device *device = dev->ratbag_device; - int res = ratbag_hidraw_output_report(device, cmd, size); + int fd = device->hidraw.fd; + int res; - if (res == 0) - return 0; + if (size < 1 || !cmd || fd < 0) + return -EINVAL; - if (res < 0) + log_buf_raw(device->ratbag, "hidpp10 write: ", cmd, size); + res = write(fd, cmd, size); + if (res < 0) { + res = -errno; log_error(device->ratbag, "Error: %s (%d)\n", strerror(-res), -res); + } - return res; + return res < 0 ? res : 0; +} + +static int +hidpp10_read_response(struct hidpp10_device *dev, uint8_t *buf, size_t size) +{ + struct ratbag_device *device = dev->ratbag_device; + int fd = device->hidraw.fd; + struct pollfd fds; + int rc; + + if (size < 1 || !buf || fd < 0) + return -EINVAL; + + fds.fd = fd; + fds.events = POLLIN; + + rc = poll(&fds, 1, 1000); + if (rc == -1) + return -errno; + + if (rc == 0) + return -ETIMEDOUT; + + rc = read(fd, buf, size); + + if (rc > 0) + log_buf_raw(device->ratbag, "input report: ", buf, rc); + + return rc >= 0 ? rc : -errno; } int @@ -141,12 +176,12 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) * loop until we get the actual answer or an error code. */ do { - ret = ratbag_hidraw_read_input_report(device, read_buffer.data, LONG_MESSAGE_LENGTH); + ret = hidpp10_read_response(dev, read_buffer.data, LONG_MESSAGE_LENGTH); /* Wait and retry if the USB timed out */ if (ret == -ETIMEDOUT) { msleep(10); - ret = ratbag_hidraw_read_input_report(device, read_buffer.data, LONG_MESSAGE_LENGTH); + ret = hidpp10_read_response(dev, read_buffer.data, LONG_MESSAGE_LENGTH); } /* Overwrite the return device index with ours. The kernel From a51cf764775979a8c3ac4b4da5118495cae4116f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 10:09:45 +1000 Subject: [PATCH 03/10] hidpp: move read/write to hidpp into the generic headers Part 2 of disentangling libratbag from the hidpp10 library. Copy the hidraw_fd into the hidpp_device struct and use that during read/write. This temporarily disables logging, will be re-enabled with an upcoming commit. Signed-off-by: Peter Hutterer --- src/hidpp-generic.c | 54 +++++++++++++++++++++++++++++++++++++++++++ src/hidpp-generic.h | 11 +++++++++ src/hidpp10.c | 56 ++++----------------------------------------- src/hidpp10.h | 1 + 4 files changed, 70 insertions(+), 52 deletions(-) diff --git a/src/hidpp-generic.c b/src/hidpp-generic.c index 15ee135c..2f1a7940 100644 --- a/src/hidpp-generic.c +++ b/src/hidpp-generic.c @@ -30,6 +30,8 @@ #include "hidpp-generic.h" +#include +#include #include #include "libratbag-private.h" @@ -201,3 +203,55 @@ hidpp20_1b04_get_physical_mapping_name(uint16_t value) return "UNKNOWN"; } +int +hidpp_write_command(struct hidpp_device *dev, uint8_t *cmd, int size) +{ + int fd = dev->hidraw_fd; + int res; + + if (size < 1 || !cmd || fd < 0) + return -EINVAL; + +#if 0 + log_buf_raw(device->ratbag, "hidpp10 write: ", cmd, size); +#endif + res = write(fd, cmd, size); + if (res < 0) { + res = -errno; +#if 0 + log_error(device->ratbag, "Error: %s (%d)\n", strerror(-res), -res); +#endif + } + + return res < 0 ? res : 0; +} + +int +hidpp_read_response(struct hidpp_device *dev, uint8_t *buf, size_t size) +{ + int fd = dev->hidraw_fd; + struct pollfd fds; + int rc; + + if (size < 1 || !buf || fd < 0) + return -EINVAL; + + fds.fd = fd; + fds.events = POLLIN; + + rc = poll(&fds, 1, 1000); + if (rc == -1) + return -errno; + + if (rc == 0) + return -ETIMEDOUT; + + rc = read(fd, buf, size); + +#if 0 + if (rc > 0) + log_buf_raw(device->ratbag, "input report: ", buf, rc); +#endif + + return rc >= 0 ? rc : -errno; +} diff --git a/src/hidpp-generic.h b/src/hidpp-generic.h index 9d3908d7..8d16a7bf 100644 --- a/src/hidpp-generic.h +++ b/src/hidpp-generic.h @@ -27,6 +27,7 @@ */ #include +#include #ifndef HIDPP_GENERIC_H #define HIDPP_GENERIC_H @@ -64,6 +65,10 @@ #define ERR_INVALID_PARAM_VALUE 0x0B #define ERR_WRONG_PIN_CODE 0x0C +struct hidpp_device { + int hidraw_fd; +}; + extern const char *hidpp_errors[0xFF]; const char * @@ -81,4 +86,10 @@ hidpp20_1b04_get_logical_mapping(uint16_t value); uint16_t hidpp20_1b04_get_logical_control_id(const struct ratbag_button_action *action); +int +hidpp_write_command(struct hidpp_device *dev, uint8_t *cmd, int size); + +int +hidpp_read_response(struct hidpp_device *dev, uint8_t *buf, size_t size); + #endif /* HIDPP_GENERIC_H */ diff --git a/src/hidpp10.c b/src/hidpp10.c index da2b8a5d..7bbdf724 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -87,55 +87,6 @@ const char *device_types[0xFF] = { [0x0A ... 0xFE] = NULL, }; -static int -hidpp10_write_command(struct hidpp10_device *dev, uint8_t *cmd, int size) -{ - struct ratbag_device *device = dev->ratbag_device; - int fd = device->hidraw.fd; - int res; - - if (size < 1 || !cmd || fd < 0) - return -EINVAL; - - log_buf_raw(device->ratbag, "hidpp10 write: ", cmd, size); - res = write(fd, cmd, size); - if (res < 0) { - res = -errno; - log_error(device->ratbag, "Error: %s (%d)\n", strerror(-res), -res); - } - - return res < 0 ? res : 0; -} - -static int -hidpp10_read_response(struct hidpp10_device *dev, uint8_t *buf, size_t size) -{ - struct ratbag_device *device = dev->ratbag_device; - int fd = device->hidraw.fd; - struct pollfd fds; - int rc; - - if (size < 1 || !buf || fd < 0) - return -EINVAL; - - fds.fd = fd; - fds.events = POLLIN; - - rc = poll(&fds, 1, 1000); - if (rc == -1) - return -errno; - - if (rc == 0) - return -ETIMEDOUT; - - rc = read(fd, buf, size); - - if (rc > 0) - log_buf_raw(device->ratbag, "input report: ", buf, rc); - - return rc >= 0 ? rc : -errno; -} - int hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) { @@ -167,7 +118,7 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) log_buf_raw(ratbag, " expected_error_dev: ", expected_error_dev.data, SHORT_MESSAGE_LENGTH); /* Send the message to the Device */ - ret = hidpp10_write_command(dev, msg->data, command_size); + ret = hidpp_write_command(&dev->base, msg->data, command_size); if (ret) goto out_err; @@ -176,12 +127,12 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) * loop until we get the actual answer or an error code. */ do { - ret = hidpp10_read_response(dev, read_buffer.data, LONG_MESSAGE_LENGTH); + ret = hidpp_read_response(&dev->base, read_buffer.data, LONG_MESSAGE_LENGTH); /* Wait and retry if the USB timed out */ if (ret == -ETIMEDOUT) { msleep(10); - ret = hidpp10_read_response(dev, read_buffer.data, LONG_MESSAGE_LENGTH); + ret = hidpp_read_response(&dev->base, read_buffer.data, LONG_MESSAGE_LENGTH); } /* Overwrite the return device index with ours. The kernel @@ -1286,6 +1237,7 @@ hidpp10_device_new(struct ratbag_device *device, int index) return NULL; dev->index = index; + dev->base.hidraw_fd = device->hidraw.fd; dev->ratbag_device = device; return dev; diff --git a/src/hidpp10.h b/src/hidpp10.h index 03f5ad35..e470c75d 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -507,6 +507,7 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, * 0 is zeroed, 1 and 2 are garbage, all above 6 is garbage */ #define HIDPP10_NUM_PROFILES 3 struct hidpp10_device { + struct hidpp_device base; struct ratbag_device *ratbag_device; unsigned index; char name[15]; From 498a2882df0cc9751319d4109799a554cf5b0caa Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 14 Oct 2015 10:01:19 +1000 Subject: [PATCH 04/10] hidpp: disentangle hidpp from libratbag We now only use libratbag for logging. Abstract that away so we can use hidpp10 separate from libratbag where needed. Signed-off-by: Peter Hutterer --- src/driver-hidpp10.c | 14 +++++- src/hidpp-generic.c | 53 ++++++++++++++++++---- src/hidpp-generic.h | 43 ++++++++++++++++++ src/hidpp10.c | 102 ++++++++++++++++++++----------------------- src/hidpp10.h | 9 ++-- 5 files changed, 152 insertions(+), 69 deletions(-) diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index b7f5d4c4..92f0a92a 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -197,12 +197,21 @@ hidpp10drv_test_hidraw(struct ratbag_device *device) return ratbag_hidraw_has_report(device, REPORT_ID_SHORT); } +static void +hidpp10_log(void *userdata, enum hidpp_log_priority priority, const char *format, va_list args) +{ + struct ratbag_device *device = userdata; + + log_msg_va(device->ratbag, priority, format, args); +} + static int hidpp10drv_probe(struct ratbag_device *device) { int rc; struct hidpp10drv_data *drv_data = NULL; struct hidpp10_device *dev = NULL; + struct hidpp_device base; rc = ratbag_find_hidraw(device, hidpp10drv_test_hidraw); if (rc == -ENODEV) { @@ -217,13 +226,16 @@ hidpp10drv_probe(struct ratbag_device *device) } drv_data = zalloc(sizeof(*drv_data)); + base.log_handler = hidpp10_log; + base.userdata = device; + base.hidraw_fd = device->hidraw.fd; /* We can treat all devices as wired devices here. If we talk to the * correct hidraw device the kernel adjusts the device index for us, * so even for unifying receiver devices we can just 0x00 as device * index. */ - dev = hidpp10_device_new_from_idx(device, WIRED_DEVICE_IDX); + dev = hidpp10_device_new_from_idx(&base, WIRED_DEVICE_IDX); if (!dev) { log_error(device->ratbag, diff --git a/src/hidpp-generic.c b/src/hidpp-generic.c index 2f1a7940..bdc615d3 100644 --- a/src/hidpp-generic.c +++ b/src/hidpp-generic.c @@ -212,15 +212,11 @@ hidpp_write_command(struct hidpp_device *dev, uint8_t *cmd, int size) if (size < 1 || !cmd || fd < 0) return -EINVAL; -#if 0 - log_buf_raw(device->ratbag, "hidpp10 write: ", cmd, size); -#endif + hidpp_log_buf_raw(dev, "hidpp write: ", cmd, size); res = write(fd, cmd, size); if (res < 0) { res = -errno; -#if 0 - log_error(device->ratbag, "Error: %s (%d)\n", strerror(-res), -res); -#endif + hidpp_log_error(dev, "Error: %s (%d)\n", strerror(-res), -res); } return res < 0 ? res : 0; @@ -248,10 +244,49 @@ hidpp_read_response(struct hidpp_device *dev, uint8_t *buf, size_t size) rc = read(fd, buf, size); -#if 0 if (rc > 0) - log_buf_raw(device->ratbag, "input report: ", buf, rc); -#endif + hidpp_log_buf_raw(dev, "hidpp read: ", buf, rc); return rc >= 0 ? rc : -errno; } + +void +hidpp_log(struct hidpp_device *dev, + enum hidpp_log_priority priority, + const char *format, + ...) +{ + va_list args; + + va_start(args, format); + dev->log_handler(dev->userdata, priority, format, args); + va_end(args); +} + +void +hidpp_log_buffer(struct hidpp_device *dev, + enum hidpp_log_priority priority, + const char *header, + uint8_t *buf, size_t len) +{ + _cleanup_free_ char *output_buf = NULL; + char *sep = ""; + unsigned int i, n; + unsigned int buf_len; + + buf_len = header ? strlen(header) : 0; + buf_len += len * 3; + buf_len += 1; /* terminating '\0' */ + + output_buf = zalloc(buf_len); + n = 0; + if (header) + n += snprintf_safe(output_buf, buf_len - n, "%s", header); + + for (i = 0; i < len; ++i) { + n += snprintf_safe(&output_buf[n], buf_len - n, "%s%02x", sep, buf[i] & 0xFF); + sep = " "; + } + + hidpp_log(dev, priority, "%s\n", output_buf); +} diff --git a/src/hidpp-generic.h b/src/hidpp-generic.h index 8d16a7bf..8d0dc121 100644 --- a/src/hidpp-generic.h +++ b/src/hidpp-generic.h @@ -27,6 +27,7 @@ */ #include +#include #include #ifndef HIDPP_GENERIC_H @@ -65,8 +66,27 @@ #define ERR_INVALID_PARAM_VALUE 0x0B #define ERR_WRONG_PIN_CODE 0x0C +/* Keep this in sync with ratbag_log_priority */ +enum hidpp_log_priority { + /** + * Raw protocol messages. Using this log level results in *a lot* of + * output. + */ + HIDPP_LOG_PRIORITY_RAW = 10, + HIDPP_LOG_PRIORITY_DEBUG = 20, + HIDPP_LOG_PRIORITY_INFO = 30, + HIDPP_LOG_PRIORITY_ERROR = 40, +}; + +typedef void (*hidpp_log_handler)(void *userdata, + enum hidpp_log_priority priority, + const char *format, va_list args) + __attribute__ ((format (printf, 3, 0))); + struct hidpp_device { int hidraw_fd; + void *userdata; + hidpp_log_handler log_handler; }; extern const char *hidpp_errors[0xFF]; @@ -92,4 +112,27 @@ hidpp_write_command(struct hidpp_device *dev, uint8_t *cmd, int size); int hidpp_read_response(struct hidpp_device *dev, uint8_t *buf, size_t size); +void +hidpp_log(struct hidpp_device *dev, + enum hidpp_log_priority priority, + const char *format, + ...) + __attribute__ ((format (printf, 3, 4))); + +void +hidpp_log_buffer(struct hidpp_device *dev, + enum hidpp_log_priority priority, + const char *header, + uint8_t *buf, size_t len); + +#define hidpp_log_raw(li_, ...) hidpp_log((li_), HIDPP_LOG_PRIORITY_RAW, __VA_ARGS__) +#define hidpp_log_debug(li_, ...) hidpp_log((li_), HIDPP_LOG_PRIORITY_DEBUG, __VA_ARGS__) +#define hidpp_log_info(li_, ...) hidpp_log((li_), HIDPP_LOG_PRIORITY_INFO, __VA_ARGS__) +#define hidpp_log_error(li_, ...) hidpp_log((li_), HIDPP_LOG_PRIORITY_ERROR, __VA_ARGS__) +#define hidpp_log_bug_kernel(li_, ...) hidpp((li_), HIDPP_LOG_PRIORITY_ERROR, "kernel bug: " __VA_ARGS__) +#define hidpp_log_buf_raw(li_, h_, buf_, len_) hidpp_log_buffer(li_, HIDPP_LOG_PRIORITY_RAW, h_, buf_, len_) +#define hidpp_log_buf_debug(li_, h_, buf_, len_) hidpp_log_buffer(li_, HIDPP_LOG_PRIORITY_DEBUG, h_, buf_, len_) +#define hidpp_log_buf_info(li_, h_, buf_, len_) hidpp_log_buffer(li_, HIDPP_LOG_PRIORITY_INFO, h_, buf_, len_) +#define hidpp_log_buf_error(li_, h_, buf_, len_) hidpp_log_buffer(li_, HIDPP_LOG_PRIORITY_ERROR, h_, buf_, len_) + #endif /* HIDPP_GENERIC_H */ diff --git a/src/hidpp10.c b/src/hidpp10.c index 7bbdf724..3bafee06 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -42,7 +42,6 @@ #include "libratbag-util.h" #include "libratbag-hidraw.h" -#include "libratbag-private.h" #define ERROR_MSG(__hidpp_msg, idx) { \ .msg = { \ @@ -90,8 +89,6 @@ const char *device_types[0xFF] = { int hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) { - struct ratbag_device *device = dev->ratbag_device; - struct ratbag *ratbag = device->ratbag; union hidpp10_message read_buffer; union hidpp10_message expected_header; union hidpp10_message expected_error_dev = ERROR_MSG(msg, msg->msg.device_idx); @@ -114,8 +111,8 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) expected_header = *msg; /* response message length doesn't depend on request length */ - log_buf_raw(ratbag, " expected_header: ?? ", &expected_header.data[1], 3); - log_buf_raw(ratbag, " expected_error_dev: ", expected_error_dev.data, SHORT_MESSAGE_LENGTH); + hidpp_log_buf_raw(&dev->base, " expected_header: ?? ", &expected_header.data[1], 3); + hidpp_log_buf_raw(&dev->base, " expected_error_dev: ", expected_error_dev.data, SHORT_MESSAGE_LENGTH); /* Send the message to the Device */ ret = hidpp_write_command(&dev->base, msg->data, command_size); @@ -149,7 +146,7 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) /* error */ if (!memcmp(read_buffer.data, expected_error_dev.data, 5)) { hidpp_err = read_buffer.msg.parameters[1]; - log_raw(ratbag, + hidpp_log_raw(&dev->base, " HID++ error from the %s (%d): %s (%02x)\n", read_buffer.msg.device_idx == RECEIVER_IDX ? "receiver" : "device", read_buffer.msg.device_idx, @@ -160,13 +157,13 @@ hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) } while (ret > 0); if (ret < 0) { - log_error(ratbag, " USB error: %s (%d)\n", strerror(-ret), -ret); + hidpp_log_error(&dev->base, " USB error: %s (%d)\n", strerror(-ret), -ret); perror("write"); goto out_err; } if (!hidpp_err) { - log_buf_raw(ratbag, " received: ", read_buffer.data, ret); + hidpp_log_buf_raw(&dev->base, " received: ", read_buffer.data, ret); /* copy the answer for the caller */ *msg = read_buffer; } @@ -207,7 +204,7 @@ hidpp10_get_hidpp_notifications(struct hidpp10_device *dev, union hidpp10_message notifications = CMD_HIDPP_NOTIFICATIONS(idx, GET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching HID++ notifications\n"); + hidpp_log_raw(&dev->base, "Fetching HID++ notifications\n"); res = hidpp10_request_command(dev, ¬ifications); if (res) @@ -228,7 +225,7 @@ hidpp10_set_hidpp_notifications(struct hidpp10_device *dev, union hidpp10_message notifications = CMD_HIDPP_NOTIFICATIONS(idx, SET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Setting HID++ notifications\n"); + hidpp_log_raw(&dev->base, "Setting HID++ notifications\n"); notifications.msg.parameters[0] = reporting_flags & 0xFF; notifications.msg.parameters[1] = (reporting_flags >> 8) & 0x1F; @@ -263,7 +260,7 @@ hidpp10_get_individual_features(struct hidpp10_device *dev, union hidpp10_message features = CMD_ENABLE_INDIVIDUAL_FEATURES(idx, GET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching individual features\n"); + hidpp_log_raw(&dev->base, "Fetching individual features\n"); res = hidpp10_request_command(dev, &features); if (res) @@ -286,7 +283,7 @@ hidpp10_set_individual_features(struct hidpp10_device *dev, union hidpp10_message mode = CMD_ENABLE_INDIVIDUAL_FEATURES(idx, SET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Setting individual features\n"); + hidpp_log_raw(&dev->base, "Setting individual features\n"); mode.msg.device_idx = dev->index; @@ -496,7 +493,7 @@ hidpp10_get_current_profile(struct hidpp10_device *dev, int8_t *current_profile) int res; int8_t page; - log_raw(dev->ratbag_device->ratbag, "Fetching current profile\n"); + hidpp_log_raw(&dev->base, "Fetching current profile\n"); res = hidpp10_request_command(dev, &profile); if (res) @@ -515,7 +512,6 @@ hidpp10_get_current_profile(struct hidpp10_device *dev, int8_t *current_profile) int hidpp10_get_profile(struct hidpp10_device *dev, int8_t number, struct hidpp10_profile *profile_return) { - struct ratbag *ratbag = dev->ratbag_device->ratbag; union _hidpp10_profile_data data; struct _hidpp10_profile *p = &data.profile; size_t i; @@ -528,7 +524,7 @@ hidpp10_get_profile(struct hidpp10_device *dev, int8_t number, struct hidpp10_pr * -> profiles are stored in the Flash */ number += 2; - log_raw(dev->ratbag_device->ratbag, "Fetching profile %d\n", number); + hidpp_log_raw(&dev->base, "Fetching profile %d\n", number); for (i = 0; i < sizeof(data); i += 16) { /* each sector contains 16 bytes of data */ @@ -589,56 +585,56 @@ hidpp10_get_profile(struct hidpp10_device *dev, int8_t number, struct hidpp10_pr } - log_buf_raw(ratbag, + hidpp_log_buf_raw(&dev->base, "+++++++++++++++++++ Profile data: +++++++++++++++++ \n", data.data, 78); - log_raw(ratbag, "Profile %d:\n", number); + hidpp_log_raw(&dev->base, "Profile %d:\n", number); for (i = 0; i < 5; i++) { - log_raw(ratbag, + hidpp_log_raw(&dev->base, "DPI mode: %dx%d dpi\n", profile.dpi_modes[i].xres, profile.dpi_modes[i].yres); - log_raw(ratbag, + hidpp_log_raw(&dev->base, "LED status: 1:%s 2:%s 3:%s 4:%s\n", (p->dpi_modes[i].led1 & 0x2) ? "on" : "off", (p->dpi_modes[i].led2 & 0x2) ? "on" : "off", (p->dpi_modes[i].led3 & 0x2) ? "on" : "off", (p->dpi_modes[i].led4 & 0x2) ? "on" : "off"); } - log_raw(ratbag, "Angle correction: %d\n", profile.angle_correction); - log_raw(ratbag, "Default DPI mode: %d\n", profile.default_dpi_mode); - log_raw(ratbag, "Refresh rate: %d\n", profile.refresh_rate); + hidpp_log_raw(&dev->base, "Angle correction: %d\n", profile.angle_correction); + hidpp_log_raw(&dev->base, "Default DPI mode: %d\n", profile.default_dpi_mode); + hidpp_log_raw(&dev->base, "Refresh rate: %d\n", profile.refresh_rate); for (i = 0; i < 13; i++) { union _hidpp10_button_binding *button = &p->buttons[i]; switch (button->any.type) { case PROFILE_BUTTON_TYPE_BUTTON: - log_raw(ratbag, + hidpp_log_raw(&dev->base, "Button %zd: button %d\n", i, ffs(hidpp10_get_unaligned_u16le(&button->button.button_flags_lsb))); break; case PROFILE_BUTTON_TYPE_KEYS: - log_raw(ratbag, + hidpp_log_raw(&dev->base, "Button %zd: key %d modifier %x\n", i, button->keyboard_keys.key, button->keyboard_keys.modifier_flags); break; case PROFILE_BUTTON_TYPE_SPECIAL: - log_raw(ratbag, + hidpp_log_raw(&dev->base, "Button %zd: special %x\n", i, ffs(hidpp10_get_unaligned_u16le(&button->special.flags1))); break; case PROFILE_BUTTON_TYPE_CONSUMER_CONTROL: - log_raw(ratbag, + hidpp_log_raw(&dev->base, "Button %zd: consumer: %x\n", i, hidpp10_get_unaligned_u16(&button->consumer_control.consumer_control1)); break; case PROFILE_BUTTON_TYPE_DISABLED: - log_raw(ratbag, "Button %zd: disabled\n", i); + hidpp_log_raw(&dev->base, "Button %zd: disabled\n", i); break; default: /* FIXME: this is the page number for the macro, @@ -677,7 +673,7 @@ hidpp10_get_led_status(struct hidpp10_device *dev, uint8_t *status = led_status.msg.parameters; int res; - log_raw(dev->ratbag_device->ratbag, "Fetching LED status\n"); + hidpp_log_raw(&dev->base, "Fetching LED status\n"); res = hidpp10_request_command(dev, &led_status); if (res) @@ -703,7 +699,7 @@ hidpp10_set_led_status(struct hidpp10_device *dev, int res; int i; - log_raw(dev->ratbag_device->ratbag, "Setting LED status\n"); + hidpp_log_raw(&dev->base, "Setting LED status\n"); for (i = 0; i < 6; i++) { switch (led[i]) { @@ -754,7 +750,7 @@ hidpp10_get_led_intensity(struct hidpp10_device *dev, uint8_t *intensity = led_intensity.msg.parameters; int res; - log_raw(dev->ratbag_device->ratbag, "Fetching LED intensity\n"); + hidpp_log_raw(&dev->base, "Fetching LED intensity\n"); res = hidpp10_request_command(dev, &led_intensity); if (res) @@ -779,7 +775,7 @@ hidpp10_set_led_intensity(struct hidpp10_device *dev, uint8_t *intensity = led_intensity.msg.parameters; int res; - log_raw(dev->ratbag_device->ratbag, "Setting LED intensity\n"); + hidpp_log_raw(&dev->base, "Setting LED intensity\n"); intensity[0] = led_intensity_in_percent[0]/10 & 0xF; intensity[0] |= (led_intensity_in_percent[1]/10 & 0xF) << 4; @@ -818,7 +814,7 @@ hidpp10_get_led_color(struct hidpp10_device *dev, union hidpp10_message led_color = CMD_LED_COLOR(idx, GET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching LED color\n"); + hidpp_log_raw(&dev->base, "Fetching LED color\n"); res = hidpp10_request_command(dev, &led_color); if (res) @@ -841,7 +837,7 @@ hidpp10_set_led_color(struct hidpp10_device *dev, union hidpp10_message led_color = CMD_LED_COLOR(idx, SET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Setting LED color\n"); + hidpp_log_raw(&dev->base, "Setting LED color\n"); led_color.msg.parameters[0] = red; led_color.msg.parameters[1] = green; @@ -874,7 +870,7 @@ hidpp10_get_optical_sensor_settings(struct hidpp10_device *dev, union hidpp10_message sensor = CMD_OPTICAL_SENSOR_SETTINGS(idx, GET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching optical sensor settings\n"); + hidpp_log_raw(&dev->base, "Fetching optical sensor settings\n"); res = hidpp10_request_command(dev, &sensor); if (res) @@ -911,7 +907,7 @@ hidpp10_get_current_resolution(struct hidpp10_device *dev, union hidpp10_message resolution = CMD_CURRENT_RESOLUTION(REPORT_ID_SHORT, idx, GET_LONG_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching current resolution\n"); + hidpp_log_raw(&dev->base, "Fetching current resolution\n"); res = hidpp10_request_command(dev, &resolution); if (res) @@ -961,7 +957,7 @@ hidpp10_get_usb_refresh_rate(struct hidpp10_device *dev, union hidpp10_message refresh = CMD_USB_REFRESH_RATE(idx, GET_REGISTER_REQ); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching USB refresh rate\n"); + hidpp_log_raw(&dev->base, "Fetching USB refresh rate\n"); res = hidpp10_request_command(dev, &refresh); if (res) @@ -979,7 +975,7 @@ hidpp10_set_usb_refresh_rate(struct hidpp10_device *dev, unsigned idx = dev->index; union hidpp10_message refresh = CMD_USB_REFRESH_RATE(idx, GET_REGISTER_REQ); - log_raw(dev->ratbag_device->ratbag, "Setting USB refresh rate\n"); + hidpp_log_raw(&dev->base, "Setting USB refresh rate\n"); refresh.msg.parameters[0] = 1000/rate; @@ -1012,9 +1008,7 @@ hidpp10_read_memory(struct hidpp10_device *dev, uint8_t page, uint8_t offset, if (page > 31) return -EINVAL; - log_raw(dev->ratbag_device->ratbag, - "Reading memory page %d, offset %#x\n", - page, offset); + hidpp_log_raw(&dev->base, "Reading memory page %d, offset %#x\n", page, offset); res = hidpp10_request_command(dev, &readmem); if (res) @@ -1032,21 +1026,21 @@ hidpp10_dump_all_pages(struct hidpp10_device *dev) uint8_t bytes[16]; int res; - log_info(dev->ratbag_device->ratbag, "::::::: Dumping memory :::::\n"); + hidpp_log_info(&dev->base, "::::::: Dumping memory :::::\n"); for (page = 0; page < 512; page++) { - log_info(dev->ratbag_device->ratbag, ":: page %d\n", page); + hidpp_log_info(&dev->base, ":: page %d\n", page); for (offset = 0; offset < 256; offset += 16) { res = hidpp10_read_memory(dev, page, offset, bytes); if (res != 0) goto out; - log_buffer(dev->ratbag_device->ratbag, RATBAG_LOG_PRIORITY_INFO, " ", bytes, ARRAY_LENGTH(bytes)); + hidpp_log_buffer(&dev->base, HIDPP_LOG_PRIORITY_INFO, " ", bytes, ARRAY_LENGTH(bytes)); } } out: - log_info(dev->ratbag_device->ratbag, "::::::: Dumping memory complete :::::\n"); + hidpp_log_info(&dev->base, "::::::: Dumping memory complete :::::\n"); } /* -------------------------------------------------------------------------- */ @@ -1111,7 +1105,7 @@ hidpp10_get_pairing_information(struct hidpp10_device *dev, union hidpp10_message pairing_information = CMD_PAIRING_INFORMATION(idx, DEVICE_PAIRING_INFORMATION); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching pairing information\n"); + hidpp_log_raw(&dev->base, "Fetching pairing information\n"); res = hidpp10_request_command(dev, &pairing_information); if (res) @@ -1133,7 +1127,7 @@ hidpp10_get_pairing_information_device_name(struct hidpp10_device *dev, union hidpp10_message device_name = CMD_PAIRING_INFORMATION(idx, DEVICE_NAME); int res; - log_raw(dev->ratbag_device->ratbag, "Fetching device name\n"); + hidpp_log_raw(&dev->base, "Fetching device name\n"); res = hidpp10_request_command(dev, &device_name); if (res) @@ -1176,7 +1170,7 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, int res; uint8_t maj, min, build; - log_raw(dev->ratbag_device->ratbag, "Fetching firmware information\n"); + hidpp_log_raw(&dev->base, "Fetching firmware information\n"); /* * This may fail on some devices @@ -1228,7 +1222,8 @@ hidpp10_get_device_info(struct hidpp10_device *dev) } static struct hidpp10_device* -hidpp10_device_new(struct ratbag_device *device, int index) +hidpp10_device_new(const struct hidpp_device *base, + int index) { struct hidpp10_device *dev; @@ -1237,20 +1232,19 @@ hidpp10_device_new(struct ratbag_device *device, int index) return NULL; dev->index = index; - dev->base.hidraw_fd = device->hidraw.fd; - dev->ratbag_device = device; + dev->base = *base; return dev; } struct hidpp10_device* -hidpp10_device_new_from_wpid(struct ratbag_device* device, uint16_t wpid) +hidpp10_device_new_from_wpid(const struct hidpp_device *base, uint16_t wpid) { struct hidpp10_device *dev; int i; for (i = 1; i < 7; i++) { - dev = hidpp10_device_new_from_idx(device, i); + dev = hidpp10_device_new_from_idx(base, i); if (!dev) continue; @@ -1264,11 +1258,11 @@ hidpp10_device_new_from_wpid(struct ratbag_device* device, uint16_t wpid) } struct hidpp10_device* -hidpp10_device_new_from_idx(struct ratbag_device *device, int idx) +hidpp10_device_new_from_idx(const struct hidpp_device *base, int idx) { struct hidpp10_device *dev; - dev = hidpp10_device_new(device, idx); + dev = hidpp10_device_new(base, idx); if (!dev) return NULL; diff --git a/src/hidpp10.h b/src/hidpp10.h index e470c75d..10fe111c 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -33,8 +33,6 @@ #include #include "hidpp-generic.h" -#include "libratbag-private.h" -#include "libratbag.h" struct hidpp10_device; @@ -57,8 +55,10 @@ union hidpp10_message { void hidpp10_device_destroy(struct hidpp10_device *dev); int hidpp10_request_command(struct hidpp10_device *device, union hidpp10_message *msg); -struct hidpp10_device *hidpp10_device_new_from_wpid(struct ratbag_device *device, uint16_t wpid); -struct hidpp10_device *hidpp10_device_new_from_idx(struct ratbag_device *device, int idx); +struct hidpp10_device *hidpp10_device_new_from_wpid(const struct hidpp_device *base, + uint16_t wpid); +struct hidpp10_device *hidpp10_device_new_from_idx(const struct hidpp_device *base, + int idx); /* -------------------------------------------------------------------------- */ /* 0x00: Enable HID++ Notifications */ @@ -508,7 +508,6 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, #define HIDPP10_NUM_PROFILES 3 struct hidpp10_device { struct hidpp_device base; - struct ratbag_device *ratbag_device; unsigned index; char name[15]; uint16_t wpid; From 35d24890aec6fe12dcb90a4e64768ca924fa38b4 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 12:38:11 +1000 Subject: [PATCH 05/10] hidpp10: drop a couple of unused fields from the hidpp10 device Some were truly unused, others were write-only. Signed-off-by: Peter Hutterer --- src/hidpp10.c | 15 ++++++++++----- src/hidpp10.h | 11 ----------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/hidpp10.c b/src/hidpp10.c index 3bafee06..2ccf244c 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -1203,20 +1203,25 @@ hidpp10_get_device_info(struct hidpp10_device *dev) uint32_t feature_mask, notifications; uint8_t reflect; int i; + uint16_t xres, yres; + uint16_t refresh_rate; + enum hidpp10_led_status led[6]; + int8_t current_profile; + struct hidpp10_profile profiles[HIDPP10_NUM_PROFILES]; hidpp10_get_individual_features(dev, &feature_mask); hidpp10_get_hidpp_notifications(dev, ¬ifications); - hidpp10_get_current_resolution(dev, &dev->xres, &dev->yres); - hidpp10_get_led_status(dev, dev->led); - hidpp10_get_usb_refresh_rate(dev, &dev->refresh_rate); + hidpp10_get_current_resolution(dev, &xres, &yres); + hidpp10_get_led_status(dev, led); + hidpp10_get_usb_refresh_rate(dev, &refresh_rate); hidpp10_get_optical_sensor_settings(dev, &reflect); - hidpp10_get_current_profile(dev, &dev->current_profile); + hidpp10_get_current_profile(dev, ¤t_profile); for (i = 0; i < HIDPP10_NUM_PROFILES; i++) - hidpp10_get_profile(dev, i, &dev->profiles[i]); + hidpp10_get_profile(dev, i, &profiles[i]); return 0; } diff --git a/src/hidpp10.h b/src/hidpp10.h index 10fe111c..41181404 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -509,17 +509,6 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, struct hidpp10_device { struct hidpp_device base; unsigned index; - char name[15]; uint16_t wpid; - uint8_t report_interval; - uint8_t device_type; - uint8_t fw_major; - uint8_t fw_minor; - uint8_t build; - uint16_t xres, yres; - uint16_t refresh_rate; - enum hidpp10_led_status led[6]; - int8_t current_profile; - struct hidpp10_profile profiles[HIDPP10_NUM_PROFILES]; }; #endif /* HIDPP_10_H */ From 70db86f0c4f6bef05cc596424133a49d4d3f8b0b Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 12:42:23 +1000 Subject: [PATCH 06/10] hidpp10: make hidpp10_request_command static For commands we support we have the wrapper functions, they're safer to use. Signed-off-by: Peter Hutterer --- src/hidpp10.c | 18 +++++++++++++++++- src/hidpp10.h | 17 ----------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/hidpp10.c b/src/hidpp10.c index 2ccf244c..574c497f 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -43,6 +43,22 @@ #include "libratbag-util.h" #include "libratbag-hidraw.h" +struct _hidpp10_message { + uint8_t report_id; + uint8_t device_idx; + uint8_t sub_id; + uint8_t address; + union { + uint8_t parameters[3]; + uint8_t string[16]; + }; +} __attribute__((packed)); + +union hidpp10_message { + struct _hidpp10_message msg; + uint8_t data[LONG_MESSAGE_LENGTH]; +}; + #define ERROR_MSG(__hidpp_msg, idx) { \ .msg = { \ .report_id = REPORT_ID_SHORT, \ @@ -86,7 +102,7 @@ const char *device_types[0xFF] = { [0x0A ... 0xFE] = NULL, }; -int +static int hidpp10_request_command(struct hidpp10_device *dev, union hidpp10_message *msg) { union hidpp10_message read_buffer; diff --git a/src/hidpp10.h b/src/hidpp10.h index 41181404..dc096da4 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -36,25 +36,8 @@ struct hidpp10_device; -struct _hidpp10_message { - uint8_t report_id; - uint8_t device_idx; - uint8_t sub_id; - uint8_t address; - union { - uint8_t parameters[3]; - uint8_t string[16]; - }; -} __attribute__((packed)); - -union hidpp10_message { - struct _hidpp10_message msg; - uint8_t data[LONG_MESSAGE_LENGTH]; -}; - void hidpp10_device_destroy(struct hidpp10_device *dev); -int hidpp10_request_command(struct hidpp10_device *device, union hidpp10_message *msg); struct hidpp10_device *hidpp10_device_new_from_wpid(const struct hidpp_device *base, uint16_t wpid); struct hidpp10_device *hidpp10_device_new_from_idx(const struct hidpp_device *base, From 9e4bafba8f26229c17544ec3a7be4f6c58062e5e Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 12:48:40 +1000 Subject: [PATCH 07/10] hidpp10: header file cleanups Signed-off-by: Peter Hutterer --- src/hidpp10.h | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/hidpp10.h b/src/hidpp10.h index dc096da4..46b63113 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -34,14 +34,24 @@ #include "hidpp-generic.h" -struct hidpp10_device; +/* FIXME: that's what my G500s supports, but only pages 3-5 are valid. + * 0 is zeroed, 1 and 2 are garbage, all above 6 is garbage */ +#define HIDPP10_NUM_PROFILES 3 -void hidpp10_device_destroy(struct hidpp10_device *dev); +struct hidpp10_device { + struct hidpp_device base; + unsigned index; + uint16_t wpid; +}; -struct hidpp10_device *hidpp10_device_new_from_wpid(const struct hidpp_device *base, - uint16_t wpid); -struct hidpp10_device *hidpp10_device_new_from_idx(const struct hidpp_device *base, - int idx); +struct hidpp10_device* +hidpp10_device_new_from_wpid(const struct hidpp_device *base, + uint16_t wpid); +struct hidpp10_device* +hidpp10_device_new_from_idx(const struct hidpp_device *base, + int idx); +void +hidpp10_device_destroy(struct hidpp10_device *dev); /* -------------------------------------------------------------------------- */ /* 0x00: Enable HID++ Notifications */ @@ -485,13 +495,4 @@ hidpp10_get_firmare_information(struct hidpp10_device *dev, uint8_t *minor, uint8_t *build_number); - -/* FIXME: that's what my G500s supports, but only pages 3-5 are valid. - * 0 is zeroed, 1 and 2 are garbage, all above 6 is garbage */ -#define HIDPP10_NUM_PROFILES 3 -struct hidpp10_device { - struct hidpp_device base; - unsigned index; - uint16_t wpid; -}; #endif /* HIDPP_10_H */ From 8a3222204e8bd3a1c7f7a1d41fcf30fa8ee46fe0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 12:53:04 +1000 Subject: [PATCH 08/10] hidpp: add hidpp_device_init() Make this into a somewhat more sensible API and default to printf as log handler. Signed-off-by: Peter Hutterer --- src/driver-hidpp10.c | 2 +- src/hidpp-generic.c | 14 ++++++++++++++ src/hidpp-generic.h | 4 ++++ src/hidpp10.h | 1 + 4 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index 92f0a92a..d049f4fa 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -226,9 +226,9 @@ hidpp10drv_probe(struct ratbag_device *device) } drv_data = zalloc(sizeof(*drv_data)); + hidpp_device_init(&base, device->hidraw.fd); base.log_handler = hidpp10_log; base.userdata = device; - base.hidraw_fd = device->hidraw.fd; /* We can treat all devices as wired devices here. If we talk to the * correct hidraw device the kernel adjusts the device index for us, diff --git a/src/hidpp-generic.c b/src/hidpp-generic.c index bdc615d3..bf8e26ce 100644 --- a/src/hidpp-generic.c +++ b/src/hidpp-generic.c @@ -290,3 +290,17 @@ hidpp_log_buffer(struct hidpp_device *dev, hidpp_log(dev, priority, "%s\n", output_buf); } + +static void +simple_log(void *userdata, enum hidpp_log_priority priority, const char *format, va_list args) +{ + vprintf(format, args); +} + +void +hidpp_device_init(struct hidpp_device *dev, int fd) +{ + dev->hidraw_fd = fd; + dev->log_handler = simple_log; + dev->userdata = NULL; +} diff --git a/src/hidpp-generic.h b/src/hidpp-generic.h index 8d0dc121..ba177b1a 100644 --- a/src/hidpp-generic.h +++ b/src/hidpp-generic.h @@ -89,6 +89,10 @@ struct hidpp_device { hidpp_log_handler log_handler; }; + +void +hidpp_device_init(struct hidpp_device *dev, int fd); + extern const char *hidpp_errors[0xFF]; const char * diff --git a/src/hidpp10.h b/src/hidpp10.h index 46b63113..a7f3379a 100644 --- a/src/hidpp10.h +++ b/src/hidpp10.h @@ -50,6 +50,7 @@ hidpp10_device_new_from_wpid(const struct hidpp_device *base, struct hidpp10_device* hidpp10_device_new_from_idx(const struct hidpp_device *base, int idx); + void hidpp10_device_destroy(struct hidpp10_device *dev); From 2186dd90c63f528bc12c8e35ea51fdfbd9319de7 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 13:00:43 +1000 Subject: [PATCH 09/10] hidpp: wrap the log handler init into a function Almost a decent API now. Signed-off-by: Peter Hutterer --- src/driver-hidpp10.c | 3 +-- src/hidpp-generic.c | 17 +++++++++++++++-- src/hidpp-generic.h | 7 ++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index d049f4fa..954cc631 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -227,8 +227,7 @@ hidpp10drv_probe(struct ratbag_device *device) drv_data = zalloc(sizeof(*drv_data)); hidpp_device_init(&base, device->hidraw.fd); - base.log_handler = hidpp10_log; - base.userdata = device; + hidpp_device_set_log_handler(&base, hidpp10_log, HIDPP_LOG_PRIORITY_RAW, device); /* We can treat all devices as wired devices here. If we talk to the * correct hidraw device the kernel adjusts the device index for us, diff --git a/src/hidpp-generic.c b/src/hidpp-generic.c index bf8e26ce..f7c5f985 100644 --- a/src/hidpp-generic.c +++ b/src/hidpp-generic.c @@ -258,6 +258,9 @@ hidpp_log(struct hidpp_device *dev, { va_list args; + if (dev->log_priority > priority) + return; + va_start(args, format); dev->log_handler(dev->userdata, priority, format, args); va_end(args); @@ -301,6 +304,16 @@ void hidpp_device_init(struct hidpp_device *dev, int fd) { dev->hidraw_fd = fd; - dev->log_handler = simple_log; - dev->userdata = NULL; + hidpp_device_set_log_handler(dev, simple_log, HIDPP_LOG_PRIORITY_INFO, NULL); +} + +void +hidpp_device_set_log_handler(struct hidpp_device *dev, + hidpp_log_handler log_handler, + enum hidpp_log_priority priority, + void *userdata) +{ + dev->log_handler = log_handler; + dev->log_priority = priority; + dev->userdata = userdata; } diff --git a/src/hidpp-generic.h b/src/hidpp-generic.h index ba177b1a..4a321b63 100644 --- a/src/hidpp-generic.h +++ b/src/hidpp-generic.h @@ -87,11 +87,16 @@ struct hidpp_device { int hidraw_fd; void *userdata; hidpp_log_handler log_handler; + enum hidpp_log_priority log_priority; }; - void hidpp_device_init(struct hidpp_device *dev, int fd); +void +hidpp_device_set_log_handler(struct hidpp_device *dev, + hidpp_log_handler log_handler, + enum hidpp_log_priority priority, + void *userdata); extern const char *hidpp_errors[0xFF]; From 0cb1fe44addbcc3e2f0c1231d14402b3472fe9c1 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 15 Oct 2015 11:41:04 +1000 Subject: [PATCH 10/10] tools: ad a hidpp10-dump-page tool to dump a HID++1.0 device's memory For protocol debugging, it's a lot easier to go have a simple helper than having to go through the whole of libratbag. And remove the page-dump helper from the main code. Note that this tool needs a /dev/hidraw device directly, not an event node. It is a debugging tool after all. Signed-off-by: Peter Hutterer --- .gitignore | 1 + src/Makefile.am | 9 +++ src/hidpp10.c | 27 --------- src/libratbag-util.h | 5 ++ tools/Makefile.am | 7 ++- tools/hidpp10-dump-page.c | 115 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 136 insertions(+), 28 deletions(-) create mode 100644 tools/hidpp10-dump-page.c diff --git a/.gitignore b/.gitignore index 1e4fc308..1ef09442 100644 --- a/.gitignore +++ b/.gitignore @@ -47,3 +47,4 @@ test-context test-device test-*.log ratbag-command +hidpp10-dump-page diff --git a/src/Makefile.am b/src/Makefile.am index 19ae3e61..a2ed81c6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,4 +1,5 @@ lib_LTLIBRARIES = libratbag.la +noinst_LTLIBRARIES = libhidpp10.la include_HEADERS = \ libratbag.h @@ -43,3 +44,11 @@ AM_CFLAGS = $(GCC_CFLAGS) DISTCLEANFILES = libratbag-version.h EXTRA_DIST = libratbag-version.h.in libratbag.sym + +# The hidpp libraries are static libraries only for in-tree tools. No API +# stability is guaranteed here. +libhidpp10_la_SOURCES = \ + hidpp-generic.h \ + hidpp-generic.c \ + hidpp10.h \ + hidpp10.c diff --git a/src/hidpp10.c b/src/hidpp10.c index 574c497f..0b93830e 100644 --- a/src/hidpp10.c +++ b/src/hidpp10.c @@ -193,9 +193,6 @@ out_err: /* HID++ 1.0 commands 10 */ /* -------------------------------------------------------------------------- */ -static inline void -hidpp10_dump_all_pages(struct hidpp10_device *dev); - /* -------------------------------------------------------------------------- */ /* 0x00: Enable HID++ Notifications */ /* -------------------------------------------------------------------------- */ @@ -1035,30 +1032,6 @@ hidpp10_read_memory(struct hidpp10_device *dev, uint8_t page, uint8_t offset, return 0; } -static inline void -hidpp10_dump_all_pages(struct hidpp10_device *dev) -{ - uint16_t page, offset; - uint8_t bytes[16]; - int res; - - hidpp_log_info(&dev->base, "::::::: Dumping memory :::::\n"); - - for (page = 0; page < 512; page++) { - hidpp_log_info(&dev->base, ":: page %d\n", page); - for (offset = 0; offset < 256; offset += 16) { - res = hidpp10_read_memory(dev, page, offset, bytes); - if (res != 0) - goto out; - - hidpp_log_buffer(&dev->base, HIDPP_LOG_PRIORITY_INFO, " ", bytes, ARRAY_LENGTH(bytes)); - } - } - -out: - hidpp_log_info(&dev->base, "::::::: Dumping memory complete :::::\n"); -} - /* -------------------------------------------------------------------------- */ /* 0xB2: Device Connection and Disconnection (Pairing) */ /* -------------------------------------------------------------------------- */ diff --git a/src/libratbag-util.h b/src/libratbag-util.h index 186dcccc..b2cb59bf 100644 --- a/src/libratbag-util.h +++ b/src/libratbag-util.h @@ -85,6 +85,10 @@ cleanup_free(void *p) { free(*(void**)p); } +static inline void +cleanup_close(int *p) { + close(*p); +} static inline void cleanup_udev_unref(struct udev **udev) { if (*udev) @@ -99,6 +103,7 @@ cleanup_udev_device_unref(struct udev_device **udev_device) { #define _cleanup_(x) __attribute__((cleanup(x))) #define _cleanup_free_ _cleanup_(cleanup_free) +#define _cleanup_close_ _cleanup_(cleanup_close) #define _cleanup_udev_unref_ _cleanup_(cleanup_udev_unref) #define _cleanup_udev_device_unref_ _cleanup_(cleanup_udev_device_unref) diff --git a/tools/Makefile.am b/tools/Makefile.am index 3ee4dddd..cfd901db 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -1,4 +1,4 @@ -noinst_PROGRAMS = +noinst_PROGRAMS = hidpp10-dump-page bin_PROGRAMS = ratbag-command noinst_LTLIBRARIES = libshared.la @@ -16,3 +16,8 @@ ratbag_command_SOURCES = ratbag-command.c ratbag_command_LDADD = ../src/libratbag.la libshared.la $(LIBUDEV_LIBS) $(LIBEVDEV_LIBS) ratbag_command_LDFLAGS = -no-install ratbag_command_CFLAGS = $(LIBUDEV_CFLAGS) $(LIBEVDEV_CFLAGS) + +hidpp10_dump_page_SOURCES = hidpp10-dump-page.c +hidpp10_dump_page_LDADD = ../src/libhidpp10.la +hidpp10_dump_page_LDFLAGS = -no-install +hidpp10_dump_page_CFLAGS = -I$(top_srcdir)/src diff --git a/tools/hidpp10-dump-page.c b/tools/hidpp10-dump-page.c new file mode 100644 index 00000000..9997fad3 --- /dev/null +++ b/tools/hidpp10-dump-page.c @@ -0,0 +1,115 @@ +/* + * Copyright © 2015 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +#include +#include +#include +#include + +#include +#include + +static inline int +dump_page(struct hidpp10_device *dev, size_t page, size_t offset) +{ + int rc = 0; + uint8_t bytes[16]; + + while (offset < 256) { + hidpp_log_info(&dev->base, "page 0x%02zx off 0x%02zx: ", page, offset); + rc = hidpp10_read_memory(dev, page, offset, bytes); + if (rc != 0) + break; + + hidpp_log_buffer(&dev->base, HIDPP_LOG_PRIORITY_INFO, " ", bytes, ARRAY_LENGTH(bytes)); + offset += 16; + } + + return rc; +} + +static inline int +dump_all_pages(struct hidpp10_device *dev) +{ + uint8_t page; + int rc = 0; + + for (page = 0; page < 31; page++) { + rc = dump_page(dev, page, 0); + if (rc != 0) + break; + } + + /* We dumped at least one page successfully and get EAGAIN, so we're + * on the last page. Overwrite the last line with a blank one so it + * doesn't look like an error */ + if (page > 0 && rc == EAGAIN) { + hidpp_log_info(&dev->base, "\r \n"); + rc = 0; + } + + return rc; +} + +void +usage(void) +{ + printf("Usage: %s [page] [offset] /dev/hidraw0\n", program_invocation_short_name); +} + +int +main(int argc, char **argv) +{ + _cleanup_close_ int fd; + const char *path; + size_t page = 0, offset = 0; + struct hidpp10_device *dev = NULL; + struct hidpp_device base; + int rc; + + if (argc < 2 || argc > 4) { + usage(); + return 1; + } + + path = argv[argc - 1]; + fd = open(path, O_RDWR); + if (fd < 0) + error(1, errno, "Failed to open path %s", path); + + hidpp_device_init(&base, fd); + dev = hidpp10_device_new_from_idx(&base, WIRED_DEVICE_IDX); + + if (argc == 2) + rc = dump_all_pages(dev); + else { + page = atoi(argv[1]); + if (argc > 3) + offset = atoi(argv[2]); + rc = dump_page(dev, page, offset); + } + + hidpp10_device_destroy(dev); + + return rc; +}