Skip to content

Fix: Enforce upper bounds for rxdelay, txdelay, and direct.txdelay#2443

Open
kizniche wants to merge 1 commit intomeshcore-dev:devfrom
kizniche:fix-rxdelay-txdelay
Open

Fix: Enforce upper bounds for rxdelay, txdelay, and direct.txdelay#2443
kizniche wants to merge 1 commit intomeshcore-dev:devfrom
kizniche:fix-rxdelay-txdelay

Conversation

@kizniche
Copy link
Copy Markdown

@kizniche kizniche commented Apr 29, 2026

Fixes #2433 by enforcing upper bounds for rxdelay, txdelay, and direct.txdelay, based on what is written in the CLI docs.

@kizniche kizniche changed the title Fnforce upper bounds for rxdelay, txdelay, and direct.txdelay Enforce upper bounds for rxdelay, txdelay, and direct.txdelay Apr 29, 2026
@kizniche kizniche changed the title Enforce upper bounds for rxdelay, txdelay, and direct.txdelay Fix: Enforce upper bounds for rxdelay, txdelay, and direct.txdelay Apr 29, 2026
@dreirund
Copy link
Copy Markdown
Contributor

dreirund commented Apr 30, 2026

Maybe increasing the limits can be useful? As There:
https://github.com/user-attachments/files/26126001/Proposal-.MeshCore.factors.in.selecting.rxdelay.txdelay.and.direct.txdelay.pdf (linked at #2053 (comment)) txdelay = 2.5 is also within the range of proposed settings.

Or just the documentation would need correction, whatever suits the intention?

@kizniche
Copy link
Copy Markdown
Author

Perhaps changing the defaults is warranted, but something that should be discussed in the issue, not the PR. This PR is simply aligning the code with the expected behavior (the docs).

@CullenShane
Copy link
Copy Markdown

Denver already recommends values that are higher than these limits: https://denvermc.com/guides/repeater-setup

@dreirund
Copy link
Copy Markdown
Contributor

Denver already recommends values that are higher than these limits: https://denvermc.com/guides/repeater-setup

So this PR would break existing use cases -- then I would say find the current maximum values and do a PR that aligns the docs with reality ;-).

@446564
Copy link
Copy Markdown
Contributor

446564 commented May 5, 2026

This should not be enforced in this way. There is no reason to have arbitrary limits, the only limit should be that of the underlying datatype. Why would we impose our own idea of a maximum on another user when it is not required for interoperability? There is nothing in the protocol or routing that this will have any effect on so there is no limit, again other than the underlying type which is likely very high (unreasonably so)

The limit should clamp to the max value for the type and the docs should be updated at the same time with maybe some language regarding a sensible value range.

@kizniche
Copy link
Copy Markdown
Author

kizniche commented May 5, 2026

For rx, there's currently a hard 32 sec runtime cap, so the current 20 limit seems arbitrary. If it were to exceed ~62, then the 32 sec cap would come into effect. There's no runtime cap for tx, so high values create proportionately higher delays.

@446564
Copy link
Copy Markdown
Contributor

446564 commented May 5, 2026

For rx, there's currently a hard 32 sec runtime cap, so the current 20 limit seems arbitrary. If it were to exceed ~62, then the 32 sec cap would come into effect. There's no runtime cap for tx, so high values create proportionately higher delays.

i didn't dig too deep, but it seems going with the 32s makes sense for rx and tx i guess if someone wants to set 1000 they can, not that they should.

@kizniche
Copy link
Copy Markdown
Author

kizniche commented May 5, 2026

Sounds reasonable. This would go nicely with additional documentation for tx/rx delay settings (#2431) so users would have some sort of guidance/bearing when changing settings.

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.

4 participants