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

Feature Request: use state_trigger with a passed function #43

Closed
dlashua opened this issue Oct 15, 2020 · 7 comments
Closed

Feature Request: use state_trigger with a passed function #43

dlashua opened this issue Oct 15, 2020 · 7 comments

Comments

@dlashua
Copy link
Contributor

dlashua commented Oct 15, 2020

I find I use this pattern quite a bit (untested):

registered_triggers = []

class FollowMe:
    def __init__(self, config):
        self.config = self.validate_config(config)

        @state_trigger(f'True or {self.config["input"]}')
        def state_trigger_update():
            self.update()

        registered_triggers.append(state_trigger_update)

    def validate_config(self, config):
        # do better
        return config

    def update(self):
        if state.get(config["input"]) == 'on':
            homeassistant.turn_on(entity_id=self.config['output'])
        else:
            homeassistant.turn_off(entity_id=self.config['output'])

It would be cleaner if I could do this:

class FollowMe:
    def __init__(self, config):
        pyscript.state_trigger(f'True or {self.config["input"]}')(self.update)

The pyscript.state_trigger method could be named something else if that made more sense. It could also take the function as a parameter (named or positional) if that's better. And this same functionality would be useful on all the decorators.

I can implement something similar in code myself like this (untested):

registered_triggers = []

def make_state_trigger(trigger, cb):
    @state_trigger(trigger)
    def inner_state_trigger():
        cb()
    
    registered_triggers.append(inner_state_trigger)

class FollowMe:
    def __init__(self, config):
        self.config = self.validate_config(config)

        make_state_trigger(
          f'True or {self.config["input"]}',
          self.update
        )

... It's just more readable, useful, and documented if it's built-in.

It would also be amazing if these triggers were removable. Something like this:

registered_triggers = {}

def make_state_trigger(trigger, cb):
    # make some unique ID in a better way
    unique_id = time.time()

    @state_trigger(trigger)
    def inner_state_trigger():
        cb()
    
    registered_triggers[unique_id] = inner_state_trigger

    def cancel_trigger():
        del registered_triggers[unique_id]

    return cancel_trigger


# in some class
cancel = make_state_trigger('domain.entity == "on"', self.update)

# later
cancel()
@craigbarratt
Copy link
Member

This relates to your comments in #30 - as you noted, pyscript doesn't yet support decorators, other than the builtin ones, which are hardcoded.

It seems like I should support decorators, and then turn the currently hardcoded decorators into functions that can be used as the current decorators, but also as you propose above.

One immediate issue, is that currently when you specify multiple decorators, it's easy for the code to know when all of them have been processed and it can then "arm" the trigger. In the function call case, you could have several nested functions with different triggers, and only the last one should arm the trigger. And the order shouldn't matter. I'm not sure yet how to deal with those issues.

A second issue is that with decorators, keeping a pointer to the function is still up to the user. It's harder to see how to do that automatically. Maybe there is a @persistent decorator that does that, and returns a cancel function? I need to think about this some more.

@dlashua
Copy link
Contributor Author

dlashua commented Oct 17, 2020

Again, I don't 100% follow the pyscript implementation (though I get a little closer every time I dig through it). I'm also pretty new to python (about a year) and have little experience writing decorators.

That being said, I imagine it would look something like this:

class state_trigger:
    def __init__(self, triggers):
        if not isinstance(triggers, list):
            triggers = [triggers]

        self.triggers = triggers
        self.func = None

    def get_triggers():
        return self.triggers

    def __call__(func):
        if isinstance(func, state_trigger):
            new_triggers = self.triggers
            new_triggers.extend(func.get_triggers())
            
            return state_trigger(new_triggers)(func)
        
        # register self.triggers with hass to call func
        ??

Obviously, __call__ would need to check for all the different kinds of triggers (event, time, etc), and it would likely create a super_trigger object that knows how to handle events, time, and state, so that these could all be chained together.

The *_active decorators are really just an if statement before it calls the passed in function. They wouldn't need special treatment as long as the documentation specified that all *_trigger decorators should come BEFORE all *_active decorators.

I think...

I might be missing a key piece though.

@dlashua
Copy link
Contributor Author

dlashua commented Oct 17, 2020

A more complete implementation might look like this?

class super_trigger:
    def __init__(self, state_triggers=[], event_triggers=[], time_triggers=[]):
        if not isinstance(state_triggers, list):
            state_triggers = [state_triggers]

        if not isinstance(event_triggers, list):
            event_triggers = [event_triggers]

        if not isinstance(time_triggers, list):
            time_triggers = [time_triggers]

        self.state_triggers = state_triggers
        self.event_triggers = event_triggers
        self.time_triggers = time_triggers

    def get_state_triggers(self):
        return self.state_triggers

    def get_time_triggers(self):
        return self.time_triggers

    def get_event_triggers(self):
        return self.event_triggers

    def __call__(self, func):
        if isinstance(func, super_trigger):
            new_state_triggers = self.state_triggers
            new_state_triggers.extend(func.get_state_triggers())

            new_event_triggers = self.event_triggers
            new_event_triggers.extend(func.get_event_triggers())

            new_time_triggers = self.time_triggers
            new_time_triggers.extend(func.get_time_triggers())

            return super_trigger(state_trigger=new_state_triggers, event_triggers=new_event_triggers, time_triggers=new_time_triggers)
        
        # register self.*_triggers with hass to call func
        ??


