-
-
Notifications
You must be signed in to change notification settings - Fork 39.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added SPLIT_HAPTIC_ENABLE to trigger haptic feedback on the slave keyboard half #16233
Conversation
…board half. For DRV2505L boards, this will send the feedback mode from master to slave. For solenoids, it will simply trigger with the current settings - it is expected that the slave half sets its own solenoid settings.
uint8_t split_haptic_play = 0xFF; | ||
|
||
static bool haptic_handlers_master(matrix_row_t master_matrix[], matrix_row_t slave_matrix[]) { | ||
bool okay = transport_write(PUT_HAPTIC, &split_haptic_play, sizeof(split_haptic_play)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anything, the entire haptic config structure should be synced over, not just the mode. Especially as the "enable" states isn't being synced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any concerns about the size of the haptic structure when sync'd? I guess it's only 8 bytes, but split transport seems sensitive to size of data being transported, at least from reading the docs.
As the patch is structured, it's not a concern that the enable state isn't sync'd, since haptic events are triggered by haptic_play(), which would only be invoked if enable status is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be fine. Mostly, it gets synced if there are changes, but that shouldn't be too often. And then forcibly synced on the forced timeout (defaults to 500ms).
So for the sync, it shouldn't be too much.
Thank you for your contribution! |
Deferred to Q3 cycle due to conflicts and unaddressed concerns. |
Thank you for your contribution! |
Thank you for your contribution! |
Hi, was there any update on this? If not, is the only fix necessary to sync the whole haptic structure rather than the mode? I'm working on a split keyboard with haptics and would love to see this get finished/merged |
For DRV2505L boards, this will send the feedback mode from master to slave. For solenoids, it will simply trigger
with the current settings - it is expected that the slave half sets its own solenoid settings.
Description
When a haptic event is triggered haptic_play() is invoked. When SPLIT_HAPTIC_ENABLE is set, a new global variable split_haptic_enable is set to the current play mode (for DRV2605L boards) or 1 (for solenoid). This then gets sent across to the slave half, where it triggers haptic_play(). split_haptic_enable is then reset to 0xFF, which is an unused waveform in the DRV2605L library, until the next haptic event.
I have tested this on my keyboard with DRV2605L boards, and it works as expected. I do not have a solenoid to test with.
Types of Changes
Issues Fixed or Closed by This PR
Checklist