python-elgato-streamdeck icon indicating copy to clipboard operation
python-elgato-streamdeck copied to clipboard

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

Open Kalle-Wirsch opened this issue 6 years ago • 17 comments

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 avatar Jan 02 '19 10:01 Kalle-Wirsch

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 avatar Jan 05 '19 22:01 Kalle-Wirsch

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.

abcminiuser avatar Jan 07 '19 09:01 abcminiuser

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

Kalle-Wirsch avatar Jan 07 '19 12:01 Kalle-Wirsch

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.

abcminiuser avatar Jan 07 '19 22:01 abcminiuser

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 avatar Jan 13 '19 21:01 Kalle-Wirsch

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 avatar Jan 16 '19 23:01 Kalle-Wirsch

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 avatar Jan 19 '19 18:01 Kalle-Wirsch

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.

Kalle-Wirsch avatar Jan 24 '19 00:01 Kalle-Wirsch

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?

abcminiuser avatar Jan 24 '19 09:01 abcminiuser

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 avatar Jan 24 '19 12:01 Kalle-Wirsch

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

Kalle-Wirsch avatar Jan 25 '19 00:01 Kalle-Wirsch

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.

abcminiuser avatar Jan 30 '19 10:01 abcminiuser

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 avatar Jan 31 '19 00:01 Kalle-Wirsch

Reintegrated StreamDeckFilter into the current version.

Please revisit and decide if this fits at all

Kalle-Wirsch avatar Feb 24 '19 17:02 Kalle-Wirsch

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.

abcminiuser avatar Feb 25 '19 08:02 abcminiuser

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. ;-)

Kalle-Wirsch avatar Feb 25 '19 22:02 Kalle-Wirsch

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.

abcminiuser avatar Feb 26 '19 09:02 abcminiuser