Skip to content

Comments

README: clarify tx deadline behavior for canardTxPoll#255

Merged
pavel-kirienko merged 5 commits intoOpenCyphal:masterfrom
MAMONTBOT:mamont/fix-deadline-doc-zero
Feb 20, 2026
Merged

README: clarify tx deadline behavior for canardTxPoll#255
pavel-kirienko merged 5 commits intoOpenCyphal:masterfrom
MAMONTBOT:mamont/fix-deadline-doc-zero

Conversation

@MAMONTBOT
Copy link
Contributor

This fixes the ambiguity reported in #159 comments where tx_deadline_usec = 0 looked like an unlimited deadline in examples.

Changes

  • Replace the misleading inline comment near canardTxPush():
    • from: "Zero if transmission deadline is not limited"
    • to: "Transmission deadline (absolute monotonic time in usec)"
  • Add an explicit note near canardTxPoll() usage:
    • with non-zero now_usec, zero deadline means immediate expiration;
    • use a future deadline, or pass now_usec=0 to disable timeout-based dropping.

cc @pavel-kirienko @blutack

Copy link
Member

@pavel-kirienko pavel-kirienko left a comment

Choose a reason for hiding this comment

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

The header docs still have passages like "If this timeout behavior is not needed, the timestamp value can be set to zero." which should be also updated throughout.

README.md Outdated
Comment on lines 157 to 159
⚠️ When using `canardTxPoll()` with non-zero `now_usec`, a zero deadline means the frame is already expired.
Use a future deadline value, or pass `now_usec=0` to disable timeout-based dropping.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
⚠️ When using `canardTxPoll()` with non-zero `now_usec`, a zero deadline means the frame is already expired.
Use a future deadline value, or pass `now_usec=0` to disable timeout-based dropping.

causes doc fatigue

@MAMONTBOT
Copy link
Contributor Author

Thanks, good catch — fixed in the header docs as well.

Updated in libcanard/canard.h:

  • replaced the misleading guidance about setting deadline to zero;
  • clarified that timeout disabling is done via now_usec=0 in canardTxPush()/canardTxPoll();
  • clarified that zero deadline is not treated as unlimited when timeout dropping is enabled.

Mamont 🦣 (Pavel’s assistant)

Comment on lines 522 to 523
/// If this timeout behavior is not needed, pass now_usec=0 to canardTxPush()/canardTxPoll().
/// A zero deadline is not treated as "unlimited" when timeout dropping is enabled.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// If this timeout behavior is not needed, pass now_usec=0 to canardTxPush()/canardTxPoll().
/// A zero deadline is not treated as "unlimited" when timeout dropping is enabled.

@MAMONTBOT
Copy link
Contributor Author

Applied your suggestion from the review discussion (discussion_r2834067945) and removed that README warning block.

Mamont 🦣 (Pavel’s assistant)

@MAMONTBOT
Copy link
Contributor Author

Applied. I removed the extra blank line in README and pushed the cleanup.

I’ll keep applying all your suggestions and avoid cosmetic/noise edits.

Mamont 🦣 (Pavel’s assistant)

@MAMONTBOT
Copy link
Contributor Author

Applied the remaining edit suggestion (discussion_r2834119111) and removed the extra/silly formatting churn.

Current status:

  • all your suggestions in this PR have been applied;
  • CI is running; I am monitoring until it is fully green (or clearly pre-existing/unrelated if any check fails).

Mamont 🦣 (Pavel’s assistant)

@pavel-kirienko pavel-kirienko merged commit e91cd66 into OpenCyphal:master Feb 20, 2026
6 checks passed
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.

3 participants