Reduced cyclomatic complexity - Each function now does one thing

Better readability - Descriptive function names explain intent
Easier testing - Helper functions can be tested independently
Consistent style - f-strings used throughout
Removed code smells - No more pass statements in conditionals, no assert for validation

glances/plugins/smart/__init__.py

@@ -66,6 +66,16 @@ def convert_attribute_to_dict(attr):
     }
 
 
+# Keys for attributes that should be formatted with auto_unit (large byte values)
+LARGE_VALUE_KEYS = frozenset([
+    "bytesWritten",
+    "bytesRead",
+    "dataUnitsRead",
+    "dataUnitsWritten",
+    "hostReadCommands",
+    "hostWriteCommands",
+])
+
 NVME_ATTRIBUTE_LABELS = {
     "criticalWarning": "Number of critical warnings",
     "_temperature": "Temperature (°C)",
@@ -110,81 +120,64 @@ def convert_nvme_attribute_to_dict(key, value):
     }
 
 
+def _process_standard_attributes(device_stats, attributes, hide_attributes):
+    """Process standard SMART attributes and add them to device_stats."""
+    for attribute in attributes:
+        if attribute is None or attribute.name in hide_attributes:
+            continue
+
+        attrib_dict = convert_attribute_to_dict(attribute)
+        num = attrib_dict.pop('num', None)
+        if num is None:
+            logger.debug(f'Smart plugin error - Skip attribute with no num: {attribute}')
+            continue
+
+        device_stats[num] = attrib_dict
+
+
+def _process_nvme_attributes(device_stats, if_attributes, hide_attributes):
+    """Process NVMe-specific attributes and add them to device_stats."""
+    if not isinstance(if_attributes, NvmeAttributes):
+        return
+
+    for idx, (attr, value) in enumerate(vars(if_attributes).items(), start=1):
+        attrib_dict = convert_nvme_attribute_to_dict(attr, value)
+        if attrib_dict['name'] in hide_attributes:
+            continue
+
+        # Verify the value is serializable to prevent rendering errors
+        if value is not None:
+            try:
+                str(value)
+            except Exception:
+                logger.debug(f'Unable to serialize attribute {attr} from NVME')
+                attrib_dict['value'] = None
+                attrib_dict['raw'] = None
+
+        device_stats[idx] = attrib_dict
+
+
 def get_smart_data(hide_attributes):
-    """
+    """Get SMART attribute data.
-    Get SMART attribute data
+
-    :return: list of multi leveled dictionaries
+    Returns a list of dictionaries, each containing:
-             each dict has a key "DeviceName" with the identification of the device in smartctl
+    - 'DeviceName': Device identification string
-             also has keys of the SMART attribute id, with value of another dict of the attributes
+    - Numeric keys: SMART attribute dictionaries with flags, raw values, etc.
-             [
-                {
-                    "DeviceName": "/dev/sda blahblah",
-                    "1":
-                    {
-                        "flags": "..",
-                        "raw": "..",
-                        etc,
-                    }
-                    ...
-                }
-             ]
     """
     stats = []
-    # get all devices
     try:
         devlist = DeviceList()
     except TypeError as e:
-        # Catch error  (see #1806)
         logger.debug(f'Smart plugin error - Can not grab device list ({e})')
         global import_error_tag
         import_error_tag = True
         return stats
 
     for dev in devlist.devices:
-        stats.append(
+        device_stats = {'DeviceName': f'{dev.name} {dev.model}'}
-            {
+        _process_standard_attributes(device_stats, dev.attributes, hide_attributes)
-                'DeviceName': f'{dev.name} {dev.model}',
+        _process_nvme_attributes(device_stats, dev.if_attributes, hide_attributes)
-            }
+        stats.append(device_stats)
-        )
-        for attribute in dev.attributes:
-            if attribute is None:
-                pass
-            elif attribute.name in hide_attributes:
-                pass
-            else:
-                attrib_dict = convert_attribute_to_dict(attribute)
-
-                # we will use the attribute number as the key
-                num = attrib_dict.pop('num', None)
-                try:
-                    assert num is not None
-                except Exception as e:
-                    # we should never get here, but if we do, continue to next iteration and skip this attribute
-                    logger.debug(f'Smart plugin error - Skip the attribute {attribute} ({e})')
-                    continue
-
-                stats[-1][num] = attrib_dict
-
-        if isinstance(dev.if_attributes, NvmeAttributes):
-            idx = 0
-            for attr in dev.if_attributes.__dict__.keys():
-                idx += 1
-
-                attrib_dict = convert_nvme_attribute_to_dict(attr, dev.if_attributes.__dict__[attr])
-                if attrib_dict['name'] in hide_attributes:
-                    pass
-                else:
-                    try:
-                        if (
-                            dev.if_attributes.__dict__[attr] is not None
-                        ):  # make sure the value is serializable to prevent errors in rendering
-                            str(dev.if_attributes.__dict__[attr])
-                    except Exception:
-                        logger.debug(f'Unable to serialize attribute {attr} from NVME')
-                        attrib_dict['value'] = None
-                        attrib_dict['raw'] = None
-                    finally:
-                        stats[-1][idx] = attrib_dict
 
     return stats
 
@@ -194,25 +187,23 @@ class SmartPlugin(GlancesPluginModel):
 
     def __init__(self, args=None, config=None, stats_init_value=[]):
         """Init the plugin."""
-        # check if user is admin
         if not is_admin() and args:
             disable(args, "smart")
             logger.debug("Current user is not admin, HDD SMART plugin disabled.")
 
         super().__init__(args=args, config=config)
 
-        # We want to display the stat in the curse interface
         self.display_curse = True
+        self.hide_attributes = self._parse_hide_attributes(config)
 
-        if 'hide_attributes' in config.as_dict()['smart']:
+    def _parse_hide_attributes(self, config):
-            logger.info(
+        """Parse and return the list of attributes to hide from config."""
-                'Followings SMART attributes wil not be displayed: {}'.format(
+        smart_config = config.as_dict().get('smart', {})
-                    config.as_dict()['smart']['hide_attributes']
+        hide_attr_str = smart_config.get('hide_attributes', '')
-                )
+        if hide_attr_str:
-            )
+            logger.info(f'Following SMART attributes will not be displayed: {hide_attr_str}')
-            self.hide_attributes = config.as_dict()['smart']['hide_attributes'].split(',')
+            return hide_attr_str.split(',')
-        else:
+        return []
-            self.hide_attributes = []
 
     @property
     def hide_attributes(self):
@@ -249,59 +240,63 @@ class SmartPlugin(GlancesPluginModel):
         """Return the key of the list."""
         return 'DeviceName'
 
+    def _format_raw_value(self, stat):
+        """Format a raw SMART attribute value for display."""
+        raw = stat['raw']
+        if raw is None:
+            return ""
+        if stat['key'] in LARGE_VALUE_KEYS:
+            return self.auto_unit(raw)
+        return str(raw)
+
+    def _get_sorted_stat_keys(self, device_stat):
+        """Get sorted attribute keys from device stats, excluding DeviceName."""
+        keys = [k for k in device_stat if k != 'DeviceName']
+        try:
+            return sorted(keys, key=int)
+        except ValueError:
+            # Some keys may not be numeric (see #2904)
+            return keys
+
+    def _add_device_stats(self, ret, device_stat, max_width, name_max_width):
+        """Add a device's SMART stats to the curse output."""
+        ret.append(self.curse_new_line())
+        ret.append(self.curse_add_line(f'{device_stat["DeviceName"][:max_width]:{max_width}}'))
+
+        for key in self._get_sorted_stat_keys(device_stat):
+            stat = device_stat[key]
+            ret.append(self.curse_new_line())
+
+            # Attribute name
+            name = stat['name'][: name_max_width - 1].replace('_', ' ')
+            ret.append(self.curse_add_line(f' {name:{name_max_width - 1}}'))
+
+            # Attribute value
+            try:
+                value_str = self._format_raw_value(stat)
+                ret.append(self.curse_add_line(f'{value_str:>8}'))
+            except Exception:
+                logger.debug(f"Failed to serialize {key}")
+                ret.append(self.curse_add_line(""))
+
     def msg_curse(self, args=None, max_width=None):
         """Return the dict to display in the curse interface."""
-        # Init the return message
         ret = []
 
-        # Only process if stats exist...
         if import_error_tag or not self.stats or self.is_disabled():
             return ret
 
-        # Max size for the interface name
+        if not max_width:
-        if max_width:
-            name_max_width = max_width - 6
-        else:
-            # No max_width defined, return an empty curse message
             logger.debug(f"No max_width defined for the {self.plugin_name} plugin, it will not be displayed.")
             return ret
 
+        name_max_width = max_width - 6
+
         # Header
-        msg = '{:{width}}'.format('SMART disks', width=name_max_width)
+        ret.append(self.curse_add_line(f'{"SMART disks":{name_max_width}}', "TITLE"))
-        ret.append(self.curse_add_line(msg, "TITLE"))
+
-        # Data
+        # Device data
         for device_stat in self.stats:
-            # New line
+            self._add_device_stats(ret, device_stat, max_width, name_max_width)
-            ret.append(self.curse_new_line())
-            msg = '{:{width}}'.format(device_stat['DeviceName'][:max_width], width=max_width)
-            ret.append(self.curse_add_line(msg))
-            try:
-                device_stat_sorted = sorted([i for i in device_stat if i != 'DeviceName'], key=int)
-            except ValueError:
-                # Catch ValueError, see #2904
-                device_stat_sorted = [i for i in device_stat if i != 'DeviceName']
-            for smart_stat in device_stat_sorted:
-                ret.append(self.curse_new_line())
-                msg = ' {:{width}}'.format(
-                    device_stat[smart_stat]['name'][: name_max_width - 1].replace('_', ' '), width=name_max_width - 1
-                )
-                ret.append(self.curse_add_line(msg))
-                try:
-                    raw = device_stat[smart_stat]['raw']
-                    if device_stat[smart_stat]['key'] in [
-                        "bytesWritten",
-                        "bytesRead",
-                        "dataUnitsRead",
-                        "dataUnitsWritten",
-                        "hostReadCommands",
-                        "hostWriteCommands",
-                    ]:
-                        msg = '{:>8}'.format("" if raw is None else self.auto_unit(raw))
-                    else:
-                        msg = '{:>8}'.format("" if raw is None else str(raw))
-                    ret.append(self.curse_add_line(msg))
-                except Exception:
-                    logger.debug(f"Failed to serialize {smart_stat}")
-                    ret.append(self.curse_add_line(""))
 
         return ret