mbox series

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

Message ID 20210727143400.32543-1-brgl@bgdev.pl
Headers show
Series libgpiod v2: C++ bindings | expand

Message

Bartosz Golaszewski July 27, 2021, 2:33 p.m. UTC
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

Comments

Jack Winch Aug. 2, 2021, 8:43 a.m. UTC | #1
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@bgdev.pl> 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

>
Bartosz Golaszewski Aug. 2, 2021, 10:33 a.m. UTC | #2
On Mon, Aug 2, 2021 at 10:43 AM Jack Winch <sunt.un.morcov@gmail.com> wrote:
>

> 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?

>


It's actually my fault. Let me explain: the development of the v2 API
for the entire project (C, C++ and Python parts) happens in the
next/libgpiod-2.0 branch of
git://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git I squash the
new changes into the big patch with the plan of resending it for a
final review once we agree on the interface (more or less anyway).
That will allow the repo to remain bisectable. Then we'll be able to
fine-tune different elements.

> 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.

>


Maybe you could wait until I post v4? I'm addressing a lot of issues
in this revision. Previous small patches have already been squashed
and they concerned the core C library anyway.

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

>


There's no target, it'll be ready when it's done.

Bart

PS How did the move go?
Jack Winch Aug. 2, 2021, 12:24 p.m. UTC | #3
On Mon, Aug 2, 2021 at 11:34 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> It's actually my fault. Let me explain: the development of the v2 API

> for the entire project (C, C++ and Python parts) happens in the

> next/libgpiod-2.0 branch of

> git://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git I squash the

> new changes into the big patch with the plan of resending it for a

> final review once we agree on the interface (more or less anyway).

> That will allow the repo to remain bisectable. Then we'll be able to

> fine-tune different elements.


Understood and sensible.  Thanks for clarifying.

> Maybe you could wait until I post v4? I'm addressing a lot of issues

> in this revision. Previous small patches have already been squashed

> and they concerned the core C library anyway.


Sounds like a plan.  I've noticed that, both in the mailing list and
your recent commits into next/libgpiod-2.0 (which is why I wondered
what the mode of operation is for these changes).  Once v4 is out, I
will look to review.

> There's no target, it'll be ready when it's done.


Again, understood and sensible.

> PS How did the move go?


Very well, thank you.  Setup with everything I've brought over to
Romania, with a little more to be shipped over very soon.  On a
somewhat related topic, I'll be reaching out to you directly over the
next few days (mainly regarding your relationship / work with
BayLibre, if that's okay).

Jack