Skip to content

Add various Filters for Key Callback and and provide a Framework to create new Filters #8

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Kalle-Wirsch
Copy link

2nd try to file a Pull Request

Removing Chattering:
New callbacks are only made after a 3ms back delay/cooldown for that key.

@Kalle-Wirsch
Copy link
Author

Hello Dean,

I implemented another feature (Tempo-mode), i.e. distinguishing between long and short key presses.
This feature is common for flight simulations, joysticks, ...

@Kalle-Wirsch Kalle-Wirsch changed the title Remove Chattering Remove Chattering + Tempo Mode Jan 5, 2019
@abcminiuser
Copy link
Owner

Thanks, the pull request looks better now.

I'm wondering if this belongs in a pluggable filter, rather than inside the core library, so that people can opt-into whatever filters they want with the parameters they want.

Something like a DebounceFilter class that takes the key change events and produces it's own filtered events, with the debounce period specified by the user. If you wanted the temp filtering on top of that, you would attach the debounce filter events to a TempoFilter class instance. Not everyone will want the filtering inside the library, and not everyone will want to use the same debounce/tempo filter intervals.

@Kalle-Wirsch
Copy link
Author

Hi Dean, I need to look into this and come back to you.

It will take some time, since started with Python 14 days ago. So I am new to filter-concepts.
But I will have a look into it and come back to you.

kind regards

@abcminiuser
Copy link
Owner

You're doing extremely well then, I didn't mean to be discouraging - I appreciate the project interest and your contribution. I'm just thinking of how to generalise this some more, to make it extensible to all the various use cases.

I'll clean up and merge this tonight, and you can keep looking at the filter concept as a good Python learning task.

@Kalle-Wirsch
Copy link
Author

Hi Dean,

sorry for the long radio frequency. I did write a lot of Python code during the holidays. Now I only find some time on the weekends, that is the only reason I said, it will take some time.

I kept thinking about it and identified several possible solutions. I now started coding my favorite solution, But it still needs some code refactoring. Most likely I will finish it next weekend.

@Kalle-Wirsch Kalle-Wirsch changed the title Remove Chattering + Tempo Mode Add various Filters for Key Callback and and provide a Framework to create new Filters Jan 16, 2019
@Kalle-Wirsch
Copy link
Author

Kalle-Wirsch commented Jan 16, 2019

Hi Dean,

I am finally kind of happy with the implementation of Filter Plugin you proposed.
There were a lot of design issues that just took some thinking and research on my side.

Let me try to explain what I had in mind

  1. Make the code compatible with your old release again, in a sense, that "old" users of the StreamDeck class do not see any changes.
  2. Create Debounce and Tempo filters
    • I actually created 4 Filters:
      • StateChangeFilter equals the old StreamDeck behavior
      • DebounceFilter allows to Filter key chattering as proposed earlier
      • TempoFilter - reports short vs long key presses
      • The Base Class CallBackFilter actually reports all key presses
  3. Filters can be exchanged on the fly by assigning each StreamDeck.filter[key]
    a different filter, with his own Debounce or Tempo timings, ...
  4. Create a plugin framework that allows to extend StreamDeck with new filters in the future
    • This can be achieved by subclassing one of the above Filters
    • The BaseClass CallbackFilter and the @manage_states decorator provide some base service
      like storing the states, times and delays since the last callback.
      This allows writing new filters with minimal effort and readable code
    • The design concept even allows to transform the boolean key states to something else like numbers
      this mapping can be implemented by overriding in the function map_states
  5. Filters do NOT include calling the callback function themselves. This makes adapting any existing
    subclasses from StreamDeck easier. e.g. It doesn't require to change StreamDeck.set_key_callback()
    To sum it up, It may seem unlogical not including the callback into the filter, but fewer hardcoded
    assumptions and dependencies between callbacks, _read() and filters will likely ease future
    extensions to filters.

How to move on:

  1. I did one mortal sin of object orientation - removing a public attribute (last_key_states) of an existing class and therefore changing the visible behavior completely. I read something about dynamic properties. I.E. I plan to fix that later.
  2. Please advice, if we should place a filter class in a sub module or inside the StreamDeck module itself.
  3. I already did some testing, but not comprehensively. Please look if there are obvious bugs I have overseen
  4. Please comment on the design decisions and implementation. You know I am new to Python, so I often guess and do the next best thing I recently got aware of.
    I am sure I would code the whole thing differently in two or three months and do not want to ruin your clean style.

@Kalle-Wirsch
Copy link
Author

