Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

S-trace
Copy link

@S-trace S-trace commented May 8, 2021

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):

buttons: (
    {
        # Tilt wheel left
        cid: 0x5b;
        action:
        {
            type: "Axis";
            axis: "REL_HWHEEL_HI_RES";
            move: -120;
            rate: 10;
        }
    },
    {
        # Tilt wheel right
        cid: 0x5d;
        action:
        {
            type: "Axis";
            axis: "REL_HWHEEL_HI_RES";
            move: 120;
            rate: 10;
        }
    }
)

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

@S-trace
Copy link
Author

S-trace commented May 8, 2021

@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).
@S-trace S-trace force-pushed the feature/action-axis branch from ddacd86 to 6555e4b Compare May 9, 2021 21:33
@S-trace
Copy link
Author

S-trace commented May 9, 2021

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?

@syepes
Copy link

syepes commented May 11, 2021

+1 Nice work, thanks for working on this

@S-trace
Copy link
Author

S-trace commented Dec 17, 2021

@PixlOne
Are there any issues with this pull request?
Do you plan to introduce this feature in your own implementation?
If both are "no" - why didn't you yet merged this pull request?

Copy link
Owner

@PixlOne PixlOne left a 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]() {
Copy link
Owner

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();
Copy link
Owner

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;
Copy link
Owner

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.

@PixlOne PixlOne force-pushed the main branch 14 times, most recently from a062ed0 to a77b328 Compare May 4, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to adjust the horizontal scroll BUTTON speed for M720?
3 participants