asterics / AsTeRICS

The Assistive Technology Rapid Integration & Construction Set
http://www.asterics.eu
Other
55 stars 27 forks source link

ASAPI's getComponentProperty and setComponentProperty do not access actual component property variables #123

Open mariokom opened 8 years ago

mariokom commented 8 years ago

The need

Implement a web client that is able to monitor specific AsTeRICS components. These components are periodically change some of their property values

The problem

The changes are not visible from any external (neither ASAPI nor REST) client.

The cause

ASAPI's getComponentProperty uses this:

DeploymentManager.instance.getCurrentRuntimeModel().getComponentProperty(componentID, key);

The line above is accessing property variables created by Deployment manager and not the component's property variables (unfortunately there are two seperate instances for the same property).

On the other hand this line:

DeploymentManager.instance.getComponentProperty(componentID, key);

is directly accessing the components property variables ( prop* ). ASAPI does not currently provide this kind of functionality.

The solution

Introduce two new ASAPI functions able to handle actual component property variables and expose them through REST api ( getExplicitComponentProperty, setExplicitComponentProperty ??? )


Comments? objections?

deinhofer commented 8 years ago

Unfortunately the naming of IComponentType, IComponentInstance and IRuntimeComponentInstance is very confusing, I think. IMHO there should be 2 levels: IComponentType: describing the metadata, type of a component/plugin IRuntimeComponentInstance: real instance of a given IComponentType

The question is, if the ASAPI method getComponentProperty should refer to the component type properties or the runtime property values. I interpreted it as runtime property values and I think that's what is actually needed. DeploymentManager.instance.getComponentProperty returns the runtime property values by calling the method IRuntimeComponentInstance.getRuntimePropertyValue (which is actually the best name I think). About the solution: I think get|setRuntimePropertyValue would be a good name. I don't think that ASAPI has to provide the metalevel of the properties directly, because if somebody is interested he/she can fetch the bundle descriptors with getComponentDescriptorsAsXml().

Cheers

Am 09.06.2016 um 17:20 schrieb mariokom:

*The need*

Implement a web client that is able to monitor specific AsTeRICS components. These components are periodically change some of their property values

*The problem*

The changes are not visible from any external (neither ASAPI nor REST) client.

*The cause*

ASAPI's getComponentProperty uses this:

DeploymentManager.instance.getCurrentRuntimeModel().getComponentProperty(componentID,
key);

The line above is accessing property variables created by Deployment manager and not the component's property variables (unfortunately there are two seperate instances for the same property).

On the other hand this line:

DeploymentManager.instance.getComponentProperty(componentID, key);

is directly accessing the components property variables ( prop* ). ASAPI does not currently provide this kind of functionality.

*The solution*

Introduce two new ASAPI functions able to handle actual component property variables and expose them through REST api ( getExplicitComponentProperty, setExplicitComponentProperty ??? )


Comments? objections?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/asterics/AsTeRICS/issues/123, or mute the thread https://github.com/notifications/unsubscribe/AEaF8t52KpvPPMaCYhxSRU2FHIoT4SGKks5qKC9QgaJpZM4IyF1f.

Martin Deinhofer, MSc Research and Development, International Projects

University of Applied Sciences Technikum Wien Hoechstaedtplatz 6, 1200 Vienna, Austria, Europe T: +43 1 333 40 77-297, F: +43 1 333 40 77-99 297 E: martin.deinhofer@technikum-wien.at I: embsys.technikum-wien.at

mariokom commented 8 years ago

I agree that the ASAPI's set|getComponentProperty are unecessary.

I will add the new functions with the names you 've proposed. With this addition - and if a plugin is implemented properly - an external client can monitor Asterics on a component level, which is something that was missing before from the API.

for example: in a MidiPlayer component, each time a note is played, write its information on property variables. A client then can monitor the property values and reproduce the same melody.

Thank you Martin for your comments

deinhofer commented 8 years ago

Hi Marios,

Please wait a second, when thinking more about the topic I think there are some misunderstandings.

