From 7c272a909d90cbf3abd15a794d843571d807a0fa Mon Sep 17 00:00:00 2001 From: Cheerpipe Date: Tue, 16 Feb 2021 12:12:36 -0300 Subject: [PATCH] Small performance and stability tweaks Performance: Don't set mode on each zone change. Stability: Fix read buffer size to avoid command corruption - Increase command delay if there is any volume change command conflict. Commits merged and amended for code style by Adam Honse --- .../LogitechG560Controller.cpp | 32 +++++++++---------- .../LogitechG560Controller.h | 29 +++++++---------- .../RGBController_LogitechG560.cpp | 29 ++++++++++------- 3 files changed, 46 insertions(+), 44 deletions(-) diff --git a/Controllers/LogitechController/LogitechG560Controller.cpp b/Controllers/LogitechController/LogitechG560Controller.cpp index 2eb1e6c7..977d9883 100644 --- a/Controllers/LogitechController/LogitechG560Controller.cpp +++ b/Controllers/LogitechController/LogitechG560Controller.cpp @@ -34,7 +34,7 @@ std::string LogitechG560Controller::GetDeviceLocation() void LogitechG560Controller::SetDirectMode(uint8_t zone) { - unsigned char usb_buf[20]; + unsigned char usb_buf[LOGI_G560_LED_PACKET_SIZE]; usb_buf[0x00] = 0x11; usb_buf[0x01] = 0xFF; @@ -45,12 +45,12 @@ void LogitechG560Controller::SetDirectMode(uint8_t zone) /*-----------------------------------------------------*\ | Send packet | \*-----------------------------------------------------*/ - fail_retry_write(dev, usb_buf, 20); + fail_retry_write(dev, usb_buf, LOGI_G560_LED_PACKET_SIZE); } void LogitechG560Controller::SetOffMode(uint8_t zone) { - unsigned char usb_buf[20]; + unsigned char usb_buf[LOGI_G560_LED_PACKET_SIZE]; usb_buf[0x00] = 0x11; usb_buf[0x01] = 0xFF; @@ -61,20 +61,19 @@ void LogitechG560Controller::SetOffMode(uint8_t zone) /*-----------------------------------------------------*\ | Send packet | \*-----------------------------------------------------*/ - fail_retry_write(dev, usb_buf, 20); + fail_retry_write(dev, usb_buf, LOGI_G560_LED_PACKET_SIZE); } void LogitechG560Controller::SendSpeakerMode ( - unsigned char zone, //0x04 - unsigned char mode, //0x05 - unsigned char speed, //0x0B // not working so far - unsigned char red, //0x06 - unsigned char green, //0x07 - unsigned char blue //0x08 + unsigned char zone, + unsigned char mode, + unsigned char red, + unsigned char green, + unsigned char blue ) { - unsigned char usb_buf[20]; + unsigned char usb_buf[LOGI_G560_LED_PACKET_SIZE]; /*-----------------------------------------------------*\ | Zero out buffer | @@ -116,13 +115,13 @@ void LogitechG560Controller::SendSpeakerMode /*-----------------------------------------------------*\ | Send packet | \*-----------------------------------------------------*/ - fail_retry_write(dev, usb_buf, 20); + fail_retry_write(dev, usb_buf, LOGI_G560_LED_PACKET_SIZE); } void LogitechG560Controller::fail_retry_write(hid_device *device, const unsigned char *data, size_t length) { - unsigned char usb_buf_out[33]; - uint8_t write_max_retry=3; + unsigned char usb_buf_out[LOGI_G560_LED_PACKET_SIZE]; + unsigned int write_max_retry = LOGI_G560_LED_COMMAND_SEND_RETRIES; do { std::this_thread::sleep_for(1ms); @@ -132,15 +131,16 @@ void LogitechG560Controller::fail_retry_write(hid_device *device, const unsigned | HID write fails if a change led color and set volume command are sent at | | the same time because RGB controller and volume control shares the same interface. | \*-------------------------------------------------------------------------------------*/ - if (ret > 0) + if(ret == 20) { std::this_thread::sleep_for(1ms); - hid_read_timeout(dev, usb_buf_out, 33, 100); + hid_read_timeout(dev, usb_buf_out, LOGI_G560_LED_PACKET_SIZE, 200); break; } else { write_max_retry--; + std::this_thread::sleep_for(10ms); } }while (write_max_retry > 0); diff --git a/Controllers/LogitechController/LogitechG560Controller.h b/Controllers/LogitechController/LogitechG560Controller.h index 2b432d1f..3259d929 100644 --- a/Controllers/LogitechController/LogitechG560Controller.h +++ b/Controllers/LogitechController/LogitechG560Controller.h @@ -16,6 +16,9 @@ #pragma once +#define LOGI_G560_LED_PACKET_SIZE 20 +#define LOGI_G560_LED_COMMAND_SEND_RETRIES 3 + enum { LOGITECH_G560_MODE_OFF = 0x00, @@ -24,38 +27,30 @@ enum LOGITECH_G560_MODE_BREATHING = 0x03, }; -/*---------------------------------------------------------------------------------------------*\ -| Speed is 1000 for fast and 20000 for slow. | -| Values are mutipled by 100 later to give lots of GUI steps. | -\*---------------------------------------------------------------------------------------------*/ -enum -{ - LOGITECH_G560_SPEED_SLOWEST = 0xC8, /* Slowest speed */ - LOGITECH_G560_SPEED_NORMAL = 0x32, /* Normal speed */ - LOGITECH_G560_SPEED_FASTEST = 0x0A, /* Fastest speed */ -}; - class LogitechG560Controller { public: LogitechG560Controller(hid_device* dev_handle, const char* path); ~LogitechG560Controller(); + std::string GetDeviceLocation(); + void SetDirectMode(uint8_t zone); void SetOffMode(uint8_t zone); + void SendSpeakerMode ( - unsigned char zone, //0x04 - unsigned char mode, //0x05 - unsigned char speed, //0x0B - unsigned char red, //0x06 - unsigned char green, //0x07 - unsigned char blue //0x08 + unsigned char zone, + unsigned char mode, + unsigned char red, + unsigned char green, + unsigned char blue ); private: hid_device* dev; std::string location; + void fail_retry_write(hid_device *device, const unsigned char *data, size_t length); }; diff --git a/Controllers/LogitechController/RGBController_LogitechG560.cpp b/Controllers/LogitechController/RGBController_LogitechG560.cpp index 1b320218..7bea882b 100644 --- a/Controllers/LogitechController/RGBController_LogitechG560.cpp +++ b/Controllers/LogitechController/RGBController_LogitechG560.cpp @@ -13,7 +13,7 @@ RGBController_LogitechG560::RGBController_LogitechG560(LogitechG560Controller* logitech_ptr) { - logitech = logitech_ptr; + logitech = logitech_ptr; name = "Logitech G560 Lightsync Speaker"; vendor = "Logitech"; @@ -114,16 +114,7 @@ void RGBController_LogitechG560::DeviceUpdateLEDs() unsigned char red = RGBGetRValue(colors[led_idx]); unsigned char grn = RGBGetGValue(colors[led_idx]); unsigned char blu = RGBGetBValue(colors[led_idx]); - - if (modes[active_mode].value==LOGITECH_G560_MODE_DIRECT) - { - logitech->SetDirectMode(leds[led_idx].value); //Required to "reset" RGB controller and start receiving color in direct mode - } - else if (modes[active_mode].value==LOGITECH_G560_MODE_OFF) - { - logitech->SetOffMode(leds[led_idx].value); - } - logitech->SendSpeakerMode((unsigned char)leds[led_idx].value, modes[active_mode].value, modes[active_mode].speed,red, grn, blu); + logitech->SendSpeakerMode((unsigned char)leds[led_idx].value, modes[active_mode].value, red, grn, blu); } } @@ -144,5 +135,21 @@ void RGBController_LogitechG560::SetCustomMode() void RGBController_LogitechG560::DeviceUpdateMode() { + for(std::size_t led_idx = 0; led_idx < leds.size(); led_idx++) + { + if(modes[active_mode].value == LOGITECH_G560_MODE_OFF) + { + logitech->SetOffMode(leds[led_idx].value); + } + else + { + /*---------------------------------------------------------*\ + | Required to "reset" RGB controller and start receiving | + | color in direct mode | + \*---------------------------------------------------------*/ + logitech->SetDirectMode(leds[led_idx].value); + } + + } DeviceUpdateLEDs(); }