Rust Implementation

[BrianPugh]

Hey @ToxicFrazzles! I just saw you working on a rust implementation! I'm not a rust developer, but I just wanted to extend an offer to help out in any way I can (answer questions, merging code into the main repo, etc). Please feel free to have discussion here, or you can DM me on discord (username: dasboss).

[tmpfs]

Hey @BrianPugh,

I've added bindings for Rust, source code. Documentation:

https://docs.rs/tamp/latest/tamp/
https://docs.rs/tamp-sys/latest/tamp_sys/

I've made sure the tests pass on Linux and MacOS and have done some simple tests on an nRF52840. I hit some issues with crashes on the nRF (when running in some quite complex firmware) but i don't know if that is related to the C bindings as it only seems to be reproducible when i enable the Softdevice on the nRF; I need to get to the bottom of it when i have more time to debug it properly.

Any comments and/or criticisms welcome of course and thanks for the tamp library 😁

[BrianPugh]

Hey @tmpfs!

Thank you for constructing these bindings! Would you have any interest in merging in your bindings to here? I'm not a rust dev, but I think we would need the following things:

1. Move everything into a rust folder.
2. Github Actions to publish crates.
3. Add a page in our docs for all the rust stuff.

---

Looking over the source code, i have some thoughts/questions (remember, not a rust developer!). Mostly nits

1. Why is there a separation for tamp and tamp-sys? I'm assuming tamp-sys is the "raw" C code, while tamp is the idiomatic rust code that invokes it?
2. Instead of using tamp_compressor_compress_cb, with no callback, you can just use tamp_compressor_compress which aliases to the same thing.

But high level, it all looks good! Can you comment on the "idiomaticness" of it within the rust ecosystem?

[tmpfs]

Hey @BrianPugh ,

Thanks! I think for these bindings it would be better to keep them external for the moment as I don't have much free time to handle moving and updating everything right now, could we just link to them in the README perhaps? I would imagine that if later there were a pure Rust implementation then that would be a better fit to live here alongside the C implementation.

You are 100% right about the -sys crate, these are the generated bindings (using bindgen) and then tamp is the higher level "safe" API. I will avoid calling the callback function with no callback, good catch!

If you are cool with just linking for now I can submit a PR with updated README if you like?

[tmpfs]

Heads up, I can't call tamp_compressor_compress as it's marked as always inline there is no symbol for me to call; how would you prefer to handle that? We could add another non-inline function for FFI calls perhaps? I've done this my side by adding a wrapper function tamp_compressor_compress_force to the bindings but maybe it would be cleaner if there was a non-inline version in tamp itself?

To answer your question about whether the code is idiomatic I would say yes as I always try my utmost to write idiomatic code and the Rust toolchain provides useful functions for formatting and linting - always open to criticism and improvement though :-) The two things that are specifically idiomatic are what you spotted earlier (the -sys for generated C bindings) and the use of the heapless library so that the library can be used on MCUs without a heap (which is important for my use case(s)). In addition, there are feature flags to only link the compressor or decompressor for further optimization as firmware will often only need to compress.

[BrianPugh]

This all sounds great! Let's just use the callback function for now as it's probably not worth the effort. And sounds good about keeping this external, I'll update the readme in a few days (i don't have access to a computer at the moment). Long term goals it would be awesome to even have a rust version of the CLI! Thanks again for your work!

[BrianPugh]

@tmpfs I'm updating the README/docs to reference your bindings, and i would love an example roundtrip compression/decompression example using the rust bindings. I was wondering if you could add a demo to your docs? Thanks!

[tmpfs]

Hey @BrianPugh , I had that before but it ended up just being a duplicate of the tests which round-trip compression/decompression using the Canterbury corpus for all type aliases. Perhaps we can point to the tests rather than duplicate that code?

[tmpfs]

Here they are: https://github.com/tmpfs/tamp-rs/blob/main/tamp/src/lib.rs#L78

[BrianPugh]

#248