From ed93a1525656603377449b076445fdf3d6d41093 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 5 Mar 2021 10:48:29 -0800 Subject: [PATCH] [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_platform_base/__init__.py | 11 ++++ sonic_platform_base/chassis_base.py | 81 +++++++++++------------- sonic_platform_base/component_base.py | 12 ++-- sonic_platform_base/device_base.py | 13 ++-- sonic_platform_base/fan_base.py | 12 ++-- sonic_platform_base/fan_drawer_base.py | 12 ++-- sonic_platform_base/module_base.py | 88 ++++++++++++-------------- sonic_platform_base/platform_base.py | 24 +++---- sonic_platform_base/psu_base.py | 50 +++++++-------- sonic_platform_base/sfp_base.py | 47 ++++++-------- sonic_platform_base/thermal_base.py | 22 +++---- sonic_platform_base/watchdog_base.py | 12 ++-- 12 files changed, 183 insertions(+), 201 deletions(-) diff --git a/sonic_platform_base/__init__.py b/sonic_platform_base/__init__.py index e69de29bb2d1..3d7f64b24423 100644 --- 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 index d5097c76fdbe..c587df192601 100644 --- 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 index ba9c975caac4..074b81def8a2 100644 --- 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 index 909449d2c95b..72f0f05add0e 100644 --- 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 index 9662f120e053..2b1259144e49 100644 --- 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 index ed9e72ea97c7..f6bebb4c8bbd 100644 --- 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 index 45c0068cb291..cb4dda7d3077 100644 --- 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, diff --git a/sonic_platform_base/platform_base.py b/sonic_platform_base/platform_base.py index 4a422161ce3b..f1a77b439281 100644 --- a/sonic_platform_base/platform_base.py +++ b/sonic_platform_base/platform_base.py @@ -1,21 +1,17 @@ -# -# platform_base.py -# -# Base class for implementing platform-specific functionality in SONiC. -# This is the root class of sonic_platform_base hierarchy. A platform -# may contain one or more chassis. Classes derived from this class provide -# the ability to interact with all available chassis on the platform. -# - -from __future__ import print_function - -import sys +""" + platform_base.py + Base class for implementing platform-specific functionality in SONiC. + This is the root class of sonic_platform_base hierarchy. A platform + may contain one or more chassis. Classes derived from this class provide + the ability to interact with all available chassis on the platform. +""" class PlatformBase(object): - # ChassisBase-derived object representing the platform's chassis - _chassis = None + def __init__(self): + # ChassisBase-derived object representing the platform's chassis + self._chassis = None def get_chassis(self): """ diff --git a/sonic_platform_base/psu_base.py b/sonic_platform_base/psu_base.py index 3c95db9b2d2e..2f1ae601c6aa 100644 --- a/sonic_platform_base/psu_base.py +++ b/sonic_platform_base/psu_base.py @@ -1,9 +1,9 @@ -# -# psu_base.py -# -# Abstract base class for implementing a platform-specific class with which -# to interact with a power supply unit (PSU) in SONiC -# +""" + psu_base.py + + Abstract base class for implementing a platform-specific class with which + to interact with a power supply unit (PSU) in SONiC +""" from . import device_base @@ -15,28 +15,22 @@ class PsuBase(device_base.DeviceBase): # Device type definition. Note, this is a constant. DEVICE_TYPE = "psu" - # List of FanBase-derived objects representing all fans - # available on the PSU - _fan_list = None - - # List of ThermalBase-derived objects representing all thermals - # available on the PSU. Put a class level _thermal_list here to - # avoid an exception when call get_num_thermals, get_all_thermals - # and get_thermal if vendor does not call PsuBase.__init__ in concrete - # PSU class - _thermal_list = [] - # Status of Master LED - psu_master_led_color = None + # This is a class attribute because there is only one master status LED + # on the platform. This attribute is shared among all objects inheriting + # from this class and can be read or set from any of them. + _psu_master_led_color = None def __init__(self): + # List of FanBase-derived objects representing all fans + # available on the PSU self._fan_list = [] # List of ThermalBase-derived objects representing all thermals # available on the PSU self._thermal_list = [] - self.psu_master_led_color = self.STATUS_LED_COLOR_OFF + PsuBase._psu_master_led_color = self.STATUS_LED_COLOR_OFF def get_num_fans(self): """ @@ -124,8 +118,8 @@ def get_voltage(self): Retrieves current PSU voltage output Returns: - A float number, the output voltage in volts, - e.g. 12.1 + A float number, the output voltage in volts, + e.g. 12.1 """ raise NotImplementedError @@ -185,7 +179,7 @@ def get_temperature(self): Returns: A float number of current temperature in Celsius up to nearest thousandth - of one degree Celsius, e.g. 30.125 + of one degree Celsius, e.g. 30.125 """ raise NotImplementedError @@ -204,8 +198,8 @@ def get_voltage_high_threshold(self): Retrieves the high threshold PSU voltage output Returns: - A float number, the high threshold output voltage in volts, - e.g. 12.1 + A float number, the high threshold output voltage in volts, + e.g. 12.1 """ raise NotImplementedError @@ -214,8 +208,8 @@ def get_voltage_low_threshold(self): Retrieves the low threshold PSU voltage output Returns: - A float number, the low threshold output voltage in volts, - e.g. 12.1 + A float number, the low threshold output voltage in volts, + e.g. 12.1 """ raise NotImplementedError @@ -237,7 +231,7 @@ def get_status_master_led(cls): Returns: A string, one of the predefined STATUS_LED_COLOR_* strings. """ - return cls.psu_master_led_color + return cls._psu_master_led_color @classmethod def set_status_master_led(cls, color): @@ -248,5 +242,5 @@ def set_status_master_led(cls, color): bool: True if status LED state is set successfully, False if not """ - cls.psu_master_led_color = color + cls._psu_master_led_color = color return True diff --git a/sonic_platform_base/sfp_base.py b/sonic_platform_base/sfp_base.py index 91a00f8d2544..a472e49051f3 100644 --- a/sonic_platform_base/sfp_base.py +++ b/sonic_platform_base/sfp_base.py @@ -1,9 +1,9 @@ -# -# sfp_base.py -# -# Abstract base class for implementing a platform-specific class with which -# to interact with a SFP module in SONiC -# +""" + sfp_base.py + + Abstract base class for implementing a platform-specific class with which + to interact with a SFP module in SONiC +""" import sys from . import device_base @@ -16,13 +16,6 @@ class SfpBase(device_base.DeviceBase): # Device type definition. Note, this is a constant. DEVICE_TYPE = "sfp" - # List of ThermalBase-derived objects representing all thermals - # available on the SFP. Put a class level _thermal_list here to - # avoid an exception when call get_num_thermals, get_all_thermals - # and get_thermal if vendor does not call SfpBase.__init__ in concrete - # SFP class - _thermal_list = [] - def __init__(self): # List of ThermalBase-derived objects representing all thermals # available on the SFP @@ -75,7 +68,7 @@ def get_transceiver_info(self): Returns: A dict which contains following keys/values : ================================================================================ - keys |Value Format |Information + keys |Value Format |Information ---------------------------|---------------|---------------------------- type |1*255VCHAR |type of SFP hardware_rev |1*255VCHAR |hardware version of SFP @@ -103,7 +96,7 @@ def get_transceiver_bulk_status(self): Returns: A dict which contains following keys/values : ======================================================================== - keys |Value Format |Information + keys |Value Format |Information ---------------------------|---------------|---------------------------- rx_los |BOOLEAN |RX loss-of-signal status, True if has RX los, False if not. tx_fault |BOOLEAN |TX fault status, True if has TX fault, False if not. @@ -211,7 +204,7 @@ def get_tx_disable_channel(self): Returns: A hex of 4 bits (bit 0 to bit 3 as channel 0 to channel 3) to represent TX channels which have been disabled in this SFP. - As an example, a returned value of 0x5 indicates that channel 0 + As an example, a returned value of 0x5 indicates that channel 0 and channel 2 have been disabled. """ raise NotImplementedError @@ -233,7 +226,7 @@ def get_power_override(self): A Boolean, True if power-override is enabled, False if disabled """ raise NotImplementedError - + def get_temperature(self): """ Retrieves the temperature of this SFP @@ -252,7 +245,7 @@ def get_voltage(self): A float representing the supply voltage in mV """ raise NotImplementedError - + def get_tx_bias(self): """ Retrieves the TX bias current of all SFP channels @@ -263,7 +256,7 @@ def get_tx_bias(self): E.g., for a tranceiver with four channels: ['110.09', '111.12', '108.21', '112.09'] """ raise NotImplementedError - + def get_rx_power(self): """ Retrieves the received optical power of all SFP channels @@ -274,7 +267,7 @@ def get_rx_power(self): E.g., for a tranceiver with four channels: ['1.77', '1.71', '1.68', '1.70'] """ raise NotImplementedError - + def get_tx_power(self): """ Retrieves the TX power of all SFP channels @@ -285,7 +278,7 @@ def get_tx_power(self): E.g., for a tranceiver with four channels: ['1.86', '1.86', '1.86', '1.86'] """ raise NotImplementedError - + def reset(self): """ Reset SFP and return all user module settings to their default srate. @@ -294,7 +287,7 @@ def reset(self): A boolean, True if successful, False if not """ raise NotImplementedError - + def tx_disable(self, tx_disable): """ Disable SFP TX for all channels @@ -307,7 +300,7 @@ def tx_disable(self, tx_disable): A boolean, True if tx_disable is set successfully, False if not """ raise NotImplementedError - + def tx_disable_channel(self, channel, disable): """ Sets the tx_disable for specified SFP channels @@ -322,7 +315,7 @@ def tx_disable_channel(self, channel, disable): A boolean, True if successful, False if not """ raise NotImplementedError - + def set_lpmode(self, lpmode): """ Sets the lpmode (low power mode) of SFP @@ -335,13 +328,13 @@ def set_lpmode(self, lpmode): A boolean, True if lpmode is set successfully, False if not """ raise NotImplementedError - + def set_power_override(self, power_override, power_set): """ Sets SFP power level using power_override and power_set Args: - power_override : + power_override : A Boolean, True to override set_lpmode and use power_set to control SFP power, False to disable SFP power control through power_override/power_set and use set_lpmode @@ -375,7 +368,7 @@ def read_eeprom(self, offset, num_bytes): def write_eeprom(self, offset, num_bytes, write_buffer): """ - write eeprom specfic bytes beginning from a random offset with size as num_bytes + write eeprom specfic bytes beginning from a random offset with size as num_bytes and write_buffer as the required bytes Args: diff --git a/sonic_platform_base/thermal_base.py b/sonic_platform_base/thermal_base.py index 75cbea19407c..5f4a5117c764 100644 --- a/sonic_platform_base/thermal_base.py +++ b/sonic_platform_base/thermal_base.py @@ -1,9 +1,9 @@ -# -# thermal_base.py -# -# Abstract base class for implementing a platform-specific class with which -# to interact with a thermal module in SONiC -# +""" + thermal_base.py + + Abstract base class for implementing a platform-specific class with which + to interact with a thermal module in SONiC +""" from . import device_base @@ -21,10 +21,10 @@ def get_temperature(self): Returns: A float number of current temperature in Celsius up to nearest thousandth - of one degree Celsius, e.g. 30.125 + of one degree Celsius, e.g. 30.125 """ raise NotImplementedError - + def get_high_threshold(self): """ @@ -50,8 +50,8 @@ def set_high_threshold(self, temperature): """ Sets the high threshold temperature of thermal - Args : - temperature: A float number up to nearest thousandth of one degree Celsius, + Args : + temperature: A float number up to nearest thousandth of one degree Celsius, e.g. 30.125 Returns: @@ -63,7 +63,7 @@ def set_low_threshold(self, temperature): """ Sets the low threshold temperature of thermal - Args : + Args : temperature: A float number up to nearest thousandth of one degree Celsius, e.g. 30.125 diff --git a/sonic_platform_base/watchdog_base.py b/sonic_platform_base/watchdog_base.py index 42ecec2a6f6e..20ce7abef430 100644 --- a/sonic_platform_base/watchdog_base.py +++ b/sonic_platform_base/watchdog_base.py @@ -1,9 +1,9 @@ -# -# watchdog_base.py -# -# Abstract base class for implementing a platform-specific class with which -# to interact with a hardware watchdog module in SONiC -# +""" + watchdog_base.py + + Abstract base class for implementing a platform-specific class with which + to interact with a hardware watchdog module in SONiC +""" class WatchdogBase: