-
Notifications
You must be signed in to change notification settings - Fork 48
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
add no_std feature #13
Conversation
As a developer who deploys to embedded devices: I would not use a library that employs lazy_static. Not only there is no reason whatsoever to calculate CRC tables at runtime when you can simply include them in the source code--it's not like they ever change--or at least do it at compile-time, but on a real-time system this would easily cause a missed deadline by whichever code path calls the corresponding CRC function first. |
Huh, I wasn't thinking about that. Two questions :
|
I feel like this is unnecessary complexity and a simple accessory binary included in the source code for posterity and verification but never normally built is more than enough. |
I created a crate for this purpose. I could definitely also make a
constructor that can be used for "scripts", which could be used to create a separate crate.
https://github.com/vitiral/build_const
|
just finished using If you would like I could put it in a script instead so the constant files could be generated whenever was convienient ( |
Fixes #11 Can this be merged? There is literally no benefit to lazy_static for this crate anymore. |
Looks like the crate author hasn't been active on github or twitter for a few months now. Maybe creating a new crate with the improvements in the open PRs could be considered. |
@mrhooray if you could give me rights on cargo I would gladly take over this repo! I totally understand that sometimes life gets hard 😄 |
@vitiral Would you be interested in making a fork of this crate and put it on crates.io to use until the author shows up? I can help out with it if needed. |
|
||
extern crate build_const; | ||
|
||
pub fn make_table_crc32(poly: u32) -> [u32; 256] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make sense to move these two make table fns to a util file/module so that we don't have to repeat them in crc32.rs and crc64.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could move them into an internal "crate" which both build.rs
and the individual modules depend on. I can do that if you would like.
I also didn't like copy/pasting them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moving make_table
out of crc32/crc64 is a breaking change since they're public
i'll see weather we could reference them in build.rs after merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking we could have an internal crate which build.rs
used and some of the functions were re-exported in crc32
and crc64
-- making it non-breaking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. let me try that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#16 was created to address this
@mrhooray it is not a breaking change, it just removes a dependency (and adds one) and allows for use in |
this adds support for
no_std
applications/libraries (like microcontrollers) by using conditional featuresOn nightly with no_std:
on stable with std: