-
-
Notifications
You must be signed in to change notification settings - Fork 271
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
Add Action type: "Axis" support for mapping buttons press to axis changes #243
base: main
Are you sure you want to change the base?
Conversation
@PixlOne Could you please review the repeat task startup part - sometimes it starts only for one axis direction, but not for another one. |
…nges This is something like Gesture mode: "Axis", but works on mouse buttons. Configure it like the following (this example will map mouse wheel tilts to vertical scrolling): ``` buttons: ( { # Tilt wheel left cid: 0x5b; action: { type: "Axis"; axis: "REL_WHEEL_HI_RES"; move: -120; rate: 100; } }, { # Tilt wheel right cid: 0x5d; action: { type: "Axis"; axis: "REL_WHEEL_HI_RES"; move: 120; rate: 100; } } ) ``` '## Axis This maps pressed buttons to axis change (scrolling etc) with configurable move distance and autorepeat rate. \### axis This is a required string/integer field that defines the axis to be changed. For a list of axis strings, refer to [linux/input-event-codes.h](https://github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h). (e.g. `axis: "REL_HWHEEL";`). Alternatively, you may use integer axis code to define the axis. \###move This is an optional integer field that defines the axis change per one repeat. Use negative values to invert scroll direction. Regular/HI_RES axis ratio is 1/120 (1 step for regular axis == 120 steps for HI_RES axis). (e.g. `move: -1;`) The default value is 1 for the regular axis and 120 for the HI_RES axis. \### rate This is an optional integer field that defines autorepeat rate (ms). (e.g. `rate: 100;`) The default value is 100 (repeat axis change 10 times per second as 1000/100 == 10).
ddacd86
to
6555e4b
Compare
I fixed the problem with repeat task startup, now it starts correctly every logid launch. Can anyone review the repeat task body about HI_RES axis changes? Is my approach (take move value, divide it by 120 and move the regular axis to resulted value and then move the HI_RES axis to remanded value) conceptually correct or not? |
+1 Nice work, thanks for working on this |
@PixlOne |
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.
Sorry for taking a while to respond, I've been a bit busy recently. I've reviewed the changes and there are a couple changes you should make before I approve this.
Action(device), _config (device, config) | ||
{ | ||
_config.repeatMutex().lock(); | ||
std::shared_ptr<task> repeatTask = std::make_shared<task>([this]() { |
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.
This code basically reserves an entire worker to the axis action regardless of the button state,
which is not ideal. We should instead:
- Initialize an axis action task in the constructor
- Queue it during the press event
- On release, alert the task that it should stop (using a condition variable)
- In the task, we should use
condition_variable::wait_until
instead of sleeping
That way, we are only using a worker when necessary.
void AxisAction::press() | ||
{ | ||
_pressed = true; | ||
_config.repeatMutex().unlock(); |
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.
A unique_lock
or a lock_guard
should be used instead in press
and release
,
int _move = 1; | ||
uint _rate = 100; | ||
int _hiResMoveMultiplier = 120; | ||
std::mutex _repeatMutex; |
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.
We shouldn't have the repeat mutex in the config class. Regardless, in the solution I described, it is not needed, so this can be removed.
a062ed0
to
a77b328
Compare
This is something like Gesture mode: "Axis", but works on mouse buttons.
Configure it like the following (this example will re-map mouse wheel tilts to horizontal scrolling with increased speed):
Axis
This maps pressed buttons to axis change (scrolling etc) with configurable move distance and autorepeat rate.
axis
This is a required string/integer field that defines the axis to be changed.
For a list of axis strings, refer to linux/input-event-codes.h. (e.g.
axis: "REL_HWHEEL_HI_RES";
).Alternatively, you may use integer axis code to define the axis.
move
This is an optional integer field that defines the axis change per one repeat. Use negative values to invert scroll direction.
Regular/HI_RES axis ratio is 1/120 (1 step for regular axis == 120 steps for HI_RES axis). (e.g.
move: -1;
)The default value is 1 for the regular axis and 120 for the HI_RES axis.
rate
This is an optional integer field that defines autorepeat rate (ms). (e.g.
rate: 100;
)The default value is 100 (repeat axis change 10 times per second).
Fixes #153
Requires #245 to map REL_HWHEEL axis