Keys at the same time #4904
Comments
|
IIRC, it's one key per scan/report. Normally. I think what you're lookin for is maybe this: #define QMK_KEYS_PER_SCAN 4Try adding that to your config.h, and see if it gets what you want. |
|
Not better with this settings. another question: |
|
The host doesn't poll the keyboard, QMK only sends reports when it needs to. It looks as though If the STM32 boards run ChibiOS then the polling rates will be in |
|
Hi, i've narrowed down this bug to this commit, which joins shared Endpoints together. First let's talk about expected results. Scanrate = 1000hz This is especially annoying in rhythm games like osu! mania, I used this program i wrote (note: it's a game that relies on having at least 1000hz frame rate): Before the bug, if i mashed 8 keys, it would report them all within 10ms, usually 4-6 different timestamps. Same timestamp would mean same usb report. After the bug, it would always report exactly 8 timestamps exactly 8ms apart. Note there are two bugs here:
|
|
For the endpoint change, you can change some of the config, and that may help: |
|
Setting Is there anything else i should be looking at? |
|
Try |
I do not see this. When I change the polling rates in *
|
Nice; just confirming that you are using FORCE_NKRO and mashing 8+ keys?
Now that i think about it, QMK sends a usb report after each key change, even if I'll do a quick retest and see what I get... |
Yes, though I do see the same behavior without NKRO (minus not seeing all 8 keys). |
|
This issue has been automatically marked as resolved because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. |
|
Dear Stale Bot, this message to let you know you're wrong. |
|
This change https://gist.github.com/xyzz/07c7473d0739cac054cb24792bcdbb04 is a quick hack I threw together to have a single report contain multiple keys. With both slamming 3 keys 3 times before the change: every key is sent as its own report at 1ms intervals. After the change: keys are combined in reports. |
|
Nice! |
|
After reading the code some more, here's the simplified version: diff --git a/tmk_core/common/action.c b/tmk_core/common/action.c
index 7fbdbd8c3..b5555757a 100644
--- a/tmk_core/common/action.c
+++ b/tmk_core/common/action.c
@@ -771,7 +771,7 @@ void register_code(uint8_t code) {
#endif
{
add_key(code);
- send_keyboard_report();
+ // send_keyboard_report();
}
}
else if
@@ -837,7 +837,7 @@ void unregister_code(uint8_t code) {
else if
IS_KEY(code) {
del_key(code);
- send_keyboard_report();
+ // send_keyboard_report();
}
else if
IS_MOD(code) {
diff --git a/tmk_core/common/keyboard.c b/tmk_core/common/keyboard.c
index 794a9152f..1f2d68c8f 100644
--- a/tmk_core/common/keyboard.c
+++ b/tmk_core/common/keyboard.c
@@ -30,6 +30,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include "sendchar.h"
#include "eeconfig.h"
#include "action_layer.h"
+#include "action_util.h"
#ifdef BACKLIGHT_ENABLE
# include "backlight.h"
#endif
@@ -331,6 +332,8 @@ void keyboard_task(void) {
MATRIX_LOOP_END:
+ send_keyboard_report();
+
#ifdef DEBUG_MATRIX_SCAN_RATE
matrix_scan_perf_task();
#endifThis however will break literally everything as so many features depend on |
|
While I can get both https://gist.github.com/xyzz/07c7473d0739cac054cb24792bcdbb04 and your simplified version to work (on my board at least), with the simplified version wireshark captures a URB_INTERRUPT every 1ms whereas the first version I only see an interrupt when I press/release a key. Not sure which is more correct or what could potentially break with this change, but I can confirm it does send multiple keys per report. |
|
Right, you can see in the modification: this sends a report every time the loop finishes scanning. This would also limit your scanrate to the USB_POLLING_INTERVAL_MS which is at most 1000hz, since send_keyboard_report() is blocking afaik.
I wish i checked this gist first, because it's exactly what i ended up implementing (send was moved to after MATRIX_LOOP_END, and i check the needs_send inside the send function). Unfortunately as xyzz said, it breaks everything that requires modifiers etc on my board which uses very minimal feature set, but passes the multiple-keys in same report test |
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean
I'm not sure what you're asking? This is a bug not a PR. There isn't a solution yet afaik (or at least no one has looked into the vast amounts of underlying mechanisms that need changing) |
|
You said
So you tested some fix (https://gist.github.com/xyzz/07c7473d0739cac054cb24792bcdbb04) and it breaks some other QMK features if I understood that right, I just want to understand what kind of features stop working and why. You said "everything that requires modifiers" but like what exactly and how it breaks. I just want to understand how other features rely on |
@ecerulm So best bet is to track #12686 since it looks like people are working on this very long lasting bug (i know it didnt exist in TMK, since i used TMK years and years ago) |
|
This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. |
github-actions
bot
added
the
stale
|
Closing due to inactivity. |
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean Gate deferred keyboard reports feature behind an optional define Create a define to enable the experimental "deferred keyboard reports" feature (the feature is off by default). - I manually tested a Fox Leaf 60 firmware with the define on and firmware with the define off. - All unit tests pass when feature define is not specified. - 9 unit tests fail when feature define is specified, keeping it in experimental status. Refactor "defer send keyboard report" feature to support macros This refactor makes the "defer send keyboard report" feature compatible with macros. Rather than modify the functionality of all "send_keyboard_report(..)" calls - the method now sends the keyboard report immediately like before. A new function called "send_keyboard_report_deferred(..)" handles the deferred support if the define is set, and I've replaced former calls to "send_keyboard_report(..)" with the new "send_keyboard_report_deferred(..)" function along the code path for handling keys through the matrix scan (this will not affect other calls, such as how macros are handled). When the feature define is included, it is now passing all but 5 unit tests. Adapt for tapped and subsequent shifted key events Tapped key events by default work only on **key release** by sending a key press event and a subsequent key release event to the host. Now with deferring the send event after the complete matrix scan these events are merged and erase each other becoming effectively a nop. The idea is to buffer the unregister event of the key release for all tapped keys (normal keys are not buffered) and send these in a separate keyboard report just after the key press events have been send. Unregister events/key codes are stored in the unregister_keycodes struct by unregister_code_buffered() which is called for all tapped key codes and send after a complete scan is completed by send_keyboard_report_buffered_unregister_keys(). Subsequent shifted key codes like pressing KC_EQL and KC_PLUS (KC_LSHIFT + KC_EQL) while holding the former need an additional key release event of KC_EQL before sending the combination again. This release event isn't defeered anymore but send as a report immediatly. All failing unit-tests have been fixed or rather adapted for this feature. As it turns out most of the failing key press cases expect one report per key event so they are false negatives. DEFER_KEYBOARD_REPORT_ENABLE was renamed to REGISTER_MULTIPLE_KEYEVENTS_ENABLE because it rather states a implementation detail but doesn't communicate purpose very well in my opinion.
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean Gate deferred keyboard reports feature behind an optional define Create a define to enable the experimental "deferred keyboard reports" feature (the feature is off by default). - I manually tested a Fox Leaf 60 firmware with the define on and firmware with the define off. - All unit tests pass when feature define is not specified. - 9 unit tests fail when feature define is specified, keeping it in experimental status. Refactor "defer send keyboard report" feature to support macros This refactor makes the "defer send keyboard report" feature compatible with macros. Rather than modify the functionality of all "send_keyboard_report(..)" calls - the method now sends the keyboard report immediately like before. A new function called "send_keyboard_report_deferred(..)" handles the deferred support if the define is set, and I've replaced former calls to "send_keyboard_report(..)" with the new "send_keyboard_report_deferred(..)" function along the code path for handling keys through the matrix scan (this will not affect other calls, such as how macros are handled). When the feature define is included, it is now passing all but 5 unit tests. Adapt for tapped and subsequent shifted key events Tapped key events by default work only on **key release** by sending a key press event and a subsequent key release event to the host. Now with deferring the send event after the complete matrix scan these events are merged and erase each other becoming effectively a nop. The idea is to buffer the unregister event of the key release for all tapped keys (normal keys are not buffered) and send these in a separate keyboard report just after the key press events have been send. Unregister events/key codes are stored in the unregister_keycodes struct by unregister_code_buffered() which is called for all tapped key codes and send after a complete scan is completed by send_keyboard_report_buffered_unregister_keys(). Subsequent shifted key codes like pressing KC_EQL and KC_PLUS (KC_LSHIFT + KC_EQL) while holding the former need an additional key release event of KC_EQL before sending the combination again. This release event isn't defeered anymore but send as a report immediatly. All failing unit-tests have been fixed or rather adapted for this feature. As it turns out most of the failing key press cases expect one report per key event so they are false negatives. DEFER_KEYBOARD_REPORT_ENABLE was renamed to REGISTER_MULTIPLE_KEYEVENTS_ENABLE because it rather states a implementation detail but doesn't communicate purpose very well in my opinion.
This change defers calling "host_keyboard_send(keyboard_report)" until after the matrix loop. This enables the ability to send multiple key changes in a single report, which also means that multiple key changes can be detected in a single polling cycle. This essentially allows us report more information in the same amount of time at no additional cost. Before this change, only one key change (a press, or release) could be sent at a time. This is because "host_keyboard_send(..)" was created as a blocking method so that data cannot be changed mid-transmission to host, and this method was formerly called immediately after any change was made to the keyboard report. Calling this method at this point meant that the remainder of the polling interval would be used to send the report to host. The impact of this can be very significant in gaming scenarios, when multiple keypresses need to be triggered simultaneously. See issue qmk#4904 for more details. I tested the change manually on a Fox Leaf 60 and a Romac, and compared directly against the same firmware without the change. I've confirmed in manual testing that this allows multiple simultaneous keypresses, while not seeming to introduce any new issues. Exact changes include: - In "send_keyboard_report(..)" method, flag the last report's state as dirty instead of immediately sending report to host - Create a new "send_keyboard_report_immediate(..)" method which is called after the matrix loop, which checks if the last report sent to host is dirty and sends an updated report if so, then sets it to clean
Is there a solution for having simultaneous hitted keys be reported in the same sub report.
for now the best i get is:
The text was updated successfully, but these errors were encountered: