Re: [PATCH V4 4/8] libgpiod: Add rust wrapper crate

[Kent Gibson <warthog618@xxxxxxxxx>]

On Thu, Jul 28, 2022 at 02:22:24PM +0530, Viresh Kumar wrote:
> On 27-07-22, 10:57, Kent Gibson wrote:
> > On Fri, Jul 08, 2022 at 05:04:57PM +0530, Viresh Kumar wrote:
>  
> > > +    /// Get the number of events the buffers stores.
> > 
> > stores -> contains
> > 
> > The capacity indicates the number that can be stored.
> > This field indicates the number the buffer currently contains.
> > 
> > And, as mentioned above, rename to len(), and return value should be
> > usize.
> > 
> > Add a set_len() method to replace the max_events parameter in
> > LineRequest::read_edge_event() to handle the unusual case where the user
> > only wants to partially fill the buffer.
> 
> So capacity is max that can be stored, len is what is currently present. What
> read_edge_event() needs is how much we want it to read, which should be <=
> capacity.
> 
> If we remove the parameter to read_edge_event(), then it must fetch the value
> from the buffer itself, which should be buffer.capacity() if user hasn't called
> set_len(), else it will be that special value the user sets.
> 

Ah, so the the problem there is after the read_edge_event() the len() is
supposed to indicate the number of events in the buffer, so if we use
that to size the read then the user will have to re-set_len() on each read
- so we are effectively back to providing the length to read on each call.

So not a good plan - my bad.

> What do you want to call the value (to be added as a field to struct
> EdgeEventBuffer) and the helper routine to fetch it ?
> 
> Maybe we should name it "max_events" and call the helpers as max_events() and
> set_max_events() instead of set_len()?
> 

I'm not even sure it makes sense to have an option to partially fill the
buffer.  It is there in the C as we have to provide the buffer size, and
so can always elect to provide a smaller size, but I'm not sure there is
any use case for it - certainly none that I am aware of.
Perhaps we should always fill the buffer to capacity.

Then we can stick with the Vec len/capacity pattern, so capacity() indicates
how many can be written during a read and len() indicates how many are
available in the buffer.  No set_len() required.

The goal was to provide an interface that is more idiomatic than
just wrapping the C API verbatim, so the idea was to provide something
with a Vec feel.

That is what I tried to do with mine[1], though that totally hides the
filling of the buffer from the user - filling it on demand when the user
requests an event (the buffer also being an iterator) when the buffer is
empty.
Something like that may be doable using the libpgiod API.

Kent.
[1]https://warthog618.github.io/gpiocdev-rs/gpiocdev/request/struct.EdgeEventBuffer.html