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

Support up to 32kB of payload using 2-byte length #31

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bojanpotocnik
Copy link
Contributor

No description provided.

target/min.c Outdated
Comment on lines 511 to 517
#else
// MSB == LSB if value is 8-bit
self->rx_frame_state = RECEIVING_LENGTH_LSB;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not required at all, but maybe useful for debugging purposes (so the state variable always represents the actual state).

target/min.h Outdated
uint8_t rx_frame_length; // Length of frame
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause 1-3 B of struct padding on >8-bit processors... I like the fact that the order of context variables is the same as reception order, but anyway... shall I move rx_frame_seqrx_frame_seq below this?

@bojanpotocnik bojanpotocnik marked this pull request as draft June 9, 2021 07:59
Control byte `rx_control` is only used to save the original payload
length as received in the reader, that is why it is renamed to
`rx_length`. `rx_frame_length` is assigned to the same value at the
beginning, but it is used as a payload data counter and decreased for
every byte of received payload.

Maximum size is limited to 0x7FFF instead of 0xFFFF to avoid changing
offset variables from uint16_t to uint32_t but still prevent overflows
when incrementing them.
@bojanpotocnik bojanpotocnik changed the title Support up to 65kB of payload using 16-bit frame length Support up to 32kB of payload using 16-bit frame length Jun 9, 2021
@bojanpotocnik bojanpotocnik changed the title Support up to 32kB of payload using 16-bit frame length Support up to 32kB of payload using 2-byte length Jun 9, 2021
@kentindell
Copy link
Collaborator

I need to look at this in a bit more detail: the goal with MIN was this is a way of talking to a lowly 8-bit microcontroller. Then transport features were added on top, which pushes out the buffering from a few bytes to a couple of hundred bytes. I don't want to lose this working on smaller devices.

@bojanpotocnik
Copy link
Contributor Author

If MAX_PAYLOAD is <= 255 then the code is the same as before. If MAX_PAYLOAD is more than UINT8_MAX then all payload-length related variables are defined as uint16_t instead of uint8_t. However, while testing this with data verification on both sides, I discovered that only 0-1024 bytes are successfully transferred - for 1025-32767 B the protocol thinks the whole transfer was successful (based on min_application_handler(len_payload) parameter) but again only 1024 B are actually transferred. This is probably because of the 10 transport bits... I need to analyse this further and add preprocessor checks, but I can only continue on this tomorrow.

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

Successfully merging this pull request may close these issues.

None yet

2 participants