-
Notifications
You must be signed in to change notification settings - Fork 176
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
[Credo][Ycable] Fix for displaying 'N/A' firmware version when NIC endpoint is power off #366
Conversation
…are version of the self-side when the NIC end is power off Signed-off-by: Xinyu <[email protected]>
@xinyulin can you paste the CLI output with this fix |
Sure!, added in the description. |
sonic_y_cable/credo/y_cable_credo.py
Outdated
self.log_error('Get firmware version error (error code:0x%04X)' % (status)) | ||
return None | ||
''' should at least return local side fw version if nic is offline''' | ||
if status == 0x1A and ((read_side[0] == 4 and target == 1) or (read_side[0] == 2 and target == 2)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can the read_side byr array be empty ?
if yes we could be causing exception here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'd better to call get_read_side() to handle all the exceptions. I will submit a commit to fix it, thanks!
Signed-off-by: Xinyu <[email protected]>
LGTM, |
sonic_y_cable/credo/y_cable_credo.py
Outdated
self.log_error('Get firmware version error (error code:0x%04X)' % (status)) | ||
return None | ||
''' should at least return local side fw version if nic is offline''' | ||
if status == 0x1A and (read_side == target): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xinyulin 0x1A can you name this status value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I made the correction in the last commit. Please review it.
… for readability Signed-off-by: Xinyu <[email protected]>
…dpoint is power off (#366) * [Credo][Ycable] Resolve the issue of being unable to obtain the firmware version of the self-side when the NIC end is power off Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] call get_read_side() to handle exceptions Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] Replace the hard-coded value with a constant variable for readability Signed-off-by: Xinyu <[email protected]> --------- Signed-off-by: Xinyu <[email protected]> Co-authored-by: Prince George <[email protected]>
…dpoint is power off (sonic-net#366) * [Credo][Ycable] Resolve the issue of being unable to obtain the firmware version of the self-side when the NIC end is power off Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] call get_read_side() to handle exceptions Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] Replace the hard-coded value with a constant variable for readability Signed-off-by: Xinyu <[email protected]> --------- Signed-off-by: Xinyu <[email protected]> Co-authored-by: Prince George <[email protected]>
Cherry-pick PR to 202205: #430 |
…dpoint is power off (#366) * [Credo][Ycable] Resolve the issue of being unable to obtain the firmware version of the self-side when the NIC end is power off Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] call get_read_side() to handle exceptions Signed-off-by: Xinyu <[email protected]> * [Credo][Ycable] Replace the hard-coded value with a constant variable for readability Signed-off-by: Xinyu <[email protected]> --------- Signed-off-by: Xinyu <[email protected]> Co-authored-by: Prince George <[email protected]>
…onic-net#366) Signed-off-by: Aman Singhal <[email protected]>
Description
This fix resolve the issue of being unable to obtain the firmware version of the self-side when the NIC end is power off.
Motivation and Context
To report the self-side firmware version for maintenance and trouble shooting.
How Has This Been Tested?
Tested on Arista-7050CX3
The original version will show 'N/A' for all endpoints If the NIC endpoint is turned off,
After applying the fix, the self-tor version can be displayed correctly.
Additional Information (Optional)