Skip to content

Fix datatype member dependency ordering during NodeSet load#293

Open
fischjo3SICKAG wants to merge 3 commits intoopen62541:masterfrom
fischjo3SICKAG:fix_loading_order
Open

Fix datatype member dependency ordering during NodeSet load#293
fischjo3SICKAG wants to merge 3 commits intoopen62541:masterfrom
fischjo3SICKAG:fix_loading_order

Conversation

@fischjo3SICKAG
Copy link
Copy Markdown
Contributor

This pull request fixes node sorting during NodeSet loading to address
open62541 issue 7784.

What changed:

  • Extended sorting readiness checks for UADataType nodes: a datatype is now only
    considered ready when all datatype member types defined in the same NodeSet are
    already available.
  • Added a regression test (loading_order_fix) with a minimal NodeSet where
    TypeA depends on TypeB but appears first in XML order.

Behavior before/after:

  • Before this fix: only one datatype could be loaded in this dependency order.
  • With this fix: both dependent datatypes are loaded successfully.

Validation:

Open question for maintainers:

  • I am not fully sure whether the memberTypesReady() implementation performs the
    correct dependency checks in all cases, especially regarding which data from
    the nodeset parameter should be used to decide that all required datatypes
    are available. A review of this function would be very helpful.

@fischjo3SICKAG
Copy link
Copy Markdown
Contributor Author

I investigated the missing binary encoding IDs for some DI datatypes.

Root cause: some NodeSets model HasEncoding only as an inverse reference on the
encoding object (IsForward="false" from encoding object to datatype). The previous
import path only checked forward HasEncoding references on the datatype node, so
binaryEncodingId could remain unset.

I pushed a fix that:

  • stores and keeps inverse references during import
    (include/NodesetLoader/NodesetLoader.h, src/Node.c,
    backends/open62541/src/import.c)
  • resolves encoding IDs from both forward and inverse HasEncoding references
    (backends/open62541/src/DataTypeImporter.c)
  • adds a regression test with a minimal NodeSet that covers both cases
    (backends/open62541/tests/dataTypeImport/compareInverseHasEncoding.c and
    backends/open62541/tests/dataTypeImport/inverseHasEncoding.xml)

The new test validates two datatypes: one with forward HasEncoding and one with
inverse HasEncoding. Both now resolve the expected binary encoding ID.

@fischjo3SICKAG
Copy link
Copy Markdown
Contributor Author

I investigated another datatype import issue in this PR: loading empty structures (Definition present, but fieldCnt = 0).

Root cause:

  • In DataTypeImporter, empty structs were not consistently treated as structured datatypes in all paths. NodeSets that define empty structures have UA_DataType instance with typeKind UA_DATATYPEKIND_EXTENSIONOBJECT, expected UA_DATATYPEKID_STRUCTURE.

Proposed fix:

  • Handle datatypes with a Definition but zero fields as regular Structure/Union types in DataTypeImporter.c

Regression test:

  • Added a dedicated test for empty structures, including inheritance from empty structures.

I would appreciate a focused review of this part because I am not fully sure whether this handling is correct for all datatype edge cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant