DellEMC: S5232, Z9264, Z9332 - Platform API fixes (sonic-net#6842)

ArunSaravananBalachandran · yxieca · commit 2bce3492ac0b · 2021-03-03T04:26:02.000Z
#### Why I did it

To incorporate the below changes in DellEMC S5232, Z9264, Z9332 platforms.

- Update thermal high threshold values
- Make watchdog API Python2 and Python3 compatible
- Fix LGTM alerts
- Z9264: Fix get_change_event timer value

#### How I did it

- Use 'universal_newlines=True' in subprocess.Popen call.
- Change the timeout in 'get_change_event' to milliseconds to match specification in sonic_platform_common/chassis_base.py
diff --git a/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/chassis.py b/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/chassis.py
@@ -87,7 +87,6 @@ def __del__(self):
 # check for this event change for sfp / do we need to handle timeout/sleep
 
     def get_change_event(self, timeout=0):
-        from time import sleep
         """
         Returns a nested dictionary containing all devices which have
         experienced a change at chassis level
@@ -116,7 +115,6 @@ def get_change_event(self, timeout=0):
                 if (now_ms - start_ms >= timeout):
                     return True, change_dict
 
-
     def get_sfp(self, index):
         """
         Retrieves sfp represented by (0-based) index <index>
@@ -136,7 +134,7 @@ def get_sfp(self, index):
             # The index will start from 0
             sfp = self._sfp_list[index-1]
         except IndexError:
-            sys.stderr.write("SFP index {} out of range (0-{})\n".format(
+            sys.stderr.write("SFP index {} out of range (1-{})\n".format(
                              index, len(self._sfp_list)))
         return sfp
 
@@ -223,6 +221,7 @@ def get_num_sfps(self):
             An integer represences the number of SFPs on the chassis.
         """
         return self._num_sfps
+
     def get_reboot_cause(self):
         """
         Retrieves the cause of the previous reboot
@@ -259,4 +258,3 @@ def get_reboot_cause(self):
             return (self.REBOOT_CAUSE_HARDWARE_OTHER, "Reset Button Cold Reboot")
         else:
             return (self.REBOOT_CAUSE_NON_HARDWARE, None)
-
diff --git a/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/thermal.py b/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/thermal.py
@@ -105,9 +105,9 @@ def get_high_threshold(self):
             Celsius up to nearest thousandth of one degree Celsius,
             e.g. 30.125
         """
-        is_valid, high_threshold = self.sensor.get_threshold("UpperCritical")
+        is_valid, high_threshold = self.sensor.get_threshold("UpperNonCritical")
         if not is_valid:
-            high_threshold = 0
+            return super(Thermal, self).get_high_threshold()
 
         return float(high_threshold)
 
@@ -134,12 +134,11 @@ def get_high_critical_threshold(self):
             thermal in Celsius up to nearest thousandth of one degree
             Celsius, e.g. 30.125
         """
-        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperNonRecoverable")
+        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperCritical")
         if not is_valid:
-            high_crit_threshold = 0
+            return super(Thermal, self).get_high_critical_threshold()
 
         return float(high_crit_threshold)
-	
 
     def set_high_threshold(self, temperature):
         """
diff --git a/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/watchdog.py b/platform/broadcom/sonic-platform-modules-dell/s5232f/sonic_platform/watchdog.py
@@ -43,7 +43,7 @@ def __init__(self):
     def _get_command_result(self, cmdline):
         try:
             proc = subprocess.Popen(cmdline.split(), stdout=subprocess.PIPE,
-                    stderr=subprocess.STDOUT)
+                                    stderr=subprocess.STDOUT, universal_newlines=True)
             stdout = proc.communicate()[0]
             proc.wait()
             result = stdout.rstrip('\n')
@@ -207,4 +207,3 @@ def get_remaining_time(self):
                 return self.timeout - diff_time
 
         return 0
-
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/chassis.py b/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/chassis.py
@@ -104,7 +104,7 @@ def _get_register(self, reg_file):
         try:
             with os.fdopen(os.open(reg_file, os.O_RDONLY)) as fd:
                 retval = fd.read()
-        except:
+        except Exception:
             pass
         retval = retval.rstrip('\r\n')
         retval = retval.lstrip(" ")
@@ -134,6 +134,8 @@ def get_change_event(self, timeout=0):
         port_dict = {}
         change_dict = {}
         change_dict['sfp'] = port_dict
+        if timeout != 0:
+            timeout = timeout / 1000
         try:
             # We get notified when there is a MSI interrupt (vector 4/5)CVR
             # Open the sysfs file and register the epoll object
@@ -174,7 +176,7 @@ def get_change_event(self, timeout=0):
                 if (retval != 0):
                     return False, change_dict
             return True, change_dict
-        except:
+        except Exception:
             return False, change_dict
         finally:
             if self.oir_fd != -1:
@@ -183,7 +185,6 @@ def get_change_event(self, timeout=0):
                 self.oir_fd.close()
                 self.oir_fd = -1
                 self.epoll = -1
-        return False, change_dict
 
     def get_sfp(self, index):
         """
@@ -281,7 +282,7 @@ def get_reboot_cause(self):
         try:
             with open(self.REBOOT_CAUSE_PATH) as fd:
                 reboot_cause = int(fd.read(), 16)
-        except:
+        except Exception:
             return (self.REBOOT_CAUSE_NON_HARDWARE, None)
 
         if reboot_cause & 0x1:
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/component.py b/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/component.py
@@ -10,7 +10,6 @@
 ########################################################################
 
 try:
-    import os
     import re
     from sonic_platform_base.component_base import ComponentBase
 
@@ -24,17 +23,18 @@ class Component(ComponentBase):
 
     CHASSIS_COMPONENTS = [
         ["BIOS", ("Performs initialization of hardware components during "
-                                                                 "booting")],
+                  "booting")],
         ["FPGA", ("Used for managing the system LEDs")],
         ["BMC", ("Platform management controller for on-board temperature "
-                         "monitoring, in-chassis power, Fan and LED control")],
+                 "monitoring, in-chassis power, Fan and LED control")],
         ["System CPLD", ("Used for managing the CPU power sequence and CPU states")],
         ["Slave CPLD 1", ("Used for managing QSFP/QSFP28 port transceivers (1-16)")],
         ["Slave CPLD 2", ("Used for managing QSFP/QSFP28 port transceivers (17-32)")],
         ["Slave CPLD 3", ("Used for managing QSFP/QSFP28 port transceivers (33-48)")],
         ["Slave CPLD 4", ("Used for managing QSFP/QSFP28 port transceivers (49-64) and SFP/SFP28 "
-                                                              "port transceivers (65 and 66)")],
-                                                                                        ]
+                          "port transceivers (65 and 66)")],
+    ]
+
     def __init__(self, component_index=0):
         self.index = component_index
         self.name = self.CHASSIS_COMPONENTS[self.index][0]
@@ -48,7 +48,6 @@ def get_name(self):
         """
         return self.name
 
-    
     def get_description(self):
         """
         Retrieves the description of the component
@@ -77,7 +76,7 @@ def get_firmware_version(self):
             if version:
                 rv = version.group(1).strip()
         return rv
-                                                                                                                                                            
+
     def install_firmware(self, image_path):
         """
         Installs firmware to the component
@@ -87,5 +86,3 @@ def install_firmware(self, image_path):
         A boolean, True if install was successful, False if not
         """
         return False
-
-
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/sfp.py b/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/sfp.py
@@ -12,11 +12,7 @@
     import os
     import time
     import struct
-    import sys
-    import getopt
-    import select
     import mmap
-    from sonic_platform_base.chassis_base import ChassisBase
     from sonic_platform_base.sfp_base import SfpBase
     from sonic_platform_base.sonic_sfp.sff8436 import sff8436InterfaceId
     from sonic_platform_base.sonic_sfp.sff8436 import sff8436Dom
@@ -976,15 +972,15 @@ def reset(self):
             reg_value = reg_value & ~mask
 
             # Convert our register value back to a hex string and write back
-            status = self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
+            self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
 
             # Sleep 1 second to allow it to settle
             time.sleep(1)
 
             reg_value = reg_value | mask
 
             # Convert our register value back to a hex string and write back
-            status = self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
+            self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
 
             return True
 
@@ -1016,7 +1012,7 @@ def set_lpmode(self, lpmode):
                 reg_value = reg_value & ~mask
 
             # Convert our register value back to a hex string and write back
-            status = self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
+            self.pci_set_value(self.BASE_RES_PATH, reg_value, port_offset)
 
             return True
 
