Improve hdparm error handling (should fix #6)

hddfancontrol/__init__.py

@@ -63,6 +63,7 @@ class Drive:
   DriveState = enum.Enum("DriveState", ("UNKNOWN", "ACTIVE_IDLE", "STANDBY", "SLEEPING"))
 
   HDPARM_GET_TEMP_HITACHI_REGEX = re.compile("drive temperature \(celsius\) is:\s*([0-9]*)")
+  HDPARM_GET_TEMP_HITACHI_ERROR_REGEX = re.compile("^SG_IO: .* sense data", re.MULTILINE)
   HDPARM_GET_MODEL_REGEX = re.compile("Model Number:\s*(.*)")
 
   def __init__(self, device_filepath, hddtemp_daemon_port):
@@ -91,16 +92,23 @@ class Drive:
 
   def supportsHitachiTempQuery(self):
     # test if drive supports hdparm -H
+    supported = True
     cmd = ("hdparm", "-H", self.device_filepath)
     try:
-      subprocess.check_call(cmd,
-                            stdin=subprocess.DEVNULL,
-                            stdout=subprocess.DEVNULL,
-                            stderr=subprocess.DEVNULL)
+      output = subprocess.check_output(cmd,
+                                       stdin=subprocess.DEVNULL,
+                                       stderr=subprocess.DEVNULL,
+                                       universal_newlines=True)
     except subprocess.CalledProcessError:
+      supported = False
+    else:
+      # catch non fatal errors
+      # see: https://github.com/Distrotech/hdparm/blob/4517550db29a91420fb2b020349523b1b4512df2/sgio.c#L308-L315
+      if __class__.HDPARM_GET_TEMP_HITACHI_ERROR_REGEX.search(output) is not None:
+        supported = False
+    if not supported:
       self.logger.warning("Drive does not allow querying temperature without going out of low power mode.")
-      return False
-    return True
+    return supported
 
   def getState(self):
     """ Get drive power state, as a DriveState enum. """

tests/__init__.py

@@ -36,11 +36,11 @@ class TestDrive(unittest.TestCase):
   def setUp(self):
     with unittest.mock.patch("hddfancontrol.os.stat") as os_stat_mock, \
             unittest.mock.patch("hddfancontrol.stat") as stat_mock, \
-            unittest.mock.patch("hddfancontrol.subprocess") as subprocess_check_call_mock, \
+            unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock, \
             unittest.mock.patch("hddfancontrol.Drive.getPrettyName") as drive_getPrettyName:
       os_stat_mock.return_value = os.stat_result
       stat_mock.stat.S_IFBLK.return_value = True
-      subprocess_check_call_mock.side_effect = subprocess.CalledProcessError(0, "")
+      subprocess_check_output_mock.side_effect = subprocess.CalledProcessError(0, "")
       drive_getPrettyName.return_value = "drive_name"
       self.drive = hddfancontrol.Drive("/dev/sdz", None)
     self.hddtemp_daemon = None
@@ -60,6 +60,36 @@ class TestDrive(unittest.TestCase):
                                                            stderr=subprocess.DEVNULL,
                                                            universal_newlines=True)
 
+  def test_supportsHitachiTempQuery(self):
+    with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock:
+      subprocess_check_output_mock.return_value = "\n/dev/sdz:\n drive temperature (celsius) is:  30\n drive temperature in range:  yes"
+      self.assertEqual(self.drive.supportsHitachiTempQuery(), True)
+      subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"),
+                                                           stdin=subprocess.DEVNULL,
+                                                           stderr=subprocess.DEVNULL,
+                                                           universal_newlines=True)
+    with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock:
+      subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: questionable sense data, results may be incorrect\n drive temperature (celsius) is: -18\n drive temperature in range: yes"
+      self.assertEqual(self.drive.supportsHitachiTempQuery(), False)
+      subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"),
+                                                           stdin=subprocess.DEVNULL,
+                                                           stderr=subprocess.DEVNULL,
+                                                           universal_newlines=True)
+    with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock:
+      subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: missing sense data, results may be incorrect\n drive temperature (celsius) is: -18\n drive temperature in range: yes"
+      self.assertEqual(self.drive.supportsHitachiTempQuery(), False)
+      subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"),
+                                                           stdin=subprocess.DEVNULL,
+                                                           stderr=subprocess.DEVNULL,
+                                                           universal_newlines=True)
+    with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock:
+      subprocess_check_output_mock.return_value = "\n/dev/sdz:\nSG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 04 51 40 00 21 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00\n drive temperature (celsius) is: -18\n drive temperature in range: yes"
+      self.assertEqual(self.drive.supportsHitachiTempQuery(), False)
+      subprocess_check_output_mock.assert_called_once_with(("hdparm", "-H", "/dev/sdz"),
+                                                           stdin=subprocess.DEVNULL,
+                                                           stderr=subprocess.DEVNULL,
+                                                           universal_newlines=True)
+
   def test_getState(self):
     with unittest.mock.patch("hddfancontrol.subprocess.check_output") as subprocess_check_output_mock:
       subprocess_check_output_mock.return_value = "\n/dev/sdz:\n drive state is:  active/idle\n"