You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In the recent discussions about hierarchy modifiers and similar things, it has emerged that our API has some flaws that should be fixed. This is mostly about the definition of connections between modules. I propose to do the following changes:
Always use of "virtual" hierarchy
We currently have two-and-a-half hierarchical views of the variable/module structure:
1a) The native C++ structure defined by C++ object ownership,
1b) a runtime version derived from this, resp. based on the owner passed as first constructor arguments, and
2) A runtime view which is derived from 1b) but takes into account the hierarchy modifiers.
1a) and 1b) must always be logically identical for standard ApplicationModules etc., but there are special cases e.g. where 1a) does not even exist (ControlSystemModule, DeviceModule - both create their variables on the fly and do not even have accessors for them).
So far so good. Problems now appear becase 1b) and 2) are easily confused. A standard ApplicationModule or ModuleGroup will use 1b) when directly used with connectTo(), or when the [] resp. () operators are used on them. Only if findTag() is used, the so-called "virtual" hierarchy 2) is created and returned as a search result.
My proposal is to always refer to 2) when connectTo() or the [] resp. () operators are used. This should make it much more obvious what happens. We should maybe find a better name for this than "virtual", though...
Prevent access to the C++ hierarchy
The original idea of ApplicationCore only had 1) in mind. The idea was to use the >> operators to define each variable connection individual, using the accessors directly by writing down the C++ class member. The ApplicationCore accessors implicitly convert into VariableNetworkNodes (and have the >> operator defined to help C++ with this), so connections can be defined with them. This approach has been basically abandoned anyway in favour of connectTo() etc, where it is no longer required to make individual connections.
My proposal is to remove the "seconds life" of the ApplicationCore accessors as a VariableNetworkNode. This does not even prevent doing individual connections, since it is still possible to access individual variables using the [] and () operators, but then through the "virtual" hierarchy instead of the C++ one. What will no longer be possible is the access to an individual accessor when it has been moved out of its native place in the C++ hiearchy and falls on the same place with another accessor in the virtual hierarchy. So one wouldn't be able to connect accessors like this somehow in a different way, but I think one shouldn't even be able to do that. One will still find a (much cleaner) solution e.g. with tags to achieve what is wanted.
Change policy from "everything is pubic" into "everything is private" for modules
This can mostly be a recommendation and will reflect itself in the examples and the ready-made modules being shipped with ApplicationCore. Since we no longer want people to make connections based on the C++ hierarchy, the argument why all members (accessors and sub-modules at least) of the modules need to be public falls away.
Further thoughts
These changes might improve the situation, but one point still smells to me: If you call connectTo() on a module, even if the module is "virtualised" first (converted into hierarchy 2)), the highest level of the connectTo() operation is still defined through the C++ hiearchy, so somehow 1a) and 1b) pop out here again. Maybe we should even disallow connectTo() on modules (other than VirtualModule) as well. We could still navigate through the entire hierarchy 2) using the [] and () operators (if we create a function with gives us the root point, (*this) would work but looks ugly), there is no need for any C++ hierarchy knowledge in the connection code as well. On the other hand, if you just have instantiated a module in the connection code (e.g. based on a configuration variable), you already have the C++ hierarchy at hand.
Final remarks on all points
We should carefully think this through. Maybe there are better solution for some aspects, or some things are still missing.
Some of the changes will definitively break existing applications, but probably this is still easy to fix at this point. We should therefore also not wait for too long to make a decision.
The text was updated successfully, but these errors were encountered:
What is also required: A better concept to identify variable endpoints (network nodes) in debug output. This may even depend on the context. When getting a test stalled exception, it would probably be best to have the full C++ hierarchy path, in other contexts we might (also?) be interested in the "virtual" variable name. It is also important that one can immediately see which notation is used there.
In the recent discussions about hierarchy modifiers and similar things, it has emerged that our API has some flaws that should be fixed. This is mostly about the definition of connections between modules. I propose to do the following changes:
Always use of "virtual" hierarchy
We currently have two-and-a-half hierarchical views of the variable/module structure:
1a) The native C++ structure defined by C++ object ownership,
1b) a runtime version derived from this, resp. based on the owner passed as first constructor arguments, and
2) A runtime view which is derived from 1b) but takes into account the hierarchy modifiers.
1a) and 1b) must always be logically identical for standard ApplicationModules etc., but there are special cases e.g. where 1a) does not even exist (ControlSystemModule, DeviceModule - both create their variables on the fly and do not even have accessors for them).
So far so good. Problems now appear becase 1b) and 2) are easily confused. A standard ApplicationModule or ModuleGroup will use 1b) when directly used with connectTo(), or when the [] resp. () operators are used on them. Only if findTag() is used, the so-called "virtual" hierarchy 2) is created and returned as a search result.
My proposal is to always refer to 2) when connectTo() or the [] resp. () operators are used. This should make it much more obvious what happens. We should maybe find a better name for this than "virtual", though...
Prevent access to the C++ hierarchy
The original idea of ApplicationCore only had 1) in mind. The idea was to use the >> operators to define each variable connection individual, using the accessors directly by writing down the C++ class member. The ApplicationCore accessors implicitly convert into VariableNetworkNodes (and have the >> operator defined to help C++ with this), so connections can be defined with them. This approach has been basically abandoned anyway in favour of connectTo() etc, where it is no longer required to make individual connections.
My proposal is to remove the "seconds life" of the ApplicationCore accessors as a VariableNetworkNode. This does not even prevent doing individual connections, since it is still possible to access individual variables using the [] and () operators, but then through the "virtual" hierarchy instead of the C++ one. What will no longer be possible is the access to an individual accessor when it has been moved out of its native place in the C++ hiearchy and falls on the same place with another accessor in the virtual hierarchy. So one wouldn't be able to connect accessors like this somehow in a different way, but I think one shouldn't even be able to do that. One will still find a (much cleaner) solution e.g. with tags to achieve what is wanted.
Change policy from "everything is pubic" into "everything is private" for modules
This can mostly be a recommendation and will reflect itself in the examples and the ready-made modules being shipped with ApplicationCore. Since we no longer want people to make connections based on the C++ hierarchy, the argument why all members (accessors and sub-modules at least) of the modules need to be public falls away.
Further thoughts
These changes might improve the situation, but one point still smells to me: If you call connectTo() on a module, even if the module is "virtualised" first (converted into hierarchy 2)), the highest level of the connectTo() operation is still defined through the C++ hiearchy, so somehow 1a) and 1b) pop out here again. Maybe we should even disallow connectTo() on modules (other than VirtualModule) as well. We could still navigate through the entire hierarchy 2) using the [] and () operators (if we create a function with gives us the root point,
(*this)
would work but looks ugly), there is no need for any C++ hierarchy knowledge in the connection code as well. On the other hand, if you just have instantiated a module in the connection code (e.g. based on a configuration variable), you already have the C++ hierarchy at hand.Final remarks on all points
We should carefully think this through. Maybe there are better solution for some aspects, or some things are still missing.
Some of the changes will definitively break existing applications, but probably this is still easy to fix at this point. We should therefore also not wait for too long to make a decision.
The text was updated successfully, but these errors were encountered: