Skip to content

Address issues #129 #131 #139 #140#141

Merged
pavel-kirienko merged 6 commits intomasterfrom
dev
May 15, 2025
Merged

Address issues #129 #131 #139 #140#141
pavel-kirienko merged 6 commits intomasterfrom
dev

Conversation

@pavel-kirienko
Copy link
Member

@pavel-kirienko pavel-kirienko commented May 9, 2025

Closes #129
Closes #131
Closes #140

@pavel-kirienko pavel-kirienko added this to the v1.0 milestone May 9, 2025
@pavel-kirienko pavel-kirienko requested a review from Copilot May 9, 2025 18:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses issues #129, #131, #139, and #140 by clarifying protocol specifications. Key changes include:

  • Enhancements to the UDP multi-frame transfer payload size description.
  • Revised definitions and behavior of transfer-ID counters, including conditions for cyclic versus monotonic identifiers.
  • Updated reserved Node-ID values for diagnostic and debugging purposes.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
specification/transport/udp/udp.tex Added clarifications regarding payload size consistency in multi-frame transfers.
specification/transport/abstract.tex Updated transfer-ID counter definitions, uniqueness conditions, and service response timeout adjustments.
specification/application/conventions.tex Revised reserved Node-ID values for diagnostic and debugging tools.
Comments suppressed due to low confidence (1)

specification/transport/abstract.tex:702

  • [nitpick] Consider rewording this condition for increased clarity. For example, rephrase to clearly indicate that the 'one million counts' threshold defines the minimum drop needed to identify a transfer-ID counter reset, and ensure that the associated footnote fully explains this rationale.
    \item has a transfer-ID value that is at least one million counts less than that of the last successfully reassembled transfer,

@thirtytwobits thirtytwobits requested a review from emrainey May 9, 2025 19:36
Copy link
Member

@thirtytwobits thirtytwobits left a comment

Choose a reason for hiding this comment

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

I don't think we should change the transfer ID in 1.0. Let's move this to 1.1 so we have more time to discuss. I think it's a slight improvement but there's some opportunity for refinement I'd like to explore.

@pavel-kirienko
Copy link
Member Author

But then should we keep the 1M count leap backward clause?

@scottdarch
Copy link

But then should we keep the 1M count leap backward clause?

No. I think we should just leave the transfer ID as is for 1.0. It's a fait accompli for anything existing anyway.

@pavel-kirienko pavel-kirienko merged commit 44521fb into master May 15, 2025
1 check passed
@pavel-kirienko pavel-kirienko deleted the dev branch May 15, 2025 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants