Skip to content

Commit ed93a15

Browse files
authored
[sonic_platform_base] Proper use of class and instance attributes (sonic-net#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.
1 parent 691de92 commit ed93a15

12 files changed

+183
-201
lines changed

sonic_platform_base/__init__.py

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
from . import chassis_base
2+
from . import component_base
3+
from . import device_base
4+
from . import fan_base
5+
from . import fan_drawer_base
6+
from . import module_base
7+
from . import platform_base
8+
from . import psu_base
9+
from . import sfp_base
10+
from . import thermal_base
11+
from . import watchdog_base

sonic_platform_base/chassis_base.py

+37-44
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
#
2-
# chassis_base.py
3-
#
4-
# Base class for implementing a platform-specific class with which
5-
# to interact with a chassis device in SONiC.
6-
#
1+
"""
2+
chassis_base.py
3+
4+
Base class for implementing a platform-specific class with which
5+
to interact with a chassis device in SONiC.
6+
"""
77

88
import sys
99
from . import device_base
@@ -26,51 +26,44 @@ class ChassisBase(device_base.DeviceBase):
2626
REBOOT_CAUSE_HARDWARE_OTHER = "Hardware - Other"
2727
REBOOT_CAUSE_NON_HARDWARE = "Non-Hardware"
2828

29-
# List of ComponentBase-derived objects representing all components
30-
# available on the chassis
31-
_component_list = None
29+
def __init__(self):
30+
# List of ComponentBase-derived objects representing all components
31+
# available on the chassis
32+
self._component_list = []
3233

33-
# List of ModuleBase-derived objects representing all modules
34-
# available on the chassis (for use with modular chassis)
35-
_module_list = None
34+
# List of ModuleBase-derived objects representing all modules
35+
# available on the chassis (for use with modular chassis)
36+
self._module_list = []
3637

37-
# List of FanBase-derived objects representing all fans
38-
# available on the chassis
39-
_fan_list = None
38+
# List of FanBase-derived objects representing all fans
39+
# available on the chassis
40+
self._fan_list = []
4041

41-
# List of FanDrawerBase-derived objects representing all fan drawers
42-
# available on the chassis
43-
_fan_drawer_list = None
42+
# List of FanDrawerBase-derived objects representing all fan drawers
43+
# available on the chassis
44+
self._fan_drawer_list = []
4445

45-
# List of PsuBase-derived objects representing all power supply units
46-
# available on the chassis
47-
_psu_list = None
46+
# List of PsuBase-derived objects representing all power supply units
47+
# available on the chassis
48+
self._psu_list = []
4849

49-
# List of ThermalBase-derived objects representing all thermals
50-
# available on the chassis
51-
_thermal_list = None
50+
# List of ThermalBase-derived objects representing all thermals
51+
# available on the chassis
52+
self._thermal_list = []
5253

53-
# List of SfpBase-derived objects representing all sfps
54-
# available on the chassis
55-
_sfp_list = None
54+
# List of SfpBase-derived objects representing all sfps
55+
# available on the chassis
56+
self._sfp_list = []
5657

57-
# Object derived from WatchdogBase for interacting with hardware watchdog
58-
_watchdog = None
58+
# Object derived from WatchdogBase for interacting with hardware watchdog
59+
self._watchdog = None
5960

60-
# Object derived from eeprom_tlvinfo.TlvInfoDecoder indicating the eeprom on the chassis
61-
_eeprom = None
61+
# Object derived from eeprom_tlvinfo.TlvInfoDecoder indicating the eeprom on the chassis
62+
self._eeprom = None
6263

63-
# System status LED
64-
_status_led = None
64+
# System status LED
65+
self._status_led = None
6566

66-
def __init__(self):
67-
self._component_list = []
68-
self._module_list = []
69-
self._fan_list = []
70-
self._psu_list = []
71-
self._thermal_list = []
72-
self._sfp_list = []
73-
self._fan_drawer_list = []
7467

7568
def get_base_mac(self):
7669
"""
@@ -472,7 +465,7 @@ def get_all_sfps(self):
472465
Retrieves all sfps available on this chassis
473466
474467
Returns:
475-
A list of objects derived from SfpBase representing all sfps
468+
A list of objects derived from SfpBase representing all sfps
476469
available on this chassis
477470
"""
478471
return self._sfp_list
@@ -565,7 +558,7 @@ def get_change_event(self, timeout=0):
565558
- True if call successful, False if not;
566559
- A nested dictionary where key is a device type,
567560
value is a dictionary with key:value pairs in the format of
568-
{'device_id':'device_event'},
561+
{'device_id':'device_event'},
569562
where device_id is the device ID for this device and
570563
device_event,
571564
status='1' represents device inserted,
@@ -574,7 +567,7 @@ def get_change_event(self, timeout=0):
574567
indicates that fan 0 has been removed, fan 2
575568
has been inserted and sfp 11 has been removed.
576569
Specifically for SFP event, besides SFP plug in and plug out,
577-
there are some other error event could be raised from SFP, when
570+
there are some other error event could be raised from SFP, when
578571
these error happened, SFP eeprom will not be avalaible, XCVRD shall
579572
stop to read eeprom before SFP recovered from error status.
580573
status='2' I2C bus stuck,

sonic_platform_base/component_base.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
#
2-
# component_base.py
3-
#
4-
# Abstract base class for implementing a platform-specific class
5-
# to interact with a chassis/module component (e.g., BIOS, CPLD, FPGA, etc.) in SONiC
6-
#
1+
"""
2+
component_base.py
3+
4+
Abstract base class for implementing a platform-specific class
5+
to interact with a chassis/module component (e.g., BIOS, CPLD, FPGA, etc.) in SONiC
6+
"""
77

88

99
class ComponentBase(object):

sonic_platform_base/device_base.py

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
#
2-
# device_base.py
3-
#
4-
# Abstract base class for interfacing with a generic type of platform
5-
# peripheral device in SONiC
6-
#
1+
"""
2+
device_base.py
3+
4+
Abstract base class for interfacing with a generic type of platform
5+
peripheral device in SONiC
6+
"""
7+
78

89
class DeviceBase(object):
910
"""

sonic_platform_base/fan_base.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
#
2-
# fan_base.py
3-
#
4-
# Abstract base class for implementing a platform-specific class with which
5-
# to interact with a fan module in SONiC
6-
#
1+
"""
2+
fan_base.py
3+
4+
Abstract base class for implementing a platform-specific class with which
5+
to interact with a fan module in SONiC
6+
"""
77

88
from . import device_base
99

sonic_platform_base/fan_drawer_base.py

+6-6
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
#
2-
# fan_drawer_base.py
3-
#
4-
# Abstract base class for implementing a platform-specific class with which
5-
# to interact with a fan drawer module in SONiC
6-
#
1+
"""
2+
fan_drawer_base.py
3+
4+
Abstract base class for implementing a platform-specific class with which
5+
to interact with a fan drawer module in SONiC
6+
"""
77

88
from . import device_base
99

sonic_platform_base/module_base.py

+41-47
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
#
2-
# module_base.py
3-
#
4-
# Base class for implementing a platform-specific class with which
5-
# to interact with a module (as used in a modular chassis) SONiC.
6-
#
1+
"""
2+
module_base.py
3+
4+
Base class for implementing a platform-specific class with which
5+
to interact with a module (as used in a modular chassis) SONiC.
6+
"""
77

88
import sys
99
from . import device_base
@@ -12,7 +12,7 @@
1212
class ModuleBase(device_base.DeviceBase):
1313
"""
1414
Base class for interfacing with a module (supervisor module, line card
15-
module, etc. (applicable for a modular chassis)
15+
module, etc. (applicable for a modular chassis)
1616
"""
1717
# Device type definition. Note, this is a constant.
1818
DEVICE_TYPE = "module"
@@ -50,31 +50,25 @@ class ModuleBase(device_base.DeviceBase):
5050
# Module reboot type to reboot FPGA complex
5151
MODULE_REBOOT_FPGA_COMPLEX = "FPGA"
5252

53-
# List of ComponentBase-derived objects representing all components
54-
# available on the module
55-
_component_list = None
56-
57-
# List of FanBase-derived objects representing all fans
58-
# available on the module
59-
_fan_list = None
60-
61-
# List of PsuBase-derived objects representing all power supply units
62-
# available on the module
63-
_psu_list = None
64-
65-
# List of ThermalBase-derived objects representing all thermals
66-
# available on the module
67-
_thermal_list = None
68-
69-
# List of SfpBase-derived objects representing all sfps
70-
# available on the module
71-
_sfp_list = None
72-
7353
def __init__(self):
54+
# List of ComponentBase-derived objects representing all components
55+
# available on the module
7456
self._component_list = []
57+
58+
# List of FanBase-derived objects representing all fans
59+
# available on the module
7560
self._fan_list = []
61+
62+
# List of PsuBase-derived objects representing all power supply units
63+
# available on the module
7664
self._psu_list = []
65+
66+
# List of ThermalBase-derived objects representing all thermals
67+
# available on the module
7768
self._thermal_list = []
69+
70+
# List of SfpBase-derived objects representing all sfps
71+
# available on the module
7872
self._sfp_list = []
7973

8074
def get_base_mac(self):
@@ -89,15 +83,15 @@ def get_base_mac(self):
8983

9084
def get_system_eeprom_info(self):
9185
"""
92-
Retrieves the full content of system EEPROM information for the module
86+
Retrieves the full content of system EEPROM information for the module
9387
9488
Returns:
9589
A dictionary where keys are the type code defined in
9690
OCP ONIE TlvInfo EEPROM format and values are their corresponding
9791
values.
98-
Ex. { 0x21’:’AG9064’, ‘0x22’:’V1.0’, ‘0x23’:’AG9064-0109867821,
99-
0x24’:’001c0f000fcd0a’, ‘0x25’:’02/03/2018 16:22:00,
100-
0x26’:’01’, ‘0x27’:’REV01’, ‘0x28’:’AG9064-C2358-16G}
92+
Ex. { '0x21': 'AG9064', '0x22': 'V1.0', '0x23': 'AG9064-0109867821',
93+
'0x24': '001c0f000fcd0a', '0x25': '02/03/2018 16:22:00',
94+
'0x26': '01', '0x27': 'REV01', '0x28': 'AG9064-C2358-16G'}
10195
"""
10296
raise NotImplementedError
10397

@@ -254,11 +248,11 @@ def get_num_fans(self):
254248

255249
def get_all_fans(self):
256250
"""
257-
Retrieves all fan modules available on this module
251+
Retrieves all fan modules available on this module
258252
259253
Returns:
260254
A list of objects derived from FanBase representing all fan
261-
modules available on this module
255+
modules available on this module
262256
"""
263257
return self._fan_list
264258

@@ -290,21 +284,21 @@ def get_fan(self, index):
290284

291285
def get_num_psus(self):
292286
"""
293-
Retrieves the number of power supply units available on this module
287+
Retrieves the number of power supply units available on this module
294288
295289
Returns:
296290
An integer, the number of power supply units available on this
297-
module
291+
module
298292
"""
299293
return len(self._psu_list)
300294

301295
def get_all_psus(self):
302296
"""
303-
Retrieves all power supply units available on this module
297+
Retrieves all power supply units available on this module
304298
305299
Returns:
306300
A list of objects derived from PsuBase representing all power
307-
supply units available on this module
301+
supply units available on this module
308302
"""
309303
return self._psu_list
310304

@@ -336,20 +330,20 @@ def get_psu(self, index):
336330

337331
def get_num_thermals(self):
338332
"""
339-
Retrieves the number of thermals available on this module
333+
Retrieves the number of thermals available on this module
340334
341335
Returns:
342-
An integer, the number of thermals available on this module
336+
An integer, the number of thermals available on this module
343337
"""
344338
return len(self._thermal_list)
345339

346340
def get_all_thermals(self):
347341
"""
348-
Retrieves all thermals available on this module
342+
Retrieves all thermals available on this module
349343
350344
Returns:
351345
A list of objects derived from ThermalBase representing all thermals
352-
available on this module
346+
available on this module
353347
"""
354348
return self._thermal_list
355349

@@ -380,20 +374,20 @@ def get_thermal(self, index):
380374

381375
def get_num_sfps(self):
382376
"""
383-
Retrieves the number of sfps available on this module
377+
Retrieves the number of sfps available on this module
384378
385379
Returns:
386-
An integer, the number of sfps available on this module
380+
An integer, the number of sfps available on this module
387381
"""
388382
return len(self._sfp_list)
389383

390384
def get_all_sfps(self):
391385
"""
392-
Retrieves all sfps available on this module
386+
Retrieves all sfps available on this module
393387
394388
Returns:
395-
A list of objects derived from PsuBase representing all sfps
396-
available on this module
389+
A list of objects derived from PsuBase representing all sfps
390+
available on this module
397391
"""
398392
return self._sfp_list
399393

@@ -431,7 +425,7 @@ def get_change_event(self, timeout=0):
431425
- True if call successful, False if not;
432426
- A nested dictionary where key is a device type,
433427
value is a dictionary with key:value pairs in the format of
434-
{'device_id':'device_event'},
428+
{'device_id':'device_event'},
435429
where device_id is the device ID for this device and
436430
device_event,
437431
status='1' represents device inserted,

0 commit comments

Comments
 (0)