feat: emberplus api expansion

[phillipivan]

Expands emberplus api by exposing internal variables, custom variables, and action recorder controls. Internal variables are exposed as read only string or int parameters, custom variables are coerced to string type parameters as the likely best compromise between the dynamic typing of companion variables and the strict typing of ember plus parameters, they are read/write.

• [x] Get list of internal and custom variables from variable controller
• [x] Define ember plus tree
• [x] Handle setValue for Custom Variables
• [x] Handle actionRecorder controls and state (mapped via serviceApi)
• [x] Monitor for changes in custom variable count and reinit server (as per changes for page count and button matrix size). NB, At present, on start up, the ember plus server is initialised before the variables are defined it seems and the service needs to be toggled before any variables are available in the tree (internal and custom).
• [x] Listen for variable change events, rather than polling for better efficiency and responsiveness
• [ ] Decide how to best handle unstable node paths for variables (between companion restarts)
• [ ] Update user docs

[phillipivan]

Each variable is defined in its own node as behaviour is poor when defined in a flat structure:

Action recorder control

[phillipivan]

@Julusian I would welcome any advice you have regarding the last two issues, and any other feedback you have.

[Julusian]

Monitor for changes in custom variable count and reinit server

I would like to see an event based flow for this. I would be tempted by making VariablesCustomVariable extend EventEmitter, (see elsewhere for how to do the types for this) with an event perhaps definitionChanged: [id: string, info: CustomVariableDefinition | null]. This would need emitting anywhere which changes the info of a variable. This api can then listen for that event (with a debounce) and reinit (if needed, some changes could be persistValue or the description changing)

the ember plus server is initialised before the variables are defined it seems

perhaps this is generally wrong, and all the apis are being started too early?
we don't start the http (admin ui) api until after everything has been initialised, perhaps all these apis should be delayed until the same point in time?

Listen for variable change events,

ah, I read the code before your comments..
Yeah, if you search for variables_changed you will see the main flow for how we handle changes elsewhere, it should be easy enough to hook into that. Maybe that event should be expanded to include an array of connectionLabels this originated from, which would allow you to check really quickly if it was an interesting update.

[phillipivan]

Monitor for changes in custom variable count and reinit server

I would like to see an event based flow for this. I would be tempted by making VariablesCustomVariable extend EventEmitter, (see elsewhere for how to do the types for this) with an event perhaps definitionChanged: [id: string, info: CustomVariableDefinition | null]. This would need emitting anywhere which changes the info of a variable. This api can then listen for that event (with a debounce) and reinit (if needed, some changes could be persistValue or the description changing)

Would this also be able to emit an event when there is a change to the internal variables? I am thinking specifically of the connection status variables which are created/deleted with each connection.

Tangentially, how practical is it to pull the variable description (for custom or internal variables). This could be used to populate the description field of the ember parameter with some more contextual info. I dont think it would be worth tearing down and re-initialising the server for a description change, however.

the ember plus server is initialised before the variables are defined it seems

perhaps this is generally wrong, and all the apis are being started too early? we don't start the http (admin ui) api until after everything has been initialised, perhaps all these apis should be delayed until the same point in time?

Thats a good idea, otherwise it is certain that on startup the emberplus api will need to be initialized twice, regardless of if this is automated or not. Is this something you would be happy to do?

Listen for variable change events,

ah, I read the code before your comments.. Yeah, if you search for variables_changed you will see the main flow for how we handle changes elsewhere, it should be easy enough to hook into that. Maybe that event should be expanded to include an array of connectionLabels this originated from, which would allow you to check really quickly if it was an interesting update.

I started to play with the variable_changed event, but haven't gotten it working successfully yet, I will look at how it is implemented elsewhere. I will have a look at also emitting a connectionLabels array.

With regards to the action recorder state, the only related variable is the action_recorder_action_count which would be insufficient to determine the run state of the recorder. Unless you are thinking of something else I have missed?

Finally, and less directly relevant to the contents of this PR: I had also wanted to expose connection variables, but given a change in variable definitions requires a server reinit (or so it seems) to handle, I'm not sure if or how this could be handled efficiently. While this is already occurring for changes to custom variables, button matrix size, page count, and connections (since that means a change to the internal variables), these are things that tends to happen during configuration, not operation, so I am semi-comfortable living with this (but only semi - since working in a large facility 'configuration time' for one part of the system may easily be operation time for another). I don't think it would be workable to do this any time a module updates its variable definitions.

[phillipivan]

I think I have addressed most of that. There is no more polling whatsoever, all updates are from event listeners. I also added one for the action recorder state.

Server restarts are automated and debounced. This also crudely addresses the initialization order issue.

I am surprised to see that some deleted variables (custom, and internal connection variables) are still returned from the getConnectionVariableDefinitions method.

Such as:

Which has been deleted:

Or: And New: From a connection list of:

While I am not especially concerned about no longer relevant variables persisting in the tree - it does bring about a subtle problem, that the node numbers of the variables may change after a full companion restart where the old variables are flushed.

However after a full companion restart we get:

The same kind of effect will be observed from renaming a connection.

Some thought needs to go into how best to handle this - as the potential for unstable ember paths will eventually cause someone heartache - and its not just potential instability of the paths for those specific variables, but all others that are normally defined after them in the list. @Julusian is it deliberate (I assume so) that these variables persist internally even after they been deleted from the user perspective?

What wont change at least, is the textual paths to the parameters, such as Companion Tree/variables/internal/action_recorder_action_count/integer

[phillipivan]

Currently my best idea for the unstable node numbering, which wouldn't really solve it, but perhaps limit the scope of impact would be to sort the internal variables into a few categories connections, pages, surfaces, date, time, system, instance, based on the variable name.

It will clutter up the code some, and is only a mitigation not a solution, so I am not sure if it would be worth the effort. Thoughts?

It may be we have to advise to use textual node paths whenever possible.

Also, since I have never had reason to use them, are the jog and t-bar variables integer or do I need to make these float? I'd prefer to avoid using floats if possible, due to issues with the implementation.

[phillipivan]

@Julusian I think this is ready for another review. I have added some documentation. Curious to know if you think it would be worth sorting the interval variables into subcategories. I have not done so for now, as it would remain at best a band aid.

[phillipivan]

Is there a practical way to pull the description of the internal variables? I have figured out how to do that for the custom variables, and map those descriptions to the respective ember nodes, but I haven't managed to find how I could do this for the internal variables. It's really just a nice to have, so if it's going to be excessively fiddly I won't pursue it.

[phillipivan]

@Julusian did you clock my question about the types for the t-bar and jog interval variables? Do I need to make these float, or will they work as ints?

[Julusian]

did you clock my question about the types for the t-bar and jog interval variables? Do I need to make these float, or will they work as ints?

I missed that, I think int should be fine. Tbh maybe this release I should try and finally remove these, as there is a better multi-surface flow that can be used instead now..

is it deliberate (I assume so) that these variables persist internally even after they been deleted from the user perspective?

No, this isn't deliberate. Its likely just a case of bad handling on our part, forgetting to consider that they might need to be have the values/definitions unset

Is there a practical way to pull the description of the internal variables?

Doesnt look like it. Perhaps some stuff needs to be exposed from VariablesInstanceDefinitions to allow this

[phillipivan]

No, this isn't deliberate. Its likely just a case of bad handling on our part, forgetting to consider that they might need to be have the values/definitions unset

Its not really critical for now, and solving it wont address the inherent numerical node instability from a dynamically generated list of variables, but perhaps this can be addressed in the future

Is there a practical way to pull the description of the internal variables?

Doesnt look like it. Perhaps some stuff needs to be exposed from VariablesInstanceDefinitions to allow this

Done.

So the only other thing, I think, might be the api initialization sequence - at the moment it spams the logs during init as it detects the new variables after it has spooled up and calls the debounced restart. But perhaps that should be in a separate PR?