Actually, the stated problem is not existent, because if open the C#-ACS, connect to the ARE and change the property of a plugin it's is changed in the runtime property instance. So actually it works, it's just the name that is not very intuitive. Secondly, the methods get/setPortProperty and get/setChannelProperty do the same for the respective category (port properties and channel properties). So if you change the names, please change it for all of them. As time is close to the next release, I think we should postpone this API incompatible step to the last phase of P4All. Additionally, let's discuss how to fully support the synchronization/data flow of property values, event listening/triggering and port data propagation between the ARE and multiple clients.

As I understand we currently have: ) REST API to set | get runtime property values ) SSE mechanism for model lifecycle events and a REST API for publish/subscribe to such events *) websocket plugin that can forward and receive connected port data between the ARE and webclients

What's still missing: ) forward / receive plugin events (through SSE?) ) Support multiple clients: If one client or the ARE gui changes a value of a property, also notify all listeners by pushing the change (otherwise they would have to poll all the time). ) forward / receive selected port data (through websocket) ) Maybe some of the features should be possible for both REST/SSE and websocket interfaces...

What do you think? Martin

Am 10.06.2016 um 15:56 schrieb mariokom:

I agree that the ASAPI's set|getComponentProperty are unecessary.

I will add the new functions with the names you 've proposed. With this addition - and if a plugin is implemented properly - an external client can monitor Asterics on a component level, which is something that was missing before from the API.

for example: in a MidiPlayer component, each time a note is played, write its information on property variables. A client then can monitor the property values and reproduce the same melody.

Thank you Martin for your comments

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/asterics/AsTeRICS/issues/123#issuecomment-225188628, or mute the thread https://github.com/notifications/unsubscribe/AEaF8twKnWIXgc8Lv7XSY-O1anvJg82zks5qKW0TgaJpZM4IyF1f.

Martin Deinhofer, MSc Research and Development, International Projects

University of Applied Sciences Technikum Wien Hoechstaedtplatz 6, 1200 Vienna, Austria, Europe T: +43 1 333 40 77-297, F: +43 1 333 40 77-99 297 E: martin.deinhofer@technikum-wien.at I: embsys.technikum-wien.at

mariokom commented 8 years ago

Hi Martin,

Sorry in advance if the following response is too technical :)

Actually, this is not what i wanted to achieve. My scenario is to change the variable from the component itself and the change to be visible from the external client. Why is the ACS able to achieve this? It changes both variable properties (AsapiSupport - line 1133, 1138) every time a client calls the setPropertyValue, but the getPropertyValue function reads what has been written by the line 1133 (which in this case there is no difference). Now, if a component changes its own variable (equivalent to the line 1138), it will not be visible by an external client. We could use the line 1133 in the component's setRuntimePropertyValue (to change both variables), but in my opinion this is a very bad practice since it creates a direct dependency from the component to the middleware. Additionally, i was not able to identify a similar behevior with the get/setPortProperty and get/setChannelProperty cases. With the above in mind, Is there something i did not understand?

All of the 'still missing' points are very good. The 'complication' is that the solution(s) - as i imagine it - needs DeploymentManager refactoring. I think at some point we should discuss these futher (may be on a skype meeting?).

Marios

mariokom commented 8 years ago

Forgot to mention that the changes are part of a P4All deliverable which must be submitted before the 10nth of July. Do we plan the next release before or after this date?

deinhofer commented 8 years ago

Hi Marios,

I think the mistake in the current implementation is that the IRuntimeModel instance is also updated with property changes, but this is IMHO not necessary. The IRuntimeModel instance only describes the initial configuration of components (initial property values,...) and the interconnections of components by data ports and event channels.

The IRuntimeComponentInstance should be the only place to hold the current states of a component. We could use the abstract class AbstractRuntimeComponentInstance#setRuntimePropertyValue to add generic functionality like change listener notification. Unfortunately this will only work, if all the plugins use super.setRuntimePropertyValue in their implementations.

Having a skype call is a good idea. The release must definitely be before July 10th, preferably one week before, because we also need it for our Summer School. Do you mean that the multiple client synchronization must be implemented for this release (2.8) already? This sounds very tough!! :-)

Cheers Martin

mariokom commented 8 years ago

Ok lets move this conversation on skype. No i was not planning to implement it for 2.8 (i can sense a small panic attack just by mentioning it :p ). I was asking just to figure out if i needed to maintain a different version of AsTeRICS just for the deliverable. Since the release will be taking place before, i think we are fine.