Fix issue: copper cable should not display DOM information (#318)

Junchao-Mellanox · web-flow · commit 2052a63d1059 · 2022-11-03T11:39:57.000-07:00
* Fix issue: copper cable should not display DOM information

* Improve unit test coverage
diff --git a/sonic_platform_base/sonic_xcvr/api/public/sff8436.py b/sonic_platform_base/sonic_xcvr/api/public/sff8436.py
@@ -13,6 +13,7 @@ class Sff8436Api(XcvrApi):
 
     def __init__(self, xcvr_eeprom):
         super(Sff8436Api, self).__init__(xcvr_eeprom)
+        self._is_copper = None
 
     def get_model(self):
         return self.xcvr_eeprom.read(consts.VENDOR_PART_NO_FIELD)
@@ -42,7 +43,7 @@ def get_transceiver_info(self):
         cable_type = "Unknown"
         for len, type in zip([smf_len, om3_len, om2_len, om1_len, cable_assembly_len], len_types):
             if len > 0:
-                cable_len = len        
+                cable_len = len
                 cable_type = type
 
         xcvr_info = {
@@ -119,8 +120,6 @@ def get_transceiver_threshold_info(self):
                               ]
         threshold_info_dict = dict.fromkeys(threshold_info_keys, 'N/A')
         thresh_support = self.get_transceiver_thresholds_support()
-        if thresh_support is None:
-            return None
         if not thresh_support:
             return threshold_info_dict
 
@@ -198,12 +197,16 @@ def get_tx_disable_channel(self):
         return self.xcvr_eeprom.read(consts.TX_DISABLE_FIELD)
 
     def get_module_temperature(self):
+        if not self.get_temperature_support():
+            return 'N/A'
         temp = self.xcvr_eeprom.read(consts.TEMPERATURE_FIELD)
         if temp is None:
             return None
         return float("{:.3f}".format(temp))
 
     def get_voltage(self):
+        if not self.get_voltage_support():
+            return 'N/A'
         voltage = self.xcvr_eeprom.read(consts.VOLTAGE_FIELD)
         if voltage is None:
             return None
@@ -216,6 +219,8 @@ def get_tx_bias(self):
         return [channel_bias for channel_bias in tx_bias.values()]
 
     def get_rx_power(self):
+        if not self.get_rx_power_support():
+            return ["N/A" for _ in range(self.NUM_CHANNELS)]
         rx_power = self.xcvr_eeprom.read(consts.RX_POWER_FIELD)
         if rx_power is None:
             return None
@@ -255,27 +260,30 @@ def set_power_override(self, power_override, power_set):
         if power_override:
             ret &= self.xcvr_eeprom.write(consts.POWER_SET_FIELD, power_set)
         return ret
-    
+
     def is_flat_memory(self):
         return self.xcvr_eeprom.read(consts.FLAT_MEM_FIELD)
 
     def get_tx_power_support(self):
         return False
 
     def get_rx_power_support(self):
-        return True
+        return not self.is_copper()
 
     def is_copper(self):
-        eth_compliance = self.xcvr_eeprom.read(consts.ETHERNET_10_40G_COMPLIANCE_FIELD)
-        if eth_compliance is None:
-            return None
-        return eth_compliance == "40GBASE-CR4"
+        if self._is_copper is None:
+            eth_compliance = self.xcvr_eeprom.read(consts.ETHERNET_10_40G_COMPLIANCE_FIELD)
+            if eth_compliance is None:
+                return None
+            else:
+                self._is_copper = eth_compliance == "40GBASE-CR4"
+        return self._is_copper
 
     def get_temperature_support(self):
-        return True
+        return not self.is_copper()
 
     def get_voltage_support(self):
-        return True
+        return not self.is_copper()
 
     def get_rx_los_support(self):
         return True
@@ -290,7 +298,7 @@ def get_tx_disable_support(self):
         return self.xcvr_eeprom.read(consts.TX_DISABLE_SUPPORT_FIELD)
 
     def get_transceiver_thresholds_support(self):
-        return not self.is_flat_memory()
+        return not self.is_copper() and not self.is_flat_memory()
 
     def get_lpmode_support(self):
         power_class = self.xcvr_eeprom.read(consts.POWER_CLASS_FIELD)
diff --git a/sonic_platform_base/sonic_xcvr/api/public/sff8636.py b/sonic_platform_base/sonic_xcvr/api/public/sff8636.py
@@ -17,6 +17,7 @@ def __init__(self, xcvr_eeprom):
         super(Sff8636Api, self).__init__(xcvr_eeprom)
         self._temp_support = None
         self._voltage_support = None
+        self._is_copper = None
 
     def get_model(self):
         return self.xcvr_eeprom.read(consts.VENDOR_PART_NO_FIELD)
@@ -49,7 +50,7 @@ def get_transceiver_info(self):
         cable_type = "Unknown"
         for len, type in zip([smf_len, om3_len, om2_len, om1_len, cable_assembly_len], len_types):
             if len > 0:
-                cable_len = len        
+                cable_len = len
                 cable_type = type
 
         xcvr_info = {
@@ -126,8 +127,6 @@ def get_transceiver_threshold_info(self):
                               ]
         threshold_info_dict = dict.fromkeys(threshold_info_keys, 'N/A')
         thresh_support = self.get_transceiver_thresholds_support()
-        if thresh_support is None:
-            return None
         if not thresh_support:
             return threshold_info_dict
         temp_thresholds = self.xcvr_eeprom.read(consts.TEMP_THRESHOLDS_FIELD)
@@ -237,6 +236,9 @@ def get_tx_bias(self):
         return [channel_bias for channel_bias in tx_bias.values()]
 
     def get_rx_power(self):
+        rx_power_support = self.get_rx_power_support()
+        if not rx_power_support:
+            return ["N/A" for _ in range(self.NUM_CHANNELS)]
         rx_power = self.xcvr_eeprom.read(consts.RX_POWER_FIELD)
         if rx_power is None:
             return None
@@ -289,23 +291,29 @@ def is_flat_memory(self):
         return self.xcvr_eeprom.read(consts.FLAT_MEM_FIELD)
 
     def get_tx_power_support(self):
+        if self.is_copper():
+            return False
         return self.xcvr_eeprom.read(consts.TX_POWER_SUPPORT_FIELD)
 
     def get_rx_power_support(self):
-        return True
+        return not self.is_copper()
 
     def is_copper(self):
-        eth_compliance = self.xcvr_eeprom.read(consts.ETHERNET_10_40G_COMPLIANCE_FIELD)
-        ext_spec_compliance = self.xcvr_eeprom.read(consts.EXT_SPEC_COMPLIANCE_FIELD)
-        if eth_compliance is None or ext_spec_compliance is None:
-            return None
-
-        return eth_compliance == "40GBASE-CR4" or \
-               "CR" in ext_spec_compliance or \
-               "ACC" in ext_spec_compliance or \
-               "Copper" in ext_spec_compliance
+        if self._is_copper is None:
+            eth_compliance = self.xcvr_eeprom.read(consts.ETHERNET_10_40G_COMPLIANCE_FIELD)
+            ext_spec_compliance = self.xcvr_eeprom.read(consts.EXT_SPEC_COMPLIANCE_FIELD)
+            if eth_compliance is None or ext_spec_compliance is None:
+                return None
+            else:
+                self._is_copper = eth_compliance == "40GBASE-CR4" or \
+                    "CR" in ext_spec_compliance or \
+                    "ACC" in ext_spec_compliance or \
+                    "Copper" in ext_spec_compliance
+        return self._is_copper
 
     def get_temperature_support(self):
+        if self.is_copper():
+            return False
         if self._temp_support is None:
             rev_compliance = self.xcvr_eeprom.read(consts.REV_COMPLIANCE_FIELD)
             # TODO: instead of checking for specific decoded value, should
@@ -317,6 +325,8 @@ def get_temperature_support(self):
         return self._temp_support
 
     def get_voltage_support(self):
+        if self.is_copper():
+            return False
         if self._voltage_support is None:
             rev_compliance = self.xcvr_eeprom.read(consts.REV_COMPLIANCE_FIELD)
             # TODO: instead of checking for specific decoded value, should
@@ -340,7 +350,7 @@ def get_tx_disable_support(self):
         return self.xcvr_eeprom.read(consts.TX_DISABLE_SUPPORT_FIELD)
 
     def get_transceiver_thresholds_support(self):
-        return not self.is_flat_memory()
+        return not self.is_copper() and not self.is_flat_memory()
 
     def get_lpmode_support(self):
         power_class = self.xcvr_eeprom.read(consts.POWER_CLASS_FIELD)
diff --git a/tests/sonic_xcvr/test_sff8436.py b/tests/sonic_xcvr/test_sff8436.py
@@ -1,4 +1,4 @@
-from mock import MagicMock
+from mock import MagicMock, patch
 
 from sonic_platform_base.sonic_xcvr.api.public.sff8436 import Sff8436Api
 from sonic_platform_base.sonic_xcvr.codes.public.sff8436 import Sff8436Codes
@@ -15,7 +15,7 @@ class TestSff8436(object):
 
     def test_api(self):
         """
-        Verify all api access valid fields    
+        Verify all api access valid fields
         """
         self.api.get_model()
         self.api.get_serial()
@@ -51,3 +51,25 @@ def test_api(self):
         self.api.get_transceiver_thresholds_support()
         self.api.get_lpmode_support()
         self.api.get_power_override_support()
+
+    def test_is_copper(self):
+        with patch.object(self.api, 'xcvr_eeprom') as mock_eeprom:
+            mock_eeprom.read = MagicMock()
+            mock_eeprom.read.return_value = None
+            assert self.api.is_copper() is None
+            mock_eeprom.read.return_value = '40GBASE-CR4'
+            assert self.api.is_copper()
+            self.api._is_copper = None
+            mock_eeprom.read.return_value = 'SR'
+            assert not self.api.is_copper()
+
+    def test_simulate_copper(self):
+        with patch.object(self.api, 'is_copper', return_value=True):
+            assert self.api.get_rx_power() == ['N/A'] * self.api.NUM_CHANNELS
+            assert self.api.get_module_temperature() == 'N/A'
+            assert self.api.get_voltage() == 'N/A'
+            assert not self.api.get_tx_power_support()
+            assert not self.api.get_rx_power_support()
+            assert not self.api.get_rx_power_support()
+            assert not self.api.get_temperature_support()
+            assert not self.api.get_voltage_support()
diff --git a/tests/sonic_xcvr/test_sff8636.py b/tests/sonic_xcvr/test_sff8636.py
@@ -1,4 +1,4 @@
-from mock import MagicMock
+from mock import MagicMock, patch
 
 from sonic_platform_base.sonic_xcvr.api.public.sff8636 import Sff8636Api
 from sonic_platform_base.sonic_xcvr.codes.public.sff8636 import Sff8636Codes
@@ -15,7 +15,7 @@ class TestSff8636(object):
 
     def test_api(self):
         """
-        Verify all api access valid fields    
+        Verify all api access valid fields
         """
         self.api.get_model()
         self.api.get_serial()
@@ -52,3 +52,22 @@ def test_api(self):
         self.api.get_lpmode_support()
         self.api.get_power_override_support()
 
+    def test_is_copper(self):
+        with patch.object(self.api, 'xcvr_eeprom') as mock_eeprom:
+            mock_eeprom.read = MagicMock()
+            mock_eeprom.read.return_value = None
+            assert self.api.is_copper() is None
+            mock_eeprom.read.return_value = '40GBASE-CR4'
+            assert self.api.is_copper()
+            self.api._is_copper = None
+            mock_eeprom.read.return_value = 'SR'
+            assert not self.api.is_copper()
+
+    def test_simulate_copper(self):
+        with patch.object(self.api, 'is_copper', return_value=True):
+            assert self.api.get_rx_power() == ['N/A'] * self.api.NUM_CHANNELS
+            assert not self.api.get_tx_power_support()
+            assert not self.api.get_rx_power_support()
+            assert not self.api.get_rx_power_support()
+            assert not self.api.get_temperature_support()
+            assert not self.api.get_voltage_support()
