feat: Make device enumeration more resilient (#72)

Author: FrogTheFrog

src/windows/win_display_device_general.cpp

@@ -50,15 +50,15 @@ namespace display_device {
       const auto source_id_index { data.m_active_source.value_or(0) };
       const auto &best_path { display_data->m_paths.at(data.m_source_id_to_path_index.at(source_id_index)) };
       const auto friendly_name { m_w_api->getFriendlyName(best_path) };
+      const bool is_active { win_utils::isActive(best_path) };
+      const auto source_mode { is_active ? win_utils::getSourceMode(win_utils::getSourceIndex(best_path, display_data->m_modes), display_data->m_modes) : nullptr };
+      const auto display_name { is_active ? m_w_api->getDisplayName(best_path) : std::string {} };  // Inactive devices can have multiple display names, so it's just meaningless use any
 
-      if (win_utils::isActive(best_path)) {
-        const auto source_mode { win_utils::getSourceMode(win_utils::getSourceIndex(best_path, display_data->m_modes), display_data->m_modes) };
-        if (!source_mode) {
-          // Error already logged
-          return {};
-        }
+      if (is_active && !source_mode) {
+        DD_LOG(warning) << "Device " << device_id << " is missing source mode!";
+      }
 
-        const auto display_name { m_w_api->getDisplayName(best_path) };
+      if (source_mode) {
         const Rational refresh_rate { best_path.targetInfo.refreshRate.Denominator > 0 ?
                                         Rational { best_path.targetInfo.refreshRate.Numerator, best_path.targetInfo.refreshRate.Denominator } :
                                         Rational { 0, 1 } };
@@ -80,7 +80,7 @@ namespace display_device {
       else {
         available_devices.push_back(
           { device_id,
-            std::string {},  // Inactive devices can have multiple display names, so it's just meaningless use any
+            display_name,
             friendly_name,
             std::nullopt });
       }

tests/unit/windows/test_win_display_device_general.cpp

@@ -201,42 +201,89 @@ TEST_F_S_MOCKED(EnumAvailableDevices) {
   EXPECT_EQ(m_win_dd.enumAvailableDevices(), expected_list);
 }
 
-TEST_F_S_MOCKED(EnumAvailableDevices, FailedToGetDisplayData) {
+TEST_F_S_MOCKED(EnumAvailableDevices, MissingSourceModes) {
+  auto pam_missing_modes { ut_consts::PAM_3_ACTIVE };
+  pam_missing_modes->m_paths.resize(2);
+  pam_missing_modes->m_modes.resize(1);
+
+  InSequence sequence;
   EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All))
     .Times(1)
-    .WillOnce(Return(ut_consts::PAM_NULL));
+    .WillOnce(Return(pam_missing_modes))
+    .RetiresOnSaturation();
 
-  EXPECT_EQ(m_win_dd.enumAvailableDevices(), display_device::EnumeratedDeviceList {});
+  for (int i = 1; i <= 2; ++i) {
+    EXPECT_CALL(*m_layer, getMonitorDevicePath(_))
+      .Times(1)
+      .WillOnce(Return("Path" + std::to_string(i)))
+      .RetiresOnSaturation();
+    EXPECT_CALL(*m_layer, getDeviceId(_))
+      .Times(1)
+      .WillOnce(Return("DeviceId" + std::to_string(i)))
+      .RetiresOnSaturation();
+    EXPECT_CALL(*m_layer, getDisplayName(_))
+      .Times(1)
+      .WillOnce(Return("DisplayName" + std::to_string(i)))
+      .RetiresOnSaturation();
+  }
+
+  EXPECT_CALL(*m_layer, getFriendlyName(_))
+    .Times(1)
+    .WillOnce(Return("FriendlyName1"))
+    .RetiresOnSaturation();
+  EXPECT_CALL(*m_layer, getDisplayName(_))
+    .Times(1)
+    .WillOnce(Return("DisplayName1"))
+    .RetiresOnSaturation();
+  EXPECT_CALL(*m_layer, getDisplayScale(_, _))
+    .Times(1)
+    .WillOnce(Return(std::nullopt))
+    .RetiresOnSaturation();
+  EXPECT_CALL(*m_layer, getHdrState(_))
+    .Times(1)
+    .WillOnce(Return(std::nullopt))
+    .RetiresOnSaturation();
+
+  EXPECT_CALL(*m_layer, getFriendlyName(_))
+    .Times(1)
+    .WillOnce(Return("FriendlyName2"))
+    .RetiresOnSaturation();
+  EXPECT_CALL(*m_layer, getDisplayName(_))
+    .Times(1)
+    .WillOnce(Return("DisplayName2"))
+    .RetiresOnSaturation();
+
+  const display_device::EnumeratedDeviceList expected_list {
+    { "DeviceId1",
+      "DisplayName1",
+      "FriendlyName1",
+      display_device::EnumeratedDevice::Info {
+        { 1920, 1080 },
+        display_device::Rational { 0, 1 },
+        display_device::Rational { 120, 1 },
+        true,
+        { 0, 0 },
+        std::nullopt } },
+    { "DeviceId2",
+      "DisplayName2",
+      "FriendlyName2",
+      std::nullopt }
+  };
+  EXPECT_EQ(m_win_dd.enumAvailableDevices(), expected_list);
 }
 
-TEST_F_S_MOCKED(EnumAvailableDevices, FailedToCollectSourceData) {
+TEST_F_S_MOCKED(EnumAvailableDevices, FailedToGetDisplayData) {
   EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All))
     .Times(1)
-    .WillOnce(Return(ut_consts::PAM_EMPTY));
+    .WillOnce(Return(ut_consts::PAM_NULL));
 
   EXPECT_EQ(m_win_dd.enumAvailableDevices(), display_device::EnumeratedDeviceList {});
 }
 
-TEST_F_S_MOCKED(EnumAvailableDevices, NoDisplayModes) {
-  auto pam_no_modes { ut_consts::PAM_3_ACTIVE };
-  pam_no_modes->m_paths.resize(1);
-  pam_no_modes->m_modes.clear();
-
+TEST_F_S_MOCKED(EnumAvailableDevices, FailedToCollectSourceData) {
   EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All))
     .Times(1)
-    .WillOnce(Return(pam_no_modes));
-  EXPECT_CALL(*m_layer, getMonitorDevicePath(_))
-    .Times(1)
-    .WillOnce(Return("Path1"));
-  EXPECT_CALL(*m_layer, getDeviceId(_))
-    .Times(1)
-    .WillOnce(Return("DeviceId1"));
-  EXPECT_CALL(*m_layer, getDisplayName(_))
-    .Times(1)
-    .WillOnce(Return("DisplayName1"));
-  EXPECT_CALL(*m_layer, getFriendlyName(_))
-    .Times(1)
-    .WillOnce(Return("FriendlyName1"));
+    .WillOnce(Return(ut_consts::PAM_EMPTY));
 
   EXPECT_EQ(m_win_dd.enumAvailableDevices(), display_device::EnumeratedDeviceList {});
 }