From 0a702eab061829807551fce47c278f1fe0a2cd32 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sun, 26 Jan 2025 15:59:32 +0200 Subject: [PATCH] fix(settings): better API test and clean persistence reset --- .../settings_manager_interface.h | 2 +- .../windows/win_display_device_interface.h | 2 +- src/windows/settings_manager_general.cpp | 7 ++++--- src/windows/win_display_device_general.cpp | 11 ++-------- .../windows/test_settings_manager_general.cpp | 9 --------- .../test_win_display_device_general.cpp | 20 +++---------------- 6 files changed, 11 insertions(+), 40 deletions(-) diff --git a/src/common/include/display_device/settings_manager_interface.h b/src/common/include/display_device/settings_manager_interface.h index c7cd885..2142e7b 100644 --- a/src/common/include/display_device/settings_manager_interface.h +++ b/src/common/include/display_device/settings_manager_interface.h @@ -98,7 +98,7 @@ namespace display_device { * * In case the settings cannot be reverted, because the display is turned or some other reason, * this allows to "accept" the current state and start from scratch, but only if the persistence was - * cleared successfuly. + * cleared successfully. * @examples * SettingsManagerInterface* iface = getIface(...); * auto result = iface->applySettings(config); diff --git a/src/windows/include/display_device/windows/win_display_device_interface.h b/src/windows/include/display_device/windows/win_display_device_interface.h index a3f4d8c..ebd82b6 100644 --- a/src/windows/include/display_device/windows/win_display_device_interface.h +++ b/src/windows/include/display_device/windows/win_display_device_interface.h @@ -127,7 +127,7 @@ namespace display_device { * @brief Set new display modes for the devices. * @param modes A map of modes to set. * @returns True if modes were set, false otherwise. - * @warning if any of the specified devices are duplicated, modes modes be provided + * @warning if any of the specified devices are duplicated, modes be provided * for duplicates too! * @examples * WinDisplayDeviceInterface* iface = getIface(...); diff --git a/src/windows/settings_manager_general.cpp b/src/windows/settings_manager_general.cpp index 758d8b4..55c2485 100644 --- a/src/windows/settings_manager_general.cpp +++ b/src/windows/settings_manager_general.cpp @@ -46,12 +46,13 @@ namespace display_device { } bool SettingsManager::resetPersistence() { - // Trying to revert one more time in case we succeed. - if (revertSettings() == RevertResult::Ok) { + DD_LOG(info) << "Trying to reset persistent display device settings."; + + const auto &cached_state {m_persistence_state->getState()}; + if (!cached_state) { return true; } - DD_LOG(info) << "Trying to reset persistent display device settings."; if (!m_persistence_state->persistState(std::nullopt)) { DD_LOG(error) << "Failed to clear persistence!"; return false; diff --git a/src/windows/win_display_device_general.cpp b/src/windows/win_display_device_general.cpp index 4c5e86a..7550511 100644 --- a/src/windows/win_display_device_general.cpp +++ b/src/windows/win_display_device_general.cpp @@ -18,17 +18,10 @@ namespace display_device { } bool WinDisplayDevice::isApiAccessAvailable() const { - const auto display_data {m_w_api->queryDisplayConfig(QueryType::All)}; - if (!display_data) { - DD_LOG(debug) << "WinDisplayDevice::isApiAccessAvailable failed while querying display data."; - return false; - } - - // Here we are supplying the retrieved display data back to SetDisplayConfig (with VALIDATE flag only, so that we make no actual changes). // Unless something is really broken on Windows, this call should never fail under normal circumstances - the configuration is 100% correct, since it was // provided by Windows. - const UINT32 flags {SDC_VALIDATE | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_VIRTUAL_MODE_AWARE}; - const LONG result {m_w_api->setDisplayConfig(display_data->m_paths, display_data->m_modes, flags)}; + const UINT32 flags {SDC_VALIDATE | SDC_USE_DATABASE_CURRENT}; + const LONG result {m_w_api->setDisplayConfig({}, {}, flags)}; DD_LOG(debug) << "WinDisplayDevice::isApiAccessAvailable result: " << m_w_api->getErrorString(result); return result == ERROR_SUCCESS; diff --git a/tests/unit/windows/test_settings_manager_general.cpp b/tests/unit/windows/test_settings_manager_general.cpp index 30dbef3..28f36ed 100644 --- a/tests/unit/windows/test_settings_manager_general.cpp +++ b/tests/unit/windows/test_settings_manager_general.cpp @@ -106,9 +106,6 @@ TEST_F_S_MOCKED(ResetPersistence, FailedToReset) { EXPECT_CALL(*m_settings_persistence_api, load()) .Times(1) .WillOnce(Return(serializeState(ut_consts::SDCS_FULL))); - EXPECT_CALL(*m_dd_api, isApiAccessAvailable()) - .Times(1) - .WillOnce(Return(false)); EXPECT_CALL(*m_settings_persistence_api, clear()) .Times(1) .WillOnce(Return(false)); @@ -120,9 +117,6 @@ TEST_F_S_MOCKED(ResetPersistence, PersistenceReset, NoCapturedDevice) { EXPECT_CALL(*m_settings_persistence_api, load()) .Times(1) .WillOnce(Return(serializeState(ut_consts::SDCS_FULL))); - EXPECT_CALL(*m_dd_api, isApiAccessAvailable()) - .Times(1) - .WillOnce(Return(false)); EXPECT_CALL(*m_settings_persistence_api, clear()) .Times(1) .WillOnce(Return(true)); @@ -138,9 +132,6 @@ TEST_F_S_MOCKED(ResetPersistence, PersistenceReset, WithCapturedDevice) { EXPECT_CALL(*m_settings_persistence_api, load()) .Times(1) .WillOnce(Return(serializeState(ut_consts::SDCS_FULL))); - EXPECT_CALL(*m_dd_api, isApiAccessAvailable()) - .Times(1) - .WillOnce(Return(false)); EXPECT_CALL(*m_settings_persistence_api, clear()) .Times(1) .WillOnce(Return(true)); diff --git a/tests/unit/windows/test_win_display_device_general.cpp b/tests/unit/windows/test_win_display_device_general.cpp index 67f89bd..c100de7 100644 --- a/tests/unit/windows/test_win_display_device_general.cpp +++ b/tests/unit/windows/test_win_display_device_general.cpp @@ -34,7 +34,7 @@ namespace { #define TEST_F_S_MOCKED(...) DD_MAKE_TEST(TEST_F, WinDisplayDeviceGeneralMocked, __VA_ARGS__) // Additional convenience global const(s) - const UINT32 FLAGS {SDC_VALIDATE | SDC_USE_SUPPLIED_DISPLAY_CONFIG | SDC_VIRTUAL_MODE_AWARE}; + const UINT32 FLAGS {SDC_VALIDATE | SDC_USE_DATABASE_CURRENT}; } // namespace TEST_F_S(NullptrLayerProvided) { @@ -50,10 +50,7 @@ TEST_F_S(IsApiAccessAvailable) { } TEST_F_S_MOCKED(IsApiAccessAvailable) { - EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All)) - .Times(1) - .WillOnce(Return(ut_consts::PAM_3_ACTIVE)); - EXPECT_CALL(*m_layer, setDisplayConfig(ut_consts::PAM_3_ACTIVE->m_paths, ut_consts::PAM_3_ACTIVE->m_modes, FLAGS)) + EXPECT_CALL(*m_layer, setDisplayConfig(std::vector{}, std::vector{}, FLAGS)) .Times(1) .WillOnce(Return(ERROR_SUCCESS)); EXPECT_CALL(*m_layer, getErrorString(ERROR_SUCCESS)) @@ -63,19 +60,8 @@ TEST_F_S_MOCKED(IsApiAccessAvailable) { EXPECT_TRUE(m_win_dd.isApiAccessAvailable()); } -TEST_F_S_MOCKED(IsApiAccessAvailable, FailedToGetDisplayData) { - EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All)) - .Times(1) - .WillOnce(Return(ut_consts::PAM_NULL)); - - EXPECT_FALSE(m_win_dd.isApiAccessAvailable()); -} - TEST_F_S_MOCKED(IsApiAccessAvailable, FailedToSetDisplayConfig) { - EXPECT_CALL(*m_layer, queryDisplayConfig(display_device::QueryType::All)) - .Times(1) - .WillOnce(Return(ut_consts::PAM_3_ACTIVE)); - EXPECT_CALL(*m_layer, setDisplayConfig(ut_consts::PAM_3_ACTIVE->m_paths, ut_consts::PAM_3_ACTIVE->m_modes, FLAGS)) + EXPECT_CALL(*m_layer, setDisplayConfig(std::vector{}, std::vector{}, FLAGS)) .Times(1) .WillOnce(Return(ERROR_ACCESS_DENIED)); EXPECT_CALL(*m_layer, getErrorString(ERROR_ACCESS_DENIED))