Skip to content
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

feat: add recipe for ua-nodeset #7312

Merged
merged 22 commits into from
Sep 28, 2021

Conversation

Mo-Tay
Copy link
Contributor

@Mo-Tay Mo-Tay commented Sep 16, 2021

Specify library name and version: lib/1.0

Building the project with the CMake Macro "ua_generate_nodeset_and_datatypes" provided in in "[https://opcua.rocks/step-9-using-a-custom-uanodeset-with-open62541/]" gave the following exception:

`CMake Error at /home/xx/.conan/data/open62541/1.2.2/sp/prerelease/package /c32e2434a9be6dcdf5509b2f4986160a64168842/lib/cmake/open62541/open62541Macros.cmake:456 (message):

open62541_TOOLS_DIR must point to the open62541 tools directory Call Stack (most recent call first): xx/xx/FooServer/CMakeLists.txt:10 (ua_generate_nodeset_and_datatypes)`

Fix:

  • open62541 OPC UA library needs to be modified to include the files in tools directory needed to create an OPC UA using the nodeset files generated from UA-ModelCompiler

  • add new library UA-Nodeset cloned from https://github.com/OPCFoundation/UA-Nodeset to have OPCUA Foundations's ua-nodeset required files

  • export the paths to tools and ua-nodeset directory as environmental variables "open62541_TOOLS_DIR and open62541_NODESET_DIR"


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@CLAassistant
Copy link

CLAassistant commented Sep 16, 2021

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@Mo-Tay Mo-Tay mentioned this pull request Sep 16, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

recipes/ua-nodeset/all/conandata.yml Outdated Show resolved Hide resolved
recipes/ua-nodeset/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ua-nodeset/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ua-nodeset/all/conanfile.py Show resolved Hide resolved
recipes/ua-nodeset/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ua-nodeset/config.yml Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

Co-authored-by: Uilian Ries <uilianries@gmail.com>
@conan-center-bot

This comment has been minimized.

@@ -0,0 +1,4 @@
sources:
"1.02-2021-07-21":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the releases, they have different prefixes, but are related to the same project. How can distribute it individually? Here you are using PADIM, but what does happen if someone want DEXPI? It looks the project is the same, but the provided package is totally different.

Or you could use the real release name for package version, or we should think about separated packages for each release, like ua-nodeset-padim/1.02

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tags are like rolling releases on the 1.04 branch.
E.g. the tag PADIM-1.02-2021-07-21 (https://github.com/OPCFoundation/UA-Nodeset/releases/tag/PADIM-1.02-2021-07-21) is for the commit OPCFoundation/UA-Nodeset@6f2b384 which is almost the newest one on the 1.04 branch.

The tag names just indicate that a specific companion specification version is part of that release. All other companion specifications released previously are still part of that too.

Therefore I do not think that it is a good idea to separate the ua-nodeset package into sub-packages.

I suggest to just use the version 1.04 and the date of the latest tag, i.e., 1.04-2021-09-15.
I will update the conandata.yml file with some more comments (and also fix the version as it is currently wrong)

@conan-center-bot

This comment has been minimized.

@@ -0,0 +1,5 @@
sources:
"1.04-2021-09-15":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have to use an official release. We only allow different names when there is no official release: https://github.com/conan-io/conan-center-index/blob/master/docs/faqs.md#what-version-should-packages-use-for-libraries-without-official-releases

I understand this project is exceptional, but still we don't track branches, but releases. Please, revert to 1.02-

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clarify:
1.04 is the current official release of the whole OPC UA NodeSets and Specification.

See for example here:
https://opcfoundation.org/developer-tools/specifications-unified-architecture/part-4-services/

You can see that the version of the specification is 1.04, i.e. the latest release.
This also matches with the Readme in UA-Nodeset repo:

The files in the 1.04 branch are also public to the OPC Foundation website. If someone is looking for the officially released version of the UANodeSets they must follow the links in the specification.

The tags on the UA-Nodeset repo indicate specific versions of Sub Specifications.

E.g.,
PADIM-1.02-2021-07-21 means that this tag includes all previously released Companion Specifications, but Updates the PADIM Specification to version 1.02.
Note that the PADIM Specification 1.02 still builds on top of the 1.04 Base Sepcification.

Yes I know that this is not straight forward, but unfortunately that's how the OPC Foundation decided to release their Companion Specs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we will follow any release scheme upstream uses, doesn't matter how weird and complex is it

Copy link
Contributor

@Pro Pro Sep 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, all clear :)

So should we then use as the version padim-1.02-2021-07-21 (i.e., the latest tag of the 1.04 branch)?

So the conan package will be ua-nodeset/padim-1.02-2021-07-21?

Just using ua-nodeset/1.02-2021-07-21 would not be correct, as one would assume that it is the 1.02 companion specification, and not 1.04 (which is included in the PADIM-1.02-2021-07-21).

I just changed the recipe to that tag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not the one who judge - you probably know better which version(s) you need.
we usually prefer latest official releases, unless there are strong reasons not to use them (e.g. breaking API changes).
if latest tag is latest release in that repository, than okay.
their versioning scheme is super confusing for the outsiders, so I would use tag name as is.
at least to distinguish base versions from PADIM versions (I have no idea what PADIM is and what's the difference from base, especially from the consumer point of view).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I completely agree that their tagging is very confusing.

IMHO using the Tag Name as version is less confusing from our side, than inventing a custom version string for conan.

SoIwpuld recommend to just use the complete tag, as it currently is 👍🏼

@Pro Pro force-pushed the feat-add-ua-nodeset branch from 238eed0 to fafc3b6 Compare September 23, 2021 13:26
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

return "source_subfolder"

def _extract_license(self):
content = tools.load(os.path.join(self.source_folder, self._source_subfolder, "AnsiC", "opcua_clientapi.c"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there are some sources (opcua_clientapi.c at very least), but I don't see anything built/compiled in the conanfile.
there is nothing included or linked in the test package as well, it only checks for some PLCopen file.
could you explain how this project is exactly used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These source files are just example files which could be used in combination with the official OPC Foundation reference ANSI C implementation, which is not open source.

The other files, especially the NodeSet2.xml files (together with their Types.xsd and NodeId.csv) files are machine-readable descriptions of an OPC UA companion specification.

The open62541 stack includes the nodeset generator, which generates C Code from these XML files, to correctly initialize an OPC UA Server with a given companion specification.

That script is added in #7314 to open62541.
So open62541 depends on ua-nodeset if one wants to create a pre-initialized OPC UA server.
ua-nodeset conan package is just delivering the resource files, and no libraries or binaries.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

All green in build 25 (49b808d2f06ad9a6fa1750f8a413303814ec6d1b):

  • ua-nodeset/padim-1.02-2021-07-21@:
    All packages built successfully! (All logs)

Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for renaming the package version, that comment in conandata.yml is good enough to understand the situation.

@conan-center-bot conan-center-bot merged commit 5be1ac2 into conan-io:master Sep 28, 2021
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.

8 participants