From 6aa4a3777e929a596b4aca24d4e8a7ca599dd8ca Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 09:30:12 +1000 Subject: [PATCH 01/10] more efficient stick test previous logic does not check if previously active before resending press event every poll also uses unnecessary local variable and multiple unnecessary assignments every poll seems much snappier now (or just me?) --- g13_stick.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/g13_stick.cpp b/g13_stick.cpp index f3a421b..c08906f 100644 --- a/g13_stick.cpp +++ b/g13_stick.cpp @@ -94,16 +94,9 @@ void G13_StickZone::dump(std::ostream &out) const { void G13_StickZone::test(const G13_ZoneCoord &loc) { if (!_action) return; - bool prior_active = _active; - _active = _bounds.contains(loc); - if (!_active) { - if (prior_active) { - // cout << "exit stick zone " << m_name << std::endl; - _action->act(false); - } - } else { - // cout << "in stick zone " << m_name << std::endl; - _action->act(true); + if (_active != _bounds.contains(loc)) { + _active = !_active; + _action->act(_active); } } From 846d22da469b1492209fca4ac7c7b2892c055af7 Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 13:23:33 +1000 Subject: [PATCH 02/10] stick efficiency tweaks Expensive coordinate transform was being performed on every poll, instead use pre-calculated zones, however calibration is no longer effective. This actually increased CPU from 3-4% to 7% due to being able to read more samples so a delay was added to the main loop to limit stick polling frequency, dropping CPU utilization to 0.7% during rapid stick movement. --- g13_device.cpp | 11 ++++++++--- g13_manager.cpp | 5 ++++- g13_stick.cpp | 17 ++++++++++------- g13_stick.hpp | 4 ++-- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/g13_device.cpp b/g13_device.cpp index 6a63c8c..adbc9f3 100644 --- a/g13_device.cpp +++ b/g13_device.cpp @@ -193,7 +193,7 @@ void G13_Device::ReadConfigFile(const std::string &filename) { std::ifstream s(filename); G13_OUT("reading configuration from " << filename); - if (s.fail()) G13_LOG(log4cpp::Priority::ERROR << strerror(errno)); + if (s.fail()) G13_LOG(log4cpp::Priority::ERROR << "Error: " << strerror(errno)); else while (s.good()) { // grab a line char buf[1024]; @@ -435,8 +435,13 @@ void G13_Device::InitCommands() { if (sscanf(remainder, "%lf %lf %lf %lf", &x1, &y1, &x2, &y2) != 4) { throw G13_CommandException("bad bounds format"); } - zone->set_bounds(G13_ZoneBounds(x1, y1, x2, y2)); - + // TODO: Add center offset and bounds (or update zone) + zone->set_bounds(G13_ZoneBounds( + (unsigned char)(x1 * UCHAR_MAX), + (unsigned char)(y1 * UCHAR_MAX), + (unsigned char)(x2 * UCHAR_MAX), + (unsigned char)(y2 * UCHAR_MAX) + )); } else if (operation == "del") { m_stick.RemoveZone(*zone); } else { diff --git a/g13_manager.cpp b/g13_manager.cpp index b7a720a..7b1bc34 100644 --- a/g13_manager.cpp +++ b/g13_manager.cpp @@ -10,6 +10,8 @@ #include #include #include +#include +#include namespace G13 { @@ -256,6 +258,7 @@ int G13_Manager::Run() { running = false; } } + std::this_thread::sleep_for(std::chrono::milliseconds(20)); } while (running); Cleanup(); @@ -283,4 +286,4 @@ void G13_Manager::setLogoFilename(const std::string &newLogoFilename) { logoFilename = newLogoFilename; } -} // namespace G13 \ No newline at end of file +} // namespace G13 diff --git a/g13_stick.cpp b/g13_stick.cpp index c08906f..e80b8d1 100644 --- a/g13_stick.cpp +++ b/g13_stick.cpp @@ -113,6 +113,11 @@ void G13_Stick::ParseJoystick(const unsigned char *buf) { // update targets if we're in calibration mode switch (m_stick_mode) { + case STICK_ABSOLUTE: + break; + case STICK_KEYS: + break; + case STICK_CALCENTER: m_center_pos = m_current_pos; return; @@ -124,13 +129,10 @@ void G13_Stick::ParseJoystick(const unsigned char *buf) { case STICK_CALBOUNDS: m_bounds.expand(m_current_pos); return; - - case STICK_ABSOLUTE: - break; - case STICK_KEYS: - break; } - +/* TODO: Transform zone so we're not constantly transforming position + * Is this even necessary? Zones could just be tweaked manually and + * north is currently unused. Four-point zones would be more powerful // determine our normalized position double dx; // = 0.5 if (m_current_pos.x <= m_center_pos.x) { @@ -153,13 +155,14 @@ void G13_Stick::ParseJoystick(const unsigned char *buf) { G13_DBG("x=" << m_current_pos.x << " y=" << m_current_pos.y << " dx=" << dx << " dy=" << dy); - G13_ZoneCoord jpos(dx, dy); + G13_ZoneCoord jpos(dx, dy);*/ if (m_stick_mode == STICK_ABSOLUTE) { _keypad.SendEvent(EV_ABS, ABS_X, m_current_pos.x); _keypad.SendEvent(EV_ABS, ABS_Y, m_current_pos.y); } else if (m_stick_mode == STICK_KEYS) { // BOOST_FOREACH (G13_StickZone& zone, m_zones) { zone.test(jpos); } + G13_ZoneCoord jpos(m_current_pos.x, m_current_pos.y); for (auto &zone : m_zones) { zone.test(jpos); } diff --git a/g13_stick.hpp b/g13_stick.hpp index 51c9729..0a778b2 100644 --- a/g13_stick.hpp +++ b/g13_stick.hpp @@ -13,8 +13,8 @@ class G13_Device; typedef Helper::Coord G13_StickCoord; typedef Helper::Bounds G13_StickBounds; -typedef Helper::Coord G13_ZoneCoord; -typedef Helper::Bounds G13_ZoneBounds; +typedef Helper::Coord G13_ZoneCoord; +typedef Helper::Bounds G13_ZoneBounds; // ************************************************************************* From bb9be8203cd020ce4f83c30bfe8248a8033ca910 Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 13:31:25 +1000 Subject: [PATCH 03/10] fix error text regression --- g13_stick.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/g13_stick.cpp b/g13_stick.cpp index e80b8d1..4cb702c 100644 --- a/g13_stick.cpp +++ b/g13_stick.cpp @@ -162,9 +162,8 @@ void G13_Stick::ParseJoystick(const unsigned char *buf) { } else if (m_stick_mode == STICK_KEYS) { // BOOST_FOREACH (G13_StickZone& zone, m_zones) { zone.test(jpos); } - G13_ZoneCoord jpos(m_current_pos.x, m_current_pos.y); for (auto &zone : m_zones) { - zone.test(jpos); + zone.test(G13_ZoneCoord(m_current_pos.x, m_current_pos.y)); } return; From 62f47d71c4ba649af836b303acc688069a290768 Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 14:24:51 +1000 Subject: [PATCH 04/10] bump up stick polling to 100Hz 50Hz was slightly noticeable --- g13_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/g13_manager.cpp b/g13_manager.cpp index 7b1bc34..db65678 100644 --- a/g13_manager.cpp +++ b/g13_manager.cpp @@ -258,7 +258,7 @@ int G13_Manager::Run() { running = false; } } - std::this_thread::sleep_for(std::chrono::milliseconds(20)); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); } while (running); Cleanup(); From 967e9efecdbc76742942c94b4d2f52e16f74ea12 Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 17:56:03 +1000 Subject: [PATCH 05/10] sync between keypress and release events Doesn't seem necessary for keys, but mouse buttons don't work unless we sync between press and release events. Applies to keydown/keyup bindings. --- g13_action.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/g13_action.cpp b/g13_action.cpp index 085140c..8e745d5 100644 --- a/g13_action.cpp +++ b/g13_action.cpp @@ -49,6 +49,7 @@ void G13_Action_Keys::act(G13_Device &g13, bool is_down) { G13_LOG(log4cpp::Priority::DEBUG << "sending KEY DOWN " << _key); } if (!_keysup.empty()) for (int i = _keys.size() - 1; i >= 0; i--) { + g13.SendEvent(EV_SYN, SYN_REPORT, 0); g13.SendEvent(EV_KEY, _keys[i], !is_down); G13_LOG(log4cpp::Priority::DEBUG << "sending KEY UP " << _keys[i]); } @@ -61,6 +62,7 @@ void G13_Action_Keys::act(G13_Device &g13, bool is_down) { g13.SendEvent(EV_KEY, _key, !is_down); G13_LOG(log4cpp::Priority::DEBUG << "sending KEY DOWN " << _key); } + g13.SendEvent(EV_SYN, SYN_REPORT, 0); for (int i = _keysup.size() - 1; i >= 0; i--) { g13.SendEvent(EV_KEY, _keysup[i], is_down); G13_LOG(log4cpp::Priority::DEBUG << "sending KEY UP " << _keysup[i]); From eb0f219f2b8a155ec9c9d7d39f2f30980b65b1fa Mon Sep 17 00:00:00 2001 From: tmackay Date: Fri, 18 Sep 2020 19:19:17 +1000 Subject: [PATCH 06/10] error text regression failed to pull before making local changes --- g13_device.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/g13_device.cpp b/g13_device.cpp index adbc9f3..8917733 100644 --- a/g13_device.cpp +++ b/g13_device.cpp @@ -193,7 +193,7 @@ void G13_Device::ReadConfigFile(const std::string &filename) { std::ifstream s(filename); G13_OUT("reading configuration from " << filename); - if (s.fail()) G13_LOG(log4cpp::Priority::ERROR << "Error: " << strerror(errno)); + if (s.fail()) G13_LOG(log4cpp::Priority::ERROR << strerror(errno)); else while (s.good()) { // grab a line char buf[1024]; From 4ffb8adf6f0f1a01fe16774bc47b640c34e75022 Mon Sep 17 00:00:00 2001 From: tmackay Date: Sat, 19 Sep 2020 10:45:47 +1000 Subject: [PATCH 07/10] relocate sleep so it doesn't execute on timeout Blocking read of USB events times out after 100ms - only sleep after an event was received to prevent "windows" of unnecessary delay after each timeout. --- g13_device.cpp | 4 ++++ g13_manager.cpp | 3 --- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/g13_device.cpp b/g13_device.cpp index 8917733..83c0942 100644 --- a/g13_device.cpp +++ b/g13_device.cpp @@ -12,6 +12,8 @@ #include "logo.hpp" #include #include +#include +#include namespace G13 { // ************************************************************************* @@ -186,6 +188,8 @@ int G13_Device::ReadKeypresses() { m_currentProfile->ParseKeys(buffer); SendEvent(EV_SYN, SYN_REPORT, 0); } + if (!error) + std::this_thread::sleep_for(std::chrono::milliseconds(10)); return 0; } diff --git a/g13_manager.cpp b/g13_manager.cpp index e045497..af1bed9 100644 --- a/g13_manager.cpp +++ b/g13_manager.cpp @@ -10,8 +10,6 @@ #include #include #include -#include -#include namespace G13 { @@ -258,7 +256,6 @@ int G13_Manager::Run() { running = false; } } - std::this_thread::sleep_for(std::chrono::milliseconds(10)); } while (running); Cleanup(); From 25bbd1fa89050c4d932764d4253121d0f4818891 Mon Sep 17 00:00:00 2001 From: tmackay Date: Sat, 19 Sep 2020 11:08:17 +1000 Subject: [PATCH 08/10] add timeout constants Added constants for G13_KEY_READ_TIMEOUT (100ms) and G13_KEY_READ_MIN_TIME (10ms) for USB blocking read timeout and minimum time between event parsing respectively. --- g13.hpp | 3 ++- g13_device.cpp | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/g13.hpp b/g13.hpp index c616959..96e73a7 100644 --- a/g13.hpp +++ b/g13.hpp @@ -19,7 +19,8 @@ namespace G13 { // const size_t G13_INTERFACE = 0; static const size_t G13_KEY_ENDPOINT = 1; static const size_t G13_LCD_ENDPOINT = 2; - // const size_t G13_KEY_READ_TIMEOUT = 0; + static const size_t G13_KEY_READ_TIMEOUT = 100; + static const size_t G13_KEY_READ_MIN_TIME = 10; // ************************************************************************* diff --git a/g13_device.cpp b/g13_device.cpp index 83c0942..2937188 100644 --- a/g13_device.cpp +++ b/g13_device.cpp @@ -174,7 +174,7 @@ int G13_Device::ReadKeypresses() { int size = 0; int error = libusb_interrupt_transfer(handle, LIBUSB_ENDPOINT_IN | G13_KEY_ENDPOINT, - buffer, G13_REPORT_SIZE, &size, 100); + buffer, G13_REPORT_SIZE, &size, G13_KEY_READ_TIMEOUT); if (error && error != LIBUSB_ERROR_TIMEOUT) { G13_ERR("Error while reading keys: " << DescribeLibusbErrorCode(error)); @@ -189,7 +189,7 @@ int G13_Device::ReadKeypresses() { SendEvent(EV_SYN, SYN_REPORT, 0); } if (!error) - std::this_thread::sleep_for(std::chrono::milliseconds(10)); + std::this_thread::sleep_for(std::chrono::milliseconds(G13_KEY_READ_MIN_TIME)); return 0; } From 1c8d34ca1d56e8680e1be7d8554c09a8a8abc429 Mon Sep 17 00:00:00 2001 From: tmackay Date: Mon, 21 Sep 2020 08:16:50 +1000 Subject: [PATCH 09/10] tidy test same logic, less code --- g13_stick.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/g13_stick.cpp b/g13_stick.cpp index 4cb702c..3d5572a 100644 --- a/g13_stick.cpp +++ b/g13_stick.cpp @@ -92,12 +92,8 @@ void G13_StickZone::dump(std::ostream &out) const { } void G13_StickZone::test(const G13_ZoneCoord &loc) { - if (!_action) - return; - if (_active != _bounds.contains(loc)) { - _active = !_active; - _action->act(_active); - } + if (_action && _active != _bounds.contains(loc)) + _action->act(_active = !_active); } G13_StickZone::G13_StickZone(G13_Stick &stick, const std::string &name, From 466e2fad614adb924469b0fbb17a3e49506d4417 Mon Sep 17 00:00:00 2001 From: tmackay Date: Mon, 21 Sep 2020 08:35:07 +1000 Subject: [PATCH 10/10] no extra delay by default Set G13_KEY_READ_MIN_TIME to non-zero value, eg. 10 (milliseconds) to reduce CPU utilization by skipping stick samples. --- g13.hpp | 2 +- g13_device.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/g13.hpp b/g13.hpp index 96e73a7..69a58a0 100644 --- a/g13.hpp +++ b/g13.hpp @@ -20,7 +20,7 @@ namespace G13 { static const size_t G13_KEY_ENDPOINT = 1; static const size_t G13_LCD_ENDPOINT = 2; static const size_t G13_KEY_READ_TIMEOUT = 100; - static const size_t G13_KEY_READ_MIN_TIME = 10; + static const size_t G13_KEY_READ_MIN_TIME = 0; // ************************************************************************* diff --git a/g13_device.cpp b/g13_device.cpp index 2937188..32eca56 100644 --- a/g13_device.cpp +++ b/g13_device.cpp @@ -188,7 +188,7 @@ int G13_Device::ReadKeypresses() { m_currentProfile->ParseKeys(buffer); SendEvent(EV_SYN, SYN_REPORT, 0); } - if (!error) + if (G13_KEY_READ_MIN_TIME && !error) std::this_thread::sleep_for(std::chrono::milliseconds(G13_KEY_READ_MIN_TIME)); return 0; }