Rework context construction

[Funky185540]

Hello,

this is an attempt to "streamline" the default Context construction. I don't mean to be rude, but from what I've seen so far, the usual way to generate instances of "objects" is by defining a new function/method. I thought that as this is a Rust library, maybe it could use a more Rust-specific way of instantiating objects... ?

Personally, as a newcomer that hasn't worked with the IIO C-library before, I found it really confusing that there are 5 or 6 different functions to generate a context. So I added an Enum for the various backend types that IIO understands and pass these as parameter to the new function.

I'm aware that this is a breaking change with respect to the current interface. Curious to hear your thoughts on the issue!

Cheers,
Andreas

[fpagliughi]

Yeah, I'm always conflicted when writing a library like this about how close to stick to the C API.
Am I trying to get Rust programmers to use IIO? Or am I trying to get IIO programmers to use Rust? Obviously both, but there are little conflicts.

The Backend enum looks like a good idea, although I might think to keep a few "obsolete" functions for at least the more common use cases to try and keep apps succinct, although, keeping things more idiomatic, maybe it would be better to drop the "create" and just use from_xxx(), like:

let ctx = iio::Context::from_uri("...");

I'll have a closer look and comment in a day or two. First, though, since this is a breaking API change, I want to push out the code that has been sitting unreleased in master as v0.3, and then this can be the start of v0.4.

[fpagliughi]

BTW, I merged as-is, but we can consider some changes before moving to a release. Some things I'm thinking:

• I'm not sure about Context::default(). This clashes with the standard Rust Default trait which assumes the call doesn't fail (i.e. returns Self, not Result<Self>)
• If we move back to a new() with no parameters for the default constructor, then perhaps make a Context::with_backend(be: Backend)
• Rather than create a URI for all the contexts and then call ffi::iio_create_context_from_uri(), perhaps we should keep the underlying calls to the individual C creation functions, i.e. why bother creating strings to call a function that just parses and splits the string based on the information we already had about the context type, and then calls the underlying, specific call. Not that context calls need to be super efficient, but it does seem a waste.
• We missed Context::create_xml_mem(&str)
• Don't panic in a library, unless something horrific happens. Just return an Error and let the application deal with it. (And yes, I know, I still have a lot f unwrap() calls to get rid of!)
• Better yet, don't even give a platform an option that it can't use. (Only provide the Local context enum variant on Linux hosts).

[Funky185540]

I'm not sure about Context::default(). This clashes with the standard Rust Default trait [ ... ]

Oh, I see! I wasn't aware of that, I just thought that's a reasonable name for the particular function...

Rather than create a URI for all the contexts and then call ffi::iio_create_context_from_uri(), perhaps we should keep the underlying calls to the individual C creation functions, i.e. why bother creating strings to call a function that just parses and splits the string based on the information we already had about the context type, and then calls the underlying, specific call. Not that context calls need to be super efficient, but it does seem a waste.

Good point. I just did it this way as it seemed less "repetitive", because now I just create a backend-specific URI and pass all URIs to the same underlying function. But that's a rather quick thing to fix.

We missed Context::create_xml_mem(&str)

True, I completely missed that, sorry!

Don't panic in a library, unless something horrific happens. Just return an Error and let the application deal with it. [ ... ] Better yet, don't even give a platform an option that it can't use. (Only provide the Local context enum variant on Linux hosts).

That's what I would have preferred, but from a quick search I couldn't find any info on how to use the cfg macro to exclude particular members of an enum... Next time I will mark the PR as WIP and ask.

Sorry for the inconveniences! If I find some time in the next few days I'll attempt to clean it up.

[fpagliughi]

No worries. I appreciate the help and fresh ideas.

I spent the day on it yesterday, and got a lot of this cleaned up. Let me know what you think.
Next, I'll look to some more examples and test with a few new piece of hardware.

I probably have a few days to focus on this project, then I'll need to move on to some other work.

[Funky185540]

Looks good!

I think that I personally would prefer to have new() as single point-of-entry for object instantiation, especially for newcomers, as there would less code/API for people to worry about. But that's a very subjective topic I assume.

Thanks for cleaning up all the bits!

[fpagliughi]

I think the enum for the back-ends works pretty well in this case, but for a struct that can be created with so many different options, trying to stretch an enum into a single new() call may be more confusing that not. Clearly, though, I was not using idiomatic Rust by sticking to the IIO "create" naming conventions. But using new(), with...(), and from...() are the Rust naming conventions:
https://rust-lang.github.io/api-guidelines/naming.html#casing-conforms-to-rfc-430-c-case

So I think we're on track now.