blackbeam / rust-crc16

Apache License 2.0
21 stars 3 forks source link

Documentation Improvements #4

Open ashfordneil opened 6 years ago

ashfordneil commented 6 years ago

First of all, thanks for the crate, it's working really well for me right now.

In my experience using it, however, I did run into some issues trying to understand how it works. I was trying to verify checksums used by different CRC libraries, and found that all of the other libraries had a set notation they agreed on for describing polynomials. Comparing that to "poly" and "check" that you list next to each polynomial, I was quite confused. I ended up diving into the source code to figure out what was going on and determine which polynomial I needed.

Firstly, can you please add (or would you be open to pull requests that add) something to the crate level documentation explicitly explaining what the "check" value for each polynomial is? It was really useful once I figured it out, but took a lot of effort to decipher and seems to be an uncommon thing to put in documentation.

Secondly, I don't really understand too much about CRC internals, but some other libraries write out polynomials in a "x^16+x^12+x^5+1" format. How does this relate to the 2 byte hex number that you have listed next to each polynomial? I'm not sure if this is background information that is expected when you want to do a CRC checksum, but putting it in the documentation would be really useful.

Finally, how do two polynomials with the same "poly" value differ? It appears that the only use of "check" is in the automated testing, which means that from a documentation perspective, X_25 and XMODEM (for example) use the same polynomial but produce different results in a way that is not clear. Can this information please be included in the documentation.

If it helps, I'm happy to submit a PR adding documentation for what I understand. However, that's unfortunately limited to the "check" value at this point in time, as I don't know much about the theory of CRCs.

blackbeam commented 6 years ago

Hi!

Firstly, can you please add (or would you be open to pull requests that add) something to the crate level documentation explicitly explaining what the "check" value for each polynomial is? It was really useful once I figured it out, but took a lot of effort to decipher and seems to be an uncommon thing to put in the documentation.

rust-crc16 uses this document as a guide for implementation. Parameters of different standards (poly, init, refin, refout, xorout and check) is also defined in this document (see section 15).

Secondly, I don't really understand too much about CRC internals, but some other libraries write out polynomials in a "x^16+x^12+x^5+1" format. How does this relate to the 2 byte hex number that you have listed next to each polynomial? I'm not sure if this is background information that is expected when you want to do a CRC checksum, but putting it in the documentation would be really useful.

poly is defined using hex of normal polynomial representation of a polynomial. (x16+x12+x5+1 in normal polynomial representation is 10000001000012, which is 102116)

Finally, how do two polynomials with the same "poly" value differ?

There is also other parameters, which may differ. For example:

define_crc_type! {
    #[doc = "X-25 ```poly=0x1021``` ```check=0x906e```"]
    #[derive(Copy, Clone, PartialEq, Eq)]
    poly=0x1021, init=0xffff, refin=True, refout=True, xorout=0xffff, check=0x906e,
    name=X_25, table=X_25_TABLE, full_name="X-25", test_name=X_25_TEST
}

define_crc_type! {
    #[doc = "XMODEM ```poly=0x1021``` ```check=0x31c3```"]
    #[derive(Copy, Clone, PartialEq, Eq)]
    poly=0x1021, init=0x0000, refin=False, refout=False, xorout=0x0000, check=0x31c3,
    name=XMODEM, table=XMODEM_TABLE, full_name="XMODEM", test_name=XMODEM_TEST
}

as you can see here both X_25 and XMODEM shares the same poly but differs in init, refin, refout and xorout.

If it helps, I'm happy to submit a PR adding documentation for what I understand.

This would be great 😁