DataType Encoding: xml_importer._get_sdef() wrongly assumes, that the first HasEncoding reference represents the default encoding.

[DrMatthiasArnold]

During parsing a node-set, StuctureDefinition objects are created for sub-data-types of data-type Structure.
Important information of a structure definition are the supported encodings (Binary, XML. JSON) and the proposed default encoding.
OPC UA Part 3 8.48 StructureDefinition defines the DefaultEncodingId as follows:

The NodeId of the default DataTypeEncoding for the DataType. The default shall always be Default  Binary encoding.

However, the current implementation wrongly assumes, that the first HasEncoding reference of the data-type node represents the default encoding, see snippet:

def _get_sdef(self, obj):
    ..
    sdef = ua.StructureDefinition()
    ..        
    for refdata in obj.refs:
        if refdata.reftype == self.session.nodes.HasEncoding.nodeid:
            # supposing that default encoding is the first one...
            sdef.DefaultEncodingId = refdata.target
            break

Since some existing "real world" companion specifications to not sort the HasEncoding references in the node-set with "Default Binary" on top (and are not mandated to do so), the wrong assumption causes runtime errors when trying to transfer such data between client and server.

In order to determine the correct DefaultNodeId, additional data of the references' target nodes is required (browse_name, display_name). As far as I understand, this data does not exist within the context of the _get_sdef() method. Imho this would either require following the references' targets within the scope of the method or doing a second pass.

Please consider additional issues in the context of handling HasEncoding e.g., xmlimporter.add_object(): Exclusion of HasEncoding target objects sometimes fails for 'Default JSON'.

[schroeder-]

As we currently only support binary, we could just set it to binary by default.

[DrMatthiasArnold]

Good point, however since the nodes representing encoding like "Default Binary" are created for each and every data-type individually (for whatever reason..), you still have to search for the individual target node with this browse_name/display_name.
I wonder why OPC UA not simply defines (3) global/static nodes, representing the supported encodings in the core node-set. But, it is what it is..

[oroulet]

That import is already slow, I am really afraid of what is going to happen if we have to do a second pass...
And yes there are probably many of that kind of assumption in the code, in fact when we started implementing these thigns we did not had access to the spec, we had to reverse engineer things...

[DrMatthiasArnold]

I just stepped through some code to get a better understanding:
As far as I understand, setting the correct DefaultEncodingId should be located somewhere around the _get_sdef() method.:
Reasoning:
The DefaultEncodingId is already utilized during the first pass:

await load_custom_struct_xml_import(node.RequestedNewNodeId, attrs)
    ..
    ua.register_extension_object(name, sdef.DefaultEncodingId, struct, node_id)

IMHO a lot of things could go wrong in trying to adjust the DefaultEncodingId afterwards..

Since I'm not an expert in this project:
Any straightforward way to already access the additional information of the HasEncoding reference target (refdata.target) while iterating through the refs of the datatype node?

[DrMatthiasArnold]

For those of you looking for a companion specification where the error occurs:
LADS Workshop Project is a demo project for the new
Laboratory and Analytical Device Standard (LADS) which was released by the OPC Foundation last week.
You find the LADS nodeset at OPCF GitHub.
While porting the LADS Workshop Project to opcua-async I did run into the different issues (plus an additional issue around HasEncoding still to be reported).
The current "update" branch, which supports OPC UA 1.05.02 provided a good starting point.