From c8220d1755cd7aa4b100634ca4f420c45ba84511 Mon Sep 17 00:00:00 2001
From: junchao <junchao@nvidia.com>
Date: Thu, 10 Mar 2022 14:16:44 +0800
Subject: [PATCH] Fix issue: psu might use wrong voltage sysfs which causes
 invalid voltage value

---
 .../mlnx-platform-api/sonic_platform/psu.py   | 57 +++++++++++++++----
 .../mlnx-platform-api/tests/test_psu.py       |  6 ++
 2 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/platform/mellanox/mlnx-platform-api/sonic_platform/psu.py b/platform/mellanox/mlnx-platform-api/sonic_platform/psu.py
index 160fd8bd7469..64aa0166086f 100644
--- a/platform/mellanox/mlnx-platform-api/sonic_platform/psu.py
+++ b/platform/mellanox/mlnx-platform-api/sonic_platform/psu.py
@@ -200,16 +200,10 @@ class Psu(FixedPsu):
     def __init__(self, psu_index):
         super(Psu, self).__init__(psu_index)
 
-        psu_voltage_out2 = os.path.join(PSU_PATH, "power/psu{}_volt_out2".format(self.index))
-        psu_voltage = os.path.join(PSU_PATH, "power/psu{}_volt".format(self.index))
-        # Workaround for psu voltage sysfs file as the file name differs among platforms
-        if os.path.exists(psu_voltage_out2):
-            self.psu_voltage = psu_voltage_out2
-        else:
-            self.psu_voltage = psu_voltage
-        self.psu_voltage_min = self.psu_voltage + "_min"
-        self.psu_voltage_max = self.psu_voltage + "_max"
-        self.psu_voltage_capability = self.psu_voltage + "_capability"
+        self._psu_voltage = None
+        self._psu_voltage_min = None
+        self._psu_voltage_max = None
+        self._psu_voltage_capability = None
 
         self.psu_current = os.path.join(PSU_PATH, self.PSU_CURRENT.format(self.index))
         self.psu_power = os.path.join(PSU_PATH, self.PSU_POWER.format(self.index))
@@ -228,6 +222,47 @@ def __init__(self, psu_index):
         from .thermal import initialize_psu_thermal
         self._thermal_list = initialize_psu_thermal(psu_index, self.get_power_available_status)
 
+    @property
+    def psu_voltage(self):
+        if not self._psu_voltage:
+            psu_voltage_out = os.path.join(PSU_PATH, "power/psu{}_volt_out2".format(self.index))
+            if os.path.exists(psu_voltage_out):
+                self._psu_voltage = psu_voltage_out
+            else:
+                psu_voltage_out = os.path.join(PSU_PATH, "power/psu{}_volt".format(self.index))
+                if os.path.exists(psu_voltage_out):
+                    self._psu_voltage = psu_voltage_out
+        
+        return self._psu_voltage
+
+    @property
+    def psu_voltage_min(self):
+        if not self._psu_voltage_min:
+            psu_voltage = self.psu_voltage
+            if psu_voltage:
+                self._psu_voltage_min = psu_voltage + "_min"
+
+        return self._psu_voltage_min
+
+    @property
+    def psu_voltage_max(self):
+        if not self._psu_voltage_max:
+            psu_voltage = self.psu_voltage
+            if psu_voltage:
+                self._psu_voltage_max = psu_voltage + "_max"
+
+        return self._psu_voltage_max
+
+    @property
+    def psu_voltage_capability(self):
+        if not self._psu_voltage_capability:
+            psu_voltage = self.psu_voltage
+            if psu_voltage:
+                self._psu_voltage_capability = psu_voltage + "_capability"
+
+        return self._psu_voltage_capability
+
+
     def get_model(self):
         """
         Retrieves the model number (or part number) of the device
@@ -272,7 +307,7 @@ def get_voltage(self):
             A float number, the output voltage in volts, 
             e.g. 12.1 
         """
-        if self.get_powergood_status():
+        if self.get_powergood_status() and self.psu_voltage:
             # TODO: should we put log_func=None here? If not do this, when a PSU is back to power, some PSU related
             # sysfs may not ready, read_int_from_file would encounter exception and log an error.
             voltage = utils.read_int_from_file(self.psu_voltage, log_func=logger.log_info)
diff --git a/platform/mellanox/mlnx-platform-api/tests/test_psu.py b/platform/mellanox/mlnx-platform-api/tests/test_psu.py
index 6de042e7bd8b..34fa70c7beca 100644
--- a/platform/mellanox/mlnx-platform-api/tests/test_psu.py
+++ b/platform/mellanox/mlnx-platform-api/tests/test_psu.py
@@ -50,6 +50,7 @@ def test_fixed_psu(self):
         assert psu.get_temperature() is None
         assert psu.get_temperature_high_threshold() is None
 
+    @mock.patch('os.path.exists', mock.MagicMock(return_value=True))
     def test_psu(self):
         psu = Psu(0)
         assert len(psu._fan_list) == 1
@@ -58,6 +59,8 @@ def test_psu(self):
             psu.psu_presence: 1,
             psu.psu_oper_status: 1,
             psu.psu_voltage: 10234,
+            psu.psu_voltage_min: 9000,
+            psu.psu_voltage_max: 12000,
             psu.psu_current: 20345,
             psu.psu_power: 30456,
             psu.psu_temp: 40567,
@@ -68,6 +71,7 @@ def mock_read_int_from_file(file_path, **kwargs):
             return mock_sysfs_content[file_path]
 
         utils.read_int_from_file = mock_read_int_from_file
+        utils.read_str_from_file = mock.MagicMock(return_value='min max')
         assert psu.get_presence() is True
         mock_sysfs_content[psu.psu_presence] = 0
         assert psu.get_presence() is False
@@ -84,6 +88,8 @@ def mock_read_int_from_file(file_path, **kwargs):
 
         mock_sysfs_content[psu.psu_oper_status] = 1
         assert psu.get_voltage() == 10.234
+        assert psu.get_voltage_high_threshold() == 12.0
+        assert psu.get_voltage_low_threshold() == 9.0
         assert psu.get_current() == 20.345
         assert psu.get_power() == 0.030456
         assert psu.get_temperature() == 40.567