Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default implementation of under/over speed checks #382

Merged
merged 5 commits into from
Jul 11, 2023
Merged

Default implementation of under/over speed checks #382

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1c73c9c
Default implementation of under/over speed checks
spilkey-cisco May 23, 2023
738ccc6
Update comment, remove requirement for float conversion
spilkey-cisco Jun 13, 2023
c0d4fcc
Merge remote-tracking branch 'origin/master' into spilkey/fan_tolerance
spilkey-cisco Jun 13, 2023
093c69f
Merge branch 'master' into spilkey/fan_tolerance
spilkey-cisco Jul 6, 2023
fa986c5
Fan test coverage
spilkey-cisco Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions sonic_platform_base/fan_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,52 @@ def get_speed_tolerance(self):
"""
raise NotImplementedError

def is_under_speed(self):
"""
Calculates if the fan speed is under the tolerated low speed threshold

Default calculation requires get_speed_tolerance to be implemented, and checks
if the current fan speed (expressed as a percentage) is lower than <get_speed_tolerance>
percent below the target fan speed (expressed as a percentage)

Returns:
A boolean, True if fan speed is under the low threshold, False if not
"""
speed = self.get_speed()
target_speed = self.get_target_speed()
tolerance = self.get_speed_tolerance()

for param, value in (('speed', speed), ('target speed', target_speed), ('speed tolerance', tolerance)):
if not isinstance(value, int):
raise TypeError(f'Fan {param} is not an integer value: {param}={value}')
if value < 0 or value > 100:
raise ValueError(f'Fan {param} is not a valid percentage value: {param}={value}')

return speed * 100 < target_speed * (100 - tolerance)

def is_over_speed(self):
"""
Calculates if the fan speed is over the tolerated high speed threshold

Default calculation requires get_speed_tolerance to be implemented, and checks
if the current fan speed (expressed as a percentage) is higher than <get_speed_tolerance>
percent above the target fan speed (expressed as a percentage)

Returns:
A boolean, True if fan speed is over the high threshold, False if not
"""
speed = self.get_speed()
Comment on lines +69 to +103
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@spilkey-cisco This is an abstract class. Where is the HLD changes?

Copy link
Contributor Author

@spilkey-cisco spilkey-cisco Jun 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that https://github.com/sonic-net/sonic-platform-common/blob/master/sonic_fan/fan_base.py is the (obsolete, from what I can tell) fan abstract base class, and the file in this changeset follows the design from https://github.com/sonic-net/SONiC/blob/master/doc/platform_api/new_platform_api.md indicating ABCs are no longer used, 'No longer abstract base classes: All abstract methods simply raise "NotImplementedError"'. The new methods are not abstract since they have a default implementation, but can be overridden with vendor specific implementations if desired. If an HLD is needed, please advise on where it should go.

Please correct me if this understanding is inaccurate or incomplete.

target_speed = self.get_target_speed()
tolerance = self.get_speed_tolerance()

for param, value in (('speed', speed), ('target speed', target_speed), ('speed tolerance', tolerance)):
if not isinstance(value, int):
raise TypeError(f'Fan {param} is not an integer value: {param}={value}')
if value < 0 or value > 100:
raise ValueError(f'Fan {param} is not a valid percentage value: {param}={value}')

return speed * 100 > target_speed * (100 + tolerance)

def set_speed(self, speed):
"""
Sets the fan speed
Expand Down