[pddf]: Modifying the PDDF common platform APIs as per the LED driver changes (#13474)

Why I did it
LED driver changed due to introduction of FPGA support. The PDDF parser and APIs need to be updated. In turn the common platform APIs also require changes.

How I did it
Changed the get/set status LED APIs for PSU, fan and fan_drawer.
Changed the color strings to plain color name. e.g. 'STATUS_LED_COLOR_GREEN' has been changed to 'green'
Added support for LED color get operation via BMC
How to verify it
Verified the new changes on Accton AS7816-64X platform.

root@sonic:/home/admin#
root@sonic:/home/admin# show platform summary
Platform: x86_64-accton_as7816_64x-r0
HwSKU: Accton-AS7816-64X
ASIC: broadcom
ASIC Count: 1
Serial Number: AAA1903AAEV
Model Number: FP3AT7664000A
Hardware Revision: N/A
root@sonic:/home/admin#
root@sonic:/home/admin# show ver |more

SONiC Software Version: SONiC.master.0-dirty-20230111.010655
Distribution: Debian 11.6
Kernel: 5.10.0-18-2-amd64
Build commit: 3176b15ae
Build date: Wed Jan 11 09:12:54 UTC 2023
Built by: fk410167@sonic-lvn-csg-006

Platform: x86_64-accton_as7816_64x-r0
HwSKU: Accton-AS7816-64X
ASIC: broadcom
ASIC Count: 1
Serial Number: AAA1903AAEV
Model Number: FP3AT7664000A
Hardware Revision: N/A
Uptime: 09:24:42 up 4 days, 22:45,  1 user,  load average: 1.97, 1.80, 1.51
Date: Mon 23 Jan 2023 09:24:42

Docker images:
REPOSITORY                    TAG                              IMAGE ID       SI
ZE
docker-orchagent              latest                           63262c7468d7   38
5MB
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin# pddf_ledutil getstatusled LOC_LED
off
root@sonic:/home/admin# pddf_ledutil getstatusled DIAG_LED
green
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin# pddf_ledutil setstatusled DIAG_LED red
True
root@sonic:/home/admin# pddf_ledutil getstatusled DIAG_LED
red
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin# pddf_ledutil setstatusled DIAG_LED amber
Invalid color
False
root@sonic:/home/admin# pddf_ledutil getstatusled DIAG_LED
red
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin# pddf_ledutil setstatusled DIAG_LED green
True
root@sonic:/home/admin# pddf_ledutil getstatusled DIAG_LED
green
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin#
root@sonic:/home/admin# pddf_ledutil getstatusled LOC_LED
off
root@sonic:/home/admin# pddf_ledutil setstatusled LOC_LED amber
True
root@sonic:/home/admin# pddf_ledutil getstatusled LOC_LED
amber
root@sonic:/home/admin# pddf_ledutil setstatusled LOC_LED off
True
root@sonic:/home/admin# pddf_ledutil getstatusled LOC_LED
off
root@sonic:/home/admin#

Author: FuzailBrcm

platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_chassis.py

@@ -193,32 +193,30 @@ def get_reboot_cause(self):
     ##############################################
     # System LED  methods
     ##############################################
+    # APIs used by PDDF. Use them for debugging front panel
+    # system LED and fantray LED issues
     def set_system_led(self, led_device_name, color):
-        result, msg = self.pddf_obj.is_supported_sysled_state(led_device_name, color)
-        if result == False:
+        """
+        Sets the color of an LED device in PDDF
+        Args:
+           led_device_name: a pre-defined LED device name list used in pddf-device.json.
+           color: A string representing the color with which to set a LED
+        Returns:
+           bool: True if the LED state is set successfully, False if not
+        """
+        result, msg = self.pddf_obj.set_system_led_color(led_device_name, color)
+        if not result and msg:
             print(msg)
-            return (False)
-
-        index = self.pddf_obj.data[led_device_name]['dev_attr']['index']
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('color', color, self.pddf_obj.get_led_cur_state_path())
-        self.pddf_obj.create_attr('dev_ops', 'set_status',  self.pddf_obj.get_led_path())
-        return (True)
+        return (result)
 
     def get_system_led(self, led_device_name):
-        if led_device_name not in self.pddf_obj.data.keys():
-            status = "[FAILED] " + led_device_name + " is not configured"
-            return (status)
-
-        index = self.pddf_obj.data[led_device_name]['dev_attr']['index']
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('dev_ops', 'get_status',  self.pddf_obj.get_led_path())
-        color = self.pddf_obj.get_led_color()
-        return (color)
+        """
+        Gets the color of an LED device in PDDF
+        Returns:
+            string: color of LED or message if failed.
+        """
+        result, output = self.pddf_obj.get_system_led_color(led_device_name)
+        return (output)
 
     ##############################################
     # Other methods

platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py

@@ -297,38 +297,31 @@ def set_speed(self, speed):
                 return status
 
     def set_status_led(self, color):
-        index = str(self.fantray_index-1)
-        led_device_name = "FANTRAY{}".format(self.fantray_index) + "_LED"
-
-        result, msg = self.pddf_obj.is_supported_sysled_state(led_device_name, color)
-        if result == False:
-            print(msg)
-            return (False)
-
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('color', color, self.pddf_obj.get_led_cur_state_path())
-        self.pddf_obj.create_attr('dev_ops', 'set_status',  self.pddf_obj.get_led_path())
-        return (True)
+        result = False
+        if self.is_psu_fan:
+            # Usually no led for psu_fan hence raise a NotImplementedError
+            raise NotImplementedError
+        else:
+            # Usually there is no led for psu_fan
+            led_device_name = "FANTRAY{}".format(self.fantray_index) + "_LED"
+            result, msg = self.pddf_obj.set_system_led_color(led_device_name, color)
+        return (result)
 
     def get_status_led(self):
-        index = str(self.fantray_index-1)
-        fan_led_device = "FANTRAY{}".format(self.fantray_index) + "_LED"
-
-        if fan_led_device not in self.pddf_obj.data.keys():
-            # Implement a generic status_led color scheme
-            if self.get_status():
-                return self.STATUS_LED_COLOR_GREEN
-            else:
-                return self.STATUS_LED_COLOR_OFF
-
-        device_name = self.pddf_obj.data[fan_led_device]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('dev_ops', 'get_status',  self.pddf_obj.get_led_path())
-        color = self.pddf_obj.get_led_color()
-        return (color)
+        if self.is_psu_fan:
+            # Usually no led for psu_fan hence raise a NotImplementedError
+            raise NotImplementedError
+        else:
+            fan_led_device = "FANTRAY{}".format(self.fantray_index) + "_LED"
+            if (not fan_led_device in self.pddf_obj.data.keys()):
+                # Implement a generic status_led color scheme
+                if self.get_status():
+                    return self.STATUS_LED_COLOR_GREEN
+                else:
+                    return self.STATUS_LED_COLOR_OFF
+
+            result, color = self.pddf_obj.get_system_led_color(fan_led_device)
+            return (color)
 
     def get_position_in_parent(self):
         """

platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan_drawer.py

@@ -78,33 +78,19 @@ def get_position_in_parent(self):
         return self.fantray_index
 
     def get_status_led(self):
-        led_device_name = "FANTRAY{}".format(self.fantray_index) + "_LED"
-
-        if led_device_name not in self.pddf_obj.data.keys():
+        fan_led_device = "FANTRAY{}".format(self.fantray_index) + "_LED"
+        if (not fan_led_device in self.pddf_obj.data.keys()):
             # Implement a generic status_led color scheme
             if self.get_status():
                 return self.STATUS_LED_COLOR_GREEN
             else:
                 return self.STATUS_LED_COLOR_OFF
 
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', str(self.fantray_index-1), self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('dev_ops', 'get_status',  self.pddf_obj.get_led_path())
-        color = self.pddf_obj.get_led_color()
+        result, color = self.pddf_obj.get_system_led_color(fan_led_device)
         return (color)
 
     def set_status_led(self, color):
         result = False
         led_device_name = "FANTRAY{}".format(self.fantray_index) + "_LED"
-        result, msg = self.pddf_obj.is_supported_sysled_state(led_device_name, color)
-        if result == False:
-            print(msg)
-            return (False)
-
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', str(self.fantray_index-1), self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('color', color, self.pddf_obj.get_led_cur_state_path())
-        self.pddf_obj.create_attr('dev_ops', 'set_status',  self.pddf_obj.get_led_path())
-        return (True)
+        result, msg = self.pddf_obj.set_system_led_color(led_device_name, color)
+        return (result)

platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_psu.py

@@ -250,23 +250,17 @@ def get_powergood_status(self):
         return self.get_status()
 
     def set_status_led(self, color):
-        index = str(self.psu_index-1)
+        if 'psu_led_color' in self.plugin_data['PSU']:
+            led_color_map = self.plugin_data['PSU']['psu_led_color']['colmap']
+            if color in led_color_map:
+                # change the color properly
+                new_color = led_color_map[color]
+                color = new_color
         led_device_name = "PSU{}".format(self.psu_index) + "_LED"
-
-        result, msg = self.pddf_obj.is_supported_sysled_state(led_device_name, color)
-        if result == False:
-            print(msg)
-            return (False)
-
-        device_name = self.pddf_obj.data[led_device_name]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('color', color, self.pddf_obj.get_led_cur_state_path())
-        self.pddf_obj.create_attr('dev_ops', 'set_status',  self.pddf_obj.get_led_path())
-        return (True)
+        result, msg = self.pddf_obj.set_system_led_color(led_device_name, color)
+        return (result)
 
     def get_status_led(self):
-        index = str(self.psu_index-1)
         psu_led_device = "PSU{}_LED".format(self.psu_index)
         if psu_led_device not in self.pddf_obj.data.keys():
             # Implement a generic status_led color scheme
@@ -275,11 +269,7 @@ def get_status_led(self):
             else:
                 return self.STATUS_LED_COLOR_OFF
 
-        device_name = self.pddf_obj.data[psu_led_device]['dev_info']['device_name']
-        self.pddf_obj.create_attr('device_name', device_name,  self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('index', index, self.pddf_obj.get_led_path())
-        self.pddf_obj.create_attr('dev_ops', 'get_status',  self.pddf_obj.get_led_path())
-        color = self.pddf_obj.get_led_color()
+        result, color = self.pddf_obj.get_system_led_color(psu_led_device)
         return (color)
 
     def get_temperature(self):

platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddfapi.py

@@ -16,19 +16,6 @@
 
 dirname = os.path.dirname(os.path.realpath(__file__))
 
-color_map = {
-    "STATUS_LED_COLOR_GREEN": "green",
-    "STATUS_LED_COLOR_RED": "red",
-    "STATUS_LED_COLOR_AMBER": "amber",
-    "STATUS_LED_COLOR_BLUE": "blue",
-    "STATUS_LED_COLOR_GREEN_BLINK": "blinking green",
-    "STATUS_LED_COLOR_RED_BLINK": "blinking red",
-    "STATUS_LED_COLOR_AMBER_BLINK": "blinking amber",
-    "STATUS_LED_COLOR_BLUE_BLINK": "blinking blue",
-    "STATUS_LED_COLOR_OFF": "off"
-}
-
-
 class PddfApi():
     def __init__(self):
         if not os.path.exists("/usr/share/sonic/platform"):
@@ -123,10 +110,12 @@ def get_led_color(self):
                 color = f.read().strip("\r\n")
         except IOError:
             return ("Error")
-
-        return (color_map[color])
+        return color
 
     def get_led_color_devtype(self, key):
+        if 'bmc' in self.data[key]:
+            return 'bmc'
+
         attr_list = self.data[key]['i2c']['attr_list']
         for attr in attr_list:
             if 'attr_devtype' in attr:
@@ -163,8 +152,8 @@ def get_led_color_from_gpio(self, led_device_name):
 
         for attr in attr_list:
             if int(attr['value'].strip(), 16) == value:
-                return(color_map[attr['attr_name']])
-        return (color_map['STATUS_LED_COLOR_OFF'])
+                return (attr['attr_name'])
+        return ("off")
 
     def get_led_color_from_cpld(self, led_device_name):
         index = self.data[led_device_name]['dev_attr']['index']
@@ -174,6 +163,12 @@ def get_led_color_from_cpld(self, led_device_name):
         self.create_attr('dev_ops', 'get_status',  self.get_led_path())
         return self.get_led_color()
 
+    def get_led_color_from_bmc(self, led_device_name):
+        for bmc_attr in self.data[led_device_name]['bmc']['ipmitool']['attr_list']:
+            if (self.bmc_get_cmd(bmc_attr) == str(int(bmc_attr['value'], 16))):
+                return (bmc_attr['attr_name'])
+        return ("off")
+
     def set_led_color_from_gpio(self, led_device_name, color):
         attr_list = self.data[led_device_name]['i2c']['attr_list']
         for attr in attr_list:
@@ -196,9 +191,9 @@ def set_led_color_from_gpio(self, led_device_name, color):
                         cmd = "echo {} > {}".format(_value, attr_path)
                         self.runcmd(cmd)
                     except Exception as e:
-                        print("Invalid gpio path : " + attr_path)
-                        return (False)
-        return (True)
+                        msg = "Invalid gpio path : " + attr_path
+                        return (False, msg)
+        return (True, "Success")
 
     def set_led_color_from_cpld(self, led_device_name, color):
         index = self.data[led_device_name]['dev_attr']['index']
@@ -207,26 +202,42 @@ def set_led_color_from_cpld(self, led_device_name, color):
         self.create_attr('index', index, self.get_led_path())
         self.create_attr('color', color, self.get_led_cur_state_path())
         self.create_attr('dev_ops', 'set_status',  self.get_led_path())
-        return (True)
+        return (True, "Success")
 
     def get_system_led_color(self, led_device_name):
         if led_device_name not in self.data.keys():
-            status = "[FAILED] " + led_device_name + " is not configured"
-            return (status)
+            msg = led_device_name + " is not configured"
+            return (False, msg)
 
         dtype = self.get_led_color_devtype(led_device_name)
 
         if dtype == 'gpio':
             color = self.get_led_color_from_gpio(led_device_name)
-        elif dtype == 'cpld':
+        elif dtype == 'bmc':
+            color = self.get_led_color_from_bmc(led_device_name)
+        else:
+            # This case takes care of CPLD as well as I2CFPGA
             color = self.get_led_color_from_cpld(led_device_name)
-        return color
+
+        return (True, color)
 
     def set_system_led_color(self, led_device_name, color):
-        result, msg = self.is_supported_sysled_state(led_device_name, color)
-        if result == False:
-            print(msg)
-            return (result)
+        # Check if the device is configured
+        if led_device_name not in self.data.keys():
+            msg = led_device_name + " is not configured"
+            return (False, msg)
+
+        # Check for the write permission
+        if 'flag' in self.data[led_device_name]['dev_attr']:
+            if self.data[led_device_name]['dev_attr']['flag'] == 'ro':
+                return (False, "Set LED operation not supported or handled separately")
+
+        found = False
+        for attr in self.data[led_device_name]['i2c']['attr_list']:
+            if attr['attr_name'] == color:
+                found = True
+        if not found:
+            return (False, "Invalid color")
 
         dtype = self.get_led_color_devtype(led_device_name)
 
@@ -391,7 +402,6 @@ def show_attr_fan_device(self, dev, ops):
                     ret.append(dsysfs_path)
         return ret
 
-    # This is only valid for LM75
     def show_attr_temp_sensor_device(self, dev, ops):
         ret = []
         if 'i2c' not in dev.keys():
@@ -412,7 +422,7 @@ def show_attr_temp_sensor_device(self, dev, ops):
                     real_name = attr['attr_name']
 
                 if 'topo_info' in dev['i2c']:
-                    path = self.show_device_sysfs(dev, ops)+"/%d-00%x/" % (int(dev['i2c']['topo_info']['parent_bus'], 0),
+                    path = self.show_device_sysfs(dev, ops)+"/%d-00%x/"%(int(dev['i2c']['topo_info']['parent_bus'], 0),
                             int(dev['i2c']['topo_info']['dev_addr'], 0))
                     if (os.path.exists(path)):
                         full_path = glob.glob(path + 'hwmon/hwmon*/' + real_name)[0]
@@ -810,14 +820,6 @@ def dev_parse(self, dev, ops):
         if attr['device_type'] == 'SYSSTAT':
             return self.sysstatus_parse(dev, ops)
 
-    def is_supported_sysled_state(self, sysled_name, sysled_state):
-        if sysled_name not in self.data.keys():
-            return False, "[FAILED] " + sysled_name + " is not configured"
-        for attr in self.data[sysled_name]['i2c']['attr_list']:
-            if attr['attr_name'] == sysled_state:
-                return True, "supported"
-        return False,  "[FAILED]: Invalid color"
-
     def create_attr(self, key, value, path):
         cmd = "echo '%s' > /sys/kernel/%s/%s" % (value,  path, key)
         self.runcmd(cmd)