Skip to content

[xcvrd] Fix outdated state machine comments for SYSTEM_NOT_READY event#821

Open
kevin94chen wants to merge 1 commit into
sonic-net:masterfrom
kevin94chen:fix/xcvrd-state-machine-comment
Open

[xcvrd] Fix outdated state machine comments for SYSTEM_NOT_READY event#821
kevin94chen wants to merge 1 commit into
sonic-net:masterfrom
kevin94chen:fix/xcvrd-state-machine-comment

Conversation

@kevin94chen

Copy link
Copy Markdown

Description

The comments in xcvrd.py describing the state machine transition for the SYSTEM_NOT_READY event while in the NORMAL state were outdated.

The comments previously indicated that the daemon would transition back to the INIT state. However, the actual implementation safely treats this condition as a fatal error and transitions to the EXIT state, relying on supervisord to cleanly restart the daemon.

This commit updates the comments to align with the actual code behavior to prevent developer confusion.

Motivation and Context

Why is this change required? What problem does it solve? The contradiction between the state machine documentation (comments) and the actual source code logic caused confusion for developers reading the codebase. By aligning the comments with the code's intended fail-fast design (treat as fatal. Exiting...), this PR improves code readability and maintainability.

How Has This Been Tested?

This is a comment-only change that aligns documentation with existing behavior. No functional code modifications were made.

Additional Information (Optional)

@linux-foundation-easycla

linux-foundation-easycla Bot commented May 25, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: kevin94chen / name: kevin94chen (6fb09ec)

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@kevin94chen kevin94chen force-pushed the fix/xcvrd-state-machine-comment branch from 541ff1f to 6fb09ec Compare May 25, 2026 03:13
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

The comments in xcvrd.py describing the state machine transition for the SYSTEM_NOT_READY event while in the NORMAL state were outdated.

The comments previously indicated that the daemon would transition back to the INIT state. However, the actual implementation safely treats this condition as a fatal error and transitions to the EXIT state, relying on supervisord to cleanly restart the daemon.

This commit updates the comments to align with the actual code behavior to prevent developer confusion.

Signed-off-by: kevin94chen <kevin94chen@gmail.com>
@kevin94chen kevin94chen force-pushed the fix/xcvrd-state-machine-comment branch from 6fb09ec to a0fabc7 Compare May 25, 2026 03:30
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates documentation in sonic-xcvrd to reflect the actual xcvrd state-machine behavior when receiving SYSTEM_NOT_READY while in the NORMAL state (fail-fast to EXIT, relying on supervisord restart), reducing developer confusion when reading the transceiver monitoring loop.

Changes:

  • Corrected the SYSTEM_NOT_READY → next-state documentation for the NORMAL state from INIT to EXIT.
  • Updated the state transition table comment to match the implemented NORMAL + SYSTEM_NOT_READYEXIT behavior.

# max retry reached, treat as fatal, transition to EXIT
# - NORMAL
# Treat as an error, transition to INIT
# Treat as fatal, transition to EXIT
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