Fix issues in cmis.get_transceiver_bulk_status (#351)

* Fix issue in cmis.get_transceiver_bulk_status 1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm 2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case Signed-off-by: Stephen Sun <stephens@nvidia.com> * Address comments: distinguish scenarios between not supporting and reading failure Signed-off-by: Stephen Sun <stephens@nvidia.com> * Adjust unit test case Signed-off-by: Stephen Sun <stephens@nvidia.com> * Remove redundant code Signed-off-by: Stephen Sun <stephens@nvidia.com> --------- Signed-off-by: Stephen Sun <stephens@nvidia.com>
sonic-net · Mar 19, 2023 · ee15357 · ee15357
1 parent 8e631e3
commit ee15357
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 13 deletions.
diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
@@ -207,8 +207,8 @@ def get_transceiver_bulk_status(self):
         for i in range(1, self.NUM_CHANNELS + 1):
             bulk_status["tx%ddisable" % i] = tx_disable[i-1] if self.get_tx_disable_support() else 'N/A'
             bulk_status["tx%dbias" % i] = tx_bias[i - 1]
-            bulk_status["rx%dpower" % i] = float("{:.3f}".format(self.mw_to_dbm(rx_power[i - 1]))) if self.get_rx_power_support() else 'N/A'
-            bulk_status["tx%dpower" % i] = float("{:.3f}".format(self.mw_to_dbm(tx_power[i - 1]))) if self.get_tx_power_support() else 'N/A'
+            bulk_status["rx%dpower" % i] = float("{:.3f}".format(self.mw_to_dbm(rx_power[i - 1]))) if rx_power[i - 1] != 'N/A' else 'N/A'
+            bulk_status["tx%dpower" % i] = float("{:.3f}".format(self.mw_to_dbm(tx_power[i - 1]))) if tx_power[i - 1] != 'N/A' else 'N/A'
 
         laser_temp_dict = self.get_laser_temperature()
         self.vdm_dict = self.get_vdm()
@@ -500,12 +500,12 @@ def get_tx_power(self):
         '''
         This function returns TX output power in mW on each media lane
         '''
-        tx_power_support = self.get_tx_power_support()
-        if tx_power_support is None:
-            return None
-
         tx_power = ["N/A" for _ in range(self.NUM_CHANNELS)]
 
+        tx_power_support = self.get_tx_power_support()
+        if not tx_power_support:
+            return tx_power
+
         if tx_power_support:
             tx_power = self.xcvr_eeprom.read(consts.TX_POWER_FIELD)
             if tx_power is not None:
@@ -520,12 +520,12 @@ def get_rx_power(self):
         '''
         This function returns RX input power in mW on each media lane
         '''
-        rx_power_support = self.get_rx_power_support()
-        if rx_power_support is None:
-            return None
-
         rx_power = ["N/A" for _ in range(self.NUM_CHANNELS)]
 
+        rx_power_support = self.get_rx_power_support()
+        if not rx_power_support:
+            return rx_power
+
         if rx_power_support:
             rx_power = self.xcvr_eeprom.read(consts.RX_POWER_FIELD)
             if rx_power is not None:

diff --git a/tests/sonic_xcvr/test_cmis.py b/tests/sonic_xcvr/test_cmis.py
@@ -398,7 +398,7 @@ def test_get_tx_power_support(self, mock_response, expected):
             [0, 0, 0, 0, 0, 0, 0, 0]
         ),
         ([False, {'OpticalPowerTx1Field': 0}], ['N/A','N/A','N/A','N/A','N/A','N/A','N/A','N/A']),
-        ([None, None], None)
+        ([True, None], None)
     ])
     def test_get_tx_power(self, mock_response, expected):
         self.api.get_tx_power_support = MagicMock()
@@ -433,7 +433,7 @@ def test_get_rx_power_support(self, mock_response, expected):
             [0, 0, 0, 0, 0, 0, 0, 0]
         ),
         ([False, {'OpticalPowerRx1Field': 0}], ['N/A','N/A','N/A','N/A','N/A','N/A','N/A','N/A']),
-        ([None, None], None)
+        ([True, None], None)
     ])
     def test_get_rx_power(self, mock_response, expected):
         self.api.get_rx_power_support = MagicMock()
@@ -1266,9 +1266,25 @@ def test_get_transceiver_info(self, mock_response, expected):
                 'N/A',
                 50, 3.3,
                 ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
+                None,
+                None,
+                False, False, False, False, False, False,
+                {'monitor value': 40},
+                None,
+            ],
+            None
+        ),
+        (
+            [
                 ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
                 ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
-                False, False, False, False, False, False,
+                ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
+                'N/A',
+                50, 3.3,
+                ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
+                ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
+                ['N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A', 'N/A'],
+                False, False, False, False, True, True,
                 {'monitor value': 40},
                 None,
             ],