FreeOpcUa / python-opcua

LGPL Pure Python OPC-UA Client and Server
http://freeopcua.github.io/
GNU Lesser General Public License v3.0
1.35k stars 658 forks source link

add_node API #236

Closed oroulet closed 8 years ago

oroulet commented 8 years ago

Hi, @destogl @bitkeeper

Looks like a lot of people are discussing these methods currently and adding extra arguments everywhere. I think we should take a step back and think about it.

Currently we have 4 methods create_object, create_variable, create_property, create_folder and create_method. All of them are available in Node class as add_xxx. Of this 2 (create_property and create_folder) are just commodity wrappers. These methods can be used to quickly populate an address space. This is however in many cases not the correct way to do it, one should create new object types and and then instantiate them

Thus I propose to add three new methods create_object_type, create_variable_type and create_data_type, that take all the necessary arguments: NodeClass, ObjectIds, etc... (Rmq: these methods are not random, they are the three missing types under types in standard address space apart from references) Then make the instantiate method work, this should reduce the need to add even more arguments to the existing methods and not add more methods to the Server class. Also remove the possibility to add objecttype as arg to create_object, this duplicate instantiate functionality it is confusing. This will also remove simplify every method and remove some bugs, it looks for example that we use ObjectTypeAttribute instead of ObjectAttribute in _create_object...

create_object(nodeid, bname) # if extra nodeid argument is present, it instantiate the given node create_variable(nodeid. bname, variant) # if extra nodeid argument is present, it instantiate the given node create_object_type(nodeid, bname, objecttype) create_variable_type(nodeid, bname, datatype) #value is always None create_reference_type TO_BE_DONE

then some simple wrappers for very common operations: create_folder(qname, baname) create_property(qname, bname, variant)

Then some methods for complex types: create_method(xxxxxx) # existing create_enum(xxxxxxx) # TO BE DONE

instantiate(parent_node, node_type, idx) #recursive instantiate but can be used for example for enum instantiate_one_node(parent_node, node_type, idx) # find better name, same as above, not recursive, maybe only internal.

maybe we can make create_object() and create_variable() call instantiate if objecttype or datatype is provided.....not sure

Populate address space without caring about types (current API)

obj = server.get_object_node()

myobj = obj_type.create_object(idx, 'MyObject')
myobj.create_variable(0, 'MySimpleVar', 0)
myobj.create_variable(0, 'MySimpleVar', 0, nodeid_of_custom_datatype)  # call instantiate in background
myobj.create_property(0, 'MyComplexVar', 0.2)

Example create custom object type, including some vars

baseobj_type = server.get_root_node().get_child(["0:Types", 
                                          "0:ObjectTypes", 
                                          "0:BaseObjectType"]) 

myobj_type = baseobj_type.create_object_type(idx, 'MyObject')
myobj_type.create_variable(0,'MySimpleVar', 0)
myobj_type.create_property(0,'MyComplexVar', 0)
myobj.create_variable(0, 'MySimpleVar', 0, nodeid_of_custom_datatype)

Create a custom Enum

# enums = server.get_node(ua.ObjectIds.Enumeration)
enums = server.get_root_node().get_child(["0:Types", 
                                          "0:DataTypes", 
                                          "0:BaseDataType", 
                                          "0:Enumeration"])

myenum_type = enums.create_data_type(idx, 'MyEnum')

enum_strings = myenum_type.create_variable(0, "EnumStrings", 
                                        [ua.LocalizedText("ok"),
                                         ua.LocalizedText("idle")])

Example create object instance object of custom type

obj_folder = server.get_node(ua.ObjectIds.ObjectsFolder)

myobj_type = server.get_root_node().get_child(["0:Types", 
                                          "0:ObjectTypes", 
                                          "0:BaseObjectType",
                                          "2:MyObject"]) 

# instanciate 
myobj_custome = obj_folder.create_object(idx, "MyObjectName", myobj_type.nodeid)

comments? I do not have time to work on this right now, but I can comment :-)

destogl commented 8 years ago

Hi @oroulet,

thanks for you comments. I partially agree with you. So I just add what should be different IMO.

Thus I propose to add three new methods create_object_type, create_variable_type and create_data_type, that take all the necessary arguments: NodeClass, ObjectIds, etc...

This is something we have in #231, but in Server class which is, if I understand good ideas of OPC UA, only plausible place to do so because:

  1. It only make sense for server to add custom types.
  2. This part is only readable for the clients (not sure for that).
  3. Adding create_xxx_type methods into node.py and exposing this to user can have fatal consequences, e.g. user adds type into Objects part of address space. For me is this something client should not be able to do.

Also remove the possibility to add objecttype as arg to create_object, this duplicate instanciate functionality is is confusing.

I would not do this, since we need this method to create custom types and instantiate objects. I would just create it as private or make it less discoverable (e.g. remove it only from node.py, but leave in manage_nodes.py)

But we need optional DataType arg in create_variable

@bitkeeper did it good in #235. After answering my comments we will merge :)

oroulet commented 8 years ago

@destogl we can have the method only available on server side if you want. Maybe put them on server object for discoverability, instead of node? 3) is not an issue, clients are not allowed unless you explicitely make them writable

