Add tx bias support check before getting tx bias of sff8436 and sff8636 by az-pz · Pull Request #586 · sonic-net/sonic-platform-common

az-pz · 2025-07-10T20:26:50Z

Description

Fix the tx bias response for sff8436 and sff8636 copper interfaces.

Motivation and Context

Tx bias of sff8436 and sff8636 interfaces would return as 0 even for copper interfaces. As copper interfaces don't support txbias, we change the return to 'N/A' for them. It keeps the behavior consistent with other DOM values.

How Has This Been Tested?

I looked at the output of the following commands on a sonic device:

sudo sfputil show eeprom : The output remained unchanged.
show interface transce eeprom : The output remained unchanged.

# In the python shell of pmon container
from sonic_platform.platform import Platform
p = Platform()
platform_chassis = p.get_chassis()
a = platform_chassis.get_all_sfps()
apis = [i.get_xcvr_api() for i in a if i]
{i.get_transceiver_info()['serial'].strip():i.get_tx_bias() for i in apis if i}
{i.get_transceiver_info()['serial'].strip():(i.is_copper(), i.get_tx_bias_support()) for i in apis if i}
{(i.get_transceiver_info()['type'].strip(),i.is_copper(), i.get_tx_bias_support(), i.is_flat_memory()) for i in apis if i}

After the change, the tx_bias of sff8436 and sff8636 copper interfaces returned as 'N/A'. The output for other interfaces remained unchanged. It was tested on a testbed with following types of txvrs:

>>>{(i.get_transceiver_info()['type'].strip(),i.is_copper(), i.get_tx_bias_support(), i.is_flat_memory()) for i in apis if i}
{('QSFP28 or later', False, True, False), ('QSFP28 or later', True, False, True), ('QSFP+ or later with SFF-8636 or SFF-8436', True, False, False), ('QSFP-DD Double Density 8X Pluggable Transceiver', True, False, True), ('QSFP+ or later with SFF-8636 or SFF-8436', False, True, False), ('QSFP-DD Double Density 8X Pluggable Transceiver', False, True, False), ('SFP/SFP+/SFP28', False, True, True)}

Before the rx_los_support change:

 [print(i.get_transceiver_info()['type'].strip(),i.is_copper(), i.get_rx_los_support(), i.get_rx_los()) for i in apis if i]
QSFP-DD Double Density 8X Pluggable Transceiver False True [True, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver True False ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A']
QSFP-DD Double Density 8X Pluggable Transceiver True False ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A']
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP28 or later False True [True, True, True, True]
QSFP28 or later False True [True, True, True, True]
QSFP28 or later True True [False, False, False, False]
QSFP28 or later True True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 False True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 False True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 True True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 True True [False, False, False, False]
SFP/SFP+/SFP28 False True [False]
SFP/SFP+/SFP28 False True [False]

After making the rx_los_support change:

>>> [print(i.get_transceiver_info()['type'].strip(),i.is_copper(), i.get_rx_los_support(), i.get_rx_los()) for i in apis if i]
QSFP-DD Double Density 8X Pluggable Transceiver False True [True, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver True False ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A']
QSFP-DD Double Density 8X Pluggable Transceiver True False ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A']
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP-DD Double Density 8X Pluggable Transceiver False True [False, False, False, False, False, False, False, False]
QSFP28 or later False True [True, True, True, True]
QSFP28 or later False True [True, True, True, True]
QSFP28 or later True False ['N/A', 'N/A', 'N/A', 'N/A']
QSFP28 or later True False ['N/A', 'N/A', 'N/A', 'N/A']
QSFP+ or later with SFF-8636 or SFF-8436 False True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 False True [False, False, False, False]
QSFP+ or later with SFF-8636 or SFF-8436 True False ['N/A', 'N/A', 'N/A', 'N/A']
QSFP+ or later with SFF-8636 or SFF-8436 True False ['N/A', 'N/A', 'N/A', 'N/A']
SFP/SFP+/SFP28 False True [False]
SFP/SFP+/SFP28 False True [False]

Additional Information (Optional)

mssonicbld · 2025-07-10T20:26:59Z

/azp run

azure-pipelines · 2025-07-10T20:27:09Z

Azure Pipelines successfully started running 1 pipeline(s).

…f8636

mssonicbld · 2025-07-10T20:49:52Z

/azp run

azure-pipelines · 2025-07-10T20:50:01Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-18T18:33:51Z

/azp run

azure-pipelines · 2025-07-18T18:34:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-18T19:16:40Z

/azp run

azure-pipelines · 2025-07-18T19:16:49Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-21T18:08:07Z

/azp run

azure-pipelines · 2025-07-21T18:08:15Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-21T18:41:14Z

/azp run

azure-pipelines · 2025-07-21T18:41:23Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-21T18:46:07Z

/azp run

azure-pipelines · 2025-07-21T18:46:15Z

Azure Pipelines successfully started running 1 pipeline(s).

mihirpat1

In the testing part of the description, can you also mention if you tested flat and/or non-flat memory transceiver for each type of SFP tested?

tests/sonic_xcvr/test_sff8436.py

tests/sonic_xcvr/test_sff8636.py

mssonicbld · 2025-07-22T18:16:44Z

/azp run

azure-pipelines · 2025-07-22T18:16:53Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2025-07-25T21:15:04Z

sonic_platform_base/sonic_xcvr/api/public/sff8636.py

@@ -398,7 +400,7 @@ def get_rx_los_support(self):
        return True


@az-pz @mihirpat1 fix get_rx_los_support() ?

@prgeor , get_rx_los_support function is only being used in cmis.py and sff8472.py . In sff8436.py and sff8636.py, they are just defined but not used anywhere in the module. I guess we don't have to fix get_rx_los_support for sff8436.py and sff8472.py.

@prgeor @mihirpat1 , I have added the fix for get_rx_los_support and get_rx_los. I have also tested it and added the result to the PR description.

mssonicbld · 2025-07-31T19:01:49Z

/azp run

azure-pipelines · 2025-07-31T19:01:59Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2025-07-31T19:30:19Z

/azp run

azure-pipelines · 2025-07-31T19:30:27Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull Request Overview

This PR fixes the tx bias response for SFF-8436 and SFF-8636 copper interfaces by making them return 'N/A' instead of 0, ensuring consistency with other DOM values for copper interfaces that don't support tx bias monitoring.

Key changes:

Updated support check methods to return False for copper interfaces
Added early return logic to DOM methods when support is not available
Enhanced test coverage for copper interface simulation

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
sonic_platform_base/sonic_xcvr/api/public/sff8636.py	Updated tx_bias_support and rx_los_support methods to return False for copper, added support checks in get_tx_bias and get_rx_los
sonic_platform_base/sonic_xcvr/api/public/sff8436.py	Updated tx_bias_support and rx_los_support methods to return False for copper, added support checks in get_tx_bias and get_rx_los
tests/sonic_xcvr/test_sff8636.py	Added test assertions for tx_bias and rx_los returning 'N/A' for copper, fixed duplicate rx_power_support assertion
tests/sonic_xcvr/test_sff8436.py	Added test assertions for tx_bias and rx_los returning 'N/A' for copper, fixed duplicate rx_power_support assertion

tests/sonic_xcvr/test_sff8636.py

tests/sonic_xcvr/test_sff8436.py

Add tx bias support check before getting tx bias of sff8436 and sff8636

005bee5

Update tx bias support check before getting tx bias of sff8436 and sf…

6cdb278

…f8636

Update unit tests.

4e8a036

Fix unit test.

02210e2

az-pz mentioned this pull request Jul 21, 2025

DOM for flat memory transceiver modules sonic-net/sonic-utilities#3950

Merged

Update unit tests for tx power.

59b9db5

Update unit tests for tx power.

81c098b

Remove comments.

c08e41c

az-pz marked this pull request as ready for review July 21, 2025 20:02

mihirpat1 requested a review from Copilot July 22, 2025 17:15

This comment was marked as outdated.

Sign in to view

mihirpat1 reviewed Jul 22, 2025

View reviewed changes

tests/sonic_xcvr/test_sff8436.py Show resolved Hide resolved

mihirpat1 reviewed Jul 22, 2025

View reviewed changes

tests/sonic_xcvr/test_sff8636.py Show resolved Hide resolved

Undo line deletion.

2762e57

az-pz requested a review from mihirpat1 July 23, 2025 17:03

mihirpat1 approved these changes Jul 23, 2025

View reviewed changes

prgeor reviewed Jul 25, 2025

View reviewed changes

Update rx los support to check for DAC.

feafb26

Fix empty line change.

29b060d

az-pz requested review from Copilot, mihirpat1 and prgeor July 31, 2025 21:21

Copilot AI reviewed Jul 31, 2025

View reviewed changes

tests/sonic_xcvr/test_sff8636.py Show resolved Hide resolved

tests/sonic_xcvr/test_sff8436.py Show resolved Hide resolved

mihirpat1 approved these changes Jul 31, 2025

View reviewed changes

prgeor approved these changes Aug 5, 2025

View reviewed changes

prgeor merged commit af47c73 into sonic-net:master Aug 5, 2025
5 checks passed

		@@ -398,7 +400,7 @@ def get_rx_los_support(self):
		return True

Conversation

az-pz commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Additional Information (Optional)

Uh oh!

mssonicbld commented Jul 10, 2025

Uh oh!

azure-pipelines bot commented Jul 10, 2025

Uh oh!

mssonicbld commented Jul 10, 2025

Uh oh!

azure-pipelines bot commented Jul 10, 2025

Uh oh!

mssonicbld commented Jul 18, 2025

Uh oh!

azure-pipelines bot commented Jul 18, 2025

Uh oh!

mssonicbld commented Jul 18, 2025

Uh oh!

azure-pipelines bot commented Jul 18, 2025

Uh oh!

mssonicbld commented Jul 21, 2025

Uh oh!

azure-pipelines bot commented Jul 21, 2025

Uh oh!

mssonicbld commented Jul 21, 2025

Uh oh!

azure-pipelines bot commented Jul 21, 2025

Uh oh!

mssonicbld commented Jul 21, 2025

Uh oh!

azure-pipelines bot commented Jul 21, 2025

Uh oh!

This comment was marked as outdated.

Uh oh!

mihirpat1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Jul 22, 2025

Uh oh!

azure-pipelines bot commented Jul 22, 2025

Uh oh!

prgeor Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

az-pz Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

az-pz Jul 31, 2025

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jul 31, 2025

Uh oh!

azure-pipelines bot commented Jul 31, 2025

Uh oh!

mssonicbld commented Jul 31, 2025

Uh oh!

azure-pipelines bot commented Jul 31, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

az-pz commented Jul 10, 2025 •

edited

Loading

az-pz Jul 29, 2025 •

edited

Loading