C++ API updated for new C-API #1112

sternmull · 2023-12-23T19:59:30Z

This updates iiopp.h for the latest changes of the C-API. It should fix issue #1046.

But be warned: I currently have no system with iio devices available for testing. So this is completely untested code!

The small example compiles and in general the code should be easy to read and review. I hope i didn't introduce any bugs. Please keep that in mind and review and test it carefully.

rgetz · 2023-12-27T16:30:21Z

since: the cpp bindings are off by default - https://github.com/analogdevicesinc/libiio/blob/61fdbe78c298570bbc0c9e9ec3ce1a7c1fa3742e/CMakeLists.txt#L651C1-L651C48

now things are working - we should turn them on for CI where we want to...

likely here for linux:

libiio/azure-pipelines.yml

Line 75 in 61fdbe7

    
                     cmake .. -Werror=dev -DCOMPILE_WARNING_AS_ERROR=ON -DENABLE_PACKAGING=ON -DPYTHON_BINDINGS=ON -DWITH_DOC=ON -DWITH_SERIAL_BACKEND=ON -DWITH_MAN=ON -DCPACK_SYSTEM_NAME=${ARTIFACTNAME}

and here for mac: https://github.com/analogdevicesinc/libiio/blob/61fdbe78c298570bbc0c9e9ec3ce1a7c1fa3742e/azure-pipelines.yml#L337C4-L337C4

and here for windows:

libiio/CI/build_win.ps1

Line 14 in 61fdbe7

    
           cmake -G "$COMPILER" -DPYTHON_EXECUTABLE:FILEPATH=$(python -c "import os, sys; print(os.path.dirname(sys.executable) + '\python.exe')") -DCMAKE_SYSTEM_PREFIX_PATH="C:" -Werror=dev -DCOMPILE_WARNING_AS_ERROR=ON -DENABLE_IPV6=ON -DWITH_USB_BACKEND=ON -DWITH_SERIAL_BACKEND=ON -DPYTHON_BINDINGS=ON -DCSHARP_BINDINGS:BOOL=$USE_CSHARP -DLIBXML2_LIBRARIES="C:\\libs\\64\\libxml2.lib" -DLIBUSB_LIBRARIES="C:\\libs\\64\\libusb-1.0.lib" -DLIBSERIALPORT_LIBRARIES="C:\\libs\\64\\libserialport.dll.a" -DLIBUSB_INCLUDE_DIR="C:\\include\\libusb-1.0" -DLIBXML2_INCLUDE_DIR="C:\\include\\libxml2" -DLIBZSTD_INCLUDE_DIR="C:\\include" -DLIBZSTD_LIBRARIES="C:\\libs\\64\\libzstd.dll.a" ..

and here for linux on ARM: https://github.com/analogdevicesinc/libiio/blob/61fdbe78c298570bbc0c9e9ec3ce1a7c1fa3742e/CI/azure/ci-ubuntu.sh

that way everything will get at least build tested on CI...

sternmull · 2023-12-27T18:44:48Z

Static code analysis passes. The Linux builds fail due to a missing C++ compiler. The windows build fails because boost is not installed.
I don't know how to fix that. I think the Linux CI had a C++ compiler back when I added the bindings. Back then the C++ bindings where not activated for the windows CI-build.

Maybe someone who has experience with this CI-stuff can take a look at it?

rgetz · 2023-12-27T20:39:54Z

Centos fails since Target "iiopp-enum" requires the language dialect "CXX17" I think it's too old to support C++17... so turning that off makes sense. I think the same for Fedora34. Maybe @tfcollins knows more when he gets back in Jan.

If BOOST is a requirement - we should check for it in the CMAKE, and tell people to install it... something like:

  find_package(Boost QUIET NO_MODULE)

  if (Boost_FOUND)
    message(STATUS "Boost ${Boost_FIND_VERSION} found.")
    if (Boost_FIND_COMPONENTS)
      message(STATUS "Found Boost components:  ${Boost_FIND_COMPONENTS}")
    endif()
  else()
    message(SEND_ERROR "You need to install BOOST, or turn off C++ bindings")
  endif()

If you need specific modules - add them rather than NO_MODULE.

I think Travis can add BOOST to the docker image in Jan.

rgetz · 2024-01-02T18:50:48Z

So, boost is not a hard requirement for windows...

The code:

#if __cplusplus < 201703L
#include <boost/optional.hpp>
#else
#include <optional>
#endif

However - Visual Studio only fills out the __cplusplus var if you tell it to. (according to the doc, we need to add /Zc:__cplusplus compiler flag to enable the __cplusplus macro). I think that is the actual solution...

With maybe some more CMAKE to see if it needs to check for boost...

sternmull · 2024-01-02T19:54:03Z

So, boost is not a hard requirement for windows...

Yes. Actually it is not required with VS 2017 and later. Fixed that and a few minor problems (warnings that showed up with MS build).

rgetz · 2024-01-02T21:17:39Z

So, then we are left with:

LinuxBuilds fedora34:

-- The CXX compiler identification is unknown
CMake Error at bindings/cpp/CMakeLists.txt:3 (project):
  No CMAKE_CXX_COMPILER could be found.

debug : Install a C++ compiler in fedora34 image. I think @tfcollins needs to do this.

LinuxBuilds centos_7:

CMake Error in bindings/cpp/CMakeLists.txt:
  Target "iiopp-enum" requires the language dialect "CXX17" (with compiler
  extensions).  But the current compiler "GNU" does not support this, or
  CMake does not know the flags to enable it.

-- The C compiler identification is GNU 4.8.5

which was released June 23, 2015, before the cpp 2017 spec was written. Either figure out how to make it work without, and have BOOST installed, or (my suggestion) just turn it off in the azure-pipelines.yml file - there is a seperate line for Centos already...

pcercuei · 2024-01-04T10:16:36Z

@sternmull thanks a lot! I'll have a proper look next week.

tfcollins · 2024-01-09T00:56:51Z

Container updated sdgtt/Dockerfiles@5ade294 Will be ready in 30 minutes

rgetz · 2024-01-09T13:23:37Z

@tfcollins - thanks - can someone with admin rights re-start the fedora build? or @sternmull can you squash things and push so things re-build?

pcercuei · 2024-01-09T13:34:32Z

All checks pass now.

pcercuei · 2024-01-11T10:13:01Z

bindings/cpp/iiopp.h

+    Stream(iio_stream * s) : p(s){}
+    operator iio_stream * () const {return p;}
+
+    Block next_block() {return const_cast<iio_block *>(impl::check(iio_stream_get_next_block(p), "iio_stream_get_next_block")); }


Won't the iio_block be deleted when it gets out of scope?

No, the Block-class does not manage the lifetime of the referenced object. It is just a shallow wrapper for calling its functions.
See the comment at the top of the header. This should explain how the C++ classes manage resources.

Ok, thanks.

pcercuei · 2024-01-11T10:33:53Z

bindings/cpp/iiopp.h

+    void enable() {impl::check(iio_buffer_enable(p), "iio_buffer_enable");}
+    void disable() {impl::check(iio_buffer_disable(p), "iio_buffer_disable");}
+    ChannelsMask channels_mask() {return iio_buffer_get_channels_mask(p);}
+    BlockPtr crate_block(size_t size) { return BlockPtr{impl::check(iio_buffer_create_block(p, size), "iio_buffer_create_block")}; }


crate_block -> create_block

Oops. Committed a fix.

Can you merge it into the first commit maybe?

I'll also need you to sign all the commits (currently only the first one is signed).

Signed-off-by: Tilman Blumhagen <tilman.blumhagen@googlemail.com>

MSVC 2017 has std::optional even if it does not yet implement C++17. See https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance?view=msvc-170https://learn.microsoft.com/en-us/cpp/overview/visual-cpp-language-conformance Signed-off-by: Tilman Blumhagen <tilman.blumhagen@googlemail.com>

Its C++ compiler is too old (or we would need boost). Signed-off-by: Tilman Blumhagen <tilman.blumhagen@googlemail.com>

sternmull · 2024-01-11T11:54:44Z

Done

pcercuei · 2024-01-12T09:48:51Z

Merged, thanks a lot!

sternmull force-pushed the main branch 2 times, most recently from 4bdee76 to 43d3e7c Compare December 27, 2023 18:23

sternmull force-pushed the main branch 4 times, most recently from 06908d2 to dd3be2f Compare January 2, 2024 19:46

pcercuei reviewed Jan 11, 2024

View reviewed changes

sternmull force-pushed the main branch from 54182d9 to c8527ed Compare January 11, 2024 10:38

sternmull added 3 commits January 11, 2024 12:53

C++ API updated for new C-API

bbc418c

Signed-off-by: Tilman Blumhagen <tilman.blumhagen@googlemail.com>

Disabled CPP_BINDINGS for CentOS 7 build

1b03de7

Its C++ compiler is too old (or we would need boost). Signed-off-by: Tilman Blumhagen <tilman.blumhagen@googlemail.com>

sternmull force-pushed the main branch from c8527ed to 1b03de7 Compare January 11, 2024 11:54

pcercuei self-requested a review January 12, 2024 08:39

pcercuei approved these changes Jan 12, 2024

View reviewed changes

pcercuei merged commit a6d3859 into analogdevicesinc:main Jan 12, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C++ API updated for new C-API #1112

C++ API updated for new C-API #1112

sternmull commented Dec 23, 2023

rgetz commented Dec 27, 2023

sternmull commented Dec 27, 2023

rgetz commented Dec 27, 2023

rgetz commented Jan 2, 2024

sternmull commented Jan 2, 2024

rgetz commented Jan 2, 2024

pcercuei commented Jan 4, 2024

tfcollins commented Jan 9, 2024

rgetz commented Jan 9, 2024

pcercuei commented Jan 9, 2024

pcercuei Jan 11, 2024

sternmull Jan 11, 2024

pcercuei Jan 11, 2024

pcercuei Jan 11, 2024

sternmull Jan 11, 2024

pcercuei Jan 11, 2024

sternmull commented Jan 11, 2024

pcercuei commented Jan 12, 2024

C++ API updated for new C-API #1112

C++ API updated for new C-API #1112

Conversation

sternmull commented Dec 23, 2023

rgetz commented Dec 27, 2023

sternmull commented Dec 27, 2023

rgetz commented Dec 27, 2023

rgetz commented Jan 2, 2024

sternmull commented Jan 2, 2024

rgetz commented Jan 2, 2024

pcercuei commented Jan 4, 2024

tfcollins commented Jan 9, 2024

rgetz commented Jan 9, 2024

pcercuei commented Jan 9, 2024

pcercuei Jan 11, 2024

Choose a reason for hiding this comment

sternmull Jan 11, 2024

Choose a reason for hiding this comment

pcercuei Jan 11, 2024

Choose a reason for hiding this comment

pcercuei Jan 11, 2024

Choose a reason for hiding this comment

sternmull Jan 11, 2024

Choose a reason for hiding this comment

pcercuei Jan 11, 2024

Choose a reason for hiding this comment

sternmull commented Jan 11, 2024

pcercuei commented Jan 12, 2024