-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Array in method result structure serialized/padded wrongly #3513
Comments
I just added a test to verify chunking on v1.0.1. See #3515. Can you make a source dump available on a private channel for easier debugging? |
Ah right. This is not a chunking issue, as the messages are rather small. In addition to the |
Of course, I can! I think it's easiest to upload the entire folder of files generated from the standard namespaces: src_generated.tar.gz I've looked into |
Found it. The Companion Spec makes us of structures with optional fields. This feature has just been completed. Let's get back to this case when the infrastructure is in place. |
General note: The nodeset compiler should abort in such cases instead of generating incomplete representations. |
I don't quite understand. RfidSighting doesn't contain any optional fields and I removed all of them from RfidScanResult making all fields mandatory instead... or so I thought. Although I haven't touched If you can give me a quick pointer on what to change in the NodeSet2, that would be appreciated. Otherwise I'll just wait for v1.1 and hope that the generator will process it without modifications. Finding a workaround is not super urgent for me. |
A few days ago, support for optional fields in structures has landed. |
I tried upgrading to open62541 v1.1-rc1 which was identical to master at the time of writing. Using the ua-nodeset schemas that open62541 includes as a submodule (branch: v1.04), the schema generator throws the following error on the AutoID schema (v1.01):
Indeed the CSV doesn't seem to be up to date and contains no entry for RfidAccessResult. I also didn't know that AutoID v1.01 has been released. Is this a work in progress? I went back to the ua-nodeset's master-branch schemas (AutoID v1.00), which result in a few warnings (even errors) but generate successfully at least:
Using these (master branch) schemas, I can get my project to compile and I can also browse my device node, but all of the methods' input and output arguments seem to refer to wrong namespace ids. Namespaces ids assigned by the schema generator (CMake calls): ua_generate_nodeset_and_datatypes(
NAME "di"
FILE_CSV "${CMAKE_SOURCE_DIR}/schemas/OpcUaDiModel.csv"
FILE_BSD "${CMAKE_SOURCE_DIR}/schemas/Opc.Ua.Di.Types.bsd"
NAMESPACE_IDX 2
FILE_NS "${CMAKE_SOURCE_DIR}/schemas/Opc.Ua.Di.NodeSet2.xml")
ua_generate_nodeset_and_datatypes(
NAME "autoid"
FILE_CSV "${CMAKE_SOURCE_DIR}/schemas/Opc.Ua.AutoID.NodeIds.csv"
FILE_BSD "${CMAKE_SOURCE_DIR}/schemas/Opc.Ua.AutoID.Types.bsd"
NAMESPACE_IDX 3
FILE_NS "${CMAKE_SOURCE_DIR}/schemas/Opc.Ua.AutoID.NodeSet2.xml"
DEPENDS "di") Consequently all of my namespace ids are as follows:
However, already below the Server object the ordering seems to be different (UaExpert screenshot): I don't suspect this to be problematic, as my working server based on open62541 v1.0 had the same ordering when browsing these nodes. Browsing the InputArguments of the Scan method, the only argument (of type ScanSettings) refers to NamespaceIndex 2: I think that should be 3 instead according to the order previously established. Consequently, I cannot test whether the serialization bug, I initially opened this ticket for, is now fixed in v1.1-rc1. Neither can I test whether optional fields and unions work now. But I did already adapt my code for them. Needless to say, the tweaked schemas I used until now for my project cause exactly the same bug on open62541 v1.1-rc1 - the schema differences are purely cosmetic, so I won't go into detail. I also tried to changing the
On open62541 v1.0, where method-calling was at least working, the namespace mapping is as expected: I suspect that a bug in the schema generator is to blame for this. |
PS: In both v1.0 and v1.1-rc1, the autoid_nodeids.h are unusable since they contain duplicate and conflicting macro definitions. I worked around this by manually picking the right the defintions into my source file. |
I suspect that lines like that in namespace_autoid_generated.c are responsible: variablenode_ns_2_i_6052_variant_DataContents[0].dataType = UA_NODEID_NUMERIC(ns[1], 3010); It should read - still did in files generated by v1.0 - Unfortunately, I cannot isolate the code in nodeset_compiler responsible for generating these faulty lines, nor do I see any obvious change in nodeset_compiler since v1.0 that could explain the difference. |
@ichrispa would you have a look at the namespace compiler? |
Hello, Thank you |
Okay. Giving this a high priority. |
Description
Implementing an AutoID server using open62541, I came across problems with arrays as part of structures returned by the Scan() method. They are apparently wrongly serialized in the binary protocol.
The Scan() method returns RfidScanResult (cf. OPC UA for AutoID, p.24), which in turn may contain several RfidSighting (p.45) structures in an array. Or as expressed in the BSD file:
The schema generator generates an UA_RfidScanResult accordingly:
The bug manifests whenever I add more than one RfidSighting to the RfidScanResult. The result you can see in this UaExpert screenshot:
The second sighting is apparently bogus. I've debugged the structure's contents of course - it's still perfectly fine when I store it into the output Variant. Neither does Valgrind show any kind memory corruption or the like.
I rather suspect a kind of padding issue after analyzing the network communication. You can refer to the attached Wireshark log in packet 13. The first RfidSighting ends in byte 00e7. But the second RfidSighting only starts at byte 00ec. There seem to be 4 padding bytes that are not accounted for and that are not expected by UaExpert's OPC UA stack. These 4 bytes are also present after the second RfidSighting instance.
This of course immediately raises the possibility of a 64-bit-only bug.
I haven't yet tried to recompile everything with
-m32
.Background Information / Reproduction Steps
Used CMake options:
Checklist
Please provide the following information:
UA_LOGLEVEL
set as low as necessary) attachedThe text was updated successfully, but these errors were encountered: