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

Input event with origin #550

Merged
merged 37 commits into from Dec 15, 2022
Merged

Conversation

jonasBoss
Copy link
Collaborator

@jonasBoss jonasBoss commented Nov 20, 2022

fix #435

  • update InputEvent to include origin and analog_threshold
  • save preset as list with InputEvent as dict inside the mapping
  • add capability to load old beta preset
  • add origin info to each InputEvent in the EventReader
  • update EventHandler's
  • make sure forward_release forwards to the correct device release_combination_keys does not always forward to the correct device (beta) #577
  • update event pipline creation to respect origin information
  • add fallback when origin is not set in mapping
  • grab devices based off of origin
    • add fallback when origin can not be found during device grabbing???
  • seperate InputEvent from InputConfig
  • tests
  • delete old stuff
  • Update documentation (the preset json has changed)

@jonasBoss
Copy link
Collaborator Author

So this is actually more than just including a origin with each event.

The main change is actually the Separation of InputEvent and InputConfig. Currently they are both very similar but it gives us much more flexibility as the InputConfig is no longer restricted by a single event or performance concerns. This can help if we ever choose to process the input on a per frame basis instead of a per event basis.
We can use the InputConfig to store mapping parameters which can be individual for each input of a mapping (e.g. the release_combination_keys flag).

I also renamed the EventCombination to InputCombination.
@sezanzeb I would like to hear your thoughts on this change.

@sezanzeb
Copy link
Owner

sezanzeb commented Dec 6, 2022

I'll look at it soon

@sezanzeb
Copy link
Owner

sezanzeb commented Dec 8, 2022

A miroboard showing how all of our models and classes are related might be great at this point

UIMapping, InputConfig, InputEvent, InputDevice, InputCombination

@sezanzeb
Copy link
Owner

sezanzeb commented Dec 8, 2022

ah,

so InputConfig is for the mapping and describes the input for a mapping

and InputEvent is for the EventHandler pipeline and describes an actual event that is happening right now?

@jonasBoss
Copy link
Collaborator Author

yeah, that is the idea.
Pylint comes with a pyreverse command which can be used to generate UML. Although, Pylint seems to be broken for me :(

@sezanzeb
Copy link
Owner

sezanzeb commented Dec 10, 2022

here is the pyreverse output:

graph

@sezanzeb
Copy link
Owner

sezanzeb commented Dec 10, 2022

How about renaming InputCombination to InputConfigCombination?

Copy link
Owner

@sezanzeb sezanzeb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think splitting it into InputConfig and InputEvent makes sense

inputremapper/configs/input_config.py Outdated Show resolved Hide resolved
inputremapper/injection/injector.py Outdated Show resolved Hide resolved
inputremapper/gui/reader_service.py Outdated Show resolved Hide resolved
inputremapper/configs/preset.py Outdated Show resolved Hide resolved
inputremapper/configs/migrations.py Outdated Show resolved Hide resolved
@sezanzeb sezanzeb added this to todo in 2.0 Dec 10, 2022
@sezanzeb sezanzeb removed this from todo in 2.0 Dec 10, 2022
@jonasBoss jonasBoss marked this pull request as ready for review December 11, 2022 14:41
readme/usage.md Outdated Show resolved Hide resolved
@sezanzeb
Copy link
Owner

sezanzeb commented Dec 14, 2022

Thanks! Is this ready to merge?

I would probably still rename origin to origin_hash though. What are your thoughts on that and #550 (comment)?

Is there a tests that checks if the hash is calculated correctly? Testing that a given generated InputConfig contains the correct hash, which is provided as hardcoded string in the test with a comment explaining where it comes from. And to test that the correct device ends up in origin.

@jonasBoss
Copy link
Collaborator Author

I will rename origin and then it is ready to merge.
I am not sure about the InputCombination it is already in the configs.input_config module. So it should be clear what it does.

As for this:

make sure forward_release forwards to the correct device

That is a bug which is present on beta since ever. I just never thought about it until this PR made it kind of obvious.
There are two possible solutions, which both will need some work.

@sezanzeb sezanzeb merged commit 0f10d06 into sezanzeb:beta Dec 15, 2022
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.

None yet

2 participants