Re: [libgpiod][PATCH v3 0/3] libgpiod v2: C++ bindings

[Jack Winch <sunt.un.morcov@xxxxxxxxx>]

Hello Bartosz,

I am most certainly being *dense* here, but which libgpiod branch
should I be applying this patch series to?  Also, what other patch
series is this particular set of patches dependent on?  I tried to
apply against master and next/libgpiod-2.0 without much success this
weekend.  Also, just for clarity, how is the next/libgpiod-2.0 branch
being used at current?  Which and at what point are you applying
patches to this particular branch?

Once I've got over this basic hurdle, I'd like to do a 'holistic'
review of this patch series.  As I've not been following changes to
either the kernel subsystem or user library as of late, I feel this
approach to be more appropriate and thorough.

Finally, what is the target deadline for completion of the v2 API?

Best,
Jack

On Tue, Jul 27, 2021 at 3:34 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> This series contains the implementation of the C++ bindings for libgpiod v2.
> In general the C++ library follows the data structure model as defined by
> the C library with one notable exception: objects that represent immutable
> snapshots of kernel data (line_info and edge & info events) are copyable
> (or rather shared behind the scenes using ::std::shared_ptr). The rest of
> the classes delete their copy constructors and assignment operators and
> are only move constructible and move assignable.
>
> All classes follow the pimpl idiom - using either shared_ptr or unique_ptr -
> and all implementations are hidden from the user for easier maintenance and
> less ABI breakage in the future.
>
> The edge_event class is a bit of a special case. While it looks the same
> as other copyable objects to the user, the implementation uses a tiny bit of
> polymorphism (although it never crosses the ABI boundary). This is done
> to make it possible to use the edge_event_buffer without any memory
> allocations like what the C API enables. The edge_event objects stored
> in the buffer only contain a raw pointer to the C object stored in the
> underlying C edge_event_buffer. The event is copied into a fully managed
> object once the copy assignment operator is called.
>
> I'm Cc'ing people who showed interest and helped me with C++ bindings
> before for review.
>
> v1 -> v2:
> Kent: I addressed most points from your review. Some are unaddressed due to
> personal preference (for instance: I still allow creating of empty line-request
> objects as they may be reused in subsequent requests). I also kept the 'watch'
> argument in get_line_info() as well as the boolean operators for chip and
> request - although with (hopefully) better documentation.
>
> v2 -> v3:
> - use scoped class enums instead of regular integer ones
> - rename getters to get_<property>() in order to avoid name conflicts with
>   the new enum types
>
> The first two patches extend the core C API of libgpiod as discussed.
>
> Bartosz Golaszewski (3):
>   API: add a function for retrieving the capacity of edge event buffers
>   API: extend the line request functionality
>   bindings: cxx: implement C++ bindings for libgpiod v2.0
>
>  Doxyfile.in                                 |   4 +-
>  bindings/cxx/Makefile.am                    |  16 +-
>  bindings/cxx/chip.cpp                       | 214 +++--
>  bindings/cxx/edge-event-buffer.cpp          | 103 +++
>  bindings/cxx/edge-event.cpp                 | 123 +++
>  bindings/cxx/examples/Makefile.am           |  12 +-
>  bindings/cxx/examples/gpiodetectcxx.cpp     |   9 +-
>  bindings/cxx/examples/gpiofindcxx.cpp       |   2 +-
>  bindings/cxx/examples/gpiogetcxx.cpp        |  12 +-
>  bindings/cxx/examples/gpioinfocxx.cpp       |  63 +-
>  bindings/cxx/examples/gpiomoncxx.cpp        |  39 +-
>  bindings/cxx/examples/gpiosetcxx.cpp        |  19 +-
>  bindings/cxx/gpiod.hpp                      | 938 +-------------------
>  bindings/cxx/gpiodcxx/Makefile.am           |  14 +
>  bindings/cxx/gpiodcxx/chip.hpp              | 180 ++++
>  bindings/cxx/gpiodcxx/edge-event-buffer.hpp | 115 +++
>  bindings/cxx/gpiodcxx/edge-event.hpp        | 125 +++
>  bindings/cxx/gpiodcxx/info-event.hpp        | 108 +++
>  bindings/cxx/gpiodcxx/line-config.hpp       | 250 ++++++
>  bindings/cxx/gpiodcxx/line-info.hpp         | 209 +++++
>  bindings/cxx/gpiodcxx/line-request.hpp      | 207 +++++
>  bindings/cxx/gpiodcxx/misc.hpp              |  49 +
>  bindings/cxx/gpiodcxx/request-config.hpp    |  97 ++
>  bindings/cxx/info-event.cpp                 |  89 ++
>  bindings/cxx/internal.hpp                   | 168 +++-
>  bindings/cxx/iter.cpp                       |  60 --
>  bindings/cxx/line-config.cpp                | 226 +++++
>  bindings/cxx/line-info.cpp                  | 150 ++++
>  bindings/cxx/line-request.cpp               | 194 ++++
>  bindings/cxx/line.cpp                       | 321 -------
>  bindings/cxx/line_bulk.cpp                  | 366 --------
>  bindings/cxx/misc.cpp                       |  18 +
>  bindings/cxx/request-config.cpp             |  80 ++
>  configure.ac                                |   1 +
>  include/gpiod.h                             |  63 +-
>  lib/edge-event.c                            |   6 +
>  lib/line-request.c                          |  50 +-
>  tools/gpioget.c                             |   3 +-
>  38 files changed, 2854 insertions(+), 1849 deletions(-)
>  create mode 100644 bindings/cxx/edge-event-buffer.cpp
>  create mode 100644 bindings/cxx/edge-event.cpp
>  create mode 100644 bindings/cxx/gpiodcxx/Makefile.am
>  create mode 100644 bindings/cxx/gpiodcxx/chip.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/edge-event-buffer.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/edge-event.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/info-event.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/line-config.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/line-info.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/line-request.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/misc.hpp
>  create mode 100644 bindings/cxx/gpiodcxx/request-config.hpp
>  create mode 100644 bindings/cxx/info-event.cpp
>  delete mode 100644 bindings/cxx/iter.cpp
>  create mode 100644 bindings/cxx/line-config.cpp
>  create mode 100644 bindings/cxx/line-info.cpp
>  create mode 100644 bindings/cxx/line-request.cpp
>  delete mode 100644 bindings/cxx/line.cpp
>  delete mode 100644 bindings/cxx/line_bulk.cpp
>  create mode 100644 bindings/cxx/misc.cpp
>  create mode 100644 bindings/cxx/request-config.cpp
>
> --
> 2.30.1
>