HMIProject / open62541

9 stars 6 forks source link

Use single module and macro for derived node attribute types #128

Closed sgoll closed 1 month ago

sgoll commented 2 months ago

Description

This adds the remaining subtypes of NodeAttributes, i.e.

sgoll commented 2 months ago

This is an alternative approach to the enum type in #126 that sticks closer to the original types from open62541. It is not clear yet which approach is better, so feel free to discuss 🙂

sgoll commented 2 months ago

But I don't want to loose the server.add_node() method which supports all attribute/node types. Could we use generics for that method and for the Node struct? Then we wouldn't require an enum, would we?

The idiomatic way to enable this use-case would be a trait. I have added this in 39e79fb30c91a352ba4964ca3a41b5a7ffe061c5. Please take a look.

LeanderGlanda commented 2 months ago

Is there any reason why you called the trait AsNodeAttributes?

There is also the display_name which requires a method on all attribute types. So a more generic name than AsNodeAttributes would be more use-describing in the future.

For fields which are only present in two Attribute types (e.g. value_rank), would you also create a trait or simply define the methods in the structs?

The changes here mostly replace how the attribute types are defined. In #126 I added a Node struct. It uses the enum Attributes, so a Node can contain any attribute type. This Node struct was used to enable the generic server.add_node() method.

Now one could use the AsNodeAttributes as trait for a generic Node struct. Like this:

pub struct Node<T: AsNodeAttributes> {
    pub node_id: ua::NodeId,
    pub parent_node_id: ua::NodeId,
    pub reference_type_id: ua::NodeId,
    pub browse_name: ua::QualifiedName,
    pub type_definition: Option<ua::NodeId>,
    pub node_context: Option<NodeContext>,
    pub attributes: T,
}

Again, a different name than AsNodeAttributes would fit better. But in general, what do you think of using this declaration for Node? Does that meet your expectation of how this should work?

LeanderGlanda commented 2 months ago

In #133 I added the changes which i described above. So maybe take a look at that to investigate whether my requested changes here are good.

LeanderGlanda commented 2 months ago

I wouldn't necessarily say an addition, but a better way to handle all the different attribute types. So it replaces code from #126.