-
Notifications
You must be signed in to change notification settings - Fork 13
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 support for arbitrary input and output bit widths #26
Comments
@Nicoretti Since I'm afraid this will be hard task without breaking current interfaces, i've re-implemented your interfaces and instead of operating on I currently have no intention on releasing this work as standalone crc library, but if you'd be interested in using it, or just as inspiration, feel free to use it anyhow re-implementation here: https://github.com/OK-DMR/ok-dmrlib/blob/master/okdmr/dmrlib/etsi/crc/crc.py This also addresses and solves the original #25 report, which was right, and i'm afraid not even the crccheck project from Martin Scharrer is capable of doing this, even crccheck operates on bytes (not bitstreams) and those references are only CRC's that operate on bytes and produce non-8bit-sized output My re-implementation should provide for any arbitrary sized input, but i've yet to build a better test-suite to catch some corner-cases, current test-suite is here: https://github.com/OK-DMR/ok-dmrlib/blob/master/okdmr/tests/dmrlib/etsi/crc/test_crc9.py All the best! |
Hi @smarek, thanks for coming back to me and providing "insperation" -> appreciate it ;D.
I think resticting the implementation to bytes (for the input) is mainly done so the implementation can use lookup tables to improve the performance. Though I think one also could implement a hybrid which feeds 8 bit (1 Byte) chunks where possible and use lookup tables for that, while dealing with chunks smaller than 8 bit and the edge cases in a bit manner. Maybe for starters I could add your implementation as an alternative if non 8 bit sized output and/or a bitwise input is needed. So Thanks a lot, also all the best for you! |
@Nicoretti cheers and thanks mate, in crc-9 use-case table-based lookup works pretty slick, only the lookup table generator function might not be written well enough for all crc sizes (since it currently creates table not on 0-255 but on 0-511 range) also from my perspective since bitarray is written in C/C++ and already provides all the bitwise operations, it's pretty easy to use and might be worth looking into replacing the Byte class basis (integers) with bitarray needs more testing, but hey, if you're happy enough, i'm as well :) |
good point(s).
That's nice, for this library though some of my major design decisions have been:
I need to think about the pros and cons |
I actually just extended the test-suite, comparing CRC16(CCIT) and CRC32 in usage, with small improvement over reversing output bytes, they result in same output However for obvious reasons, table based implementations are currently far-more-inferior to bit-by-bit processing Also i probably have similar point-of-view on using dependencies, however i had to settle on "do not add unnecessary dependencies", thus allowing myself to use bitarray library, since it's widely supported, used and tested, but for anything that can be solved without library, or the library itself is so tiny/insignificant, i guess it's better to use (or write from scratch) the module internally thank you |
Well update, i just got it working for arbitrary bit-sized input (see OK-DMR/ok-dmrlib@840291e ), and lookup tables are created with optimal size (for crc[1-15] it's self size, for crcs like [16,32,...] it's greatest possible divisor, eg. byte for crc16/crc32, 9 bits for crc18, etc.), and that's it, it's fully working, compatible with your current crc implementation (both table and processing byte-by-byte where applicable), and test contains sanity check, that our libraries provide same output for same input+algorithm If you'd have anything in mind to change/update, let me know, but I consider current state stable enough to be used world-wide |
nice, thx for sharing |
e.g. 1,2,3,5...,9,..,.63,...
currently
amount_of_bits(input) % 8 == 0
must hold trueRelevant other work (see also #25)
The text was updated successfully, but these errors were encountered: