On Wed, May 22, 2024 at 2:41 PM Kent Gibson <warthog618@xxxxxxxxx> wrote: > > On Wed, May 22, 2024 at 12:59:39PM +0200, Bartosz Golaszewski wrote: > > On Fri, May 17, 2024 at 2:37 PM Kent Gibson <warthog618@xxxxxxxxx> wrote: > > > > > > > > > Oh, I agree - that makes total sense. The direction I was headed felt wrong, > > > so I figured I must've misunderstood what you meant. > > > > > > I'll re-organise it into a separate unit. > > > > > > Does that unit have to act through the core API, or can we give it > > > access to the internals? > > > > If we can avoid it accessing the internals, that would be awesome. > > Unless you hit a road-block, please try to keep the core separate. > > > > If you want the ext functions to store config then that would rule out the > shortcut constructors returning a gpiod_line_request. > Actually this is why I initially proposed to have another object wrapping gpiod_line_request. I don't really want to extend it unless we make use of it in the core. We can always provide gpiod_ext_state_get_request() or something. We could call this wrapper gpiod_ext_state so that we can cram whatever persistence we will require as the ext library grows (which it inevitably will). > I was thinking that the ext could be a friend of core and get access to > some things not generally accessible, in this case allowing ext to store > the config inside the request. > I would rather it be a wrapper around libgpiod which simplifies operating on the core API. > Without that, the two options we have is to rebuild the config from line > info, which you don't like, or wrapping the gpiod_line_request and have > the wrapper provide an accessor to the contained gpiod_line_request if > the user wants to use core API, which I'm not thrilled about. > But that seems to be the only option. > If I were to rank the solutions here from best to worst (personal opinion) I'd go: 1. Wrapper for extended state providing accessor to gpiod_line_request and storing the config 2. Rebuilding the config from info 3. Storing the config in gpiod_line_request > > > And where do you stand wrt the idea of adding a config pointer to struct > > > gpiod_line_request itself? > > > > > > > We'd need to make a deep copy, otherwise it could be destroyed and the > > pointer would be left dangling, right? > > > > No, as the config is constructed and added by ext - the user can't actually > see it. It would only be accessed indirectly via the ext functions. > If using only the core API then it would always be NULL. > Ugh, I really don't like having a useless pointer in this struct. It's not about memory - the 8 bytes are irrelevant - but the logic of it. Making concessions in the very core of the library for what is essentially an outer shell around it just doesn't sound right. > Well unless we were to provide accessors to make it accessible to the > user, and then which way to go would be open to debate. There are other > functions in libgpiod that pass ownership, so copying isn't the only > option. I haven't put much thought into that though, as I wasn't planning > to go there. > For now that doesn't seem required. At worst we can provide a function to lookup individual per offset settings from the ext state object. Settings can be copied alright. Bart