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

xmodem: Only increment sequence number on ACK #52

Merged
merged 1 commit into from
May 1, 2023

Conversation

nekromant
Copy link
Contributor

Otherwise a double fault causes transfer to be terminated
because of a sequence error

Signed-off-by: Andrew Andrianov andrew@ncrmnt.org

Otherwise a double fault causes transfer to be terminated
because of a sequence error

Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org>
@nekromant nekromant mentioned this pull request Aug 18, 2021
@nekromant
Copy link
Contributor Author

That was sitting in the closet for a while. If I remember xmodem correctly we have to increment sequence only on ACK. If we get NACK, we resend 'as is'. Otherwise, things got broken after two successive NACKs for me.

@jquast
Copy link
Collaborator

jquast commented May 1, 2023

understood, I agree that you are correct, that sequence number only increments on ACK, from http://textfiles.com/programming/ymodem.txt

Where [sequence] is then next window sequence number (between H000 and H003) after the [sequence] of the last good block. The receiver will discard up to 3 Xmodem packets received after the NAK is transmitted until it receives the packet with the sequence number that had previously been nak'ed by the receiver. The receiver will not send a second NAK until another packet with the same sequence number is received which is also invalid or a timeout has occurred.

  1. When the transmitter receives a NAK[sequence], it will complete transmission of any XMODEM block currently being transmitted and then begin re- transmission starting with the block which was nak'ed.

@jquast jquast merged commit 6bae7be into tehmaze:master May 1, 2023
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.

2 participants