From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Kent Gibson <warthog618@gmail.com>
Cc: linux-gpio@vger.kernel.org, Bartosz Golaszewski <brgl@bgdev.pl>
Subject: Re: [libgpiod][RFC] helper functions for basic use cases
Date: Wed, 15 May 2024 06:54:15 -0700 [thread overview]
Message-ID: <CAMRc=MeBGJwyKBTYD1PQkk940t6ECsBxHCprjFUx1KFSKMe7fw@mail.gmail.com> (raw)
In-Reply-To: <20240515091848.GA86661@rigel>
On Wed, 15 May 2024 11:18:48 +0200, Kent Gibson <warthog618@gmail.com> said:
>
> Sure thing. This is what I have at the moment (the declarations are as
> per earlier, just renamed. And I just noticed some variables I haven't
> renamed. I'll add it to the todo list.):
>
> diff --git a/lib/line-request.c b/lib/line-request.c
> index b76b3d7..5af23e0 100644
> --- a/lib/line-request.c
> +++ b/lib/line-request.c
> @@ -305,3 +305,200 @@ gpiod_line_request_read_edge_events(struct gpiod_line_request *request,
>
> return gpiod_edge_event_buffer_read_fd(request->fd, buffer, max_events);
> }
> +
> +static struct gpiod_line_request *
> +ext_request(const char *path, unsigned int offset,
> + enum gpiod_line_direction direction,
> + enum gpiod_line_value value)
> +{
> + struct gpiod_line_request *request = NULL;
> + struct gpiod_line_settings *settings;
> + struct gpiod_line_config *line_cfg;
> + struct gpiod_chip *chip;
> + int ret;
> +
> + chip = gpiod_chip_open(path);
> + if (!chip)
> + return NULL;
> +
> + settings = gpiod_line_settings_new();
> + if (!settings)
> + goto close_chip;
> +
> + gpiod_line_settings_set_direction(settings, direction);
> + if (direction == GPIOD_LINE_DIRECTION_OUTPUT)
> + gpiod_line_settings_set_output_value(settings, value);
> +
> + line_cfg = gpiod_line_config_new();
> + if (!line_cfg)
> + goto free_settings;
> +
> + ret = gpiod_line_config_add_line_settings(line_cfg, &offset, 1,
> + settings);
> + if (ret)
> + goto free_line_cfg;
> +
> + request = gpiod_chip_request_lines(chip, NULL, line_cfg);
> +
> +free_line_cfg:
> + gpiod_line_config_free(line_cfg);
> +
> +free_settings:
> + gpiod_line_settings_free(settings);
> +
> +close_chip:
> + gpiod_chip_close(chip);
> +
> + return request;
> +}
> +
> +GPIOD_API struct gpiod_line_request *
> +gpiod_ext_request_input(const char *path, unsigned int offset)
> +{
> + return ext_request(path, offset, GPIOD_LINE_DIRECTION_INPUT, 0);
> +}
> +
> +GPIOD_API struct gpiod_line_request *
> +gpiod_ext_request_output(const char *path, unsigned int offset,
> + enum gpiod_line_value value)
> +{
> + return ext_request(path, offset, GPIOD_LINE_DIRECTION_OUTPUT, value);
> +}
> +
> +static struct gpiod_line_settings *
> +ext_line_settings(struct gpiod_line_request * olr)
> +{
> + struct gpiod_line_settings *settings = NULL;
> + struct gpiod_line_info *line_info;
> + struct gpiod_chip *chip;
> + char path[32];
> +
> + assert(olr);
> +
> + if (olr->num_lines != 1) {
> + errno = EINVAL;
> + return NULL;
> + }
> +
> + /*
> + * This is all decidedly non-optimal, as generally the user has the
> + * config available from when they made the request, but here we need to
> + * rebuild it from the line info...
> + */
Yeah, I hate it...
I would assume hogging memory for config structs is still cheaper than all these
operations needed to reread the settings for a line. Not to mention the fact
that if we ever extend the settings, we'll need to remember to update this
routine too.
The users of the _ext_ functions most likely wouldn't care whether the context
is stored in struct gpiod_line_request or struct gpiod_ext_request or otherwise.
> + memcpy(path, "/dev/", 5);
> + strncpy(&path[5], olr->chip_name, 26);
> + chip = gpiod_chip_open(path);
> + if (!chip)
> + return NULL;
> +
> + // get info
> + line_info = gpiod_chip_get_line_info(chip, olr->offsets[0]);
> + gpiod_chip_close(chip);
> + if (!line_info)
> + return NULL;
> +
> + if (gpiod_line_info_get_direction(line_info) != GPIOD_LINE_DIRECTION_INPUT) {
> + errno = EINVAL;
> + goto free_info;
> + }
> +
> + settings = gpiod_line_settings_new();
> + if (!settings)
> + goto free_info;
> +
> + gpiod_line_settings_set_direction(settings, GPIOD_LINE_DIRECTION_INPUT);
> + gpiod_line_settings_set_bias(settings,
> + gpiod_line_info_get_bias(line_info));
> + gpiod_line_settings_set_edge_detection(settings,
> + gpiod_line_info_get_edge_detection(line_info));
> + gpiod_line_settings_set_debounce_period_us(settings,
> + gpiod_line_info_get_debounce_period_us(line_info));
> +
> +free_info:
> + gpiod_line_info_free(line_info);
> +
> + return settings;
> +}
> +
>
> Oh, I also added this:
>
> +/**
> + * @brief The size required to contain a single edge event.
> + * @return The size in bytes.
> + */
> +size_t gpiod_edge_event_size();
> +
>
> So users can perform the event read themselves without requiring any
> knowledge of event buffers (at the moment they can't determine the size
> required to perform the read).
>
Can you post an example of how this is used?
> I also intend to provide an updated set of examples that use the ext API.
> They should go in examples/ext?
>
I think the code should go into ext/, the gpiod-ext.h header can go right next
to gpiod.h in include/ and the examples can be in the same examples/ directory,
just call them something_something_ext.c to indicate they use the simpler API.
Does that sound right?
Bart
next prev parent reply other threads:[~2024-05-15 13:54 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-07 2:21 [libgpiod][RFC] helper functions for basic use cases Kent Gibson
2024-05-10 18:37 ` Bartosz Golaszewski
2024-05-11 1:11 ` Kent Gibson
2024-05-13 8:28 ` Bartosz Golaszewski
2024-05-13 10:13 ` Kent Gibson
2024-05-14 13:31 ` Bartosz Golaszewski
2024-05-14 13:38 ` Kent Gibson
2024-05-15 8:26 ` Bartosz Golaszewski
2024-05-15 9:18 ` Kent Gibson
2024-05-15 13:37 ` Kent Gibson
2024-05-15 13:54 ` Bartosz Golaszewski [this message]
2024-05-15 14:14 ` Kent Gibson
2024-05-16 0:19 ` Kent Gibson
2024-05-16 13:55 ` Kent Gibson
2024-05-17 10:53 ` Bartosz Golaszewski
2024-05-17 12:37 ` Kent Gibson
2024-05-22 10:59 ` Bartosz Golaszewski
2024-05-22 12:41 ` Kent Gibson
2024-05-24 14:20 ` Bartosz Golaszewski
2024-05-15 7:06 ` Martin Hundebøll
2024-05-15 9:32 ` Kent Gibson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMRc=MeBGJwyKBTYD1PQkk940t6ECsBxHCprjFUx1KFSKMe7fw@mail.gmail.com' \
--to=brgl@bgdev.pl \
--cc=linux-gpio@vger.kernel.org \
--cc=warthog618@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).