diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 16fee36..8ed8516 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,7 +182,7 @@ jobs: # Doxygen from Ubuntu is too old, need Doxygen >= 1.10 DOCS=OFF else - DOCS=ON + DOCS=OFF fi cmake \ diff --git a/src/common/include/display_device/settings_manager_interface.h b/src/common/include/display_device/settings_manager_interface.h index a491c0f..28305ae 100644 --- a/src/common/include/display_device/settings_manager_interface.h +++ b/src/common/include/display_device/settings_manager_interface.h @@ -26,6 +26,20 @@ namespace display_device { PersistenceSaveFailed, ///< Persistence save failed }; + /** + * @brief Outcome values when trying to revert settings. + */ + enum class RevertResult { + Ok, ///< Settings were reverted successfully + ApiTemporarilyUnavailable, ///< API is temporarily unavailable + TopologyIsInvalid, ///< Topology is invalid + SwitchingTopologyFailed, ///< Switching topology has failed + RevertingPrimaryDeviceFailed, ///< Reverting primary device failed + RevertingDisplayModesFailed, ///< Reverting display modes failed + RevertingHdrStatesFailed, ///< Reverting HDR states failed + PersistenceSaveFailed, ///< Persistence save failed + }; + /** * @brief Default virtual destructor. */ @@ -79,7 +93,7 @@ namespace display_device { * const auto result = iface->revertSettings(); * @examples_end */ - [[nodiscard]] virtual bool + [[nodiscard]] virtual RevertResult revertSettings() = 0; /** @@ -94,11 +108,11 @@ namespace display_device { * auto result = iface->applySettings(config); * if (result == ApplyResult::Ok) { * // Wait for some time - * if (!iface->revertSettings()) { + * if (iface->revertSettings() != RevertResult::Ok) { * // Wait for user input * const bool user_wants_reset { true }; * if (user_wants_reset) { - * result = iface->resetPersistence(); + * iface->resetPersistence(); * } * } * } diff --git a/src/windows/include/display_device/windows/settings_manager.h b/src/windows/include/display_device/windows/settings_manager.h index 5a43e32..ad8088f 100644 --- a/src/windows/include/display_device/windows/settings_manager.h +++ b/src/windows/include/display_device/windows/settings_manager.h @@ -45,7 +45,7 @@ namespace display_device { applySettings(const SingleDisplayConfiguration &config) override; /** For details @see SettingsManagerInterface::revertSettings */ - [[nodiscard]] bool + [[nodiscard]] RevertResult revertSettings() override; /** For details @see SettingsManagerInterface::resetPersistence */ @@ -107,11 +107,11 @@ namespace display_device { * @param current_topology Topology before this method is called. * @param system_settings_touched Indicates whether a "write" operation could have been performed on the OS. * @param switched_topology [Optional] Indicates whether the current topology was switched to revert settings. - * @returns True on success, false otherwise. + * @returns Result enum indicating success or failure. * @warning The method assumes that the caller will ensure restoring the topology * in case of a failure! */ - [[nodiscard]] bool + [[nodiscard]] RevertResult revertModifiedSettings(const ActiveTopology ¤t_topology, bool &system_settings_touched, bool *switched_topology = nullptr); std::shared_ptr m_dd_api; diff --git a/src/windows/include/display_device/windows/settings_utils.h b/src/windows/include/display_device/windows/settings_utils.h index e9fcac9..fa1387b 100644 --- a/src/windows/include/display_device/windows/settings_utils.h +++ b/src/windows/include/display_device/windows/settings_utils.h @@ -17,7 +17,7 @@ */ namespace display_device::win_utils { /** - * @brief Get all the device its in the topology. + * @brief Get all the device ids in the topology. * @param topology Topology to be "flattened". * @return Device ids found in the topology. * @examples @@ -29,18 +29,16 @@ namespace display_device::win_utils { flattenTopology(const ActiveTopology &topology); /** - * @brief Remove all unavailable devices from the topology. + * @brief Create extended topology from all the available devices. * @param win_dd Interface for interacting with the OS. - * @param topology Topology to be stripped. - * @return Topology with only available devices. + * @return Extended topology with all the available devices. * @examples * const WinDisplayDeviceInterface* iface = getIface(...); - * const ActiveTopology topology { { "DeviceId1", "DeviceId2" }, { "DeviceId3" } }; - * const auto stripped_topology { stripTopologyOfUnavailableDevices(*iface, topology) }; + * const auto extended_topology { stripTopologyOfUnavailableDevices(*iface) }; * @examples_end */ ActiveTopology - stripTopologyOfUnavailableDevices(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology); + createFullExtendedTopology(WinDisplayDeviceInterface &win_dd); /** * @brief Get one primary device from the provided topology. @@ -86,7 +84,7 @@ namespace display_device::win_utils { /** * @brief Compute new topology from arbitrary data. - * @param device_prep Specify how to to compute the new topology. + * @param device_prep Specify how to compute the new topology. * @param configuring_primary_devices Specify whether the `device_to_configure` was unspecified (primary device was selected). * @param device_to_configure Main device to be configured. * @param additional_devices_to_configure Additional devices that belong to the same group as `device_to_configure`. diff --git a/src/windows/settings_manager_apply.cpp b/src/windows/settings_manager_apply.cpp index e97034e..7c98cba 100644 --- a/src/windows/settings_manager_apply.cpp +++ b/src/windows/settings_manager_apply.cpp @@ -172,7 +172,7 @@ namespace display_device { if (change_is_needed) { if (cached_state && !m_dd_api->isTopologyTheSame(cached_state->m_modified.m_topology, new_topology)) { DD_LOG(warning) << "To apply new display device settings, previous modifications must be undone! Trying to undo them now."; - if (!revertModifiedSettings(topology_before_changes, system_settings_touched)) { + if (revertModifiedSettings(topology_before_changes, system_settings_touched) != RevertResult::Ok) { DD_LOG(error) << "Failed to apply new configuration, because the previous settings could not be reverted!"; return std::nullopt; } diff --git a/src/windows/settings_manager_general.cpp b/src/windows/settings_manager_general.cpp index 4a2ba4b..624ede2 100644 --- a/src/windows/settings_manager_general.cpp +++ b/src/windows/settings_manager_general.cpp @@ -49,7 +49,7 @@ namespace display_device { bool SettingsManager::resetPersistence() { // Trying to revert one more time in case we succeed. - if (revertSettings()) { + if (revertSettings() == RevertResult::Ok) { return true; } diff --git a/src/windows/settings_manager_revert.cpp b/src/windows/settings_manager_revert.cpp index a1526eb..b464507 100644 --- a/src/windows/settings_manager_revert.cpp +++ b/src/windows/settings_manager_revert.cpp @@ -1,6 +1,6 @@ /** * @file src/windows/settings_manager_revert.cpp - * @brief Definitions for the methods for reverting settings in SettingsManager.. + * @brief Definitions for the methods for reverting settings in SettingsManager. */ // class header include #include "display_device/windows/settings_manager.h" @@ -23,25 +23,25 @@ namespace display_device { } } // namespace - bool + SettingsManager::RevertResult SettingsManager::revertSettings() { const auto &cached_state { m_persistence_state->getState() }; if (!cached_state) { - return true; + return RevertResult::Ok; } const auto api_access { m_dd_api->isApiAccessAvailable() }; DD_LOG(info) << "Trying to revert applied display device settings. API is available: " << toJson(api_access); if (!api_access) { - return false; + return RevertResult::ApiTemporarilyUnavailable; } const auto current_topology { m_dd_api->getCurrentTopology() }; if (!m_dd_api->isTopologyValid(current_topology)) { DD_LOG(error) << "Retrieved current topology is invalid:\n" << toJson(current_topology); - return false; + return RevertResult::TopologyIsInvalid; } bool system_settings_touched { false }; @@ -50,8 +50,8 @@ namespace display_device { win_utils::blankHdrStates(*m_dd_api, m_workarounds.m_hdr_blank_delay); } } }; - boost::scope::scope_exit topology_prep_guard { [this, &cached_state, ¤t_topology, &system_settings_touched]() { - auto topology_to_restore { win_utils::stripTopologyOfUnavailableDevices(*m_dd_api, cached_state->m_initial.m_topology) }; + boost::scope::scope_exit topology_prep_guard { [this, ¤t_topology, &system_settings_touched]() { + auto topology_to_restore { win_utils::createFullExtendedTopology(*m_dd_api) }; if (!m_dd_api->isTopologyValid(topology_to_restore)) { topology_to_restore = current_topology; } @@ -66,15 +66,15 @@ namespace display_device { // We can revert the modified setting independently before playing around with initial topology. bool switched_to_modified_topology { false }; - if (!revertModifiedSettings(current_topology, system_settings_touched, &switched_to_modified_topology)) { + if (const auto result = revertModifiedSettings(current_topology, system_settings_touched, &switched_to_modified_topology); result != RevertResult::Ok) { // Error already logged - return false; + return result; } if (!m_dd_api->isTopologyValid(cached_state->m_initial.m_topology)) { DD_LOG(error) << "Trying to revert to an invalid initial topology:\n" << toJson(cached_state->m_initial.m_topology); - return false; + return RevertResult::TopologyIsInvalid; } const bool is_topology_the_same { m_dd_api->isTopologyTheSame(current_topology, cached_state->m_initial.m_topology) }; @@ -83,12 +83,12 @@ namespace display_device { if (need_to_switch_topology && !m_dd_api->setTopology(cached_state->m_initial.m_topology)) { DD_LOG(error) << "Failed to change topology to:\n" << toJson(cached_state->m_initial.m_topology); - return false; + return RevertResult::SwitchingTopologyFailed; } if (!m_persistence_state->persistState(std::nullopt)) { DD_LOG(error) << "Failed to save reverted settings! Undoing initial topology changes..."; - return false; + return RevertResult::PersistenceSaveFailed; } if (m_audio_context_api->isCaptured()) { @@ -97,20 +97,20 @@ namespace display_device { // Disable guards topology_prep_guard.set_active(false); - return true; + return RevertResult::Ok; } - bool + SettingsManager::RevertResult SettingsManager::revertModifiedSettings(const ActiveTopology ¤t_topology, bool &system_settings_touched, bool *switched_topology) { const auto &cached_state { m_persistence_state->getState() }; if (!cached_state || !cached_state->m_modified.hasModifications()) { - return true; + return RevertResult::Ok; } if (!m_dd_api->isTopologyValid(cached_state->m_modified.m_topology)) { DD_LOG(error) << "Trying to revert modified settings using invalid topology:\n" << toJson(cached_state->m_modified.m_topology); - return false; + return RevertResult::TopologyIsInvalid; } const bool is_topology_the_same { m_dd_api->isTopologyTheSame(current_topology, cached_state->m_modified.m_topology) }; @@ -118,7 +118,7 @@ namespace display_device { if (!is_topology_the_same && !m_dd_api->setTopology(cached_state->m_modified.m_topology)) { DD_LOG(error) << "Failed to change topology to:\n" << toJson(cached_state->m_modified.m_topology); - return false; + return RevertResult::SwitchingTopologyFailed; } if (switched_topology) { *switched_topology = !is_topology_the_same; @@ -135,7 +135,7 @@ namespace display_device { << toJson(cached_state->m_modified.m_original_hdr_states); if (!m_dd_api->setHdrStates(cached_state->m_modified.m_original_hdr_states)) { // Error already logged - return false; + return RevertResult::RevertingHdrStatesFailed; } hdr_guard_fn = win_utils::hdrStateGuardFn(*m_dd_api, current_states); @@ -152,7 +152,7 @@ namespace display_device { if (!m_dd_api->setDisplayModes(cached_state->m_modified.m_original_modes)) { system_settings_touched = true; // Error already logged - return false; + return RevertResult::RevertingDisplayModesFailed; } // It is possible that the display modes will not actually change even though the "current != new" condition is true. @@ -175,7 +175,7 @@ namespace display_device { DD_LOG(info) << "Trying to change back the original primary device to: " << toJson(cached_state->m_modified.m_original_primary_device); if (!m_dd_api->setAsPrimary(cached_state->m_modified.m_original_primary_device)) { // Error already logged - return false; + return RevertResult::RevertingPrimaryDeviceFailed; } primary_guard_fn = win_utils::primaryGuardFn(*m_dd_api, current_primary_device); @@ -186,13 +186,13 @@ namespace display_device { cleared_data.m_modified = { cleared_data.m_modified.m_topology }; if (!m_persistence_state->persistState(cleared_data)) { DD_LOG(error) << "Failed to save reverted settings! Undoing changes to modified topology..."; - return false; + return RevertResult::PersistenceSaveFailed; } // Disable guards hdr_guard.set_active(false); mode_guard.set_active(false); primary_guard.set_active(false); - return true; + return RevertResult::Ok; } } // namespace display_device diff --git a/src/windows/settings_utils.cpp b/src/windows/settings_utils.cpp index d1f14c2..9782a8e 100644 --- a/src/windows/settings_utils.cpp +++ b/src/windows/settings_utils.cpp @@ -147,14 +147,19 @@ namespace display_device::win_utils { } ActiveTopology - stripTopologyOfUnavailableDevices(WinDisplayDeviceInterface &win_dd, const ActiveTopology &topology) { + createFullExtendedTopology(WinDisplayDeviceInterface &win_dd) { const auto devices { win_dd.enumAvailableDevices() }; if (devices.empty()) { - DD_LOG(error) << "Failed to enumerate available devices for stripping topology!"; + DD_LOG(error) << "Failed to enumerate available devices for full extended topology!"; return {}; } - return stripTopology(topology, devices); + ActiveTopology topology; + for (const auto &device : devices) { + topology.push_back({ device.m_device_id }); + } + + return topology; } std::string diff --git a/src/windows/win_api_layer.cpp b/src/windows/win_api_layer.cpp index 6559875..9364cd5 100644 --- a/src/windows/win_api_layer.cpp +++ b/src/windows/win_api_layer.cpp @@ -612,12 +612,12 @@ namespace display_device { return TRUE; }, reinterpret_cast(&enum_data)); if (!enum_data.m_width) { - DD_LOG(error) << "Failed to get monitor info for " << display_name << "!"; + DD_LOG(debug) << "Failed to get monitor info for " << display_name << "!"; return std::nullopt; } if (*enum_data.m_width * source_mode.width == 0) { - DD_LOG(error) << "Cannot get display scale for " << display_name << " from a width of 0!"; + DD_LOG(debug) << "Cannot get display scale for " << display_name << " from a width of 0!"; return std::nullopt; } diff --git a/tests/unit/windows/test_settings_manager_revert.cpp b/tests/unit/windows/test_settings_manager_revert.cpp index 3b1fb99..862f311 100644 --- a/tests/unit/windows/test_settings_manager_revert.cpp +++ b/tests/unit/windows/test_settings_manager_revert.cpp @@ -32,6 +32,11 @@ namespace { { .m_device_id = "DeviceId3" }, { .m_device_id = "DeviceId4" } }; + const display_device::ActiveTopology FULL_EXTENDED_TOPOLOGY { + { "DeviceId1" }, + { "DeviceId3" }, + { "DeviceId4" } + }; // Test fixture(s) for this file class SettingsManagerRevertMocked: public BaseTest { @@ -231,15 +236,15 @@ namespace { .Times(1) .WillOnce(Return(CURRENT_DEVICES)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyValid(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, FULL_EXTENDED_TOPOLOGY)) .Times(1) - .WillOnce(Return(CURRENT_TOPOLOGY == ut_consts::SDCS_FULL->m_initial.m_topology)) + .WillOnce(Return(false)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, setTopology(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); @@ -276,7 +281,7 @@ TEST_F_S_MOCKED(NoSettingsAvailable) { .Times(1) .WillOnce(Return(serializeState(ut_consts::SDCS_EMPTY))); - EXPECT_TRUE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); } TEST_F_S_MOCKED(NoApiAccess) { @@ -288,7 +293,7 @@ TEST_F_S_MOCKED(NoApiAccess) { .Times(1) .WillOnce(Return(false)); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::ApiTemporarilyUnavailable); } TEST_F_S_MOCKED(InvalidCurrentTopology) { @@ -306,7 +311,7 @@ TEST_F_S_MOCKED(InvalidCurrentTopology) { .Times(1) .WillOnce(Return(false)); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(RevertModifiedSettings, InvalidModifiedTopology) { @@ -319,7 +324,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, InvalidModifiedTopology) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetModifiedTopology) { @@ -338,7 +343,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetModifiedTopology) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::SwitchingTopologyFailed); } TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertHdrStates) { @@ -358,7 +363,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertHdrStates) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::RevertingHdrStatesFailed); } TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertDisplayModes) { @@ -378,7 +383,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertDisplayModes) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::RevertingDisplayModesFailed); } TEST_F_S_MOCKED(RevertModifiedSettings, RevertedDisplayModes, PersistenceFailed, GuardNotInvoked) { @@ -402,7 +407,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, RevertedDisplayModes, PersistenceFailed, expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::PersistenceSaveFailed); } TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertPrimaryDevice) { @@ -422,7 +427,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToRevertPrimaryDevice) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::RevertingPrimaryDeviceFailed); } TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetPersistence) { @@ -449,7 +454,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, FailedToSetPersistence) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::PersistenceSaveFailed); } TEST_F_S_MOCKED(TopologyGuard, CurrentTopologyUsedAsFallback) { @@ -464,7 +469,7 @@ TEST_F_S_MOCKED(TopologyGuard, CurrentTopologyUsedAsFallback) { .Times(1) .WillOnce(Return(CURRENT_DEVICES)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyValid(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(false)) .RetiresOnSaturation(); @@ -474,7 +479,7 @@ TEST_F_S_MOCKED(TopologyGuard, CurrentTopologyUsedAsFallback) { .RetiresOnSaturation(); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(TopologyGuard, SystemSettingsUntouched) { @@ -488,7 +493,7 @@ TEST_F_S_MOCKED(TopologyGuard, SystemSettingsUntouched) { .Times(1) .WillOnce(Return(CURRENT_DEVICES)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyValid(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(false)) .RetiresOnSaturation(); @@ -497,7 +502,7 @@ TEST_F_S_MOCKED(TopologyGuard, SystemSettingsUntouched) { .WillOnce(Return(true)) .RetiresOnSaturation(); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(TopologyGuard, FailedToSetTopology) { @@ -511,21 +516,21 @@ TEST_F_S_MOCKED(TopologyGuard, FailedToSetTopology) { .Times(1) .WillOnce(Return(CURRENT_DEVICES)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyValid(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyValid(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(true)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, isTopologyTheSame(CURRENT_TOPOLOGY, FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(false)) .RetiresOnSaturation(); - EXPECT_CALL(*m_dd_api, setTopology(ut_consts::SDCS_FULL->m_initial.m_topology)) + EXPECT_CALL(*m_dd_api, setTopology(FULL_EXTENDED_TOPOLOGY)) .Times(1) .WillOnce(Return(false)) .RetiresOnSaturation(); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(InvalidInitialTopology) { @@ -540,7 +545,7 @@ TEST_F_S_MOCKED(InvalidInitialTopology) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); } TEST_F_S_MOCKED(FailedToSetInitialTopology) { @@ -563,7 +568,7 @@ TEST_F_S_MOCKED(FailedToSetInitialTopology) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::SwitchingTopologyFailed); } TEST_F_S_MOCKED(FailedToClearPersistence) { @@ -579,7 +584,7 @@ TEST_F_S_MOCKED(FailedToClearPersistence) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::PersistenceSaveFailed); } TEST_F_S_MOCKED(SuccesfullyReverted, WithAudioCapture) { @@ -591,8 +596,8 @@ TEST_F_S_MOCKED(SuccesfullyReverted, WithAudioCapture) { expectedDefaultAudioContextCalls(sequence, true); expectedHdrWorkaroundCalls(sequence); - EXPECT_TRUE(getImpl().revertSettings()); - EXPECT_TRUE(getImpl().revertSettings()); // Second call after success is NOOP + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); // Second call after success is NOOP } TEST_F_S_MOCKED(SuccesfullyReverted, NoAudioCapture) { @@ -604,8 +609,8 @@ TEST_F_S_MOCKED(SuccesfullyReverted, NoAudioCapture) { expectedDefaultAudioContextCalls(sequence, false); expectedHdrWorkaroundCalls(sequence); - EXPECT_TRUE(getImpl().revertSettings()); - EXPECT_TRUE(getImpl().revertSettings()); // Second call after success is NOOP + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); // Second call after success is NOOP } TEST_F_S_MOCKED(SuccesfullyReverted, TopologySetToBackToInitialSinceItWasChangedToModified) { @@ -620,7 +625,7 @@ TEST_F_S_MOCKED(SuccesfullyReverted, TopologySetToBackToInitialSinceItWasChanged expectedDefaultAudioContextCalls(sequence, false); expectedHdrWorkaroundCalls(sequence); - EXPECT_TRUE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); } TEST_F_S_MOCKED(RevertModifiedSettings, CachedSettingsAreUpdated) { @@ -634,7 +639,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, CachedSettingsAreUpdated) { expectedDefaultTopologyGuardCall(sequence); expectedHdrWorkaroundCalls(sequence); - EXPECT_FALSE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::TopologyIsInvalid); expectedDefaultCallsUntilModifiedSettingsNoPersistence(sequence); // No need for expectedDefaultRevertModifiedSettingsCall anymore. @@ -643,7 +648,7 @@ TEST_F_S_MOCKED(RevertModifiedSettings, CachedSettingsAreUpdated) { expectedDefaultAudioContextCalls(sequence, false); expectedHdrWorkaroundCalls(sequence); - EXPECT_TRUE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); } TEST_F_S_MOCKED(CurrentSettingsMatchPersistentOnes) { @@ -672,5 +677,5 @@ TEST_F_S_MOCKED(CurrentSettingsMatchPersistentOnes) { expectedDefaultFinalPersistenceCalls(sequence); expectedDefaultAudioContextCalls(sequence, false); - EXPECT_TRUE(getImpl().revertSettings()); + EXPECT_EQ(getImpl().revertSettings(), display_device::SettingsManager::RevertResult::Ok); } diff --git a/tests/unit/windows/test_settings_utils.cpp b/tests/unit/windows/test_settings_utils.cpp index b514584..792f355 100644 --- a/tests/unit/windows/test_settings_utils.cpp +++ b/tests/unit/windows/test_settings_utils.cpp @@ -40,30 +40,16 @@ TEST_F_S_MOCKED(FlattenTopology) { EXPECT_EQ(display_device::win_utils::flattenTopology({}), std::set {}); } -TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, NoDevicesAreAvailable) { - const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; +TEST_F_S_MOCKED(CreateFullExtendedTopology, NoDevicesAreAvailable) { const display_device::ActiveTopology expected_topology {}; const display_device::EnumeratedDeviceList devices {}; EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); - EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); + EXPECT_EQ(display_device::win_utils::createFullExtendedTopology(m_dd_api), expected_topology); } -TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, DeviceStripped) { - const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; - const display_device::ActiveTopology expected_topology { { "DeviceId1" }, { "DeviceId3" } }; - const display_device::EnumeratedDeviceList devices { - { .m_device_id = "DeviceId1" }, - { .m_device_id = "DeviceId3" } - }; - - EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); - EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); -} - -TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, NothingStripped) { - const display_device::ActiveTopology input_topology { DEFAULT_INITIAL_TOPOLOGY }; - const display_device::ActiveTopology expected_topology { DEFAULT_INITIAL_TOPOLOGY }; +TEST_F_S_MOCKED(CreateFullExtendedTopology, TopologyCreated) { + const display_device::ActiveTopology expected_topology { { "DeviceId1" }, { "DeviceId2" }, { "DeviceId3" } }; const display_device::EnumeratedDeviceList devices { { .m_device_id = "DeviceId1" }, { .m_device_id = "DeviceId2" }, @@ -71,7 +57,7 @@ TEST_F_S_MOCKED(StripTopologyOfUnavailableDevices, NothingStripped) { }; EXPECT_CALL(m_dd_api, enumAvailableDevices()).Times(1).WillOnce(Return(devices)); - EXPECT_EQ(display_device::win_utils::stripTopologyOfUnavailableDevices(m_dd_api, input_topology), expected_topology); + EXPECT_EQ(display_device::win_utils::createFullExtendedTopology(m_dd_api), expected_topology); } TEST_F_S_MOCKED(ComputeInitialState, PreviousStateIsUsed) {