def state_trigger(triggers):
    return super_trigger(state_triggers=triggers)

def event_trigger(triggers):
    return super_trigger(event_triggers=triggers)

def time_trigger(triggers):
    return super_trigger(time_triggers=triggers)

@dlashua
Copy link
Contributor Author

dlashua commented Oct 17, 2020

And, I imagine if you didn't want to stipulate that *_active decorators come AFTER *_trigger decorators, then those, too, could be included in super_trigger (both in __init__ with named parameters, as well as in __call__ with .extend()), And in the "??" section. Instead of sending "func" to HASS to call, you could send a method that checks all the *_active conditions and only calls func if they pass.

@dlashua
Copy link
Contributor Author

dlashua commented Oct 17, 2020

And I think __call__ could just extend self.state_triggers and friends in place and then return self?

@dlashua
Copy link
Contributor Author

dlashua commented Oct 17, 2020

With *_active...

class super_trigger:
    def __init__(
            self,
            state_triggers=[],
            event_triggers=[],
            time_triggers=[],
            state_active=[],
            event_active=[],
            time_active=[]
            ):

        if not isinstance(state_triggers, list):
            state_triggers = [state_triggers]

        if not isinstance(event_triggers, list):
            event_triggers = [event_triggers]

        if not isinstance(time_triggers, list):
            time_triggers = [time_triggers]

        if not isinstance(state_active, list):
            state_active = [state_active]

        if not isinstance(event_triggers, list):
            event_active = [event_active]

        if not isinstance(time_triggers, list):
            time_active = [time_active]

        self.state_triggers = state_triggers
        self.event_triggers = event_triggers
        self.time_triggers = time_triggers
        self.state_active = state_active
        self.event_active = event_active
        self.time_active = time_active


    def __call__(self, func):
        if isinstance(func, super_trigger):
            self.state_triggers.extend(func.state_triggers)
            self.event_triggers.extend(func.event_triggers)
            self.time_triggres.extend(func.time_triggers)
            self.state_active.extend(func.state_active)
            self.event_active.extend(func.event_active)
            self.time_active.extend(func.time_active)

            return self

        # we only get here if we didn't get a super_trigger

        def state_and_time_pass_me_to_hass(**params):
            for active in self.state_active:
                # check condition return if false
                pass

            for active in self.time_active:
                # check condition return if false
                pass

            func(**params)

        def event_pass_me_to_hass(**params):
            for active in self.event_active:
                # check condition return if false
                pass
 
            for active in self.state_active:
                # check condition return if false
                pass

            for active in self.time_active:
                # check condition return if false
                pass

            func(**params)

        # register event, state, and time triggers with hass passing in above methods
        ??
        

        

def state_trigger(triggers):
    return super_trigger(state_triggers=triggers)

def event_trigger(triggers):
    return super_trigger(event_triggers=triggers)

def time_trigger(triggers):
    return super_trigger(time_triggers=triggers)

def state_active(active):
    return super_trigger(state_active=active)

def event_active(active):
    return super_trigger(event_active=active)

def time_active(active):
    return super_trigger(time_active=active)

@dlashua
Copy link
Contributor Author

dlashua commented Dec 10, 2020

I've created a piece of working example code to follow another path for this. I also realize that I missed an important piece in my example code above... returning the handler. Ooops. The code below has actually been tested and is working.

The concept is, each decorator can be a "filter" (like state_active), a "generator" (like state_trigger), or a modifier (which pyscript doesn't do (yet?), but I left those bits in anyway so you could see them).

The "func_vars" is what would get passed around (instead of the "value" I'm using in this example). And, of course, instead of an event emitter, we have the HASS buss for states and events.

https://gist.github.com/dlashua/b2f510a794cedfb370a0418b8592765e

Each class, when __call__ed, stores the function it was passed and returns its handler. This allows the decorators to be chained.

With this in place, the biggest difference between how pyscript works now and how it would work with this is that the location of "state_active" in the chain would matter. Since "state_trigger" would be "generating" messages, if "state_active" was above it, then it would never be consulted. Some more complex code could be written to allow state_active to pass its filter handlers down the chain with the handler checking to see if it's the last in the chain and calling all of the filters first if it is. But, this wouldn't work well when also faced with user supplied decorators because they'd have to extend a certain object type to be detectable. If this is important to you, though, I can work out the details and provide sample code on how that could work as well.

craigbarratt added a commit that referenced this issue Dec 26, 2020
@dlashua dlashua closed this as completed Feb 10, 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

No branches or pull requests

2 participants