refactor(encoder): use behavior queue to archive key taps for encoder #1658
Conversation
Remove k_msleep() call used in sensor trigger callback that's meant to simulate a key tap (press and release), instead we can use the existing behavior queue mechanism, where a press and a release events are queued up separately. The trick is to add .binging_pressed and .binding_released callbacks for the sensor driver api, which will gets called when the queued events are processed accordingly.
| k_msleep(5); | ||
|
|
||
| return ZMK_EVENT_RAISE(zmk_keycode_state_changed_from_encoded(keycode, false, timestamp)); | ||
| zmk_behavior_queue_add(keycode_position, *binding, true, 20); |
There was a problem hiding this comment.
This will delay both the press and release by 20 ms instead of immediately pressing and then releasing after 20 ms. Was that intended?
|
|
||
| return ZMK_EVENT_RAISE(zmk_keycode_state_changed_from_encoded(keycode, false, timestamp)); | ||
| zmk_behavior_queue_add(keycode_position, *binding, true, 20); | ||
| return zmk_behavior_queue_add(keycode_position, *binding, false, 20); |
There was a problem hiding this comment.
Can we make this hold duration configurable?
| @@ -36,26 +73,22 @@ static int on_sensor_binding_triggered(struct zmk_behavior_binding *binding, | |||
|
|
|||
| switch (value.val1) { | |||
| case 1: | |||
| keycode = binding->param1; | |||
| keycode_position = 1; | |||
There was a problem hiding this comment.
This would cause every encoder to overlap with the second and third keys in the keymap.
I had a change of my own which was going to be part of a larger PR, but since it would fix this problem for you, I've split it out and created #1659. This gives each sensor a virtual key position which it can use for queueing behaviors.
| struct zmk_behavior_binding_event event) { | ||
| uint32_t keycode; | ||
|
|
||
| switch (event.position) { |
There was a problem hiding this comment.
As noted below, using the position like this would make it appear to be the same key position as the second and third keys in the keymap.
Maybe a better way to handle this would be to rebase onto #1275, and then instead of giving the sensor behavior itself press/release functions and needing some way to identify whether a press/release corresponds to clockwise or counterclockwise, you would just queue up a press/release for the wrapped behavior?
There was a problem hiding this comment.
|
@joelspadin Thank you so much for reviewing and your feedback! |
Remove k_msleep() call used in sensor trigger callback that's meant to simulate a key tap (press and release), instead we can use the existing behavior queue mechanism, where a press and a release events are queued up separately. The trick is to add .binging_pressed and .binding_released callbacks for the sensor driver api, which will get called when the queued events are processed accordingly.
Board/Shield Check-list
.zmk.ymlmetadata file added&pro_microused in favor of&pro_micro_d/aif applicable.conffile has optional extra features commented out