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

libiec61850: new recipe #26096

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

libiec61850: new recipe #26096

wants to merge 33 commits into from

Conversation

inPhraZ
Copy link
Contributor

@inPhraZ inPhraZ commented Dec 3, 2024

Summary

Changes to recipe: libiec61850/1.5.3

Motivation

Adds libiec61850: an open-source library for the IEC 61850 protocols.

https://github.com/mz-automation/libiec61850

Details


Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

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

Hi! Thanks a lot for taking the time to implement this recipe, we appreciate it. I have some minor comments, and an important point with the vendorized dependencies, but this looks good as a starting point :)

recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libiec61850/all/test_package/test_package.cpp Outdated Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Show resolved Hide resolved
@danimtb
Copy link
Member

danimtb commented Dec 12, 2024

Hi @inPhraZ!

I have contributed some changes to your PR. The library had some issues packaging the correct configuration for shared/static (the library builds both by default). There were some issues with the cmake install as well mixing both configurations, so I replaced them with the copy calls.

I tested on windows both shared and static and they seem to work. Let's see what the CI says. Thank you!

@inPhraZ
Copy link
Contributor Author

inPhraZ commented Dec 12, 2024

Thanks @danimtb
I really appreciate your help!

danimtb
danimtb previously approved these changes Dec 13, 2024
czoido
czoido previously approved these changes Dec 13, 2024
Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

@@ -0,0 +1,4 @@
sources:
"1.5.3":
url: "https://github.com/mz-automation/libiec61850/archive/refs/tags/v1.5.3.tar.gz"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the most recent version available, 1.6.0 seems to have been released a few months ago:
https://github.com/mz-automation/libiec61850/releases/tag/v1.6.0 - any reason why we wouldnt start with the most recent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

any reason why we wouldnt start with the most recent?

Yes, the latest release version (1.6.0) has an issue where one of the required header files is missing from the installation path, causing the test_package to fail. Although this issue has been addressed and fixed in this PR, it has not yet been included in an official release.

recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Show resolved Hide resolved
recipes/libiec61850/all/conanfile.py Outdated Show resolved Hide resolved
@inPhraZ inPhraZ dismissed stale reviews from czoido and danimtb via 6627bd9 December 13, 2024 15:36
@czoido czoido requested a review from jcar87 December 16, 2024 06:38
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.

5 participants