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

the limit of 254 max bytes doesn't avoid the 0x7E value in the packet #101

Open
uncle-buzz opened this issue Nov 28, 2022 · 3 comments
Open

Comments

@uncle-buzz
Copy link

Describe the bug
Not really a bug, but a feature not fully operational: the COBS function is preventing having any byte in the payload with the same value as the start byte (0x7E).

Sorry if I misunderstood how the library works, following statements are based on what I have understood and could be wrong, so correct me if so.

I don't know exactly what it's supposed to avoid, because the start byte value is interpreted only at the state "find_start_byte", so if this value is encountered somewhere else, there will not be a problem. But if for any reason the start byte missing, any byte with the value 0x7E will be interpreted as a new start byte and will run the parse of the packet, but obviously, CRC checking will fail since the byte after the false payload will be used and won't be able to validate the payload, nor the stop byte will follow this virtual CRC byte since it's not the real CRC byte...

But if we want to avoid any 0x7E value in the packet, the maximum size of the payload could not be 0xFE but 0x7C :
Since COBS is applied only on payload bytes, neither the size byte nor the COBS overhead byte is changed, so the payload size can't be equal to the start byte 0x7E and should stay lower. But if no byte in the payload has the 0x7E value, the COBS byte value will be 0x7E with a payload of 0x7D bytes. So maximum payload size should be 0x7C bytes to avoid the value at COBS overhead byte or payload size byte equals the start byte value.

If I'm not wrong, the choice of 0x7E as a start byte is from the design of the binary writing displaying a "pretty" symmetrical "01111110" word, and is an arbitrary choice of a meaningless logical value, right?

Expected behavior
To maximize the payload size and avoid any start byte value in the packet, the start byte value should be 0xFF, the maximum COBS overhead byte value could be 0xFE and so the maximum payload size could be 0xFD (253 bytes).

But anyway, the CRC byte could randomly be equal to the start byte so this feature would not be 100% proof. ID byte is software controlled so it's not an issue with 254 maximum values.

SUBSIDIARY QUESTION:
Is the size packet limited only by the COBS algorithm? Would it be possible to remove the COBS function and write the payload size on 2 bytes? (It would be a different library of course, but I wonder if there are other reasons to limit the size or if I can go with 2 bytes for size, and use a bigger packet depending on the buffer size I can afford)

@vshymanskyy
Copy link

Agree. The packet structure is weird and redundant. Would love to see more details on this.

@PowerBroker2
Copy link
Owner

Not sure why I didn't notice this issue before, but you do raise good points! I might eventually update or remove COBS, but the lib works well already and I have other things I'm working on atm.

@PowerBroker2
Copy link
Owner

I have other things I'm working on atm

Also, PRs are always welcome

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

3 participants