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

Refactored injection #263

Merged
merged 211 commits into from
Apr 17, 2022
Merged

Refactored injection #263

merged 211 commits into from
Apr 17, 2022

Conversation

jonasBoss
Copy link
Collaborator

@jonasBoss jonasBoss commented Jan 17, 2022

New Features

  • Multiple Trigger points for mapping EV_ABS or EV_REL to buttons
  • Simultaneous mapping of EV_ABS or EV_REL to buttons and other analog axis
  • All EV_ABS event codes can be mapped to axis and or buttons
    • No limitation for mapping only joystick to mouse or scroll wheel
  • Individually (per mapping) configurable mapping parameters
    • trigger point, deadzone, gain, expo

Changes

  1. replaced all Consumer classes with multiple MappingHandler classes
  2. injecting release events to the forwarded UInput before combinations trigger
  3. reworked macro if_singel to work with new injection architecture
  4. renamed Mapping class to Preset
  5. created new Mapping class responsible for mapping a event or event combination to an output action
  6. moved multiple configuration classes to new configs directory
  7. renamed ConsumerControl to EventReader
  8. replaced all evdev.InputEvent objects with custom inputremapper.InputEvent objects
  9. Replaced the Key class with a EventCombination class of type Tuple[InputEvent]

TODO

  • Integrate the new Mapping class into the Preset
  • Implement Setting for automatic key release see also Combinations involving Modifiers on Wayland. Modifier key not released. #229
  • make the Editor-Part of the GUI interact with the new Mapping instead of the Preset
  • Patch the GUI to create Mapping objects for Joystick to Mouse
  • modify Injector._grab_device to purely rely on mappings, no longer use maps_joystick()
  • expose the Mapping in the preset files
  • rework the mapping_parser
  • migrations
  • make sure to release any keys, center any axis before stopping the injection
  • Tests, Tests, Tests, Tests
  • Update documentation

Future (probably not this PR)

  • Update GUI To expose the new functionality
  • Implement RelToAbsHandler
  • Implement RelToRelHandler
  • Implement AbsToAbsHandler
  • Implement Offset for axis to axis handlers

inputremapper/injection/context.py Outdated Show resolved Hide resolved
inputremapper/injection/consumer_control.py Outdated Show resolved Hide resolved
D-Pad and joystick to button or key or macro mapping works now
@sezanzeb
Copy link
Owner

created beta branch

@sezanzeb
Copy link
Owner

@clausbertels (#7 (comment)) might have some ideas on how to improve the user interface :). See #263 (comment) and below for screenshots of the current state

@jonasBoss jonasBoss changed the base branch from main to beta April 16, 2022 08:37
@jonasBoss jonasBoss marked this pull request as ready for review April 16, 2022 20:49
@jonasBoss
Copy link
Collaborator Author

I just implemented the last back-end changes, this should now have the same functionality as the main branch plus some more.
I think it is ready for merge into the beta branch.
As we discussed, I will open new PRs for the work on the front-end.

@sezanzeb sezanzeb merged commit 1a2b2d7 into sezanzeb:beta Apr 17, 2022
@sezanzeb
Copy link
Owner

sezanzeb commented May 8, 2022

what happened to tests like test_hold_two and other hold tests?

@sezanzeb
Copy link
Owner

Can't find test_filter_trigger_spam in beta

@jonasBoss
Copy link
Collaborator Author

jonasBoss commented May 12, 2022

I removed quite a few tests form the test_keycode_mapper.py since we no longer have a KeycodeMapper most of them are now in the test_event_pipeline.py but some are completely missing for various reasons. Obviously tests concerned with the internal working of KeycodeMapper are removed. Some tests seemed to be duplicate of other tests.

For example the various TestKeycodeMapper.test_hold_xxx: Those seem to test the same behavior which is tested in TestMacros.test_hold_xxx in addition to that they test running multiple macros at once, triggering mappings with combinations and single keys ...

Those are things we no longer need to test specifically for the hold macro or even macros in general. The event-pipeline separates the Input handling from the actions a input triggers. Therefore we don't need to specifically test all the ways we can trigger a macro we only need to test all the different ways to trigger an action, whether this action is a macro or a key does not make a difference because the same system for input handling is used. There are test like test_combination_keycode_macro_mix, test_combination, test_rel_to_btn and more, which make sure the input handling works as expected.

As for the test_filter_trigger_spam the linux kernel documentation states

The input protocol is a stateful protocol. Events are emitted only when values of event codes have changed. However, the state is maintained within the Linux input subsystem; drivers do not need to maintain the state and may attempt to emit unchanged values without harm.

So we don't actually need it, and I think there is at least test_wheel_combination_release_failure which includes assertions to make sure that duplicate scroll (EV_REL) events are not propagated.

There might still be tests I missed or other valid reasons I did not think of. So I am happy to add more tests where necessary.

In general I try to keep the tests focused on specific behaviors and not write tests that test multiple (possibly unrelated) behaviors at once, that way they are hopefully easier to understand (shorter) and maintain.

@sezanzeb
Copy link
Owner

Thanks for explaining!

as for test_filter_trigger_spam, looking back in history the test once looked like this

        for i in range(1, 20):
            handle_keycode(_key_to_code, {}, InputEvent(*trigger, i), uinput)

so it was testing various values, like when slowly pressing a trigger. I don't know why it was changed to 1.

@sezanzeb
Copy link
Owner

sezanzeb commented May 14, 2022

Ah, because at some point the old should_map_event_as_btn function was taking care of setting the value of joystick/trigger events to 0 or 1. That means handle_keycode was actually receiving lots of events with the same value.

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.

2 participants