bindings: cxx: implement C++ bindings for libgpiod v2.0

This rewrites the C++ bindings for libgpiod in order to work with v2.0 version of the C API. The C++ standard use is C++17 which is well supported in GCC. The documentation covers the entire API so for details please refer to it, the tests and example programs.

Signed-off-by Bartosz Golaszewski <[email protected]>

---
v4 -> v5:

While this is technically the fifth iteration of C++ bindings I'm posting, the change are so many, it doesn't make sense to list them here - especially since the C API changed in the meantime too. This time the tests have been rewritten as well so the bindings can actually be tested using gpio-sim.

 Doxyfile.in                                 |   4 +-
 bindings/cxx/Makefile.am                    |  23 +-
 bindings/cxx/chip-info.cpp                  |  74 ++
 bindings/cxx/chip.cpp                       | 213 +++--
 bindings/cxx/edge-event-buffer.cpp          | 115 +++
 bindings/cxx/edge-event.cpp                 | 135 +++
 bindings/cxx/examples/Makefile.am           |  12 +-
 bindings/cxx/examples/gpiodetectcxx.cpp     |  10 +-
 bindings/cxx/examples/gpiofindcxx.cpp       |   2 +-
 bindings/cxx/examples/gpiogetcxx.cpp        |  19 +-
 bindings/cxx/examples/gpioinfocxx.cpp       |  64 +-
 bindings/cxx/examples/gpiomoncxx.cpp        |  53 +-
 bindings/cxx/examples/gpiosetcxx.cpp        |  33 +-
 bindings/cxx/exception.cpp                  | 119 +++
 bindings/cxx/gpiod.hpp                      | 944 +-------------------
 bindings/cxx/gpiodcxx/Makefile.am           |  18 +
 bindings/cxx/gpiodcxx/chip-info.hpp         | 105 +++
 bindings/cxx/gpiodcxx/chip.hpp              | 179 ++++
 bindings/cxx/gpiodcxx/edge-event-buffer.hpp | 129 +++
 bindings/cxx/gpiodcxx/edge-event.hpp        | 137 +++
 bindings/cxx/gpiodcxx/exception.hpp         | 158 ++++
 bindings/cxx/gpiodcxx/info-event.hpp        | 123 +++
 bindings/cxx/gpiodcxx/line-config.hpp       | 564 ++++++++++++
 bindings/cxx/gpiodcxx/line-info.hpp         | 176 ++++
 bindings/cxx/gpiodcxx/line-request.hpp      | 221 +++++
 bindings/cxx/gpiodcxx/line.hpp              | 274 ++++++
 bindings/cxx/gpiodcxx/misc.hpp              |  44 +
 bindings/cxx/gpiodcxx/request-config.hpp    | 163 ++++
 bindings/cxx/gpiodcxx/timestamp.hpp         | 122 +++
 bindings/cxx/info-event.cpp                 | 102 +++
 bindings/cxx/internal.cpp                   |  28 +
 bindings/cxx/internal.hpp                   | 208 ++++-
 bindings/cxx/iter.cpp                       |  60 --
 bindings/cxx/line-config.cpp                | 685 ++++++++++++++
 bindings/cxx/line-info.cpp                  | 189 ++++
 bindings/cxx/line-request.cpp               | 224 +++++
 bindings/cxx/line.cpp                       | 331 ++-----
 bindings/cxx/line_bulk.cpp                  | 366 --------
 bindings/cxx/misc.cpp                       |  20 +
 bindings/cxx/request-config.cpp             | 174 ++++
 bindings/cxx/tests/Makefile.am              |  27 +-
 bindings/cxx/tests/check-kernel.cpp         |  48 +
 bindings/cxx/tests/gpio-mockup.cpp          | 153 ----
 bindings/cxx/tests/gpio-mockup.hpp          |  94 --
 bindings/cxx/tests/gpiod-cxx-test.cpp       |  55 --
 bindings/cxx/tests/gpiosim.cpp              | 264 ++++++
 bindings/cxx/tests/gpiosim.hpp              |  69 ++
 bindings/cxx/tests/helpers.cpp              |  37 +
 bindings/cxx/tests/helpers.hpp              |  36 +
 bindings/cxx/tests/tests-chip-info.cpp      |  91 ++
 bindings/cxx/tests/tests-chip.cpp           | 219 +++--
 bindings/cxx/tests/tests-edge-event.cpp     | 417 +++++++++
 bindings/cxx/tests/tests-event.cpp          | 280 ------
 bindings/cxx/tests/tests-info-event.cpp     | 198 ++++
 bindings/cxx/tests/tests-iter.cpp           |  21 -
 bindings/cxx/tests/tests-line-config.cpp    | 270 ++++++
 bindings/cxx/tests/tests-line-info.cpp      | 140 +++
 bindings/cxx/tests/tests-line-request.cpp   | 494 ++++++++++
 bindings/cxx/tests/tests-line.cpp           | 467 ----------
 bindings/cxx/tests/tests-misc.cpp           |  78 ++
 bindings/cxx/tests/tests-request-config.cpp | 155 ++++
 configure.ac                                |   1 +
 62 files changed, 7270 insertions(+), 2964 deletions(-)
 create mode 100644 bindings/cxx/chip-info.cpp
 create mode 100644 bindings/cxx/edge-event-buffer.cpp
 create mode 100644 bindings/cxx/edge-event.cpp
 create mode 100644 bindings/cxx/exception.cpp
 create mode 100644 bindings/cxx/gpiodcxx/Makefile.am
 create mode 100644 bindings/cxx/gpiodcxx/chip-info.hpp
 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/exception.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/line.hpp
 create mode 100644 bindings/cxx/gpiodcxx/misc.hpp
 create mode 100644 bindings/cxx/gpiodcxx/request-config.hpp
 create mode 100644 bindings/cxx/gpiodcxx/timestamp.hpp
 create mode 100644 bindings/cxx/info-event.cpp
 create mode 100644 bindings/cxx/internal.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_bulk.cpp
 create mode 100644 bindings/cxx/misc.cpp
 create mode 100644 bindings/cxx/request-config.cpp
 create mode 100644 bindings/cxx/tests/check-kernel.cpp
 delete mode 100644 bindings/cxx/tests/gpio-mockup.cpp
 delete mode 100644 bindings/cxx/tests/gpio-mockup.hpp
 delete mode 100644 bindings/cxx/tests/gpiod-cxx-test.cpp
 create mode 100644 bindings/cxx/tests/gpiosim.cpp
 create mode 100644 bindings/cxx/tests/gpiosim.hpp
 create mode 100644 bindings/cxx/tests/helpers.cpp
 create mode 100644 bindings/cxx/tests/helpers.hpp
 create mode 100644 bindings/cxx/tests/tests-chip-info.cpp

 create mode 100644 bindings/cxx/tests/tests-edge-event.cpp
 delete mode 100644 bindings/cxx/tests/tests-event.cpp
 create mode 100644 bindings/cxx/tests/tests-info-event.cpp
 delete mode 100644 bindings/cxx/tests/tests-iter.cpp
 create mode 100644 bindings/cxx/tests/tests-line-config.cpp
 create mode 100644 bindings/cxx/tests/tests-line-info.cpp
 create mode 100644 bindings/cxx/tests/tests-line-request.cpp
 delete mode 100644 bindings/cxx/tests/tests-line.cpp

 create mode 100644 bindings/cxx/tests/tests-misc.cpp
 create mode 100644 bindings/cxx/tests/tests-request-config.cpp

bindings/cxx/examples/gpiomoncxx.cpp +15 -18

index 4d6ac6e..db053dd 100644
@@ -3,29 +3,27 @@
 
 /* Simplified C++ reimplementation of the gpiomon tool. */
 
-#include <gpiod.hpp>
-
+#include <chrono>
 #include <cstdlib>
+#include <gpiod.hpp>
 #include <iostream>
 
 namespace {
 
-void print_event(const ::gpiod::line_event& event)
+void print_event(const ::gpiod::edge_event& event)
 {
-	if (event.event_type == ::gpiod::line_event::RISING_EDGE)
+	if (event.type() == ::gpiod::edge_event::event_type::RISING_EDGE)
 		::std::cout << " RISING EDGE";
-	else if (event.event_type == ::gpiod::line_event::FALLING_EDGE)
-		::std::cout << "FALLING EDGE";
 	else
-		throw ::std::logic_error("invalid event type");
+		::std::cout << "FALLING EDGE";
 
 	::std::cout << " ";
 
-	::std::cout << ::std::chrono::duration_cast<::std::chrono::seconds>(event.timestamp).count();
+	::std::cout << event.timestamp_ns() / 1000000000;
 	::std::cout << ".";
-	::std::cout << event.timestamp.count() % 1000000000;
+	::std::cout << event.timestamp_ns() % 1000000000;
 
-	::std::cout << " line: " << event.source.offset();
+	::std::cout << " line: " << event.line_offset();
 
 	::std::cout << ::std::endl;
 }

@@ -39,25 +37,36 @@ int main(int argc, char **argv)
 		return EXIT_FAILURE;
 	}
 
-	::std::vector<unsigned int> offsets;
+	::gpiod::line::offsets offsets;
 	offsets.reserve(argc);
 	for (int i = 2; i < argc; i++)
 		offsets.push_back(::std::stoul(argv[i]));
 
 	::gpiod::chip chip(argv[1]);
-	auto lines = chip.get_lines(offsets);
-
-	lines.request({
-		argv[0],
-		::gpiod::line_request::EVENT_BOTH_EDGES,
-		0,
-	});
+	auto request = chip.request_lines(
+			::gpiod::request_config({
+				{ ::gpiod::request_config::property::OFFSETS, offsets},
+				{ ::gpiod::request_config::property::CONSUMER, "gpiomoncxx"},
+			}),
+			::gpiod::line_config({

bindings/cxx/examples/gpiomoncxx.cpp +1 -2

+	::gpiod::edge_event_buffer buffer;
 
 	for (;;) {
-		auto events = lines.event_wait(::std::chrono::seconds(1));
-		if (events) {

bindings/cxx/gpiodcxx/chip-info.hpp new file +0 -0

new file mode 100644
index 0000000..9313e88

+ * @param out Output stream to write to.
+ * @param chip GPIO chip to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const chip_info& chip);

bindings/cxx/gpiodcxx/chip-info.hpp new file +8 -0

+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_CHIP_INFO_HPP__ */

bindings/cxx/gpiodcxx/chip.hpp new file +147 -0

new file mode 100644
index 0000000..7cc2156
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file chip.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_CHIP_HPP__
+#define __LIBGPIOD_CXX_CHIP_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <chrono>
+#include <cstddef>
+#include <iostream>
+#include <filesystem>
+#include <memory>
+
+#include "line.hpp"
+
+namespace gpiod {
+
+class chip_info;
+class info_event;
+class line_config;
+class line_info;
+class line_request;
+class request_config;
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Represents a GPIO chip.
+ */
+class chip
+{
+public:
+
+	/**
+	 * @brief Instantiates a new chip object by opening the device file
+	 *        indicated by the path argument.
+	 * @param path Path to the device file to open.
+	 */
+	explicit chip(const ::std::filesystem::path& path);
+
+	chip(const chip& other) = delete;
+
+	/**
+	 * @brief Move constructor.
+	 * @param other Object to move.
+	 */
+	chip(chip&& other) noexcept;
+
+	~chip(void);
+
+	chip& operator=(const chip& other) = delete;
+
+	/**
+	 * @brief Move assignment operator.
+	 * @param other Object to move.
+	 * @return Reference to self.
+	 */
+	chip& operator=(chip&& other) noexcept;
+
+	/**
+	 * @brief Check if this object is valid.
+	 * @return True if this object's methods can be used, false otherwise.
+	 *         False usually means the chip was closed. If the user calls
+	 *         any of the methods of this class on an object for which this
+	 *         operator returned false, a logic_error will be thrown.
+	 */
+	explicit operator bool(void) const noexcept;
+
+	/**
+	 * @brief Close the GPIO chip device file and free associated resources.
+	 * @note The chip object can live after calling this method but any of
+	 *       the chip's mutators will throw a logic_error exception.
+	 */
+	void close(void);
+
+	/**
+	 * @brief Get the filesystem path that was used to open this GPIO chip.
+	 * @return Path to the underlying character device file.
+	 */
+	::std::filesystem::path path(void) const;
+
+	/**
+	 * @brief Get information about the chip.
+	 * @return New chip_info object.
+	 */
+	chip_info get_info(void) const;
+
+	/**
+	 * @brief Retrieve the current snapshot of line information for a
+	 *        single line.
+	 * @param offset Offset of the line to get the info for.
+	 * @return New ::gpiod::line_info object.
+	 */
+	line_info get_line_info(line::offset offset) const;
+
+	/**
+	 * @brief Wrapper around ::gpiod::chip::get_line_info that retrieves
+	 *        the line info and starts watching the line for changes.
+	 * @param offset Offset of the line to get the info for.
+	 * @return New ::gpiod::line_info object.
+	 */
+	line_info watch_line_info(line::offset offset) const;
+
+	/**
+	 * @brief Stop watching the line at given offset for info events.
+	 * @param offset Offset of the line to get the info for.
+	 */
+	void unwatch_line_info(line::offset offset) const;
+
+	/**
+	 * @brief Get the file descriptor associated with this chip.
+	 * @return File descriptor number.
+	 */
+	int fd(void) const;
+
+	/**
+	 * @brief Wait for line status events on any of the watched lines
+	 *        exposed by this chip.
+	 * @param timeout Wait time limit in nanoseconds.
+	 * @return True if at least one event is ready to be read. False if the
+	 *         wait timed out.
+	 */
+	bool wait_info_event(const ::std::chrono::nanoseconds& timeout) const;
+
+	/**
+	 * @brief Read a single line status change event from this chip.
+	 * @return New info_event object.
+	 */
+	info_event read_info_event(void) const;
+
+	/**
+	 * @brief Map a GPIO line's name to its offset within the chip.
+	 * @param name Name of the GPIO line to map.
+	 * @return Offset of the line within the chip or -1 if the line with
+	 *         given name is not exposed by this chip.
+	 */
+	int find_line(const ::std::string& name) const;

bindings/cxx/gpiodcxx/chip.hpp new file +22 -0

+private:
+
+	struct impl;
+
+	::std::unique_ptr<impl> _m_priv;
+};
+
+/**
+ * @brief Stream insertion operator for GPIO chip objects.
+ * @param out Output stream to write to.
+ * @param chip GPIO chip to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const chip& chip);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_CHIP_HPP__ */

+++ b/bindings/cxx/gpiodcxx/line-request.hpp

bindings/cxx/gpiodcxx/chip.hpp new file +195 -0

@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line-request.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+#define __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <chrono>
+#include <cstddef>
+#include <iostream>
+#include <memory>
+
+#include "misc.hpp"
+
+namespace gpiod {
+
+class chip;
+class edge_event;
+class edge_event_buffer;
+class line_config;
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Stores the context of a set of requested GPIO lines.
+ */
+class line_request
+{
+public:
+
+	line_request(const line_request& other) = delete;
+
+	/**
+	 * @brief Move constructor.
+	 * @param other Object to move.
+	 */
+	line_request(line_request&& other) noexcept;
+
+	~line_request(void);
+
+	line_request& operator=(const line_request& other) = delete;
+
+	/**
+	 * @brief Move assignment operator.
+	 * @param other Object to move.
+	 */
+	line_request& operator=(line_request&& other) noexcept;
+
+	/**
+	 * @brief Check if this object is valid.
+	 * @return True if this object's methods can be used, false otherwise.
+	 *         False usually means the request was released. If the user
+	 *         calls any of the methods of this class on an object for
+	 *         which this operator returned false, a logic_error will be
+	 *         thrown.
+	 */
+	explicit operator bool(void) const noexcept;
+
+	/**
+	 * @brief Release the GPIO chip and free all associated resources.
+	 * @note The object can still be used after this method is called but
+	 *       using any of the mutators will result in throwing
+	 *       a logic_error exception.
+	 */
+	void release(void);
+
+	/**
+	 * @brief Get the number of requested lines.
+	 * @return Number of lines in this request.
+	 */
+	::std::size_t num_lines(void) const;
+
+	/**
+	 * @brief Get the list of offsets of requested lines.
+	 * @return List of hardware offsets of the lines in this request.
+	 */
+	line::offsets offsets(void) const;
+
+	/**
+	 * @brief Get the value of a single requested line.
+	 * @param offset Offset of the line to read within the chip.
+	 * @return Current line value.
+	 */
+	line::value get_value(line::offset offset);
+
+	/**
+	 * @brief Get the values of a subset of requested lines.
+	 * @param offsets Vector of line offsets
+	 * @return Vector of lines values with indexes of values corresponding
+	 *         to those of the offsets.
+	 */
+	line::values get_values(const line::offsets& offsets);
+
+	/**
+	 * @brief Get the values of all requested lines.
+	 * @return List of read values.
+	 */
+	line::values get_values(void);
+
+	/**
+	 * @brief Get the values of a subset of requested lines into a vector
+	 *        supplied by the caller.
+	 * @param offsets Vector of line offsets.
+	 * @param values Vector for storing the values. Its size must be at
+	 *               least that of the offsets vector. The indexes of read
+	 *               values will correspond with those in the offsets
+	 *               vector.
+	 */
+	void get_values(const line::offsets& offsets, line::values& values);
+
+	/**
+	 * @brief Get the values of all requested lines.
+	 * @param values Array in which the values will be stored. Must hold
+	 *               at least the number of elements returned by
+	 *               line_request::num_lines.
+	 */
+	void get_values(line::values& values);
+
+	/**
+	 * @brief Set the value of a single requested line.
+	 * @param offset Offset of the line to set within the chip.
+	 * @param value New line value.
+	 */
+	void set_value(line::offset offset, line::value value);
+
+	/**
+	 * @brief Set the values of a subset of requested lines.
+	 * @param values Vector containing a set of offset->value mappings.
+	 */
+	void set_values(const line::value_mappings& values);
+
+	/**
+	 * @brief Set the values of a subset of requested lines.
+	 * @param offsets Vector containing the offsets of lines to set.
+	 * @param values Vector containing new values with indexes
+	 *               corresponding with those in the offsets vector.
+	 */
+	void set_values(const line::offsets& offsets, const line::values& values);
+
+	/**
+	 * @brief Set the values of all requested lines.
+	 * @param values Array of new line values. The size must be equal to
+	 *               the value returned by line_request::num_lines.
+	 */
+	void set_values(const line::values& values);
+
+	/**
+	 * @brief Apply new config options to requested lines.
+	 * @param config New configuration.
+	 */
+	void reconfigure_lines(const line_config& config);
+
+	/**
+	 * @brief Get the file descriptor number associated with this line
+	 *        request.
+	 * @return File descriptor number.
+	 */
+	int fd(void) const;
+
+	/**
+	 * @brief Wait for edge events on any of the lines requested with edge
+	 *        detection enabled.
+	 * @param timeout Wait time limit in nanoseconds.
+	 * @return True if at least one event is ready to be read. False if the
+	 *         wait timed out.
+	 */
+	bool wait_edge_event(const ::std::chrono::nanoseconds& timeout) const;
+
+	/**
+	 * @brief Read a number of edge events from this request up to the
+	 *        maximum capacity of the buffer.
+	 * @param buffer Edge event buffer to read events into.
+	 * @return Number of events read.
+	 */
+	::std::size_t read_edge_event(edge_event_buffer& buffer);
+
+	/**
+	 * @brief Read a number of edge events from this request.
+	 * @param buffer Edge event buffer to read events into.
+	 * @param max_events Maximum number of events to read. Limited by the
+	 *                   capacity of the buffer.
+	 * @return Number of events read.
+	 */
+	::std::size_t read_edge_event(edge_event_buffer& buffer, ::std::size_t max_events);
+

bindings/cxx/gpiodcxx/chip.hpp new file +16 -0

+
+/**
+ * @brief Stream insertion operator for line requests.
+ * @param out Output stream to write to.
+ * @param request Line request object to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const line_request& request);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_LINE_REQUEST_HPP__ */

bindings/cxx/gpiodcxx/line.hpp new file +72 -0

new file mode 100644
index 0000000..8e8a984
@@ -0,0 +1,274 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_HPP__
+#define __LIBGPIOD_CXX_LINE_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <ostream>
+#include <utility>
+#include <vector>
+
+namespace gpiod {
+
+/**
+ * @brief Namespace containing various type definitions for GPIO lines.
+ */
+namespace line {
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Wrapper around unsigned int for representing line offsets.
+ */
+class offset
+{
+public:
+	/**
+	 * @brief Constructor with implicit conversion from unsigned int.
+	 * @param off Line offset.
+	 */
+	offset(unsigned int off = 0) : _m_offset(off) {	}
+
+	/**
+	 * @brief Copy constructor.
+	 * @param other Object to copy.
+	 */
+	offset(const offset& other) = default;
+
+	/**
+	 * @brief Move constructor.
+	 * @param other Object to move.
+	 */
+	offset(offset&& other) = default;
+
+	~offset(void) = default;
+
+	/**
+	 * @brief Assignment operator.
+	 * @param other Object to copy.
+	 * @return Reference to self.
+	 */
+	offset& operator=(const offset& other) = default;
+
+	/**
+	 * @brief Move assignment operator.
+	 * @param other Object to move.
+	 * @return Reference to self.
+	 */
+	offset& operator=(offset&& other) noexcept = default;
+
+	/**
+	 * @brief Conversion operator to `unsigned int`.

bindings/cxx/gpiodcxx/line.hpp new file +7 -0

+TEST_CASE("line_request stream insertion operator works", "[line-request]")
+{
+	::gpiosim::chip sim({{ simprop::NUM_LINES, 4 }});
+	::gpiod::chip chip(sim.dev_path());
+
+	auto request = chip.request_lines(
+		::gpiod::request_config({

bindings/cxx/gpiodcxx/line.hpp new file +5 -0

+TEST_CASE("request_config stream insertion operator works", "[request-config]")
+{
+	::gpiod::request_config cfg({
+		{ property::CONSUMER, "foobar" },
+		{ property::OFFSETS, offsets({ 0, 1, 2, 3 })},

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+	REQUIRE_THAT(buf.str(), Catch::Equals(expected));
+}
+
+} /* namespace */

configure.ac +1 -0

index f8d34ed..ab03673 100644
@@ -239,6 +239,7 @@ AC_CONFIG_FILES([Makefile
 		 bindings/cxx/libgpiodcxx.pc
 		 bindings/Makefile
 		 bindings/cxx/Makefile
+		 bindings/cxx/gpiodcxx/Makefile
 		 bindings/cxx/examples/Makefile
 		 bindings/cxx/tests/Makefile
 		 bindings/python/Makefile

Signature · 1 line

2.30.1

Quoted 5 lines · Bartosz Golaszewski

This rewrites the C++ bindings for libgpiod in order to work with v2.0 version of the C API. The C++ standard use is C++17 which is well supported in GCC. The documentation covers the entire API so for details please refer to it, the tests and example programs.

So C++17 for cxx bindings, but still C89 for core? Maybe time to switch that to C99?

Quoted 1 line

Signed-off-by Bartosz Golaszewski <[email protected]>

Would've been nice to split this into several patches to make it more manageable. As it is I'm reluctant to suggest any changes as I really don't want to see this go to a v6 :-(.

Maybe a patch each for headers, impl, tests and examples? If nothing else that would be a better order than what we find below. My comments below may be misordered for the same reason.

Alternatively, I don't have any major issues with this patch, so I would also be ok with applying it as is and refining it from there.

Btw, I no longer actively use C++, so I'm not a good judge of what is idomatic, I'm just pointing out what looks odd to me.

No tests for line.cpp?
Should still be some tests for the streaming operators, even if it is just to provide an example of what to expect.

Quoted 3 lines

 create mode 100644 bindings/cxx/tests/tests-misc.cpp
 create mode 100644 bindings/cxx/tests/tests-request-config.cpp

[snip]

Quoted 71 lines

bindings/cxx/examples/gpiomoncxx.cpp +20 -18

index 4d6ac6e..db053dd 100644
@@ -3,29 +3,27 @@
 
 /* Simplified C++ reimplementation of the gpiomon tool. */
 
-#include <gpiod.hpp>
-
+#include <chrono>
 #include <cstdlib>
+#include <gpiod.hpp>
 #include <iostream>
 
 namespace {
 
-void print_event(const ::gpiod::line_event& event)
+void print_event(const ::gpiod::edge_event& event)
 {
-     if (event.event_type == ::gpiod::line_event::RISING_EDGE)
+     if (event.type() == ::gpiod::edge_event::event_type::RISING_EDGE)
              ::std::cout << " RISING EDGE";
-     else if (event.event_type == ::gpiod::line_event::FALLING_EDGE)
-             ::std::cout << "FALLING EDGE";
      else
-             throw ::std::logic_error("invalid event type");
+             ::std::cout << "FALLING EDGE";
 
      ::std::cout << " ";
 
-     ::std::cout << ::std::chrono::duration_cast<::std::chrono::seconds>(event.timestamp).count();
+     ::std::cout << event.timestamp_ns() / 1000000000;
      ::std::cout << ".";
-     ::std::cout << event.timestamp.count() % 1000000000;
+     ::std::cout << event.timestamp_ns() % 1000000000;
 
-     ::std::cout << " line: " << event.source.offset();
+     ::std::cout << " line: " << event.line_offset();
 
      ::std::cout << ::std::endl;
 }

@@ -39,25 +37,36 @@ int main(int argc, char **argv)
              return EXIT_FAILURE;
      }
 
-     ::std::vector<unsigned int> offsets;
+     ::gpiod::line::offsets offsets;
      offsets.reserve(argc);
      for (int i = 2; i < argc; i++)
              offsets.push_back(::std::stoul(argv[i]));
 
      ::gpiod::chip chip(argv[1]);
-     auto lines = chip.get_lines(offsets);
-
-     lines.request({
-             argv[0],
-             ::gpiod::line_request::EVENT_BOTH_EDGES,
-             0,
-     });
+     auto request = chip.request_lines(
+                     ::gpiod::request_config({
+                             { ::gpiod::request_config::property::OFFSETS, offsets},
+                             { ::gpiod::request_config::property::CONSUMER, "gpiomoncxx"},
+                     }),
+                     ::gpiod::line_config({
+                             {
+                                     ::gpiod::line_config::property::DIRECTION,
+                                     ::gpiod::line::direction::INPUT
+                             },
+                             {

bindings/cxx/examples/gpiomoncxx.cpp int main(int argc, char **argv) · line 61

Quoted 5 lines

bindings/cxx/examples/gpiomoncxx.cpp +5 -0

+                                     ::gpiod::line_config::property::EDGE,
+                                     ::gpiod::line::edge::BOTH
+                             }
+                     }));
+

Would be good to close the chip to highlight the fact that the chip is not required once the lines are requested.

Or use ::gpiod::request_lines(path,request_config,line_config) to request the lines.

Quoted 5 lines

bindings/cxx/examples/gpiomoncxx.cpp +1 -2

+     ::gpiod::edge_event_buffer buffer;
 
      for (;;) {
-             auto events = lines.event_wait(::std::chrono::seconds(1));
-             if (events) {

bindings/cxx/examples/gpiomoncxx.cpp int main(int argc, char **argv) · line 72

Quoted 10 lines

bindings/cxx/examples/gpiomoncxx.cpp +5 -2

-                     for (auto& it: events)
-                             print_event(it.event_read());
+             if (request.wait_edge_event(::std::chrono::seconds(5))) {
+                     request.read_edge_event(buffer);
+
+                     for (const auto& event: buffer)
+                             print_event(event);
              }
      }
 

What is the purpose of the wait_edge_event() here? Wouldn't read_edge_event() block until the next event?

This example should be minimal and demonstrate how the code should normally be used. e.g.

for (const auto& event: request.events_iter())
print_event(event);

(assuming the addition of an iterator wrapping request and event buffer)

If you want to showcase more complex examples then provide them separately.

[snip]

bindings/cxx/examples/gpiomoncxx.cpp

Quoted 9 lines

bindings/cxx/examples/gpiomoncxx.cpp +9 -0

+     /**
+      * @brief Request a set of lines for exclusive usage.
+      * @param req_cfg Request config object.
+      * @param line_cfg Line config object.
+      * @return New line_request object.
+      */
+     line_request request_lines(const request_config& req_cfg,
+                                const line_config& line_cfg);
+

A common use case is to just want to request a line, so a top-level version of this that hides the chip object from the user might be nice. i.e.

line_request request_lines(const ::std::filesystem::path& path,
const request_config& req_cfg, const line_config& line_cfg);

Also applies to C API.

Quoted 23 lines

bindings/cxx/examples/gpiomoncxx.cpp +22 -0

+private:
+
+     struct impl;
+
+     ::std::unique_ptr<impl> _m_priv;
+};
+
+/**
+ * @brief Stream insertion operator for GPIO chip objects.
+ * @param out Output stream to write to.
+ * @param chip GPIO chip to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const chip& chip);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_CHIP_HPP__ */

[snip]

Quoted 197 lines

+++ b/bindings/cxx/gpiodcxx/line-request.hpp

bindings/cxx/examples/gpiomoncxx.cpp +195 -0

@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line-request.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+#define __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <chrono>
+#include <cstddef>
+#include <iostream>
+#include <memory>
+
+#include "misc.hpp"
+
+namespace gpiod {
+
+class chip;
+class edge_event;
+class edge_event_buffer;
+class line_config;
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Stores the context of a set of requested GPIO lines.
+ */
+class line_request
+{
+public:
+
+     line_request(const line_request& other) = delete;
+
+     /**
+      * @brief Move constructor.
+      * @param other Object to move.
+      */
+     line_request(line_request&& other) noexcept;
+
+     ~line_request(void);
+
+     line_request& operator=(const line_request& other) = delete;
+
+     /**
+      * @brief Move assignment operator.
+      * @param other Object to move.
+      */
+     line_request& operator=(line_request&& other) noexcept;
+
+     /**
+      * @brief Check if this object is valid.
+      * @return True if this object's methods can be used, false otherwise.
+      *         False usually means the request was released. If the user
+      *         calls any of the methods of this class on an object for
+      *         which this operator returned false, a logic_error will be
+      *         thrown.
+      */
+     explicit operator bool(void) const noexcept;
+
+     /**
+      * @brief Release the GPIO chip and free all associated resources.
+      * @note The object can still be used after this method is called but
+      *       using any of the mutators will result in throwing
+      *       a logic_error exception.
+      */
+     void release(void);
+
+     /**
+      * @brief Get the number of requested lines.
+      * @return Number of lines in this request.
+      */
+     ::std::size_t num_lines(void) const;
+
+     /**
+      * @brief Get the list of offsets of requested lines.
+      * @return List of hardware offsets of the lines in this request.
+      */
+     line::offsets offsets(void) const;
+
+     /**
+      * @brief Get the value of a single requested line.
+      * @param offset Offset of the line to read within the chip.
+      * @return Current line value.
+      */
+     line::value get_value(line::offset offset);
+
+     /**
+      * @brief Get the values of a subset of requested lines.
+      * @param offsets Vector of line offsets
+      * @return Vector of lines values with indexes of values corresponding
+      *         to those of the offsets.
+      */
+     line::values get_values(const line::offsets& offsets);
+
+     /**
+      * @brief Get the values of all requested lines.
+      * @return List of read values.
+      */
+     line::values get_values(void);
+
+     /**
+      * @brief Get the values of a subset of requested lines into a vector
+      *        supplied by the caller.
+      * @param offsets Vector of line offsets.
+      * @param values Vector for storing the values. Its size must be at
+      *               least that of the offsets vector. The indexes of read
+      *               values will correspond with those in the offsets
+      *               vector.
+      */
+     void get_values(const line::offsets& offsets, line::values& values);
+
+     /**
+      * @brief Get the values of all requested lines.
+      * @param values Array in which the values will be stored. Must hold
+      *               at least the number of elements returned by
+      *               line_request::num_lines.
+      */
+     void get_values(line::values& values);
+
+     /**
+      * @brief Set the value of a single requested line.
+      * @param offset Offset of the line to set within the chip.
+      * @param value New line value.
+      */
+     void set_value(line::offset offset, line::value value);
+
+     /**
+      * @brief Set the values of a subset of requested lines.
+      * @param values Vector containing a set of offset->value mappings.
+      */
+     void set_values(const line::value_mappings& values);
+
+     /**
+      * @brief Set the values of a subset of requested lines.
+      * @param offsets Vector containing the offsets of lines to set.
+      * @param values Vector containing new values with indexes
+      *               corresponding with those in the offsets vector.
+      */
+     void set_values(const line::offsets& offsets, const line::values& values);
+
+     /**
+      * @brief Set the values of all requested lines.
+      * @param values Array of new line values. The size must be equal to
+      *               the value returned by line_request::num_lines.
+      */
+     void set_values(const line::values& values);
+
+     /**
+      * @brief Apply new config options to requested lines.
+      * @param config New configuration.
+      */
+     void reconfigure_lines(const line_config& config);
+
+     /**
+      * @brief Get the file descriptor number associated with this line
+      *        request.
+      * @return File descriptor number.
+      */
+     int fd(void) const;
+
+     /**
+      * @brief Wait for edge events on any of the lines requested with edge
+      *        detection enabled.
+      * @param timeout Wait time limit in nanoseconds.
+      * @return True if at least one event is ready to be read. False if the
+      *         wait timed out.
+      */
+     bool wait_edge_event(const ::std::chrono::nanoseconds& timeout) const;
+
+     /**
+      * @brief Read a number of edge events from this request up to the
+      *        maximum capacity of the buffer.
+      * @param buffer Edge event buffer to read events into.
+      * @return Number of events read.
+      */
+     ::std::size_t read_edge_event(edge_event_buffer& buffer);
+
+     /**
+      * @brief Read a number of edge events from this request.
+      * @param buffer Edge event buffer to read events into.
+      * @param max_events Maximum number of events to read. Limited by the
+      *                   capacity of the buffer.
+      * @return Number of events read.
+      */
+     ::std::size_t read_edge_event(edge_event_buffer& buffer, ::std::size_t max_events);
+

bindings/cxx/examples/gpiomoncxx.cpp line 205

Quoted 10 lines

bindings/cxx/examples/gpiomoncxx.cpp +10 -0

+private:
+
+     line_request(void);
+
+     struct impl;
+
+     ::std::unique_ptr<impl> _m_priv;
+
+     friend chip;
+};

Just wanting to get events from a request is a common use case, and having to use an edge_event_buffer is tedious (refer to the gpiomoncxx example)
How about an iterator that wraps the request and an edge_event_buffer and provides events?
I refer to this elsewhere as request.events_iter().

Quoted 99 lines

bindings/cxx/examples/gpiomoncxx.cpp +16 -0

+
+/**
+ * @brief Stream insertion operator for line requests.
+ * @param out Output stream to write to.
+ * @param request Line request object to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const line_request& request);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_LINE_REQUEST_HPP__ */

bindings/cxx/gpiodcxx/line.hpp new file +77 -0

new file mode 100644
index 0000000..8e8a984
@@ -0,0 +1,274 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_HPP__
+#define __LIBGPIOD_CXX_LINE_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <ostream>
+#include <utility>
+#include <vector>
+
+namespace gpiod {
+
+/**
+ * @brief Namespace containing various type definitions for GPIO lines.
+ */
+namespace line {
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Wrapper around unsigned int for representing line offsets.
+ */
+class offset
+{
+public:
+     /**
+      * @brief Constructor with implicit conversion from unsigned int.
+      * @param off Line offset.
+      */
+     offset(unsigned int off = 0) : _m_offset(off) { }
+
+     /**
+      * @brief Copy constructor.
+      * @param other Object to copy.
+      */
+     offset(const offset& other) = default;
+
+     /**
+      * @brief Move constructor.
+      * @param other Object to move.
+      */
+     offset(offset&& other) = default;
+
+     ~offset(void) = default;
+
+     /**
+      * @brief Assignment operator.
+      * @param other Object to copy.
+      * @return Reference to self.
+      */
+     offset& operator=(const offset& other) = default;
+
+     /**
+      * @brief Move assignment operator.
+      * @param other Object to move.
+      * @return Reference to self.
+      */
+     offset& operator=(offset&& other) noexcept = default;
+
+     /**
+      * @brief Conversion operator to `unsigned int`.
+      */
+     operator unsigned int(void) const noexcept
+     {
+             return this->_m_offset;
+     }

bindings/cxx/gpiodcxx/line.hpp line 82

Quoted 5 lines

bindings/cxx/gpiodcxx/line.hpp new file +5 -0

+
+private:
+     unsigned int _m_offset;
+};
+

Wrapping unsigned int in a class seems like overkill. Is this just to get the streaming operators for offsets to work?

[snip]

bindings/cxx/gpiodcxx/line.hpp

Quoted 3 lines

bindings/cxx/gpiodcxx/line.hpp new file +3 -0

+GPIOD_CXX_API line::offsets line_request::offsets(void) const
+{
+     this->_m_priv->throw_if_released();

redundant as this->num_lines() also does it.

bindings/cxx/gpiodcxx/line.hpp

Quoted 9 lines

bindings/cxx/gpiodcxx/line.hpp new file +9 -0

+
+     ::std::vector<unsigned int> buf(this->num_lines());
+     line::offsets offsets(this->num_lines());
+
+     ::gpiod_line_request_get_offsets(this->_m_priv->request.get(), buf.data());
+
+     for (unsigned int i = 0; i < this->num_lines(); i++)
+             offsets[i] = buf[i];
+

Cache num_lines locally rather than calling num_lines() several times.

[snip]

bindings/cxx/gpiodcxx/line.hpp

Quoted 13 lines

bindings/cxx/gpiodcxx/line.hpp new file +8 -3

+template<typename T>
+::std::ostream& insert_vector(::std::ostream& out,
+                           const ::std::string& name, const ::std::vector<T>& vec)
 {
-     this->_m_line = nullptr;
-     this->_m_owner.reset();
-}
+     out << name << "([";
+     ::std::copy(vec.begin(), ::std::prev(vec.end()),
+                 ::std::ostream_iterator<T>(out, ", "));
+     out << vec.back();
+     out << "])";

What formatting are you after for vectors?
Is the double bracketing necessary?

[snip]

Quoted 13 lines

bindings/cxx/gpiodcxx/line.hpp new file +13 -0

+TEST_CASE("line_request stream insertion operator works", "[line-request]")
+{
+     ::gpiosim::chip sim({{ simprop::NUM_LINES, 4 }});
+     ::gpiod::chip chip(sim.dev_path());
+
+     auto request = chip.request_lines(
+             ::gpiod::request_config({
+                     { reqprop::OFFSETS, offsets({ 3, 1, 0, 2 }) }
+             }),
+             ::gpiod::line_config()
+     );
+
+     ::std::stringstream buf, expected;

bindings/cxx/gpiodcxx/line.hpp

Quoted 4 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+
+     expected << "line_request(num_lines=4, line_offsets=[offsets([3, 1, 0, 2])], fd=" <<
+                 request.fd() << ")";
+

Still not sure what formatting you are going for with the vectors. And the line_offsets=[] is not consistent with the offsets=() used in request_config.

[snip]

Quoted 11 lines

bindings/cxx/gpiodcxx/line.hpp new file +11 -0

+TEST_CASE("request_config stream insertion operator works", "[request-config]")
+{
+     ::gpiod::request_config cfg({
+             { property::CONSUMER, "foobar" },
+             { property::OFFSETS, offsets({ 0, 1, 2, 3 })},
+             { property::EVENT_BUFFER_SIZE, 32 }
+     });
+
+     ::std::stringstream buf;
+
+     buf << cfg;

bindings/cxx/gpiodcxx/line.hpp

Quoted 4 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+
+     ::std::string expected("request_config(consumer='foobar', num_offsets=4, "
+                            "offsets=(offsets([0, 1, 2, 3])), event_buffer_size=32)");
+

The streaming output stutters.
"offsets=(offsets([0, 1, 2, 3]))" should be "offsets=([0, 1, 2, 3])"? And even then one of the sets of brackets is redundant.

Quoted 17 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+     REQUIRE_THAT(buf.str(), Catch::Equals(expected));
+}
+
+} /* namespace */

configure.ac +1 -1

index f8d34ed..ab03673 100644
@@ -239,6 +239,7 @@ AC_CONFIG_FILES([Makefile
               bindings/cxx/libgpiodcxx.pc
               bindings/Makefile
               bindings/cxx/Makefile
+              bindings/cxx/gpiodcxx/Makefile
               bindings/cxx/examples/Makefile
               bindings/cxx/tests/Makefile
               bindings/python/Makefile
--

Quoted 2 lines

2.30.1

Phew. So nothing major.

Cheers,
Kent.

Quoted 8 lines · Kent Gibson

Quoted 5 lines · Bartosz Golaszewski

This rewrites the C++ bindings for libgpiod in order to work with v2.0 version of the C API. The C++ standard use is C++17 which is well supported in GCC. The documentation covers the entire API so for details please refer to it, the tests and example programs.

Quoted 3 lines

So C++17 for cxx bindings, but still C89 for core? Maybe time to switch that to C99?

I'm not saying no, but what would we gain from that in C that isn't already provided by the gnu89 standard we're using? Declaring variables in for loops if all I can think of and that's not really useful for us as we're not providing lots of for_each type macros.

Quoted 10 lines

Quoted 1 line

Signed-off-by Bartosz Golaszewski <[email protected]>

Would've been nice to split this into several patches to make it more manageable. As it is I'm reluctant to suggest any changes as I really don't want to see this go to a v6 :-(.

Maybe a patch each for headers, impl, tests and examples? If nothing else that would be a better order than what we find below. My comments below may be misordered for the same reason.

Will do in the next iteration.

Quoted 6 lines

Alternatively, I don't have any major issues with this patch, so I would also be ok with applying it as is and refining it from there.

Btw, I no longer actively use C++, so I'm not a good judge of what is idomatic, I'm just pointing out what looks odd to me.

Yeah I need to make a list of C++ people who contributed code to libgpiod before and Cc them next time.

[snip]

Quoted 5 lines

No tests for line.cpp?
Should still be some tests for the streaming operators, even if it is just to provide an example of what to expect.

I ran the code with gcov and all those stream operators are already tested in other places, no need to be that redundant. I even skipped the tests for stream operators on purpose for objects logically "embedded" in other objects (like chip_info).

Quoted 77 lines

Quoted 3 lines

 create mode 100644 bindings/cxx/tests/tests-misc.cpp
 create mode 100644 bindings/cxx/tests/tests-request-config.cpp

[snip]

Quoted 71 lines

bindings/cxx/examples/gpiomoncxx.cpp +20 -18

index 4d6ac6e..db053dd 100644
@@ -3,29 +3,27 @@
 
 /* Simplified C++ reimplementation of the gpiomon tool. */
 
-#include <gpiod.hpp>
-
+#include <chrono>
 #include <cstdlib>
+#include <gpiod.hpp>
 #include <iostream>
 
 namespace {
 
-void print_event(const ::gpiod::line_event& event)
+void print_event(const ::gpiod::edge_event& event)
 {
-     if (event.event_type == ::gpiod::line_event::RISING_EDGE)
+     if (event.type() == ::gpiod::edge_event::event_type::RISING_EDGE)
              ::std::cout << " RISING EDGE";
-     else if (event.event_type == ::gpiod::line_event::FALLING_EDGE)
-             ::std::cout << "FALLING EDGE";
      else
-             throw ::std::logic_error("invalid event type");
+             ::std::cout << "FALLING EDGE";
 
      ::std::cout << " ";
 
-     ::std::cout << ::std::chrono::duration_cast<::std::chrono::seconds>(event.timestamp).count();
+     ::std::cout << event.timestamp_ns() / 1000000000;
      ::std::cout << ".";
-     ::std::cout << event.timestamp.count() % 1000000000;
+     ::std::cout << event.timestamp_ns() % 1000000000;
 
-     ::std::cout << " line: " << event.source.offset();
+     ::std::cout << " line: " << event.line_offset();
 
      ::std::cout << ::std::endl;
 }

@@ -39,25 +37,36 @@ int main(int argc, char **argv)
              return EXIT_FAILURE;
      }
 
-     ::std::vector<unsigned int> offsets;
+     ::gpiod::line::offsets offsets;
      offsets.reserve(argc);
      for (int i = 2; i < argc; i++)
              offsets.push_back(::std::stoul(argv[i]));
 
      ::gpiod::chip chip(argv[1]);
-     auto lines = chip.get_lines(offsets);
-
-     lines.request({
-             argv[0],
-             ::gpiod::line_request::EVENT_BOTH_EDGES,
-             0,
-     });
+     auto request = chip.request_lines(
+                     ::gpiod::request_config({
+                             { ::gpiod::request_config::property::OFFSETS, offsets},
+                             { ::gpiod::request_config::property::CONSUMER, "gpiomoncxx"},
+                     }),
+                     ::gpiod::line_config({
+                             {
+                                     ::gpiod::line_config::property::DIRECTION,
+                                     ::gpiod::line::direction::INPUT
+                             },
+                             {

Quoted 9 lines

bindings/cxx/examples/gpiomoncxx.cpp int main(int argc, char **argv) · line 61

Quoted 5 lines

bindings/cxx/examples/gpiomoncxx.cpp +5 -0

+                                     ::gpiod::line_config::property::EDGE,
+                                     ::gpiod::line::edge::BOTH
+                             }
+                     }));
+

Would be good to close the chip to highlight the fact that the chip is not required once the lines are requested.

There's a special test case for that already elsewhere.

Quoted 19 lines

Or use ::gpiod::request_lines(path,request_config,line_config) to request the lines.

Quoted 15 lines

bindings/cxx/examples/gpiomoncxx.cpp +6 -4

+     ::gpiod::edge_event_buffer buffer;
 
      for (;;) {
-             auto events = lines.event_wait(::std::chrono::seconds(1));
-             if (events) {
-                     for (auto& it: events)
-                             print_event(it.event_read());
+             if (request.wait_edge_event(::std::chrono::seconds(5))) {
+                     request.read_edge_event(buffer);
+
+                     for (const auto& event: buffer)
+                             print_event(event);
              }
      }
 

Quoted 3 lines

What is the purpose of the wait_edge_event() here? Wouldn't read_edge_event() block until the next event?

Indeed and it would block forever if the event doesn't arrive. The call is to make sure there's an even available.

Quoted 8 lines

This example should be minimal and demonstrate how the code should normally be used. e.g.

for (const auto& event: request.events_iter())
print_event(event);

(assuming the addition of an iterator wrapping request and event buffer)

Quoted 4 lines

If you want to showcase more complex examples then provide them separately.

[snip]

I'm not how that would work - would it create a new buffer every time we read events? Doesn't that defeat the entire purpose of the event buffer of preallocating memory for the events?

Quoted 14 lines

bindings/cxx/examples/gpiomoncxx.cpp

Quoted 9 lines

bindings/cxx/examples/gpiomoncxx.cpp +9 -0

+     /**
+      * @brief Request a set of lines for exclusive usage.
+      * @param req_cfg Request config object.
+      * @param line_cfg Line config object.
+      * @return New line_request object.
+      */
+     line_request request_lines(const request_config& req_cfg,
+                                const line_config& line_cfg);
+

A common use case is to just want to request a line, so a top-level version of this that hides the chip object from the user might be nice. i.e.

Quoted 5 lines

line_request request_lines(const ::std::filesystem::path& path,
const request_config& req_cfg, const line_config& line_cfg);

Also applies to C API.

I replied about this under the C patch series.

Quoted 340 lines

Quoted 23 lines

bindings/cxx/examples/gpiomoncxx.cpp +22 -0

+private:
+
+     struct impl;
+
+     ::std::unique_ptr<impl> _m_priv;
+};
+
+/**
+ * @brief Stream insertion operator for GPIO chip objects.
+ * @param out Output stream to write to.
+ * @param chip GPIO chip to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const chip& chip);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_CHIP_HPP__ */

[snip]

Quoted 197 lines

+++ b/bindings/cxx/gpiodcxx/line-request.hpp

bindings/cxx/examples/gpiomoncxx.cpp +195 -0

@@ -0,0 +1,221 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line-request.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+#define __LIBGPIOD_CXX_LINE_REQUEST_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <chrono>
+#include <cstddef>
+#include <iostream>
+#include <memory>
+
+#include "misc.hpp"
+
+namespace gpiod {
+
+class chip;
+class edge_event;
+class edge_event_buffer;
+class line_config;
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Stores the context of a set of requested GPIO lines.
+ */
+class line_request
+{
+public:
+
+     line_request(const line_request& other) = delete;
+
+     /**
+      * @brief Move constructor.
+      * @param other Object to move.
+      */
+     line_request(line_request&& other) noexcept;
+
+     ~line_request(void);
+
+     line_request& operator=(const line_request& other) = delete;
+
+     /**
+      * @brief Move assignment operator.
+      * @param other Object to move.
+      */
+     line_request& operator=(line_request&& other) noexcept;
+
+     /**
+      * @brief Check if this object is valid.
+      * @return True if this object's methods can be used, false otherwise.
+      *         False usually means the request was released. If the user
+      *         calls any of the methods of this class on an object for
+      *         which this operator returned false, a logic_error will be
+      *         thrown.
+      */
+     explicit operator bool(void) const noexcept;
+
+     /**
+      * @brief Release the GPIO chip and free all associated resources.
+      * @note The object can still be used after this method is called but
+      *       using any of the mutators will result in throwing
+      *       a logic_error exception.
+      */
+     void release(void);
+
+     /**
+      * @brief Get the number of requested lines.
+      * @return Number of lines in this request.
+      */
+     ::std::size_t num_lines(void) const;
+
+     /**
+      * @brief Get the list of offsets of requested lines.
+      * @return List of hardware offsets of the lines in this request.
+      */
+     line::offsets offsets(void) const;
+
+     /**
+      * @brief Get the value of a single requested line.
+      * @param offset Offset of the line to read within the chip.
+      * @return Current line value.
+      */
+     line::value get_value(line::offset offset);
+
+     /**
+      * @brief Get the values of a subset of requested lines.
+      * @param offsets Vector of line offsets
+      * @return Vector of lines values with indexes of values corresponding
+      *         to those of the offsets.
+      */
+     line::values get_values(const line::offsets& offsets);
+
+     /**
+      * @brief Get the values of all requested lines.
+      * @return List of read values.
+      */
+     line::values get_values(void);
+
+     /**
+      * @brief Get the values of a subset of requested lines into a vector
+      *        supplied by the caller.
+      * @param offsets Vector of line offsets.
+      * @param values Vector for storing the values. Its size must be at
+      *               least that of the offsets vector. The indexes of read
+      *               values will correspond with those in the offsets
+      *               vector.
+      */
+     void get_values(const line::offsets& offsets, line::values& values);
+
+     /**
+      * @brief Get the values of all requested lines.
+      * @param values Array in which the values will be stored. Must hold
+      *               at least the number of elements returned by
+      *               line_request::num_lines.
+      */
+     void get_values(line::values& values);
+
+     /**
+      * @brief Set the value of a single requested line.
+      * @param offset Offset of the line to set within the chip.
+      * @param value New line value.
+      */
+     void set_value(line::offset offset, line::value value);
+
+     /**
+      * @brief Set the values of a subset of requested lines.
+      * @param values Vector containing a set of offset->value mappings.
+      */
+     void set_values(const line::value_mappings& values);
+
+     /**
+      * @brief Set the values of a subset of requested lines.
+      * @param offsets Vector containing the offsets of lines to set.
+      * @param values Vector containing new values with indexes
+      *               corresponding with those in the offsets vector.
+      */
+     void set_values(const line::offsets& offsets, const line::values& values);
+
+     /**
+      * @brief Set the values of all requested lines.
+      * @param values Array of new line values. The size must be equal to
+      *               the value returned by line_request::num_lines.
+      */
+     void set_values(const line::values& values);
+
+     /**
+      * @brief Apply new config options to requested lines.
+      * @param config New configuration.
+      */
+     void reconfigure_lines(const line_config& config);
+
+     /**
+      * @brief Get the file descriptor number associated with this line
+      *        request.
+      * @return File descriptor number.
+      */
+     int fd(void) const;
+
+     /**
+      * @brief Wait for edge events on any of the lines requested with edge
+      *        detection enabled.
+      * @param timeout Wait time limit in nanoseconds.
+      * @return True if at least one event is ready to be read. False if the
+      *         wait timed out.
+      */
+     bool wait_edge_event(const ::std::chrono::nanoseconds& timeout) const;
+
+     /**
+      * @brief Read a number of edge events from this request up to the
+      *        maximum capacity of the buffer.
+      * @param buffer Edge event buffer to read events into.
+      * @return Number of events read.
+      */
+     ::std::size_t read_edge_event(edge_event_buffer& buffer);
+
+     /**
+      * @brief Read a number of edge events from this request.
+      * @param buffer Edge event buffer to read events into.
+      * @param max_events Maximum number of events to read. Limited by the
+      *                   capacity of the buffer.
+      * @return Number of events read.
+      */
+     ::std::size_t read_edge_event(edge_event_buffer& buffer, ::std::size_t max_events);
+

bindings/cxx/examples/gpiomoncxx.cpp line 205

Quoted 10 lines

bindings/cxx/examples/gpiomoncxx.cpp +10 -0

+private:
+
+     line_request(void);
+
+     struct impl;
+
+     ::std::unique_ptr<impl> _m_priv;
+
+     friend chip;
+};

Just wanting to get events from a request is a common use case, and having to use an edge_event_buffer is tedious (refer to the gpiomoncxx example)
How about an iterator that wraps the request and an edge_event_buffer and provides events?
I refer to this elsewhere as request.events_iter().

Quoted 99 lines

bindings/cxx/examples/gpiomoncxx.cpp +16 -0

+
+/**
+ * @brief Stream insertion operator for line requests.
+ * @param out Output stream to write to.
+ * @param request Line request object to insert into the output stream.
+ * @return Reference to out.
+ */
+::std::ostream& operator<<(::std::ostream& out, const line_request& request);
+
+/**
+ * @}
+ */
+
+} /* namespace gpiod */
+
+#endif /* __LIBGPIOD_CXX_LINE_REQUEST_HPP__ */

bindings/cxx/gpiodcxx/line.hpp new file +77 -0

new file mode 100644
index 0000000..8e8a984
@@ -0,0 +1,274 @@
+/* SPDX-License-Identifier: LGPL-3.0-or-later */
+/* SPDX-FileCopyrightText: 2021-2022 Bartosz Golaszewski <[email protected]> */
+
+/**
+ * @file line.hpp
+ */
+
+#ifndef __LIBGPIOD_CXX_LINE_HPP__
+#define __LIBGPIOD_CXX_LINE_HPP__
+
+#if !defined(__LIBGPIOD_GPIOD_CXX_INSIDE__)
+#error "Only gpiod.hpp can be included directly."
+#endif
+
+#include <ostream>
+#include <utility>
+#include <vector>
+
+namespace gpiod {
+
+/**
+ * @brief Namespace containing various type definitions for GPIO lines.
+ */
+namespace line {
+
+/**
+ * @ingroup gpiod_cxx
+ * @{
+ */
+
+/**
+ * @brief Wrapper around unsigned int for representing line offsets.
+ */
+class offset
+{
+public:
+     /**
+      * @brief Constructor with implicit conversion from unsigned int.
+      * @param off Line offset.
+      */
+     offset(unsigned int off = 0) : _m_offset(off) { }
+
+     /**
+      * @brief Copy constructor.
+      * @param other Object to copy.
+      */
+     offset(const offset& other) = default;
+
+     /**
+      * @brief Move constructor.
+      * @param other Object to move.
+      */
+     offset(offset&& other) = default;
+
+     ~offset(void) = default;
+
+     /**
+      * @brief Assignment operator.
+      * @param other Object to copy.
+      * @return Reference to self.
+      */
+     offset& operator=(const offset& other) = default;
+
+     /**
+      * @brief Move assignment operator.
+      * @param other Object to move.
+      * @return Reference to self.
+      */
+     offset& operator=(offset&& other) noexcept = default;
+
+     /**
+      * @brief Conversion operator to `unsigned int`.
+      */
+     operator unsigned int(void) const noexcept
+     {
+             return this->_m_offset;
+     }

Quoted 9 lines

bindings/cxx/gpiodcxx/line.hpp line 82

Quoted 5 lines

bindings/cxx/gpiodcxx/line.hpp new file +5 -0

+
+private:
+     unsigned int _m_offset;
+};
+

Wrapping unsigned int in a class seems like overkill. Is this just to get the streaming operators for offsets to work?

Yes. Otherwise we'd be providing a generic stream operator for all vector specializations. And this wrapping doesn't really do much other than providing a new type within the gpiod namespace. The constructor with implicit conversion and the operator unsigned int() make sure of this.

Quoted 55 lines

[snip]

bindings/cxx/gpiodcxx/line.hpp

Quoted 3 lines

bindings/cxx/gpiodcxx/line.hpp new file +3 -0

+GPIOD_CXX_API line::offsets line_request::offsets(void) const
+{
+     this->_m_priv->throw_if_released();

redundant as this->num_lines() also does it.

bindings/cxx/gpiodcxx/line.hpp

Quoted 9 lines

bindings/cxx/gpiodcxx/line.hpp new file +9 -0

+
+     ::std::vector<unsigned int> buf(this->num_lines());
+     line::offsets offsets(this->num_lines());
+
+     ::gpiod_line_request_get_offsets(this->_m_priv->request.get(), buf.data());
+
+     for (unsigned int i = 0; i < this->num_lines(); i++)
+             offsets[i] = buf[i];
+

Cache num_lines locally rather than calling num_lines() several times.

[snip]

bindings/cxx/gpiodcxx/line.hpp

Quoted 13 lines

bindings/cxx/gpiodcxx/line.hpp new file +8 -3

+template<typename T>
+::std::ostream& insert_vector(::std::ostream& out,
+                           const ::std::string& name, const ::std::vector<T>& vec)
 {
-     this->_m_line = nullptr;
-     this->_m_owner.reset();
-}
+     out << name << "([";
+     ::std::copy(vec.begin(), ::std::prev(vec.end()),
+                 ::std::ostream_iterator<T>(out, ", "));
+     out << vec.back();
+     out << "])";

What formatting are you after for vectors?
Is the double bracketing necessary?

[snip]

Quoted 13 lines

bindings/cxx/gpiodcxx/line.hpp new file +13 -0

+TEST_CASE("line_request stream insertion operator works", "[line-request]")
+{
+     ::gpiosim::chip sim({{ simprop::NUM_LINES, 4 }});
+     ::gpiod::chip chip(sim.dev_path());
+
+     auto request = chip.request_lines(
+             ::gpiod::request_config({
+                     { reqprop::OFFSETS, offsets({ 3, 1, 0, 2 }) }
+             }),
+             ::gpiod::line_config()
+     );
+
+     ::std::stringstream buf, expected;

Quoted 9 lines

bindings/cxx/gpiodcxx/line.hpp

Quoted 4 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+
+     expected << "line_request(num_lines=4, line_offsets=[offsets([3, 1, 0, 2])], fd=" <<
+                 request.fd() << ")";
+

Still not sure what formatting you are going for with the vectors. And the line_offsets=[] is not consistent with the offsets=() used in request_config.

Yep need to make that consistent, thanks.

Quoted 13 lines

[snip]

Quoted 11 lines

bindings/cxx/gpiodcxx/line.hpp new file +11 -0

+TEST_CASE("request_config stream insertion operator works", "[request-config]")
+{
+     ::gpiod::request_config cfg({
+             { property::CONSUMER, "foobar" },
+             { property::OFFSETS, offsets({ 0, 1, 2, 3 })},
+             { property::EVENT_BUFFER_SIZE, 32 }
+     });
+
+     ::std::stringstream buf;
+
+     buf << cfg;

Quoted 9 lines

bindings/cxx/gpiodcxx/line.hpp

Quoted 4 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+
+     ::std::string expected("request_config(consumer='foobar', num_offsets=4, "
+                            "offsets=(offsets([0, 1, 2, 3])), event_buffer_size=32)");
+

The streaming output stutters.
"offsets=(offsets([0, 1, 2, 3]))" should be "offsets=([0, 1, 2, 3])"? And even then one of the sets of brackets is redundant.

Actually maybe it should be offsets=gpiod::offsets(0, 1, 2 ,3)? I would like to indicate the type and not make it a generic streaming operator for vectors like what Qt provides.

Quoted 22 lines

Quoted 17 lines

bindings/cxx/gpiodcxx/line.hpp new file +4 -0

+     REQUIRE_THAT(buf.str(), Catch::Equals(expected));
+}
+
+} /* namespace */

configure.ac +1 -1

index f8d34ed..ab03673 100644
@@ -239,6 +239,7 @@ AC_CONFIG_FILES([Makefile
               bindings/cxx/libgpiodcxx.pc
               bindings/Makefile
               bindings/cxx/Makefile
+              bindings/cxx/gpiodcxx/Makefile
               bindings/cxx/examples/Makefile
               bindings/cxx/tests/Makefile
               bindings/python/Makefile
--

Quoted 2 lines

2.30.1

Phew. So nothing major.

Quoted 2 lines

Cheers,
Kent.

Thanks Kent! Wherever there's no comment, I will address the issue.

Bart

Quoted 6 lines

bindings/cxx/examples/gpiomoncxx.cpp +1 -1

+     ::gpiod::edge_event_buffer buffer;
 
      for (;;) {
-             auto events = lines.event_wait(::std::chrono::seconds(1));

bindings/cxx/examples/gpiomoncxx.cpp int main(int argc, char **argv) · line 71

Quoted 10 lines

bindings/cxx/examples/gpiomoncxx.cpp +5 -3

-             if (events) {
-                     for (auto& it: events)
-                             print_event(it.event_read());
+             if (request.wait_edge_event(::std::chrono::seconds(5))) {
+                     request.read_edge_event(buffer);
+
+                     for (const auto& event: buffer)
+                             print_event(event);
              }
      }

What is the purpose of the wait_edge_event() here? Wouldn't read_edge_event() block until the next event?

This example should be minimal and demonstrate how the code should normally be used. e.g.

for (const auto& event: request.events_iter())
print_event(event);

Quoted 1 line · Kent Gibson

[snip]

Quoted 23 lines

Quoted 6 lines

bindings/cxx/examples/gpiomoncxx.cpp +1 -1

+     ::gpiod::edge_event_buffer buffer;
 
      for (;;) {
-             auto events = lines.event_wait(::std::chrono::seconds(1));

bindings/cxx/examples/gpiomoncxx.cpp int main(int argc, char **argv) · line 71

Quoted 10 lines

bindings/cxx/examples/gpiomoncxx.cpp +5 -3

-             if (events) {
-                     for (auto& it: events)
-                             print_event(it.event_read());
+             if (request.wait_edge_event(::std::chrono::seconds(5))) {
+                     request.read_edge_event(buffer);
+
+                     for (const auto& event: buffer)
+                             print_event(event);
              }
      }

What is the purpose of the wait_edge_event() here? Wouldn't read_edge_event() block until the next event?

This example should be minimal and demonstrate how the code should normally be used. e.g.

Quoted 3 lines

for (const auto& event: request.events_iter())
print_event(event);

We're making the request's file descriptor non-blocking in the C library. Do you think we should keep it in blocking mode?

I'm no longer sure why I did that honestly.

Maybe a request config flag for that?