Skip to contentSkip to conversation

bindings: cxx: fix potential glitch in gpiosetcxx

PATCH In review Bartosz Golaszewski 2 participants 4 messages Jun 12, 2023 GPIO

lore
Share & tools
lore
#0 lore raw

cover bindings: cxx: fix potential glitch in gpiosetcxx

From: Bartosz Golaszewski <[email protected]>

Kent,

This is an alternative approach to the problem you raised with your gpioset patches. It adds a new helper method that allows to set all output values at request-time using the line_config's set_output_values() method.

Please take a look and let me know what you think.

Bart

Bartosz Golaszewski (2):
bindings: cxx: provide request_builder::set_output_values()
bindings: cxx: examples: fix potential glitch in gpiosetcxx

 bindings/cxx/examples/gpiosetcxx.cpp      | 3 +--
 bindings/cxx/gpiodcxx/request-builder.hpp | 8 ++++++++
 bindings/cxx/request-builder.cpp          | 8 ++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)
Signature · 1 line
2.39.2
#1 lore raw

PATCH 1/2 bindings: cxx: provide request_builder::set_output_values()

From: Bartosz Golaszewski <[email protected]>

Implement a new method in the request_builder class that allows to set output values at request-time by calling the set_output_values() method of the internal line_config object.

---
 bindings/cxx/gpiodcxx/request-builder.hpp | 8 ++++++++
 bindings/cxx/request-builder.cpp          | 8 ++++++++
 2 files changed, 16 insertions(+)
bindings/cxx/gpiodcxx/request-builder.hpp +8 -0
index 3216216..90cc8f0 100644
@@ -114,6 +114,14 @@ public:
 	 */
 	request_builder& add_line_settings(const line::offsets& offsets, const line_settings& settings);
 
+	/**
+	 * @brief Set output values for a number of lines in the line config
+	 *        stored by this object.
+	 * @param values Buffer containing the output values.
+	 * @return Reference to self.
+	 */
+	request_builder& set_output_values(const line::values& values);
+
 	/**
 	 * @brief Make the line request.
 	 * @return New line_request object.
bindings/cxx/request-builder.cpp +8 -0
index 6a1a487..d11e47b 100644
@@ -106,6 +106,14 @@ request_builder::add_line_settings(const line::offsets& offsets, const line_sett
 	return *this;
 }
 
+GPIOD_CXX_API request_builder&
+request_builder::set_output_values(const line::values& values)
+{
+	this->_m_priv->line_cfg.set_output_values(values);
+
+	return *this;
+}
+
 GPIOD_CXX_API line_request request_builder::do_request()
 {
 	line_request_ptr request(::gpiod_chip_request_lines(
Signature · 1 line
2.39.2
#2 lore raw

PATCH 2/2 bindings: cxx: examples: fix potential glitch in gpiosetcxx

In reply to Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

gpiosetcxx requests lines without setting their output value, and so sets them all inactive, and subsequently sets them to their requested value. This can result in glitches on lines which were active and are set active.

As this is example code, it is also important to demonstrate that the output value can be set by the request itself.

Request the lines with the correct output values set in the request itself.

---
 bindings/cxx/examples/gpiosetcxx.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
bindings/cxx/examples/gpiosetcxx.cpp +1 -2
index dde5379..f46cb85 100644
@@ -44,10 +44,9 @@ int main(int argc, char **argv)
 			::gpiod::line_settings()
 				.set_direction(::gpiod::line::direction::OUTPUT)
 		)
+		.set_output_values(values)
 		.do_request();
 
-	request.set_values(values);
-
 	::std::cin.get();
 
 	return EXIT_SUCCESS;
Signature · 1 line
2.39.2
Reviewed-by on the v1 series
#3 lore raw
In reply to Bartosz Golaszewski
Quoted 10 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Kent,

This is an alternative approach to the problem you raised with your gpioset patches. It adds a new helper method that allows to set all output values at request-time using the line_config's set_output_values() method.

Please take a look and let me know what you think.

That works for me - it is waaaay less painful than the per-line approach.

for the series.

Though come to think of it, it could use tests as well. But you can always add some subsequently.

Btw, this is also an example of the outcome I expected to see from expanding the examples - identify common pain points and extend the API to address them.

Cheers,
Kent.