I think we only need data type and objectids args for create_object_type and create_variable type. Then users will use instantiate function to create special nodes like enum. This will enforce good practice

Maybe we need create enum type method too..

oroulet commented 8 years ago

updated proposition.

destogl commented 8 years ago

Then I will merge #231 and create new example with @bitkeeper. After that we go after instantiate method.

No problem for the time, I am also doing it "in few minutes slots".

Just please confirm if I got you right: Semantics of create_object_type is actually "create object with type"? It that case it make sense to put it into node.py, but probably will be in logical collision with instantiate...

oroulet commented 8 years ago

@destogl The difference is is more than simply an attribute. Look at code, in create_object we call add_node with ua.ObjectAttributes structure while to create an object type we should call add_node with ObjectTypeAttritbute, they are almost similar but not completely and they may get more differences in the future.

That is also why I am confident we should have different methods, they are different call argument at lower level.

The instanciate method will then create an object from an object type by copying all attributes (and referemces) of an object type created with ObjectTypeAttritbutes to an object we create with ObjectAttrtibutes.

btw looks like _create_object uses ObjectTypeAttribute, this is a bug...should be ObjectAttrtibute. Hopefully we will fix a bunch of these bugs by splitting the functions

bitkeeper commented 8 years ago

A little bit late reaction, also have limit time ... sorry for the long story below: I agree for the most part with both your stories, but I have some suggestions.

Starting point: Object is a instance of a type.

This reflected by the way a standard address is organized:

Root
 |-Objects
 |-Types
   |-DataTypes
   |-EventTypes
   |-ObjectTypes
   |-...
 |-Views 

All types are created as a subtype from an existing one.

Node.create_xxx_type then would create type as 'child'. Where the relation(reference) is typical a HasSubtype. NodeClass is not required as argument because it could be taken from the parent Node.

Node.create_xxx then would create instance of a xxx type as child. Where the relation(reference) is typical an Organizes or HasComponent.

No need for a separate public instanciate method, this would be the responsiblity from the Node.create_xxx. If you want an empty one, just you a base type like BaseObjectType. This maps also more to the expectation most users will have, at least from what I see in feedback of the users.

Btw create_data_type shouldn't be confuesed with create_variable_type. Most users will create new data types and create variables with that data type, but almost never create new variable types.

#basic instances creation
Node.create_object(nodeid, bname, objecttype)
Node.create_variable(nodeid. bname, value, datatype) # in case of a variant the datatype is optional

# new types creation
Node.create_object_type(nodeid, bname)  # is then a replacement for add_subtype
Node.create_data_type(nodeid, bname) # in case of a variant the datatype is optional

#then some simple wrappers for very common operations:
Node.create_folder(qname, baname)
Node.create_property(qname, bname, value, datatype)

To prevent illegal/wrong use of the create_method_types. The createxxx(type) method can be protected to check if the parent is a legal one (create_datatype_type can only be performed on a parent that has the NodeClass set to DataType etc)

I don't think you directly need create_enum_type and other specialized datatype functions; If you go that way; there are still a lot of different basic datatypes to cover.

Should the create custom event type scheme be equalized with this new naming scheme ?

To finish my two cents; some examples:

Example create custom data type (enum)

# I don't use the predefined ObjectIds in the example, because in this way it 
# also clear how to extend an other already custom type.
# enums = server.get_node(ua.ObjectIds.Enumeration)
enums = server.get_root_node().get_child(["0:Types", 
                                          "0:DataTypes", 
                                          "0:BaseDataType", 
                                          "0:Enumeration"])

myenum_type = enums.create_data_type(idx, 'MyEnum')

enum_strings = myenum_type.create_variable(0, "EnumStrings", 
                                        [ua.LocalizedText("ok"),
                                         ua.LocalizedText("idle")])

Example create custom object type, including some vars

baseobj_type = server.get_root_node().get_child(["0:Types", 
                                          "0:ObjectTypes", 
                                          "0:BaseObjectType"]) 

myobj_type = baseobj_type.create_object_type(idx, 'MyObject')
myobj_type.create_variable(0,'MySimpleVar', 0)
myobj_type.create_variable(0,'MyComplexVar', 0, nodeid_of_custom_datatype)

Example create object instance object of BaseObjectType

obj_folder = server.get_node(ua.ObjectIds.ObjectsFolder)

# is equivalant to:
# obj_folder.create_object(idx, "MySimpleObjectName", ua.ObjectIds.BaseObjectType)
obj_simple  = obj_folder.create_object(idx, "MySimpleObjectName")

Example create object instance object of custom type

obj_folder = server.get_node(ua.ObjectIds.ObjectsFolder)

myobj_type = server.get_root_node().get_child(["0:Types", 
                                          "0:ObjectTypes", 
                                          "0:BaseObjectType",
                                          "2:MyObject"]) 

# this will also create instances of MySimpleVar and MyComplexVar
myobj_custome = obj_folder.create_object(idx, "MyObjectName", myobj_type.nodeid)
destogl commented 8 years ago

@bitkeeper: I agree with everything except with the example about creating custom object types (or any other types). For me it should look like:

myobj_type = etype = server.create_custom_object_type(2, 'MyObject', ua.ObjectIds.BaseObjectType, [('MySimpleVar', ua.VariantType.Int), ('MyComplexVar', ua.VariantType.Int, nodeid_of_custom_datatype)])

This is done for the custom events see server.py, create_custom_event_type method (I made this extension already in #233 (it is missing to add custom data types, but this I will do after finishing discussion). The reason for this is that Server should create all data types and there should not be possibility for the client to add custom types.

I also agree with @oroulet that we add creat_xxx_type functions which will accept additional datatype argument. This functions are not for creating types but for creating stuff with defined types. They will be call the same functions _create_xxx as the create_xxx functions. It is just to make API for further users clear.

If you could update #235 with create_xxx_type functions it would be great :)

oroulet commented 8 years ago

@bitkeeper I mainly agree with your examples and I am integrating them in first post with one change: I still want want an instantiate method :-) because in your example the instantiation is only implicit in create_object and in fact instantiate possibly many other things than objects

I see the create_xxx methods as simple function to create standard objects and variables when people do not care about creating types then instantiating. Then use the create_xxx_type to create type trees which can get quite complex (the standard Server object tree for example) Then instantiate them using instantiate function. or maybe implicitly inside create_object

@destogl your proposed syntax only support generating objects with one layer, this is too restrictive. It is already an issue with Events but while I am guessing deep Events tree will be rare, deep type tree will be/are common

@destogl The spec support create types on client side so I think we should not disable it. Anyway on a standard server clients will not be allowed to create types in Types tree, but they can create their own under Objects if they want (Thinking about it maybe they currently can but this is a bug we should not allow for that in default setup :-) )

oroulet commented 8 years ago

Trying to write the examples I understand that we would like the create_xxx methods to instantiate. This really require to finish the instantiate function first... and see if it does not bring unexpected difficulties.

Or maybe we should make the create_xxx variable only set datatype or ObjectId and not try to instantiate children (same as the already proposed PRs). This is a completely different code but might be confusing for new users Then make the instantiate function work for the more general case like instantiating the entire Server tree

bitkeeper commented 8 years ago

@destogl About creating the object and providing a list of vars to create: And what about the methods, child objects, properties and references ? Providing them all would create a kind of switch army knife method with a lot of arguments. And sometimes you also need to set a specific namespace for a variable, where does this fit ?

So I prefer to leave them all out and just add few lines of extra code.

bitkeeper commented 8 years ago

@oroulet Haha your are the boss, so it is your decision in the end ;-) It seems mainly a discussion about the terminology ... I just try to plead for my view angle one more time and then I shut up :-)

The orginal proposed scheme (for clarity only the example of objects is provided):

create_object  # create object instance of BaseObjectType
create_object_type # create object instance of any type, but don't create the full interface (like vars, props etc)
create_custom_object_type # create a new type of an object not instance
instantiate # the same as create_object_type, but node create the full interface

Try to imagine a discussion with a new user of the library about the use of the high level functions:

My proposed scheme:

create_object  # create object instance of any type, but default BaseObjectType at the same time 
create_object_type # create a new type as subtype of the current node

Every object instance is of a certain type, even if not specified then it is the basic type.

Reduces the 4 functions to only 2 clear defined functions:

Create_object/add_object is already compatible with existing version when you leave out the optional objecttype, because an instance of ObjectBaseType contains an empty interface. Only user which already uses the additional argument for objectype have to change the code, being available for only a very short time I don't think this is a problem.

If you really need it to create an object instance, without instantiate stuff, for some voodo functionality, don't make it an high level function of the node. Call it directly from manage_nodes.

Of course you can reuse the already existing function names:

add_object  # create object instance of any type, but default BaseObjectType at the same time 
add_object_subtype # create a new type as subtype of the current node

Phuff congratulations you reached the end, please shoot !

Offtopic: Personally I would prefer defining the types in a xml address space (by hand or by tool) above coding the by hand. And then import the xml file and create the instance from code. Static nodes can even be created in the xml address space. It makes it easier to edit, review, share and reduces the code. With an XSLT tranformation it is even possible to generate the python code from it.

oroulet commented 8 years ago

@bitkeeper do you want create_object to also instantiate children of type object?

bitkeeper commented 8 years ago

@oroulet yes I would like to.

zerox1212 commented 8 years ago

I'm not on the same level as you guys, but as a user I typically want to operate like this.

  1. Add any custom types to the server address space. I would think server.create_object_type would work good, but if you think clients should have access then I don't care where the function is located. In other words I just want to make a new object types what inherits something like BaseObjectType, add some variables/properities to it and give it a meaningful name.
  2. After that when I make a new node, I just want to call create_object and tell it what object type it should be an instance of. Maybe could even make a method on the node like you guys are talking about myObj.creat_child_obj(name, obj_type)

Looking forward to what happens.

oroulet commented 8 years ago

I am creating PR for create __xxx_type methods, then we will have a look at instantiating, anyway internally we have no choice but browse recursively tree and copy attributes.