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: Bring back callbacks to IActionTypeListener and ITriggerTypeListener #342

Open
avargaskun opened this issue Feb 19, 2023 · 0 comments
Labels
App Misbehavior This problem is causing HomeSeer to not do what was expected. NOT FOR PLUGINS HS Core This issue involves changes to core HS systems. NOT FOR PLUGINS To Do This issue is in need of attention

Comments

@avargaskun
Copy link

avargaskun commented Feb 19, 2023

Environment

HomeSeer

OS

All

HS Version

v4.2.17.0

Development

PSDK Version

v.1.4.3.0

Language

All

IDE

VSCode

Dev OS

Mac

Page

https://homeseer.github.io/Plugin-SDK-Docs/api/HomeSeer.PluginSdk.Events.ActionTypeCollection.IActionTypeListener.html
https://homeseer.github.io/Plugin-SDK-Docs/api/HomeSeer.PluginSdk.Events.TriggerTypeCollection.ITriggerTypeListener.html

Plugin

HSBuddy

Problem

Description

This is a feature request to add back some (or all) the callbacks to IActionTypeListener and ITriggerTypeListener - specifically those used during the plug-in's handling of:

  • ActionBuildUI
  • ActionProcessPostUI
  • TriggerBuildUI
  • TriggerProcessPostUI

The new AbstractActionType2 and AbstractTriggerType2 are an important improvement since it is easier to construct dynamic JUI views on the fly on every plug-in invocation. However, it's still wasteful to even build the JUI views as part of ActionConfigured or ActionFormatUI since nothing is being changed or shown to the user. The deserialized Dictionary<string,string> with the selected values is sufficient information to handle those calls, saving the time that it would have taken to build the JUI views. In the case of HSBuddy this is especially important because some views need enumerating all devices in the server which can be expensive in some systems.

This would bring a big performance improvement to the rendering of the top-level events.html page which does not show any of the trigger/action JUI views nor allows user to make changes until they navigate to an individual event.

Expected Behavior

ActionTypeCollection and TriggerTypeCollection invoke the callbacks in IActionTypeListener and ITriggerTypeListener (respectively) when different operations are occurring.

For example: When the user navigates to an individual event with an action owned by a plug-in then the plug-in's IActionTypeListener.OnBuildActionUi callback would get invoked. That seems to have been the case earlier according to some code that is currently commented out.

Other notes

I am currently using a workaround: In my HSPI class I am overriding the methods like ActionBuildUI and setting a flag in my HSPI class which is then read by my action class.

  • In my HSPI class:
public class HSPI : IActionTypeListener, AbstractPlugin
{
    // ...
    public ShouldBuildActionUI { get; set; }
    public new string ActionBuildUI(TrigActInfo actInfo)
    {
        ShouldBuildActionUI = true;
        try { base.ActionBuildUI(actInfo); }
        finally { ShouldBuildActionUI = false; } 
    }
    public new EventUpdateReturnData ActionProcessPostUI(Dictionary<string, string> postData, TrigActInfo actInfo)
    {
        ShouldBuildActionUI = true;
        try { base.ActionProcessPostUI(postData, actInfo); }
        finally { ShouldBuildActionUI = false; } 
    }
    // ...
}
  • In my action class
public class MyAction : AbstractActionType2
{
    // ...
    private Dictionary<string, string> _viewValues = null;
    protected override void OnInstantiateAction(Dictionary<string, string> viewIdValuePairs)
    {
        _viewValues = new Dictionary<string, string>(viewIdValuePairs);
        if ((ActionListener as HSPI).ShouldBuildActionUI)
        {
            // Only build all the JUI views if they are meant to be used
            ConfigPage = BuildMyConfigPage();
        }
    }
    // ...
}

This workaround is a lucky hack that happens to work because the HSCF framework ignores the fact that the methods being overrided aren't virtual. Also, having to use flags like this in the HSPI which is a singleton is a recipe for disaster if multiple calls to methods like ActionBuildUI were to happen in parallel.

A more elegant solution with the proper callbacks in IActionTypeListener would look like:

public class HSPI : IActionTypeListener, AbstractPlugin
{
    // ...
    AbstractActionType IActionTypeListener.OnBuildActionUi(AbstractActionType action)
    {
        if (action is MyAction) ((MyAction)action).BuildMyConfigPage();
        return action;
    }
    AbstractActionType IActionTypeListener.BeforeActionConfigChange(AbstractActionType action)
    {
        if (action is MyAction) ((MyAction)action).BuildMyConfigPage();
        return action;
    }
    // ...
}
@avargaskun avargaskun added App Misbehavior This problem is causing HomeSeer to not do what was expected. NOT FOR PLUGINS HS Core This issue involves changes to core HS systems. NOT FOR PLUGINS To Do This issue is in need of attention labels Feb 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App Misbehavior This problem is causing HomeSeer to not do what was expected. NOT FOR PLUGINS HS Core This issue involves changes to core HS systems. NOT FOR PLUGINS To Do This issue is in need of attention
Projects
None yet
Development

No branches or pull requests

1 participant