| Message ID | 20230113215210.616812-1-brgl@bgdev.pl |
|---|---|
| Headers | show |
| Series | treewide: continue beating libgpiod v2 into shape for an upcoming release | expand |
On Fri, Jan 13, 2023 at 10:51:55PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Certain parts of the README file still refer to concepts removed from > libgpiod v2. Update whatever needs updating. ... > The minimum kernel version required to run the tests can be checked in the > tests/gpiod-test.c source file (it's subject to change if new features are > -added to the kernel). The tests work together with the gpio-mockup kernel > -module which must be enabled. NOTE: the module must not be built-in. A helper > -library - libgpiomockup - is included to enable straightforward interaction > -with the module. > +added to the kernel). The tests work together with the gpio-sim kernel which Mistakenly removed word 'module' ? > +must either be built-in or available for loading using kmod. A helper > +library - libgpiosim - is included to enable straightforward interaction with > +the module. ... > -Both C++ and Python bindings also include their own test-suites. Both reuse the > -libgpiomockup library to avoid code duplication when interacting with > -gpio-mockup. > +C++, Rust and Python bindings also include their own test-suites. Both reuse the "Both" out of three? > +libgpiosim library to avoid code duplication when interacting with gpio-sim.
On Fri, Jan 13, 2023 at 10:51:56PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The name 'ret' if very common for local variables so change it to _ret > in test helper macros to avoid potential shadowing. Makes sense! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > tests/gpiod-test-helpers.h | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/tests/gpiod-test-helpers.h b/tests/gpiod-test-helpers.h > index 2d86345..b40b820 100644 > --- a/tests/gpiod-test-helpers.h > +++ b/tests/gpiod-test-helpers.h > @@ -118,11 +118,11 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(struct_gpiod_edge_event_buffer, > #define gpiod_test_line_config_add_line_settings_or_fail(_line_cfg, _offsets, \ > _num_offsets, _settings) \ > do { \ > - gint ret = gpiod_line_config_add_line_settings(_line_cfg, \ > - _offsets, \ > - _num_offsets, \ > - _settings); \ > - g_assert_cmpint(ret, ==, 0); \ > + gint _ret = gpiod_line_config_add_line_settings(_line_cfg, \ > + _offsets, \ > + _num_offsets, \ > + _settings); \ > + g_assert_cmpint(_ret, ==, 0); \ > gpiod_test_return_if_failed(); \ > } while (0) > > @@ -147,9 +147,9 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(struct_gpiod_edge_event_buffer, > > #define gpiod_test_reconfigure_lines_or_fail(_request, _line_cfg) \ > do { \ > - gint ret = gpiod_line_request_reconfigure_lines(_request, \ > - _line_cfg); \ > - g_assert_cmpint(ret, ==, 0); \ > + gint _ret = gpiod_line_request_reconfigure_lines(_request, \ > + _line_cfg); \ > + g_assert_cmpint(_ret, ==, 0); \ > gpiod_test_return_if_failed(); \ > } while (0) > > -- > 2.37.2 >
On Fri, Jan 13, 2023 at 10:52:06PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Extend line_config to expose a new method - set_output_values() - which > wraps the new C function for setting multiple output values at once. ... Side Q: Does documentation describe the order in which lines are being set? Or is it solely specified by a kernel driver for a hardware? (I can imagine that this may be not so trivial as long as the input parameters, i.e. line offsets, are not sorted and hardware supports full bank atomic write, this may have a lot of interesting side effects.)
On 13-01-23, 22:52, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Request config is not necessary to request lines. In C API we accept > a NULL pointer, in C++ it's not necessary to assign a request_config > to the request builder, in Python the consumer and event buffer size > arguments are optional. Let's make rust bindings consistent and not > require the request config to be always present. Convert the argument > in request_lines to Option and update the rest of the code. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > bindings/rust/libgpiod/examples/gpio_events.rs | 2 +- > .../libgpiod/examples/gpio_threaded_info_events.rs | 2 +- > bindings/rust/libgpiod/examples/gpioget.rs | 2 +- > bindings/rust/libgpiod/examples/gpiomon.rs | 2 +- > bindings/rust/libgpiod/examples/gpioset.rs | 2 +- > bindings/rust/libgpiod/src/chip.rs | 10 ++++++++-- > bindings/rust/libgpiod/tests/common/config.rs | 2 +- > bindings/rust/libgpiod/tests/info_event.rs | 2 +- > 8 files changed, 15 insertions(+), 9 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On 13-01-23, 22:52, Bartosz Golaszewski wrote: > diff --git a/bindings/rust/libgpiod/src/line_config.rs b/bindings/rust/libgpiod/src/line_config.rs > > + /// Set output values for a number of lines. > + pub fn set_output_values(&mut self, values: &[Value]) -> Result<&mut Self> { > + let mut mapped_values = Vec::new(); > + for value in values { > + mapped_values.push(value.value()); > + } Can be rewritten as this too: let values: Vec<gpiod::gpiod_line_value> = values.iter().map(|val| val.value()).collect(); > + > + let ret = unsafe { > + gpiod::gpiod_line_config_set_output_values(self.config, mapped_values.as_ptr(), > + values.len() as u64) > + }; > + > + if ret == -1 { > + Err(Error::OperationFailed( > + OperationType::LineConfigSetOutputValues, > + errno::errno(), > + )) > + } else { > + Ok(self) > + } > + }
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> This series contains an assortment of smaller and bigger changes. Some are just simple updates to docs, some fix bugs and we have two more changes to the API: unifying the get_offsets functions for line config and line request as well as providing an interface for setting multiple output values at once in line_config before requesting lines. Rust bindings have also been updated slightly so Viresh please make sure to take a look and review. I really hope this is the last set of bigger changes and that I'll be able to tag an RC and release v2 in the next couple of weeks. Bartosz Golaszewski (16): README: update for libgpiod v2 tests: avoid shadowing local variables with common names in macros build: unify the coding style of source files lists in Makefiles treewide: unify gpiod_line_config/request_get_offsets() functions doc: update docs for libgpiod v2 bindings: cxx: prepend all C symbols with the scope resolution operator bindings: cxx: allow to copy line_settings tests: fix the line config reset test case tests: add a helper for reading back line settings from line config core: provide gpiod_line_config_set_output_values() gpioset: use gpiod_line_config_set_output_values() bindings: cxx: add line_config.set_output_values() bindings: python: provide line_config.set_output_values() bindings: rust: make request_config optional in Chip.request_lines() bindings: rust: make mutators return &mut self bindings: rust: provide line_config.set_output_values() README | 32 ++--- bindings/cxx/Makefile.am | 32 ++--- bindings/cxx/examples/Makefile.am | 12 +- bindings/cxx/gpiodcxx/line-config.hpp | 7 ++ bindings/cxx/gpiodcxx/line-settings.hpp | 13 +- bindings/cxx/internal.hpp | 3 +- bindings/cxx/line-config.cpp | 33 +++-- bindings/cxx/line-request.cpp | 10 +- bindings/cxx/line-settings.cpp | 85 +++++++++---- bindings/cxx/tests/Makefile.am | 36 +++--- bindings/cxx/tests/tests-line-config.cpp | 51 ++++++++ bindings/cxx/tests/tests-line-settings.cpp | 43 +++++++ bindings/python/gpiod/chip.py | 6 + bindings/python/gpiod/ext/line-config.c | 64 ++++++++++ bindings/python/gpiod/ext/request.c | 8 +- bindings/python/tests/tests_line_request.py | 14 +++ .../rust/libgpiod/examples/gpio_events.rs | 4 +- .../examples/gpio_threaded_info_events.rs | 8 +- bindings/rust/libgpiod/examples/gpioget.rs | 6 +- bindings/rust/libgpiod/examples/gpiomon.rs | 4 +- bindings/rust/libgpiod/examples/gpioset.rs | 6 +- bindings/rust/libgpiod/src/chip.rs | 10 +- bindings/rust/libgpiod/src/lib.rs | 1 + bindings/rust/libgpiod/src/line_config.rs | 83 +++++++------ bindings/rust/libgpiod/src/line_request.rs | 22 ++-- bindings/rust/libgpiod/src/request_config.rs | 8 +- bindings/rust/libgpiod/tests/common/config.rs | 10 +- bindings/rust/libgpiod/tests/info_event.rs | 8 +- bindings/rust/libgpiod/tests/line_config.rs | 76 +++++++++--- bindings/rust/libgpiod/tests/line_request.rs | 99 +++++++-------- .../rust/libgpiod/tests/request_config.rs | 2 +- configure.ac | 1 + include/gpiod.h | 104 ++++++++++++---- lib/Makefile.am | 27 ++-- lib/line-config.c | 98 +++++++++++---- lib/line-request.c | 23 ++-- man/Makefile.am | 8 +- tests/Makefile.am | 34 ++--- tests/gpiod-test-helpers.h | 36 ++++-- tests/tests-line-config.c | 116 +++++++++++++----- tests/tests-line-request.c | 10 +- tools/gpioset.c | 21 ++-- 42 files changed, 879 insertions(+), 395 deletions(-)