Dear Dean,

  • I added @Property for backward compatibility with the last revision.
  • Added some minor changes to StreamDeckFilter

Works from my side. let me know what you think of it

@Kalle-Wirsch
Copy link
Author

Hi Dean,

I observed, that json.Encoder support will be crucial to make any button settings persistent, that include a StreamDeckFilter object.
So I added support functions for that.

@abcminiuser
Copy link
Owner

Fancy! Certainly a lot more involved than I was expecting. We have a long weekend coming up in a day and a bit - I'll have a proper go through then and try to fully understand your implementation.

Some questions:

  • How do you expect the end-user to configure the filters, once they have a StreamDeck instance?
  • What are the expected uses of the JSON functions? They don't seem to be doing anything specific to actual JSON data.
  • Do you think the new classes belong in a new Filter folder, like the existing Transport?

@Kalle-Wirsch
Copy link
Author

Hi Dean,
I will go trough your questions in detail tonight.

Q1&2 show that I need to provide a more detailed documentation and an example
Q3: Will be necessary if there is a separate documentation and examples

I appreciate very much, that you take some time to look into the implementation.
Due to the early status of the "Filter" project, und lacking Python experience some things might be too unconventional and might need some refactoring before being published for a bigger audience.

@Kalle-Wirsch
Copy link
Author

Hello Dean,

  • I added an example and more comments, documentation
  • found some bugs and fixed one
  • I observed, that DebounceFilter doesn't work well with long key delays.
    It can swallow key releases. This feels. weird in your example

I have done the best to create a base for you to revise the whole stuff at the weekend.
I hope it doesn't contain too many typos, inconsistencies and alike.

Regarding the example:

  • for now, I chose to just change yours as a base.
  • We should put it in a subfolder

Regarding Q2.
I think StreamDeck.set_key_filter(self, key, filter) would be the best choice

@abcminiuser
Copy link
Owner

Had a look at this the other day - I dig the functional style of programming. It seems to interact poorly with the demo at the moment with the more exotic filters, as they are only triggering on button state changes (so pressing the button with some of the delay filters applied doesn't reset the button state after a while).

I need to look into proper event scheduling in python, as I think for these to work natively we would need to schedule future callbacks to, for example, reset the key states after the button is released and the filter period has elapsed. Originally I never bothered to implement anything like this, as I expected the application layer using my API to implement whatever scheduling it deems necessary.

The easiest way I can see to do this would be to go all-in on asyncio and use it's native scheduler for the filters. I've tried to avoid asyncio stuff up until now despite being a big fan as I'm not sure what versions of Python most users have installed, but if it's an optional part of the library I can't imaging too many people complaining.

@Kalle-Wirsch
Copy link
Author

Hi Dean,
So how do we proceed?

I also observed that we need timed callbacks for DebounceFilter (see my previous comment).
This could either be handled by StreamDeckFilter or by StreamDeck itself.

I understood, that you like to look into that.

Here is a stupid question:

  • Can't we just create a loop,
  • use set_key_callback_async
  • and register additional future callbacks with loop.call_later
  • So Streamdeck-Filter can simply be extended with this timing behavior
  • and any additional application-level callbacks can register their own callbacks too

isn't that the whole idea of set_key_callback_async?

@Kalle-Wirsch Kalle-Wirsch reopened this Feb 24, 2019
@Kalle-Wirsch
Copy link
Author

Reintegrated StreamDeckFilter into the current version.

Please revisit and decide if this fits at all

@abcminiuser
Copy link
Owner

Oops - I was on a two week holiday and mean to get back to this when I got back a week ago, sorry!

As you've seen, I've been fiddling with the project structure somewhat. For the filtering, I'm thinking of putting these in a Filters subdirectory as an optional component, sort of like the new ImageHelpers subdirectory. By default, the library won't filter anything, but if a user wants they can wrap the StreamDeck callback in one of your filters to get their desired behavior. That keeps the API surface simple inside the core, but makes opting in the filters seamless.

@Kalle-Wirsch
Copy link
Author

Thx Dean,

I didn't mean to be pushy. I took off from coding for 3 weeks myself and now restarted and wanted to know about the plans. ;-)

@abcminiuser
Copy link
Owner

No it's my fault for failing to follow up, between long travel times and multiple projects at work I don't have a lot of time or brain space these days :(.

I don't have any real concrete plans currently, but with the refactoring I did for the StreamDeck Mini support plus the helper classes and PyPi packaging I think the optional filter helpers is the way to go, since it gives the best flexibility and allows the library users to integrate it into their own event loops/timer threads to handle the time-dependent filters.

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