Incorrect instantiation of ObjectType from an imported XML model

[FMeinicke]

Describe the bug

I'm trying to instantiate an Object using an ObjectType defined in an XML NodeSet file created with UaModeler. The result differs from what UaModeler creates when I instantiate the same ObjectType directly in the model. Specifically, Organizes references from FunctionalGroups to Nodes that are Components of the ObjectType itself or e.g. a ParameterSet/MethodSet create new Nodes if I instantiate the type using asyncua, but not if instantiated directly in UaModeler.

To Reproduce

I created a very stripped-down example in UaModeler that illustrates what I mean. The example can also be found on GitHub for anyone willing to play around with this themselves: https://github.com/FMeinicke/asyncua-references-instantiate-issue

The gist is that I have a custom Device type derived from OPC UA DI's DeviceType. This type overrides the Identification FunctionalGroup, the ParameterSet, and the MethodSet. For all of these, my type changes the ModellingRule to Mandatory. Additionally, I create the Operational FunctionalGroup.
In the Identification FunctionalGroup, I add Organizes references to the Properties of the Component type which are made Mandatory by the Device type.
The ParameterSet gets a Variable and a Property as its child nodes, and the MethodSets gets a method. All of these are references from the Operational FunctionalGroup using Organizes references again.

The final model looks like this:

To compare the instantiation of UaModeler to the one from asyncua, I also add an instance of MyDeviceType to the DeviceSet Object in UaModeler.

Using asyncua, I create a basic server that loads the OPC UA DI NodeSet XML and my custom one (note that I have to edit the XML file created by UaModeler to make it compatible with asyncua because UaModeler uses OPC UA v1.05.03 but asyncua only works with v1.05.02 - I left this out here for brevity).

import asyncio
import logging
from pathlib import Path

import asyncua
from asyncua import Server, ua

_logger = logging.getLogger(__name__)


async def load_nodesets(server: Server) -> tuple:
    nodeset_dir = Path(__file__).parent.joinpath("nodesets")

    _logger.info("Importing OPC UA DI nodeset...")
    await server.import_xml(nodeset_dir.joinpath("Opc.Ua.Di.v1.05.03.NodeSet2.xml"))
    opc_ua_di_ns = await server.get_namespace_index("http://opcfoundation.org/UA/DI/")

    _logger.info("Patching example nodeset...")
    # ...

    _logger.info("Importing example nodeset...")
    await server.import_xml(nodeset_dir.joinpath("asyncua-references-instantiate-issue.xml"))
    example_ns = await server.get_namespace_index("http://example.com/UA/")

    return opc_ua_di_ns, example_ns

After this, I instantiate a second Object in the DeviceSet Object using MyDeviceType:

async def instantiate_my_device(
    device_name: str, server: Server, opc_ua_di_ns: int, example_ns: int
) -> asyncua.common.Node:
    type_node = await server.nodes.base_object_type.get_child(
        (
            f"{opc_ua_di_ns}:TopologyElementType",
            f"{opc_ua_di_ns}:ComponentType",
            f"{opc_ua_di_ns}:DeviceType",
            f"{example_ns}:MyDeviceType",
        )
    )

    _logger.info(f"Instantiating new device node for {device_name}...")

    device_set_node = await server.nodes.objects.get_child(f"{opc_ua_di_ns}:DeviceSet")

    return await device_set_node.add_object(
        ua.NodeId(5501, example_ns),
        ua.QualifiedName(device_name, example_ns),
        type_node,
        instantiate_optional=False,
    )

And the main function, just for completeness:

async def main() -> None:
    server = Server()
    await server.init()
    server.set_endpoint("opc.tcp://0.0.0.0:4840/example/server/")

    opc_ua_di_ns, example_ns = await load_nodesets(server)

    await instantiate_my_device("MyDevice2", server, opc_ua_di_ns, example_ns)

    _logger.info("Starting server!")
    async with server:
        _logger.info("Server started!")
        while True:
            await asyncio.sleep(1)


if __name__ == "__main__":
    logging.basicConfig(level=logging.DEBUG)

    try:
        asyncio.run(main())
    except KeyboardInterrupt:
        print()

I then connect to the server using UaExpert.

Expected behavior

The two instances of MyDeviceType should be equal. Instead, only the instance created using UaModeler is correct.
As expected, it only creates nodes once and correctly uses the Organizes references to reference them from the FunctionalGroups as described above.
The instance created with asyncua doesn't show the same behavior. Instead, it creates multiple nodes within the ParameterSet/MethodSet and the FunctionalGroups.

As an example, this is the DeviceManual and SomeMethod nodes in the instance created in UaModeler. Here the NodeIds of the bodes in the FunctionalGroups are equal to the NodeIds of the nodes in the Object/MethodSet, which means that it's the same node being referenced.

On the other hand, in the instance created with asyncua, the NodeIds differ, indicating that it's not the same node but a different one. This is a problem since I rely on the fact that I can reference the same node from multiple places (e.g. FunctionalGroups) to provide structure to the parameters and methods in the ParameterSet and MethodSet.

Another thing you'll notice is that the Identification FunctionalGroup is empty despite the DeviceType making some of the Properties (which are Optional in the base TopologyElementType) Mandatory. I think the problem here is that the instantiate logic in asyncua only looks at the ModellingRule of the node in the base type where it is originally defined but not at subtypes, which might override them. What's strange though, is that the Identification FunctionalGroup is also Optional in TopologyElement but then MyDeviceType makes it Mandatory - this is correctly handled by asyncua. And even though MyDeviceType also overrides DeviceRevision, Manufacturer, and Model (which are all referenced by the Identification FunctionalGroup), they're not instantiated.

My current workflow is to create the instances directly in the model using UaModeler, but I'd like to be able to dynamically instantiate Objects from my types using asyncua.

I looked into the code which handles the instantiation of nodes but I'm not sure how it could be changed to prevent instantiating duplicate nodes. This was my first idea: Since the ObjectType should be using the correct references to the correct nodes (meaning it doesn't have duplicate nodes), then the instantiation logic should be looking at the nodes it encounters in the type and check if their NodeIds are equal to other nodes in the type which it already instantiated and in this case don't instantiate another node but rather create the necessary reference to the already instantiated node. This sounds relatively simple in theory, but I feel this leads to quite some changes in the instantiation logic (due to the necessary bookkeeping of which nodes have been instantiated from which nodes in the type, etc.).
Since I haven't been using asyncua for too long,

So, my first question is, are my observations correct, and would you agree that the instantiation logic needs improvement?
And secondly, would my idea work to produce the desired result, and if so, could someone provide some guidance for implementing that change? Then I would try to propose a PR.

Version

Python-Version: 3.10.11 on Windows 11

opcua-asyncio Version (e.g. master branch, 0.9): 1.1.5