From 1c73c9cb61135e9c47d18f1e0022051b5d447298 Mon Sep 17 00:00:00 2001 From: Scott Pilkey Date: Tue, 23 May 2023 16:06:31 -0700 Subject: [PATCH 1/3] Default implementation of under/over speed checks --- sonic_platform_base/fan_base.py | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/sonic_platform_base/fan_base.py b/sonic_platform_base/fan_base.py index 2b1259144..91052f506 100644 --- a/sonic_platform_base/fan_base.py +++ b/sonic_platform_base/fan_base.py @@ -66,6 +66,44 @@ def get_speed_tolerance(self): """ raise NotImplementedError + def is_under_speed(self): + """ + Calculates if the fan speed is under the tolerated low speed threshold + + 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('Fan {0} is not an integer value: {0}={1}'.format(param, value)) + if value < 0 or value > 100: + raise ValueError('Fan {0} is not a valid percentage value: {0}={1}'.format(param, value)) + + return speed < target_speed * (1 - float(tolerance) / 100) + + def is_over_speed(self): + """ + Calculates if the fan speed is over the tolerated high speed threshold + + Returns: + A boolean, True if fan speed is over the high 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('Fan {0} is not an integer value: {0}={1}'.format(param, value)) + if value < 0 or value > 100: + raise ValueError('Fan {0} is not a valid percentage value: {0}={1}'.format(param, value)) + + return speed > target_speed * (1 + float(tolerance) / 100) + def set_speed(self, speed): """ Sets the fan speed From 738ccc6fc9b61535b852ca12953bc9591e5e5bc7 Mon Sep 17 00:00:00 2001 From: Scott Pilkey Date: Tue, 13 Jun 2023 15:51:50 -0700 Subject: [PATCH 2/3] Update comment, remove requirement for float conversion --- sonic_platform_base/fan_base.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sonic_platform_base/fan_base.py b/sonic_platform_base/fan_base.py index 91052f506..1eaa28859 100644 --- a/sonic_platform_base/fan_base.py +++ b/sonic_platform_base/fan_base.py @@ -70,6 +70,10 @@ 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 + 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 """ @@ -79,16 +83,20 @@ def is_under_speed(self): for param, value in (('speed', speed), ('target speed', target_speed), ('speed tolerance', tolerance)): if not isinstance(value, int): - raise TypeError('Fan {0} is not an integer value: {0}={1}'.format(param, value)) + raise TypeError(f'Fan {param} is not an integer value: {param}={value}') if value < 0 or value > 100: - raise ValueError('Fan {0} is not a valid percentage value: {0}={1}'.format(param, value)) + raise ValueError(f'Fan {param} is not a valid percentage value: {param}={value}') - return speed < target_speed * (1 - float(tolerance) / 100) + 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 + 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 """ @@ -98,11 +106,11 @@ def is_over_speed(self): for param, value in (('speed', speed), ('target speed', target_speed), ('speed tolerance', tolerance)): if not isinstance(value, int): - raise TypeError('Fan {0} is not an integer value: {0}={1}'.format(param, value)) + raise TypeError(f'Fan {param} is not an integer value: {param}={value}') if value < 0 or value > 100: - raise ValueError('Fan {0} is not a valid percentage value: {0}={1}'.format(param, value)) + raise ValueError(f'Fan {param} is not a valid percentage value: {param}={value}') - return speed > target_speed * (1 + float(tolerance) / 100) + return speed * 100 > target_speed * (100 + tolerance) def set_speed(self, speed): """ From fa986c54f0645c0d9827f994e10f81b3fe08a6f8 Mon Sep 17 00:00:00 2001 From: Scott Pilkey Date: Mon, 10 Jul 2023 13:32:51 -0700 Subject: [PATCH 3/3] Fan test coverage --- tests/fan_base_test.py | 158 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 tests/fan_base_test.py diff --git a/tests/fan_base_test.py b/tests/fan_base_test.py new file mode 100644 index 000000000..af3055e28 --- /dev/null +++ b/tests/fan_base_test.py @@ -0,0 +1,158 @@ +''' +Test FanBase module +''' + +from unittest import mock +from sonic_platform_base.fan_base import FanBase + +class TestFanBase: + ''' + Collection of FanBase test methods + ''' + + @staticmethod + def test_is_under_speed(): + ''' + Test fan.is_under_speed default implementation + ''' + fan = FanBase() + fan.get_speed = mock.MagicMock(return_value=100) + fan.get_target_speed = mock.MagicMock(return_value=50) + fan.get_speed_tolerance = mock.MagicMock(return_value=10) + + for func in (fan.get_speed, fan.get_target_speed, fan.get_speed_tolerance): + return_val = func() + + # Check type and bounds errors + for value, exc_type in ((None, TypeError), (-1, ValueError), (101, ValueError)): + func.return_value = value + expected_exception = False + try: + fan.is_under_speed() + except Exception as exc: + expected_exception = isinstance(exc, exc_type) + assert expected_exception + + # Reset function return value + func.return_value = return_val + + # speed=100, minimum tolerated speed=45, not under speed + assert not fan.is_under_speed() + + # speed=46, minimum tolerated speed=45, not under speed + fan.get_speed.return_value = 46 + assert not fan.is_under_speed() + + # speed=45, minimum tolerated speed=45, not under speed + fan.get_speed.return_value = 45 + assert not fan.is_under_speed() + + # speed=44, minimum tolerated speed=45, under speed + fan.get_speed.return_value = 44 + assert fan.is_under_speed() + + # speed=44, minimum tolerated speed=40, not under speed + fan.get_speed_tolerance.return_value = 20 + assert not fan.is_under_speed() + + # speed=41, minimum tolerated speed=40, not under speed + fan.get_speed.return_value = 41 + assert not fan.is_under_speed() + + # speed=40, minimum tolerated speed=40, not under speed + fan.get_speed.return_value = 40 + assert not fan.is_under_speed() + + # speed=39, minimum tolerated speed=40, under speed + fan.get_speed.return_value = 39 + assert fan.is_under_speed() + + # speed=1, minimum tolerated speed=40, under speed + fan.get_speed.return_value = 1 + assert fan.is_under_speed() + + @staticmethod + def test_is_over_speed(): + ''' + test fan.is_over_speed default implementation + ''' + fan = FanBase() + fan.get_speed = mock.MagicMock(return_value=1) + fan.get_target_speed = mock.MagicMock(return_value=50) + fan.get_speed_tolerance = mock.MagicMock(return_value=10) + + for func in (fan.get_speed, fan.get_target_speed, fan.get_speed_tolerance): + return_val = func() + + # Check type and bounds errors + for value, exc_type in ((None, TypeError), (-1, ValueError), (101, ValueError)): + func.return_value = value + expected_exception = False + try: + fan.is_under_speed() + except Exception as exc: + expected_exception = isinstance(exc, exc_type) + assert expected_exception + + # Reset function return value + func.return_value = return_val + + # speed=1, maximum tolerated speed=55, not over speed + assert not fan.is_over_speed() + + # speed=54, maximum tolerated speed=55, not over speed + fan.get_speed.return_value = 54 + assert not fan.is_over_speed() + + # speed=55, maximum tolerated speed=55, not over speed + fan.get_speed.return_value = 55 + assert not fan.is_over_speed() + + # speed=56, maximum tolerated speed=55, over speed + fan.get_speed.return_value = 56 + assert fan.is_over_speed() + + # speed=56, maximum tolerated speed=60, not over speed + fan.get_speed_tolerance.return_value = 20 + assert not fan.is_over_speed() + + # speed=59, maximum tolerated speed=60, not over speed + fan.get_speed.return_value = 59 + assert not fan.is_over_speed() + + # speed=60, maximum tolerated speed=60, not over speed + fan.get_speed.return_value = 60 + assert not fan.is_over_speed() + + # speed=61, maximum tolerated speed=60, over speed + fan.get_speed.return_value = 61 + assert fan.is_over_speed() + + # speed=100, maximum tolerated speed=60, over speed + fan.get_speed.return_value = 100 + assert fan.is_over_speed() + + @staticmethod + def test_fan_base(): + ''' + Verify unimplemented methods + ''' + fan = FanBase() + not_implemented_methods = [ + (fan.get_direction,), + (fan.get_speed,), + (fan.get_target_speed,), + (fan.get_speed_tolerance,), + (fan.set_speed, 50), + (fan.set_status_led, 'green'), + (fan.get_status_led,)] + + for method in not_implemented_methods: + expected_exception = False + try: + func = method[0] + args = method[1:] + func(*args) + except Exception as exc: + expected_exception = isinstance(exc, NotImplementedError) + assert expected_exception