-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improvement for static array support #48
Comments
No I want to thank you for wanting to improve the crate. I think it makes sense to go back to arrays to keep the implementation a bit closer to the c code. In fact we already rely on retain_mut being in the standard library, which was added in 1.61, so the minimum supported rust version is not an issue for this. It might be a while until I have time to work on it though. |
How much workload do you think it is? I am willing to raise a PR for this. |
And would you mind if I rewrite the |
It's probably not much work, perhaps you only need to revert to the code in the old comment. But then some work to verify that it works properly.
I would love it, have been thinking about it many times myself. Perhaps as a separate PR? |
I came into a problem with static array support when using r2r.
As is described in the ROS 2 interfaces, ROS message supports static arrays where the array size is known at compile time.
For example, if I have a message
ValueGroup.msg
,The corresponding C struct is
However, the generated Rust binding uses
Vec
.And the publisher panics if it receives a
ValueGroup
with an incorrect size.I saw a comment from 4 years ago. https://github.com/sequenceplanner/r2r/blob/master/r2r_msg_gen/src/lib.rs#L392
// actually lets use a vector anyway because its more convenient with traits. assert on the fixed size instead!
However, since "const generic" has been stable for a long time (since Rust 1.51), is it possible to bring the static array back?
Again thanks for writing and maintaining this awesome crate! Forgive me if I raise issues too frequently.
The text was updated successfully, but these errors were encountered: