[sonic_platform_base] Proper use of class and instance attributes (#173)

#### Description - Use class and instance attributes properly. Only use class attributes for data which should be shared among all instances. - Import all modules in `__init__.py` - Remove unused imports - Clean up whitespace #### Motivation and Context Proper object-oriented programming. This could prevent issues where attributes are shared between instances when they were not meant to be. This also fixes a bug with `PsuBase.psu_master_led_color`, where it is defined as both a class attribute and an instance attribute. With this change, some vendors' APIs can be cleaned up, because they redefined class attributes as instance attributes. That code can be removed. However, note that vendor APIs MUST call the base class initializer in their initializer methods, or the instance attributes will not be available and will raise exceptions.
sonic-net · Mar 5, 2021 · ed93a15 · ed93a15
1 parent 691de92
commit ed93a15
Show file tree

Hide file tree

Showing 12 changed files with 183 additions and 201 deletions.
diff --git a/sonic_platform_base/__init__.py b/sonic_platform_base/__init__.py
@@ -0,0 +1,11 @@
+from . import chassis_base 
+from . import component_base
+from . import device_base
+from . import fan_base
+from . import fan_drawer_base
+from . import module_base
+from . import platform_base
+from . import psu_base
+from . import sfp_base
+from . import thermal_base
+from . import watchdog_base
diff --git a/sonic_platform_base/chassis_base.py b/sonic_platform_base/chassis_base.py
@@ -1,9 +1,9 @@
-#
-# chassis_base.py
-#
-# Base class for implementing a platform-specific class with which
-# to interact with a chassis device in SONiC.
-#
+"""
+    chassis_base.py
+
+    Base class for implementing a platform-specific class with which
+    to interact with a chassis device in SONiC.
+"""
 
 import sys
 from . import device_base
@@ -26,51 +26,44 @@ class ChassisBase(device_base.DeviceBase):
     REBOOT_CAUSE_HARDWARE_OTHER = "Hardware - Other"
     REBOOT_CAUSE_NON_HARDWARE = "Non-Hardware"
 
-    # List of ComponentBase-derived objects representing all components
-    # available on the chassis
-    _component_list = None
+    def __init__(self):
+        # List of ComponentBase-derived objects representing all components
+        # available on the chassis
+        self._component_list = []
 
-    # List of ModuleBase-derived objects representing all modules
-    # available on the chassis (for use with modular chassis)
-    _module_list = None
+        # List of ModuleBase-derived objects representing all modules
+        # available on the chassis (for use with modular chassis)
+        self._module_list = []
 
-    # List of FanBase-derived objects representing all fans
-    # available on the chassis
-    _fan_list = None
+        # List of FanBase-derived objects representing all fans
+        # available on the chassis
+        self._fan_list = []
 
-    # List of FanDrawerBase-derived objects representing all fan drawers
-    # available on the chassis
-    _fan_drawer_list = None
+        # List of FanDrawerBase-derived objects representing all fan drawers
+        # available on the chassis
+        self._fan_drawer_list = []
 
-    # List of PsuBase-derived objects representing all power supply units
-    # available on the chassis
-    _psu_list = None
+        # List of PsuBase-derived objects representing all power supply units
+        # available on the chassis
+        self._psu_list = []
 
-    # List of ThermalBase-derived objects representing all thermals
-    # available on the chassis
-    _thermal_list = None
+        # List of ThermalBase-derived objects representing all thermals
+        # available on the chassis
+        self._thermal_list = []
 
-    # List of SfpBase-derived objects representing all sfps
-    # available on the chassis
-    _sfp_list = None
+        # List of SfpBase-derived objects representing all sfps
+        # available on the chassis
+        self._sfp_list = []
 
-    # Object derived from WatchdogBase for interacting with hardware watchdog
-    _watchdog = None
+        # Object derived from WatchdogBase for interacting with hardware watchdog
+        self._watchdog = None
 
-    # Object derived from eeprom_tlvinfo.TlvInfoDecoder indicating the eeprom on the chassis
-    _eeprom = None
+        # Object derived from eeprom_tlvinfo.TlvInfoDecoder indicating the eeprom on the chassis
+        self._eeprom = None
 
-    # System status LED
-    _status_led = None
+        # System status LED
+        self._status_led = None
 
-    def __init__(self):
-        self._component_list = []
-        self._module_list = []
-        self._fan_list = []
-        self._psu_list = []
-        self._thermal_list = []
-        self._sfp_list = []
-        self._fan_drawer_list = []
 
     def get_base_mac(self):
         """
@@ -472,7 +465,7 @@ def get_all_sfps(self):
         Retrieves all sfps available on this chassis
 
         Returns:
-            A list of objects derived from SfpBase representing all sfps 
+            A list of objects derived from SfpBase representing all sfps
             available on this chassis
         """
         return self._sfp_list
@@ -565,7 +558,7 @@ def get_change_event(self, timeout=0):
                 - True if call successful, False if not;
                 - A nested dictionary where key is a device type,
                   value is a dictionary with key:value pairs in the format of
-                  {'device_id':'device_event'}, 
+                  {'device_id':'device_event'},
                   where device_id is the device ID for this device and
                         device_event,
                              status='1' represents device inserted,
@@ -574,7 +567,7 @@ def get_change_event(self, timeout=0):
                       indicates that fan 0 has been removed, fan 2
                       has been inserted and sfp 11 has been removed.
                   Specifically for SFP event, besides SFP plug in and plug out,
-                  there are some other error event could be raised from SFP, when 
+                  there are some other error event could be raised from SFP, when
                   these error happened, SFP eeprom will not be avalaible, XCVRD shall
                   stop to read eeprom before SFP recovered from error status.
                       status='2' I2C bus stuck,

diff --git a/sonic_platform_base/component_base.py b/sonic_platform_base/component_base.py
@@ -1,9 +1,9 @@
-#
-# component_base.py
-#
-# Abstract base class for implementing a platform-specific class
-# to interact with a chassis/module component (e.g., BIOS, CPLD, FPGA, etc.) in SONiC
-#
+"""
+    component_base.py
+
+    Abstract base class for implementing a platform-specific class
+    to interact with a chassis/module component (e.g., BIOS, CPLD, FPGA, etc.) in SONiC
+"""
 
 
 class ComponentBase(object):

diff --git a/sonic_platform_base/device_base.py b/sonic_platform_base/device_base.py
@@ -1,9 +1,10 @@
-#
-# device_base.py
-#
-# Abstract base class for interfacing with a generic type of platform
-# peripheral device in SONiC
-#
+"""
+    device_base.py
+
+    Abstract base class for interfacing with a generic type of platform
+    peripheral device in SONiC
+"""
+
 
 class DeviceBase(object):
     """

diff --git a/sonic_platform_base/fan_base.py b/sonic_platform_base/fan_base.py
@@ -1,9 +1,9 @@
-#
-# fan_base.py
-#
-# Abstract base class for implementing a platform-specific class with which
-# to interact with a fan module in SONiC
-#
+"""
+    fan_base.py
+
+    Abstract base class for implementing a platform-specific class with which
+    to interact with a fan module in SONiC
+"""
 
 from . import device_base
 

diff --git a/sonic_platform_base/fan_drawer_base.py b/sonic_platform_base/fan_drawer_base.py
@@ -1,9 +1,9 @@
-#
-# fan_drawer_base.py
-#
-# Abstract base class for implementing a platform-specific class with which
-# to interact with a fan drawer module in SONiC
-#
+"""
+    fan_drawer_base.py
+
+    Abstract base class for implementing a platform-specific class with which
+    to interact with a fan drawer module in SONiC
+"""
 
 from . import device_base
 

diff --git a/sonic_platform_base/module_base.py b/sonic_platform_base/module_base.py
@@ -1,9 +1,9 @@
-#
-# module_base.py
-#
-# Base class for implementing a platform-specific class with which
-# to interact with a module (as used in a modular chassis) SONiC.
-#
+"""
+    module_base.py
+
+    Base class for implementing a platform-specific class with which
+    to interact with a module (as used in a modular chassis) SONiC.
+"""
 
 import sys
 from . import device_base
@@ -12,7 +12,7 @@
 class ModuleBase(device_base.DeviceBase):
     """
     Base class for interfacing with a module (supervisor module, line card
-    module, etc. (applicable for a modular chassis) 
+    module, etc. (applicable for a modular chassis)
     """
     # Device type definition. Note, this is a constant.
     DEVICE_TYPE = "module"
@@ -50,31 +50,25 @@ class ModuleBase(device_base.DeviceBase):
     # Module reboot type to reboot FPGA complex
     MODULE_REBOOT_FPGA_COMPLEX = "FPGA"
 
-    # List of ComponentBase-derived objects representing all components
-    # available on the module
-    _component_list = None
-
-    # List of FanBase-derived objects representing all fans
-    # available on the module 
-    _fan_list = None
-
-    # List of PsuBase-derived objects representing all power supply units
-    # available on the module
-    _psu_list = None
-
-    # List of ThermalBase-derived objects representing all thermals
-    # available on the module
-    _thermal_list = None
-
-    # List of SfpBase-derived objects representing all sfps
-    # available on the module
-    _sfp_list = None
-
     def __init__(self):
+        # List of ComponentBase-derived objects representing all components
+        # available on the module
         self._component_list = []
+
+        # List of FanBase-derived objects representing all fans
+        # available on the module
         self._fan_list = []
+
+        # List of PsuBase-derived objects representing all power supply units
+        # available on the module
         self._psu_list = []
+
+        # List of ThermalBase-derived objects representing all thermals
+        # available on the module
         self._thermal_list = []
+
+        # List of SfpBase-derived objects representing all sfps
+        # available on the module
         self._sfp_list = []
 
     def get_base_mac(self):
@@ -89,15 +83,15 @@ def get_base_mac(self):
 
     def get_system_eeprom_info(self):
         """
-        Retrieves the full content of system EEPROM information for the module 
+        Retrieves the full content of system EEPROM information for the module
 
         Returns:
             A dictionary where keys are the type code defined in
             OCP ONIE TlvInfo EEPROM format and values are their corresponding
             values.
-            Ex. { ‘0x21’:’AG9064’, ‘0x22’:’V1.0’, ‘0x23’:’AG9064-0109867821’,
-                  ‘0x24’:’001c0f000fcd0a’, ‘0x25’:’02/03/2018 16:22:00’,
-                  ‘0x26’:’01’, ‘0x27’:’REV01’, ‘0x28’:’AG9064-C2358-16G’}
+            Ex. { '0x21': 'AG9064', '0x22': 'V1.0', '0x23': 'AG9064-0109867821',
+                  '0x24': '001c0f000fcd0a', '0x25': '02/03/2018 16:22:00',
+                  '0x26': '01', '0x27': 'REV01', '0x28': 'AG9064-C2358-16G'}
         """
         raise NotImplementedError
 
@@ -254,11 +248,11 @@ def get_num_fans(self):
 
     def get_all_fans(self):
         """
-        Retrieves all fan modules available on this module 
+        Retrieves all fan modules available on this module
 
         Returns:
             A list of objects derived from FanBase representing all fan
-            modules available on this module 
+            modules available on this module
         """
         return self._fan_list
 
@@ -290,21 +284,21 @@ def get_fan(self, index):
 
     def get_num_psus(self):
         """
-        Retrieves the number of power supply units available on this module 
+        Retrieves the number of power supply units available on this module
 
         Returns:
             An integer, the number of power supply units available on this
-            module 
+            module
         """
         return len(self._psu_list)
 
     def get_all_psus(self):
         """
-        Retrieves all power supply units available on this module 
+        Retrieves all power supply units available on this module
 
         Returns:
             A list of objects derived from PsuBase representing all power
-            supply units available on this module 
+            supply units available on this module
         """
         return self._psu_list
 
@@ -336,20 +330,20 @@ def get_psu(self, index):
 
     def get_num_thermals(self):
         """
-        Retrieves the number of thermals available on this module 
+        Retrieves the number of thermals available on this module
 
         Returns:
-            An integer, the number of thermals available on this module 
+            An integer, the number of thermals available on this module
         """
         return len(self._thermal_list)
 
     def get_all_thermals(self):
         """
-        Retrieves all thermals available on this module 
+        Retrieves all thermals available on this module
 
         Returns:
             A list of objects derived from ThermalBase representing all thermals
-            available on this module 
+            available on this module
         """
         return self._thermal_list
 
@@ -380,20 +374,20 @@ def get_thermal(self, index):
 
     def get_num_sfps(self):
         """
-        Retrieves the number of sfps available on this module 
+        Retrieves the number of sfps available on this module
 
         Returns:
-            An integer, the number of sfps available on this module 
+            An integer, the number of sfps available on this module
         """
         return len(self._sfp_list)
 
     def get_all_sfps(self):
         """
-        Retrieves all sfps available on this module 
+        Retrieves all sfps available on this module
 
         Returns:
-            A list of objects derived from PsuBase representing all sfps 
-            available on this module 
+            A list of objects derived from PsuBase representing all sfps
+            available on this module
         """
         return self._sfp_list
 
@@ -431,7 +425,7 @@ def get_change_event(self, timeout=0):
                 - True if call successful, False if not;
                 - A nested dictionary where key is a device type,
                   value is a dictionary with key:value pairs in the format of
-                  {'device_id':'device_event'}, 
+                  {'device_id':'device_event'},
                   where device_id is the device ID for this device and
                         device_event,
                              status='1' represents device inserted,