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

[Python/clang] Add missing core functions #1300

Merged

Conversation

DownerCase
Copy link
Contributor

@DownerCase DownerCase commented Jan 14, 2024

Description

Adds the missing core functions from ecal_core.h (GetVersion, IsInitialized, SetUnitName) to ecal_clang and the python wrapper.

I had to implement the non-string version of GetVersion as getversion_components due to the obvious name already being taken.

Tested locally and all works perfectly.

Tangents:

  • Don't fully understand why you have two C interfaces (ecal_clang and ecal/cimpl )
  • mon_pubmonitoring and mon_publogging need reimplementing for v5.13 due to the partial API removal.

Related issues

Closes #1239

Cherry-pick to

  • none

@DownerCase
Copy link
Contributor Author

CUSTOMBUILD : Sphinx version error :  [D:\a\ecal\_build\complete\doc\documentation_sphinx.vcxproj]
  The sphinxcontrib.applehelp extension used by this project needs at least Sphinx v5.0; it therefore cannot be built with this version.

Is this caused by the PR?

@DownerCase DownerCase marked this pull request as ready for review January 14, 2024 12:21
@FlorianReimold
Copy link
Member

The error that you posted is a Sphinx version missmatch, which is documentation related. We use Sphinx to build the documentation.

@KerstinKeller
Copy link
Contributor

@DownerCase We have this problem on all GH action builds atm (#1301), will try to solve ASAP and then rebuild the PR. Thanks for your contribution anyways.

@DownerCase
Copy link
Contributor Author

@DownerCase We have this problem on all GH action builds atm (#1301), will try to solve ASAP and then rebuild the PR. Thanks for your contribution anyways.

Thanks, looks like you solved it!

Copy link
Contributor

@hannemn hannemn left a comment

Choose a reason for hiding this comment

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

I just had a look at the newly added function and tested it in python. Everything looks good from my side!

@FlorianReimold FlorianReimold merged commit c532f86 into eclipse-ecal:master Jan 17, 2024
7 of 8 checks passed
KerstinKeller added a commit that referenced this pull request Jan 18, 2024
* Doc: Downgraded sphinx related packages in requirements.txt (#1302)

* The most recent release of various sphinx related packages need Sphinx 5 or up, but our old sphinx-book-theme 0.3.3 requires Sphinx 4
For a proper solution see #1303

* CMake: Fixed Findqwt.cmake (#1306)

* [Python/clang] Add missing core functions (#1300)

Adds the missing core functions from ecal_core.h (GetVersion, IsInitialized, SetUnitName) to ecal_clang and the python wrapper.

* Measurement API: Introduce experimental namespace to mark new api as not yet stable. (#1307)

---------

Co-authored-by: Florian Reimold <[email protected]>
Co-authored-by: DownerCase <[email protected]>
@KerstinKeller
Copy link
Contributor

@DownerCase, could you please sign the ECA even though we already merged the commit to master?
Please follow this link and click ECA sign on the right side of the page
image

@DownerCase
Copy link
Contributor Author

@KerstinKeller Sorry! Should be good now! The validation tool is shows it as accepted now 😄

@DownerCase DownerCase deleted the python_add_core_functions branch January 20, 2024 12:31
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.

IsInitialized Python API
4 participants