Safe Rust native wrapper around MXL C API

[pac-work]

Hello, I am working on safe Rust wrapper around the C MXL library interface.
The goals are for the moment following:

• Hide all the unsafe stuff inside.
• Provide more Rust-native like experience (async API based on streams?).

I would like to discuss the following:

• Would there be interest to make this more transparent and potentially in the future included in the official SDK?
• Is there anybody else who would like to join this effort?

[pac-work]

I have published what we have so far for the Rust bindings:

https://github.com/pac-work/mxl/tree/rust-bindings

There is a lot of work to do. If you wish to contribute, please refer to the https://github.com/pac-work/mxl/blob/rust-bindings/rust-bindings/README.md for inspiration.

[pedro-alves-ferreira]

Thanks for the feedback, @cwtchan! Yes, that is the goal. The idea is to merge it to @pac-work 's branch and then see how to proceed regarding contributing this to the main mxl branch or a separate repo.

[pac-work]

@pedro-alves-ferreira Looking through the commits (not yet in detail), but I will post my comments here...

84eecc1 (don't expose destroy method) - That was a deliberate choice to be able to test it. I agree that it does not have to be (and should not be) public, but in my eyes, it definitely belongs there and should be explicit, so that one can check that it is working. Not doing so may, especially with the rule to not use unwrap, hide errors.

[pedro-alves-ferreira]

PR here: pac-work#1

[pedro-alves-ferreira]

@pedro-alves-ferreira Looking through the commits (not yet in detail), but I will post my comments here...

84eecc1 (don't expose destroy method) - That was a deliberate choice to be able to test it. I agree that it does not have to be (and should not be) public, but in my eyes, it definitely belongs there and should be explicit, so that one can check that it is working. Not doing so may, especially with the rule to not use unwrap, hide errors.

Right. I had my doubts there and I wanted to discuss them with you.
What I suggest is that I add it back but chage the signature from:

pub fn destroy(&mut self) -> Result<()>

to

pub fn destroy(self) -> Result<()>

The reasoning is that it must consume the instance, otherwise it will be unsafe, as any call to other methods would crash.
Same for 2a32c41.

[pac-work]

Great suggestion @pedro-alves-ferreira ! Sounds good to me...

I continued with the discussion inside the PR.

[vt-tv]

My 2 cents : we should wait for the upcoming synchronization API changes before issuing a PR to the main mxl repo. We know these APIs will change soon.

Additionally we would need to :

• Add support for the rust language in all the devcontainers (VSCode plugins for rust could be added to the devcontainer.json as well.) and all relevant Dockerfiles.
• Modify the clang-format file for rust (if needed)
• Doxygen documentation/config?
• CMakelist integration. the bindings could be built at part of the build and published as a build artefact, automated tests should be run and reported in the github pipeline results.

[pedro-alves-ferreira]

I like modules and tend to use them a lot, so I agree. I'd avoid nesting them too much but something like:

• instance
• flow (including grain and sample)
• reader (all readers)
• writer (all writers)

looks like a good starting point.

[cwtchan]

I was thinking more along the lines of:

mxl
└──src
         └── api.rs
         ├── config.rs
         ├── flow.rs
         ├── flow
         │        ├── reader.rs
         │        └── writer.rs
         ├── grain.rs
         ├── grain
         │        ├── data.rs
         │        ├── reader.rs
         │        ├── write_access.rs
         │        └── writer.rs
...etc

[pedro-alves-ferreira]

Yes, I think I like that better.

[pac-work]

I think I like Pedro's proposal tiny bit more, but no strong preference. I can live with both.

Just a question - in the second proposal, where would the "generic" of "factory" class for readers and writers go?

[cwtchan]

I think these could live as a top level module?