OPCFoundation / UA-ModelCompiler

ModelCompiler converts XML files into C# and ANSI C
MIT License
151 stars 94 forks source link

Model crashes when modeluri does not start with http://opcfoundation.org/UA/ and/or model uri is shorter than 28 chars. #125

Closed maxschiffer closed 1 year ago

maxschiffer commented 1 year ago

The Modelcompiler crashes, when code for a model is generated thats uri does not start with http://opcfoundation.org/UA/ and/or model uri is shorter than 28 chars.

This is due to the implementation of the "private static Namespace CreateNamespace(ModelTableEntry model)" method in the NodeSetToModelDesign.cs file. The method assumes all modeluris start with http://opcfoundation.org/UA/ and does a substring with the length of said uri (http://opcfoundation.org/UA/):

private static Namespace CreateNamespace(ModelTableEntry model)
        {
            var ns = new Namespace()
            {
                Name = model.ModelUri.Substring(Namespaces.OpcUa.Length).Replace("/", " ").Trim().Replace(" ", "."),
                Value = model.ModelUri,
                XmlNamespace = model.XmlSchemaUri,
                PublicationDate = model.PublicationDate.ToString("yyyy-MM-ddT00:00:00Z"),
                Version = model.Version
            };

            if (ns.Value == Opc.Ua.Namespaces.OpcUa)
            {
                ns.XmlNamespace = Opc.Ua.Namespaces.OpcUaXsd;
            }

            ns.XmlPrefix = ns.Prefix = "Opc.Ua." + ns.Name;
            ns.Name = ns.Name.Replace(".", "");

            if (ns.Value == Namespaces.OpcUa)
            {
                ns.Name = "OpcUa";
                ns.XmlPrefix = ns.Prefix = "Opc.Ua";
            }

            return ns;
        }

This could easily be fixed by adding a switchcase and just using the name supplied from the commandline.

-d2 "<pathToNodeSet2.xml>,<Namespace.YourModel>,**<Name/Shortname>**" or implementing some logic like this:

private static Namespace CreateNamespace(ModelTableEntry model)
        {
            Namespace ns = null;
            if (model.ModelUri.StartsWith(Namespaces.OpcUa))
            {
                ns = new Namespace()
                {
                    Name = model.ModelUri.Substring(Namespaces.OpcUa.Length).Replace("/", " ").Trim().Replace(" ", "."),
                    Value = model.ModelUri,
                    XmlNamespace = model.XmlSchemaUri,
                    PublicationDate = model.PublicationDate.ToString("yyyy-MM-ddT00:00:00Z"),
                    Version = model.Version
                };
                ns.XmlPrefix = ns.Prefix = "Opc.Ua." + ns.Name;
                ns.Name = ns.Name.Replace(".", "");
            }
            else
            {
                ns = new Namespace()
                {
                    Name = model.ModelUri.Replace("http://", "").Replace("/", " ").Trim().Replace(" ", "."),
                    Value = model.ModelUri,
                    XmlNamespace = model.XmlSchemaUri,
                    PublicationDate = model.PublicationDate.ToString("yyyy-MM-ddT00:00:00Z"),
                    Version = model.Version
                };
                ns.XmlPrefix = ns.Name;
                ns.Name = ns.Name.Replace(".", " ");
            }

            if (ns.Value == Namespaces.OpcUa)
            {
                ns.Name = "OpcUa";
                ns.XmlPrefix = ns.Prefix = "Opc.Ua";
            }

            return ns;
        }
opcfoundation-org commented 1 year ago

Why don't you just fix the information model? It is a bad practice to have the same name for an output argument as an input argument. It is quite likely that doing so will cause issues or other code generators.

maxschiffer commented 1 year ago

EDIT: This is just the issue for the namespace issue, not the method argument pull request in the OPCFM Reposity!

Your reply is not at all related to the issue! It is related to the PR this issue is mentioned in...


Original Reply:

If its bad practice, why not forbid it in the OPC specification? Also there is no other way to implement a "ref" argument in OPC UA.

If for instance I'd like to request a specific Id/Name for something and pass it on to the method and the systems interal logic doesn't allow this or the name is already taken so it generates a new name/id and returns it. Instead of just throwing an error and having the client resubmit it.

I'll bring this up in the working group, but I still consider this a valid usecase.

What about the second, namespace related part of the PR?

opcfoundation-org commented 1 year ago

It is now an explicit requirement that all Argument names be unique:

Argument names shall be unique within the scope of the Method.

https://reference.opcfoundation.org/Core/Part3/v105/docs/5.7

The requirement was added when the extended argument metadata was added to the specification. The extended metadata depends on Nodes with BrowseNames equal to the Method arguments and it would not work if the arguments were not unique.

In the base specification we prefix out arguments with 'revised' when the Server needs to return the value it actually used.

maxschiffer commented 1 year ago

Alright, so it should generate valid code when using the -version v104 or v103 flag? 😆 Just kidding, I wanted to clarify this before talking the working group.

Still, the main issue remains: ModelCompiler crashes when modeluri does not start with http://opcfoundation.org/UA/ and/or model uri is shorter than 28 chars. I'll modify my PR to only fix this.

maxschiffer commented 1 year ago

Pull request is now updated https://github.com/OPCF-Members/UA-ModelCompiler/pull/23

maxschiffer commented 1 year ago

PR was merged into the private repository and should soon be synced in the public main branch.

Closing this issue now.