[cmis] Optimize cmis.get_error_description speed for passive module

sonic-net · Dec 25, 2024 · 8eb3462 · 8eb3462
1 parent 9ca0f69
commit 8eb3462
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 44 deletions.
diff --git a/sonic_platform_base/sonic_xcvr/api/public/cmis.py b/sonic_platform_base/sonic_xcvr/api/public/cmis.py
@@ -3132,21 +3132,22 @@ def stage_custom_si_settings(self, host_lanes_mask, optics_si_dict):
         return True
 
     def get_error_description(self):
-        dp_state = self.get_datapath_state()
-        conf_state = self.get_config_datapath_hostlane_status()
-        for lane in range(self.NUM_CHANNELS):
-            name = "{}_{}_{}".format(consts.STAGED_CTRL_APSEL_FIELD, 0, lane + 1)
-            appl = self.xcvr_eeprom.read(name)
-            if (appl is None) or ((appl >> 4) == 0):
-                continue
-
-            name = "DP{}State".format(lane + 1)
-            if dp_state[name] != CmisCodes.DATAPATH_STATE[4]:
-                return dp_state[name]
-
-            name = "ConfigStatusLane{}".format(lane + 1)
-            if conf_state[name] != CmisCodes.CONFIG_STATUS[1]:
-                return conf_state[name]
+        if not self.is_flat_memory():
+            dp_state = self.get_datapath_state()
+            conf_state = self.get_config_datapath_hostlane_status()
+            for lane in range(self.NUM_CHANNELS):
+                name = "{}_{}_{}".format(consts.STAGED_CTRL_APSEL_FIELD, 0, lane + 1)
+                appl = self.xcvr_eeprom.read(name)
+                if (appl is None) or ((appl >> 4) == 0):
+                    continue
+
+                name = "DP{}State".format(lane + 1)
+                if dp_state[name] != CmisCodes.DATAPATH_STATE[4]:
+                    return dp_state[name]
+
+                name = "ConfigStatusLane{}".format(lane + 1)
+                if conf_state[name] != CmisCodes.CONFIG_STATUS[1]:
+                    return conf_state[name]
 
         state = self.get_module_state()
         if state != CmisCodes.MODULE_STATE[3]:

diff --git a/tests/sonic_xcvr/test_cmis.py b/tests/sonic_xcvr/test_cmis.py
@@ -3038,35 +3038,63 @@ def test_set_module_OutputAmplitudeTargetRx_settings(self):
         assert self.api.xcvr_eeprom.write.call_count == 4
 
     def test_get_error_description(self):
-        self.api.get_module_state = MagicMock()
-        self.api.get_module_state.return_value = 'ModuleReady'
-        self.api.get_datapath_state = MagicMock()
-        self.api.get_datapath_state.return_value = {
-            'DP1State': 'DataPathActivated',
-            'DP2State': 'DataPathActivated',
-            'DP3State': 'DataPathActivated',
-            'DP4State': 'DataPathActivated',
-            'DP5State': 'DataPathActivated',
-            'DP6State': 'DataPathActivated',
-            'DP7State': 'DataPathActivated',
-            'DP8State': 'DataPathActivated'
-        }
-        self.api.get_config_datapath_hostlane_status = MagicMock()
-        self.api.get_config_datapath_hostlane_status.return_value = {
-            'ConfigStatusLane1': 'ConfigSuccess',
-            'ConfigStatusLane2': 'ConfigSuccess',
-            'ConfigStatusLane3': 'ConfigSuccess',
-            'ConfigStatusLane4': 'ConfigSuccess',
-            'ConfigStatusLane5': 'ConfigSuccess',
-            'ConfigStatusLane6': 'ConfigSuccess',
-            'ConfigStatusLane7': 'ConfigSuccess',
-            'ConfigStatusLane8': 'ConfigSuccess'
-        }
-        self.api.xcvr_eeprom.read = MagicMock()
-        self.api.xcvr_eeprom.read.return_value = 0x10
-
-        result = self.api.get_error_description()
-        assert result is 'OK'
+        with patch.object(self.api, 'is_flat_memory') as mock_method:
+            mock_method.return_value = False
+            self.api.get_module_state = MagicMock()
+            self.api.get_module_state.return_value = 'ModuleReady'
+            self.api.get_datapath_state = MagicMock()
+            self.api.get_datapath_state.return_value = {
+                'DP1State': 'DataPathActivated',
+                'DP2State': 'DataPathActivated',
+                'DP3State': 'DataPathActivated',
+                'DP4State': 'DataPathActivated',
+                'DP5State': 'DataPathActivated',
+                'DP6State': 'DataPathActivated',
+                'DP7State': 'DataPathActivated',
+                'DP8State': 'DataPathActivated'
+            }
+            self.api.get_config_datapath_hostlane_status = MagicMock()
+            self.api.get_config_datapath_hostlane_status.return_value = {
+                'ConfigStatusLane1': 'ConfigSuccess',
+                'ConfigStatusLane2': 'ConfigSuccess',
+                'ConfigStatusLane3': 'ConfigSuccess',
+                'ConfigStatusLane4': 'ConfigSuccess',
+                'ConfigStatusLane5': 'ConfigSuccess',
+                'ConfigStatusLane6': 'ConfigSuccess',
+                'ConfigStatusLane7': 'ConfigSuccess',
+                'ConfigStatusLane8': 'ConfigSuccess'
+            }
+            self.api.xcvr_eeprom.read = MagicMock()
+            self.api.xcvr_eeprom.read.return_value = 0x10
+
+            result = self.api.get_error_description()
+            assert result is 'OK'
+
+            self.api.get_config_datapath_hostlane_status.return_value = {
+                'ConfigStatusLane1': 'ConfigRejected',
+                'ConfigStatusLane2': 'ConfigRejected',
+                'ConfigStatusLane3': 'ConfigRejected',
+                'ConfigStatusLane4': 'ConfigRejected',
+                'ConfigStatusLane5': 'ConfigRejected',
+                'ConfigStatusLane6': 'ConfigRejected',
+                'ConfigStatusLane7': 'ConfigRejected',
+                'ConfigStatusLane8': 'ConfigRejected'
+            }
+            result = self.api.get_error_description()
+            assert result is 'ConfigRejected'
+
+            self.api.get_datapath_state.return_value = {
+                'DP1State': 'DataPathDeactivated',
+                'DP2State': 'DataPathActivated',
+                'DP3State': 'DataPathActivated',
+                'DP4State': 'DataPathActivated',
+                'DP5State': 'DataPathActivated',
+                'DP6State': 'DataPathActivated',
+                'DP7State': 'DataPathActivated',
+                'DP8State': 'DataPathActivated'
+            }
+            result = self.api.get_error_description()
+            assert result is 'DataPathDeactivated'
 
     def test_random_read_fail(self):
         def mock_read_raw(offset, size):