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

crc checksum input #51

Open
sschulz-t opened this issue Mar 5, 2024 · 3 comments
Open

crc checksum input #51

sschulz-t opened this issue Mar 5, 2024 · 3 comments

Comments

@sschulz-t
Copy link

Regarding

**Checksum:** 2 bytes CRC16-CCITT-False (unsigned)

In this case i assume the input is the chunk num and chunk data, however the specification should specify it clearly that no assumptions are needed.

@gsasikumar
Copy link
Collaborator

I thought the table was providing the clarity. But I agree we can make this change to provide better clarity. I will open a PR on the same by tomorrow.

@sschulz-t
Copy link
Author

I think this should be written more precise. I was looking at this:

openid4vp_ble/main.md

Lines 336 to 344 in e5370d5

| Chunk sequence no | Chunk payload | Checksum value of data |
|-----------------------|-------------------------------------|---------------------------|
| (2 bytes) | (upto MTU-4 bytes - 2 bytes) | (2 bytes) |
**Chunk Sequence No:** Running unsigned counter for the chunk and starts with 1 (Max 65535)
**Chunk Payload:** Chunk data.
**Checksum:** 2 bytes CRC16-CCITT-False (unsigned)

The table says checksum of "data" and the text below the table specifies chunk payload as chunk data. One could easily misinterpret this and not include the seqNo in the crc.

I can work out a revised definition and create a PR if you want.

@gsasikumar
Copy link
Collaborator

@sschulz-t Yes it would be great if you can.

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

No branches or pull requests

2 participants