JKISoftware / JKI-State-Machine-Objects

Object-oriented framework for LabVIEW based on the JKI State Machine
BSD 3-Clause "New" or "Revised" License
96 stars 55 forks source link

Removed ValidateCharacters.vi from Register Subsystem.vi #13

Closed 3esmit closed 7 years ago

3esmit commented 8 years ago

SMO namining convention suggest to use dot to separate, but Registry removes dots from names, so when I need to find a shared resouce I need to call it without dots.
I don't see any usage of this, just more complication. Also, this seems to make it incompatitble with onStart.vi call of ListSharedResources.vi when using dots on the class name. I think it would always create a new instance of that shared resource; Currently I'm working SMO without ValidateCharacters.vi.

3esmit commented 8 years ago

JKI repository currently have this vis. image Added note to identify the VI removed.

Assume this enumarateStaticDependencies.vi: image

Endurance.DB is used in a lot of subsystems, including Endurance.Test, Endurance.UI.Config.User, etc. All this subsystems are somehow nested to Endurance.lvclass SMO.

enumarateStaticDependencies.vi is called by onStart.vi image

3esmit commented 8 years ago

Also fixing the onStart.vi scope of Shared Instance verification

This: image

Becomes this: image

francois-normandin commented 8 years ago

Hi Ricardo, I believe you are confusing two types of dependencies: inheritance and composition. The dot notation is a convention for inheritance. However, there is no single convention that I know of for composition/aggregation patterns. Certain characters, including dots, can cause conflicts in maintaining an easy to parse composition dependency lineage.

For example, let's assume you have an instrument derived from "Instrument.lvclass" and it depends on a subsystem of type "tool.modelA.lvclass". The naming convention for inheritance informs us that "tool" is a generalization of the "modelA" class. However, it does not inform you that the "Instrument" class depends (through composition) on the "modelA" class.

image

Take this screenshot above from LaunchDependency.vi. When you launch a dependency, statically or dynamically-defined, the "owner attribute" is constructed to include the lineage of the object, i.e. its ownership tree. In this example, the Qualified Name of the tool.modelA object created by the Instrument object would be "Instrument.toolmodelA". Note the use of a single dot to separate the different levels in the ownership tree. This lineage is constructed from the unique name of the object, not its class name on disk. By removing the validate character method in the registry, you are affecting the ability of the framework to construct a unique signature that would allow it to dynamically route messages to an object based on the composition of the system at runtime. There are no conventions for composition. We could have used "/" to separate the object layers in the composition tree, but then it would also potentially conflict with a developer's wish to extend the framework using web services and require encoding or escape characters. The choice of the dot is to anticipate that future extensions of the framework will not force encoding schemes to be used for things such as configuration management, URL pasrsing or dynamic message routing to service remote calls through a generic server, etc.

It is by no means a universal solution, but this is the one currently chosen for the framework. Before making changes that affect the behavior of a bunch of other use cases, we should have a discussion about advantages and disadvantages of using this notation for composition. I'm happy to entertain this discussion and see how it impacts present and future development of SMOs.

Shared Resources

You are using the list of shared resources to access objects by the name of their classes, assuming there will only be one instance in memory and any point in time. This is an assumption that is not always valid. It might be for your specific application, but this is rarely the case for larger applications. For example, what if you have two identical pumps connected on COM1 and COM2? Both could be spawned from the same class but started with different configurations (serial port, baud rate, etc.). If you rely on a class name, you would not be able to differentiate at runtime without hardcoding the names. This is why a dependency should not be defined with its class name and why the dot notation does not apply to composition as a universal means of defining the objects. Your system could look like this, if you were to not give an explicit name to your dependencies:

Now, consider how you would store your persistent configuration for these objects. The first pump connected to COM1 would be labeled "Instrument.Pump" in your configuration string, whereas the second pump connected to COM2 would be lebeled "Instrument.Pump_001". What if you need to disable the first pump in your system, temporarily to make some tests? If you do not specifically name your dependencies and rely strictly on the class name at creation, you would be creating only one pump and therefore would have Instrument.Pump... which might load the wrong configuration file and try to connect to COM1 instead of the intended COM2, unless Instrument has correctly saved the configuration where you explicitely create your pump at runtime using the name "Pump_0001" in "enumerateStaticDependencies".

Concept of Shared Resources: Aggregation The proposed fix to onStart( ) is, from my perspective, removing a feature from the framework. The concept of shared resource is to reuse already existing components in a system instead of starting up a new one. In essence, launching a dependency can result in two things: the caller creates a new dependency and controls its lifetime (composition) or the caller uses a dependency owned by another object instance (aggregation). If your object does not find an object already created because it does not have the correct name, then the problem needs to be corrected upstream by specifyng the name that will be recognized.

Think of a smartphone with a single camera that can be used by multiple apps. The phone's top-level app will likely have a resource manager which creates, when needed, an object of type camera and gives permission to apps to make use of it. This resource manager creates the camera object, starts it when needed and allows apps that need it to request access to this shared resource. If a second app needs to use the camera, it does too. Neither of those apps "own" the camera object. They depend on the camera through aggregation. If you allow all apps to create their own camera instances, you'll create resource conflicts. This is the idea of assigning a resource as "shared".

You are correct that your Endurance Database is shared by multiple subsystems and should be aggregated in your objects. The implementation I recommend is to define your dependency cluster with the name EnduranceDB (no dot), such that you can load that from configuration within your "enumertateStaticDependencies" method if the name might change over time.

It is by design that the static dependencies check that a statically-defined object it needs is already a shared resource or not. If it is, the caller adds the existing object to its dependencies through aggregation. If it is not, it creates it and adds it to its dependencies through composition.

It is the responsibility of the object's owner to determine whether or not an object is shared. The proposed fix suggests that we should reverse this responsibility. The "shared resource" attribute as defined in "enumerateStaticDependencies" represents the desired behavior that the caller SMO wants to give to its dependency, should it be the owner of its lifetime. If it instead finds that the dependency already exists (and is not its owner), you will notice in the LaunchDependency method that the attribute will not be changed, because the shared key will not match...

3esmit commented 8 years ago

I see that remove invalid chars is added for a reason that is not well understood by me. I will keep that discussion in the issue you opened specially for this. About ListSharedResouces always trying to find object, even if is not marked as shared... It means that when setting up dependencies, I just need to set up as shared the "main" enumarateStaticDependencies, and the others would find it even if is setted shared resources to false? This seems to be very hacky, but I was always setting all my shared resources to T in main SMO and nested SMOs, but it seems that if it was false it would work anyway. My system works with a UI.ClimateSimulation, that is a UI to setup a setpoint table for values. It is used as non shared resouces by two different SMOs, the UI.Template and the UI.Config.General. I would imagine that if one of them would be shared, all of them would be pointing to the solo shared instance. I did change to prevent future problems, as I'm repeating names of SMOs instances if they are the same class and the only instance of this class in that level, as I find easier to do this way.

francois-normandin commented 8 years ago

This seems to be very hacky, but I was always setting all my shared resources to T in main SMO and nested SMOs, but it seems that if it was false it would work anyway.

I don't think it is a hack. Once the owner of a resource sets the "Shared = True" flag, then that resource becomes available to others. By having ownership on a subsystem, the caller remains the only one to be able to control its lifetime (using a shared key created from the dependency's GUID). All other subsystems that find it by name (or regex) from the list of shared resources available, will be able to use its public API.

I would imagine that if one of them would be shared, all of them would be pointing to the solo shared instance.

Yes, if one subsystem is a shared resources and subsequently launched subsystems find a perfect match for both the name of the object and its type, then it would use that instance already created.

I think I see how you understood the shared resource concept now. You thought that the enumerateStaticDependencies method was setting the shared resource flag and meant "please use a shared resource if you find it" when it actually means "please make this dependency available to others if I'm the first to create it". Both meanings make sense, from different perspective :-)

In this case, the enumeration of dependencies is a declaration of what they are. Here are some thoughts. Let me know if those make sense and it could be a start for documenting this feature...

image

1- Declare what are the subsystems that this SMO depends on. This method declares all the names and types of objects it needs to operate and what are the attributes/behaviors of those objects when created.

For each dependency needed: 2- Check if a shared resource with the exact name exists?

3- If none have been found, propagate the object type as defined in enumerateStaticDependencies. If a shared resource matches the name provided, check if it matches the type as well. If it does, propagate the shared resource reference. If it does not, propagate the type as defined in enumerateStaticDependencies.

4- Launch the dependency. If the dependency is a shared resource, add it to dependencies list (aggregation). If the dependency does not exist yet, create it, take ownership and start it with the attributes as defined (composition). If the "Shared Resource" attribute of the newly launched dependency is TRUE, then it becomes available for any subsequent objects to use.