Skip to content
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

🔧 Refactor CRC-16 Configuration Naming and Expand Variants #148

Closed
Nicoretti opened this issue Mar 24, 2024 · 4 comments · Fixed by #157
Closed

🔧 Refactor CRC-16 Configuration Naming and Expand Variants #148

Nicoretti opened this issue Mar 24, 2024 · 4 comments · Fixed by #157
Assignees
Milestone

Comments

@Nicoretti
Copy link
Owner

Nicoretti commented Mar 24, 2024

Summary

The current Crc16.CCITT implementation in our library aligns with CRC-16/XMODEM, which isn't listed among the alternative names/"Aliases" for "CCITT". One of the common names this configuration is known by is KERMIT (see here). We need to ensure CCITT provides the correct configuration or is replaced by at least one of the valid known alternative names/aliases.

Details

  • Rename Crc16.CCITT to Crc16.XMODEM to accurately represent its configuration.
  • Introduce Crc16.CCITT with the appropriate configuration.
  • Bump major version.
  • Introduce additional predefined CRC-16 configuration aliases, such as Crc16.KERMIT (optional).

References

Expected Outcome:
The library will accurately represent various CRC-16 configurations, reducing confusion and increasing usability.

@IsaacDynamo
Copy link

Hi,

The proposed changes would be a nice improvement. I was surprised by the Crc16.CCITT behavior as well (#145).

Do you by any change have a source for Considering that "CCITT" refers to a group of related configurations rather than a single, specific one.? I'm trying to make sense of the CRC behavior in some other system and this sounds like an interesting reference.

@Nicoretti
Copy link
Owner Author

Nicoretti commented Apr 11, 2024

@IsaacDynamo sry, apparently the first section in this issue was formulated badly and is misleading.
I will update it.

@Nicoretti Nicoretti added the bug label Apr 11, 2024
@Nicoretti Nicoretti self-assigned this Apr 11, 2024
@Nicoretti Nicoretti added this to the 7.0.0 milestone Apr 11, 2024
@Nicoretti
Copy link
Owner Author

@IsaacDynamo this comment, may be interesting for you too.

@cmcqueen
Copy link

A tangential note about CCITT: There is further confusion about what algorithm is the "real CCITT CRC", to do with its initial value — whether the initial value is 0xFFFF or the "augmented" value of 0x1D0F (0xFFFF passed through the 16-bit shift register).

It's yet another reason to be cautious about providing a predefined definition for CCITT.

See

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants