Add integration test for response curve action plugin

[code-monet]

Along the lines of what we discussed earlier for testing one or more actions using IntermediateOutputs, here's a PR!

It runs on GitHub actions

What's your preference on how to test different configurations of the action(s)? E.g. for response curve, change the curve type and the control points. We could modify the action plugin instance via Python, or load a different profile for each case from the xml directory. I'm leaning towards modifying via Python - somewhat less code coverage, but more maintainable.

[WhiteMagic]

That looks nice and definitely will allow covering awkward combinations, which Gremlin now allows with the arbitrary nesting of actions.

As for how to configure, yeah I think Python might be easier and more concise. Creating XML files with the UI works but is awkward and reading/modifying them is a pain with the number of indirections now present. Also doing it with Python would give more control should we want to vary a base action or similar.

[code-monet]

That looks nice and definitely will allow covering awkward combinations, which Gremlin now allows with the arbitrary nesting of actions.

As for how to configure, yeah I think Python might be easier and more concise. Creating XML files with the UI works but is awkward and reading/modifying them is a pain with the number of indirections now present. Also doing it with Python would give more control should we want to vary a base action or similar.

Let me give another stab at creating a profile in Python - I might be missing something, but it looks like I'd have to duplicate some of the logic that's currently in the QML files, and it seemed like it would end up being a lot of code overall.

[code-monet]

I think I'm a little too deep in the weeds here, trying to create the profile from Python. So far I have this fixture:

@pytest.fixture(scope="module")
def profile_setup() -> None:
    # This is important for locally-run tests where the last used profile could
    # get loaded, if present in configuration.
    backend.Backend().profile = pr = profile.Profile()

    # Create intermediate output axis for both input and output.
    io = intermediate_output.IntermediateOutput()
    io.reset()
    io.create(types.InputType.JoystickAxis, label=_INPUT_IO_AXIS_LABEL)
    io.create(types.InputType.JoystickAxis, label=_OUTPUT_IO_AXIS_LABEL)

    p_manager = plugin_manager.PluginManager()
    # Create response curve action.
    response_curve_action = p_manager.create_instance(
        response_curve.ResponseCurveData.name,
        types.InputType.JoystickAxis
    )

    # Create intermediate output mappings.
    map_to_io_action = p_manager.create_instance(
        map_to_io.MapToIOData.name,
        types.InputType.JoystickAxis
    )
    map_to_io_action.io_input_guid = io[_OUTPUT_IO_AXIS_LABEL]

    # Add actions to profile.
    root_action = p_manager.create_instance(root.RootData.name, types.InputType.JoystickAxis)
    root_action.insert_action(response_curve_action, "children")
    root_action.insert_action(map_to_io_action, "children")
    input_item_ = profile.InputItem(pr.library)
    input_item_binding_ = profile.InputItemBinding(input_item_)
    input_item_binding_.root_action = root_action
    input_item_binding_.device_id = dill.UUID_IntermediateOutput
    input_item_binding_.input_id = io[_INPUT_IO_AXIS_LABEL]
    input_item_binding_.input_type = types.InputType.JoystickAxis
    input_item_binding_.mode = mode_manager.ModeManager().current.name

Is this generally the right idea? What should happen next?

[WhiteMagic]

I think I'm a little too deep in the weeds here, trying to create the profile from Python. So far I have this fixture:

@pytest.fixture(scope="module")
def profile_setup() -> None:
    # This is important for locally-run tests where the last used profile could
    # get loaded, if present in configuration.
    backend.Backend().profile = pr = profile.Profile()

    # Create intermediate output axis for both input and output.
    io = intermediate_output.IntermediateOutput()
    io.reset()
    io.create(types.InputType.JoystickAxis, label=_INPUT_IO_AXIS_LABEL)
    io.create(types.InputType.JoystickAxis, label=_OUTPUT_IO_AXIS_LABEL)

    p_manager = plugin_manager.PluginManager()
    # Create response curve action.
    response_curve_action = p_manager.create_instance(
        response_curve.ResponseCurveData.name,
        types.InputType.JoystickAxis
    )

    # Create intermediate output mappings.
    map_to_io_action = p_manager.create_instance(
        map_to_io.MapToIOData.name,
        types.InputType.JoystickAxis
    )
    map_to_io_action.io_input_guid = io[_OUTPUT_IO_AXIS_LABEL]

    # Add actions to profile.
    root_action = p_manager.create_instance(root.RootData.name, types.InputType.JoystickAxis)
    root_action.insert_action(response_curve_action, "children")
    root_action.insert_action(map_to_io_action, "children")
    input_item_ = profile.InputItem(pr.library)
    input_item_binding_ = profile.InputItemBinding(input_item_)
    input_item_binding_.root_action = root_action
    input_item_binding_.device_id = dill.UUID_IntermediateOutput
    input_item_binding_.input_id = io[_INPUT_IO_AXIS_LABEL]
    input_item_binding_.input_type = types.InputType.JoystickAxis
    input_item_binding_.mode = mode_manager.ModeManager().current.name

Is this generally the right idea? What should happen next?

Yes, that is one way that should work. The other way which one could take (not sure it's better or easier at all though) is to directly work with the gremlin.profile.Profile, gremlin.profile.Library, etc. which would then force you to recreate some of the various from_xml() functionalities, so probably not any easier but just different :-)

Once you have a valid profile, you should be able to run something like this to get Gremlin into the "active" state.

from gremlin import code_runner
cr = code_runner.CodeRunner()
cr.start(pr, mode_manager.ModeManager().current.name)

After that, all events should be processed by the profile, and the corresponding actions be executed. And then we hit a snag I just discovered :-) Currently, there is no way to emit events (Qt Signals) directly from the IO system. The update method only changes the internal state but does not emit signals. The only places they are emitted is in the MapToIOFunctor. That's kind of an oversight and the emit part of the functor should be moved into the actual IntermediateOutput.Input.update() method such that they emit events no matter how an update is called. With that change then all the test code would need to do is do various .update calls on the IO input instance of choice.

I hope that all makes sense, if not let me know. I can also draw up a diagram of how the whole Profile thing connects together, as the entire Library, Input, InputBinding, etc. are quite a handful to wrap one's head around.

[WhiteMagic]

Here is a rough outline of how the profile classes and actions interact with each other. I didn't show where the model and functor of the actions go as that's UI and response to events.

[code-monet]

Thanks for the outline! That does help.

I've been a little busy but I do plan to get this PR sorted out; for now can you confirm a few things:

1. For the "something broken with profiles loading or persisting these IO inputs", would you prefer fixing that before this PR goes in, or should I take a stab at it? Sounds like my "fix" isn't a fix and would break something else later/in other use cases I haven't tried.
2. You shared a snippet for running just the code runner without the app, which would presumably run into the issue with events. However, I currently have it running the app as well, and it does work fine on the GitHub workflows, so I think we could just go with that. It would be nice to have the backend capable of being run without the app, but probably not something we can address soon?
3. So the code snippet I showed above where I added an input item and binding to the profile, wasn't really working; I think I'm missing some step. Looking at your chart, is it the link between the input item binding and the RootData? If I dump the profile I just have:

<?xml version="1.0" ?>
<profile version="14">
    <inputs/>
    <library/>
    <modes>
        <mode>Default</mode>
    </modes>
    <scripts/>
</profile>

so I think I need to add something to the "inputs" section, but as far as I can tell, the code to do that is across a few files including the QMLs?

[WhiteMagic]

For the first point, it's probably easier if I take a look at it as I know that part of the code better so don't have to first understand what the goal was to begin with.

Yes Qt may be unhappy to emit signals if certain aspects of the stack are missing. I haven't looked into what the minimal components are that one needs to get the signal/slot mechanism to work. A bare bones QApplication instance might be sufficient though.

The profile is a bit (lot) convoluted, and quirky to handle all the things I want and that's before the UI has to make sense of it and handle user interactions :-) The Profile class just holds everything together and has a few random helpers. The two big groups that matter are the Library and the collection of InputItem instances.

As a preface, you have to think of actions as a tree structure that can be nested arbitrarily deep but does not allow cycles (the code should check for that iirc).

Library has one entry for each action that has been configured. Actions themselves can refer to other actions if they need that for their functionality, examples include condition, tempo, chain, etc, effectively everything that was a container in R13 plus some others. There is a special action called Root which just allows an InputBinding (more on that guy later) to hold a sequence of actions (think first response curve, then remap the resulting value). This root action is hidden from the user but was needed to make the UI side not get even worse.

InputItem refers to an individual physical input (axis, button, hat). They don't do much other than letting Gremlin know which input has been configured (avoiding the endless empty lists in R13). They are tied to actions via one or more InputItemBinding instances.

InputItemBinding Gremlin lets you define multiple independent sets of "reactions" to run for events on an input. For example, on an axis one set of actions applies a response curve before remapping the value to a vJoy axis while a second set of actions treats the input as a button and triggers a macro. Each of those two "behaviors" are captured by an InputItemBinding instance each. These are what actually hold the reference to a Root action which in turn would hold references to a bunch of other actions.

Each action has an id (UUID) that is used to refer to it from other actions or the InputItemBinding.

The actual code of adding the needed entry to the <input> tag should be in one place but the call originates from QML. IT's mainly this one function and I would not recommend to use it as the things it does are rather simple (create object and update references).

1. The QML triggers the creation of an InputItemBinding when the InputConfiguration.qml triggers the ui.profile.InputItemModel.newActionSequence slot.
   • This has an instance of the InputItem of the currently selected input in the UI.
2. Create an InputItemBinding instance and give it the InputItem instance as parent.
3. Update the InputItem set of bindings as well as the previous step only told the binding about the input.
   Adding actions now will place them inside the binding item and the input item just knows about it via the chain.

An example of what happens when an action is added is shown in gremlin.ui.action_model.ActionModel.appendAction. That does roughly what you have in your example. Use the PluginManager to create the actual action instance and then put it in the correct "container" of the "parent" action. So "children" for the Root action but could be "true actions" and "false actions" for the condition action, for example.

[code-monet]

Thanks for the help! I have the code working now, and I think it is "as it should be". Let me know what you think.

I will add more tests cases to this PR tomorrow, ran out of time for today. Meanwhile if you have any thoughts let me know.

[code-monet]

Today I was looking at how to change the response curve for testing purposes; part of this logic is implemented in ResponseCurveModel, which is a UI element. I dug around a bit, and it seems I could:

1. Create my own ResponseCurveModel and use that to configure the "data" instance.
2. Create my own InputItemBindingModel, let it create the ResponseCurveModel, and then use that to configure the data instance
3. Ignore ResponseCurveModel and configure the data instance directly from the test code, reimplementing (a lot of the) model code.
4. Refactor the ResponseCurveModel class to separate the business logic from the Qt decorations.
5. Try to get the ResponseCurveModel instance as created by the app itself.

I think I should do (4), but in that case I should probably go about that in a separate PR. Otherwise, (1) could also work and could probably be done with less code changes. I don't think the other options are promising/desirable. Thoughts?

[WhiteMagic]

I'll try to take a look at it later today. As for the response curve, it depends a bit on what parts you want to test. Though that entire action is probably the most complex one because it has high demands on being really fast so it gives instant feedback without lagging the UI out but also awkward coordinate frame conversions.

The gremlin.spline module is what actually holds the real curves and everything the QML model does around that is to help it behave and convert back and forth "correctly". In theory your option 4 is the best but it might cause a lot of headaches as most of the "logic" in there is presentation rather than backend data. Getting an actual instance might be tricky as it's nested quite a way down in the UI but I've never tried. Option 3 probably would defeat the point if the intent is to test the action and what it does on the UI side as then we'd need to keep test and QML model in sync.

I'd say depending on what level you want to test to be at I'd either try to refactor the "coordinate conversion logic" into the spling module. Or alternatively simply ignore it as the QML model has no bearing on the execution of the action itself as it modifies the curve only. Whatever the values specified via the UI are is what the functor will use, so if we don't care to make sure the UI doesn't break we can ignore the mess there and just test with fixed curve parametrizations.

[WhiteMagic]

You could also see if using Backend.newProfile works as that ensures everything Gremlin does for a new profile is also done here.

[WhiteMagic]

Looks good to me. Obviously writing the set for the profile is a bit awkward but I don't think there is much in the way of making it simpler other than having a bunch of helper functions. Though should those become useful they can always be added later.

Just as a heads up, I'll likely rename the IntermediateOutput class do VirtualDevice, I never liked the name and it's awkward because IO usually means input-output which is just weird here. I likely will also drop the uuid for the input_id on them as it adds a complications for probably no real benefit. This all comes from realizing there were a few holes in how those inputs were used/treated in Gremlin. The profile will have to be expanded to have a new section for those virtual device inputs (what the "device" actual has as inputs) and then the inputs with direct action sequences attached to them, i.e. defined in the device tab. Other actions that refer to it like in remap to io don't add those entries which is fine but caused issues when serializing everything as I hadn't thought far enough ahead.

[code-monet]

Let's go ahead and merge this PR, if you don't mind; I just rebased it on top of your latest commits. Integration tests are passing, although one unit test is broken, presumably because of commit e2c1694; unrelated to this PR.

I'll try approach 4 this weekend and expand the test coverage.

IO rename sounds good to me, agreed that it was confusing earlier. But consider that VirtualDevice might be too much like vJoy :) how about MemoryDevice, CacheDevice or IntermediateDevice?

[WhiteMagic]

Yeah naming things is hard and had similar thoughts about similarity to vJoy but that's kind of what the thing is but yeah more pondering :-)