@@ -1035,12 +1031,6 @@ def tx_disable_channel(self, channel, disable):
         """
         return False
 
-    def tx_disable_channel(self, channel, disable):
-        """
-        Sets the tx_disable for specified SFP channels
-        """
-        return False
-
     def set_power_override(self, power_override, power_set):
         """
         Sets SFP power level using power_override and power_set
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/thermal.py b/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/thermal.py
@@ -11,7 +11,7 @@
 
 try:
     from sonic_platform_base.thermal_base import ThermalBase
-    from sonic_platform.ipmihelper import IpmiSensor, IpmiFru
+    from sonic_platform.ipmihelper import IpmiSensor
 except ImportError as e:
     raise ImportError(str(e) + "- required module not found")
 
@@ -105,9 +105,9 @@ def get_high_threshold(self):
             Celsius up to nearest thousandth of one degree Celsius,
             e.g. 30.125
         """
-        is_valid, high_threshold = self.sensor.get_threshold("UpperCritical")
+        is_valid, high_threshold = self.sensor.get_threshold("UpperNonCritical")
         if not is_valid:
-            high_threshold = 0
+            return super(Thermal, self).get_high_threshold()
 
         return float(high_threshold)
 
@@ -135,9 +135,9 @@ def get_high_critical_threshold(self):
             thermal in Celsius up to nearest thousandth of one degree
             Celsius, e.g. 30.125
         """
-        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperNonRecoverable")
+        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperCritical")
         if not is_valid:
-            high_crit_threshold = 0
+            return super(Thermal, self).get_high_critical_threshold()
 
         return float(high_crit_threshold)
 
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/watchdog.py b/platform/broadcom/sonic-platform-modules-dell/z9264f/sonic_platform/watchdog.py
@@ -10,8 +10,6 @@
 ########################################################################
 
 try:
-    import sys
-    import struct
     import ctypes
     import subprocess
     from sonic_platform_base.watchdog_base import WatchdogBase
@@ -45,7 +43,7 @@ def __init__(self):
     def _get_command_result(self, cmdline):
         try:
             proc = subprocess.Popen(cmdline.split(), stdout=subprocess.PIPE,
-                    stderr=subprocess.STDOUT)
+                                    stderr=subprocess.STDOUT, universal_newlines=True)
             stdout = proc.communicate()[0]
             proc.wait()
             result = stdout.rstrip('\n')
@@ -139,8 +137,6 @@ def arm(self, seconds):
             self.timeout = seconds
             return seconds
 
-        return -1
-
     def disarm(self):
         """
         Disarm the hardware watchdog
@@ -211,4 +207,3 @@ def get_remaining_time(self):
                 return self.timeout - diff_time
 
         return 0
-
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9332f/sonic_platform/chassis.py b/platform/broadcom/sonic-platform-modules-dell/z9332f/sonic_platform/chassis.py
@@ -200,7 +200,7 @@ def get_sfp(self, index):
             # The index will start from 0
             sfp = self._sfp_list[index-1]
         except IndexError:
-            sys.stderr.write("SFP index {} out of range (0-{})\n".format(
+            sys.stderr.write("SFP index {} out of range (1-{})\n".format(
                              index, len(self._sfp_list)))
         return sfp
 
diff --git a/platform/broadcom/sonic-platform-modules-dell/z9332f/sonic_platform/thermal.py b/platform/broadcom/sonic-platform-modules-dell/z9332f/sonic_platform/thermal.py
@@ -111,9 +111,9 @@ def get_high_threshold(self):
             Celsius up to nearest thousandth of one degree Celsius,
             e.g. 30.125
         """
-        is_valid, high_threshold = self.sensor.get_threshold("UpperCritical")
+        is_valid, high_threshold = self.sensor.get_threshold("UpperNonCritical")
         if not is_valid:
-            high_threshold = 0
+            return super(Thermal, self).get_high_threshold()
 
         return float(high_threshold)
 
@@ -140,9 +140,9 @@ def get_high_critical_threshold(self):
             thermal in Celsius up to nearest thousandth of one degree
             Celsius, e.g. 30.125
         """
-        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperNonRecoverable")
+        is_valid, high_crit_threshold = self.sensor.get_threshold("UpperCritical")
         if not is_valid:
-            high_crit_threshold = 0
+            return super(Thermal, self).get_high_critical_threshold()
 
         return float(high_crit_threshold)
 	
