Maassoft / ColorControl

Easily change NVIDIA display settings and/or control LG TV's
GNU General Public License v3.0
829 stars 39 forks source link

List of LG devices empty on startup (until Refresh button is pressed) #312

Open lahma0 opened 5 months ago

lahma0 commented 5 months ago

Every time the application is restarted, the list of LG devices is empty upon startup, making the app completely non-functional until the 'Refresh' button is manually clicked. I replied to a previous issue that reported the same/similar behavior but I think you probably missed it so I'm just re-posting that same information here. See below:

Despite my TV being added as a device in device manager, the problem persists: 295359173-abe20c17-6eb6-4a0f-98a1-c01ebf2b8fbe 295359251-b14c3569-937e-43ba-ac5c-64ef879e393e

Furthermore, adding the device as a "Custom" device, changes nothing. Every time the application is restarted, or the machine is rebooted, the device list shows up as empty. Only after clicking the refresh button do my previously added "Custom" devices show up in the list. This effectively means that the only way the application can properly control my TV is if I remember to open the app manually after every app restart or system restart and click the Refresh button.

I would like to also add that this isn't a result of some recent change or update. It has always been this way as far as I can remember. The only unique thing about my system that I can think of that might be noteworthy is the fact that I use a virtualized network adapter (Hyper-V Virtual Ethernet Adapter). The only reason I mention this is because the app previously didn't work with virtualized network adapters because you were manually filtering out these adapters. I reported this bug and you fixed the problem very quickly (thanks!). I doubt it is related but thought I'd mention it just in case.

If you're busy or overwhelmed with other tasks, I would be happy to download the source code myself and track down the issue and fix it. I can then submit a pull request for you to review and accept the changes. While I suspect it would be much easier for you to implement a quick fix since you're familiar with the code base, I would be more than happy to do it myself if that would be helpful. Just let me know.

I suspect a quick and dirty fix would be to simply invoke the function connected to the 'Refresh' button after the app is fully loaded. Here is what it looks like after the app is restarted: 295361237-24610111-4686-4ac3-a7e1-8fe0272346b3

Here is what it looks like after clicking the 'Refresh' button: 295361368-af17cf52-15a1-421a-8573-06dafc2d7c2e

Please let me know if there is anything else I can do to help. I really appreciate all the hard work you've put into this app!

lahma0 commented 5 months ago

So I looked into this issue further, debugging the app with dnSpy, and I've discovered that this issue is being caused by a race condition. Whenever I attach a debugger, cbxLgDevices is being successfully loaded with the items saved in LgConfig.json. However, if I don't have a debugger attached, the execution order/timing is different enough that cbxLgDevices is not being loaded with any items. These kinds of issues are notoriously difficult to analyze and debug and, please don't think I'm being critical here or trying to be ugly, but your code base is kind of a mess, making it even more difficult to track down what's ultimately causing this problem. LG devices are being retrieved and set to the cbxLgDevices ComboBox like 3 or 4 times in a row, triggering the same set of cascading actions to happen multiple times for no reason (that I can tell anyways). Outside of refactoring a bunch of code to try to clean up the order in which some of these tasks are executed (which would likely fix the bug anyways), the only (very) dirty way I can quickly come up with to work around this bug is to simply set up a Task on a timer which will automatically execute the method connected to the Refresh button a few seconds after program startup. Obviously, this is an awful solution, and definitely isn't the right way, but it is about the only quick solution I can come up with. I think for now, I'll just use a loader which will inject a few lines of code on startup to do exactly this. If anyone else comes across this issue and would like to use the loader I created to bypass this issue, just let me know, and I'll post a link to it here.

Berutoron commented 5 months ago

Just chiming in to say that I set up ColorControl today (latest version, i.e. 9.8.0.1, on Windows 11, latest update, with an LG C3 42" on the latest firmware) and I'm having the exact same issue as you @lahma0 : empty device list until I press Refresh. A few more notes that might help us troubleshoot this:

  1. With the PC on LAN and the TV on WiFi, the TV wasn't auto-detected at all. I had to manually add it through the Custom option for it to even show up. Once I reboot the PC, the TV doesn't show up in the list anymore, but like you, it does after I press Refresh
  2. I tried having both the PC and the TV on LAN, and while that did allow me to auto-detect the TV, it didn't fix the main issue: empty list until Refresh is pressed
  3. You said "Every time the application is restarted, the list of LG devices is empty upon startup, making the app completely non-functional until the 'Refresh' button is manually clicked" but my experience has been more of a glass half-full kind of thing. Like you, given that the device list is initially empty, I would expect the TV to stay on if I simply boot to desktop and then immediately turn off the PC. Logic says that if ColorControl just has an empty list initially, then the TV isn't detected, and so an immediate shutdown (without even opening the ColorControl interface) wouldn't turn off the TV... Except in my case, it does! Meaning the Refresh issue only breaks the "Power on after startup" part of the process.

With regards to 3 especially, is your experience different from mine?

Two more odd things I've noticed: a. When I open ColorControl for the first time after boot, I always get the prompt "The elevation method is set to Windows Service but it is not running. Do you want to start it now?"... Except the task is already running if I take a look at the Task Scheduler (and I suspect that's why "Power off on shutdown" still works). b. Even if I press Yes and even if the Task is running according to the task scheduler, a visit to the Options menu shows that Elevation-method says "Use Windows service (installed, not running)" with a "Start" button next to it. Clicking that button does nothing by the way.

Have you had this issue too?

Unfortunately I'm not savvy enough to help beyond this point. But it would be great if this were fixed because everything else works like a charm.

lahma0 commented 5 months ago

Hi @Berutoron, in regards to item 3, I'm honestly not sure. All I know for sure is that my TV won't power on/off according to my "Windows power settings" unless I manually click the Refresh button. I turn my machine off so infrequently, I haven't really established if mine behaves the same as you described.

With regard to the Windows service issues you described, it was honestly such a buggy mess, that I long ago just created my own task in Task Scheduler to handle starting/restarting the app, bypassing all of the issues that were present in the built-in mechanism. But yes, I definitely had lots of issues when I tried to use it initially (multiple instances of the app, the app failing to recognize the service was running, the service crashing frequently, etc and so on). Here's how I set up my tasks if you're interested:

We will create 2 tasks. One for starting ColorControl on logon and another for auto restarting ColorControl if it crashes/exits or the process is stopped for any reason. To make it easy, let's create the tasks with Powershell (I'm using Powershell 7 but I think classic Windows Powershell should work as well). Make sure to run powershell as admin and replace any of the values inside brackets with their appropriate values.

Task: ColorControl-StartOnLogon Powershell Command:

$action = New-ScheduledTaskAction -Execute "[FULL_PATH_TO_COLOR_CONTROL_EXE]" -Argument "--scheduled" -WorkingDirectory "[FULL_PATH_TO_COLOR_CONTROL_DIRECTORY]"
$trigger = New-ScheduledTaskTrigger -AtLogon
$principal = New-ScheduledTaskPrincipal -UserId "[COMPUTER_NAME]\[USERNAME]" -RunLevel Highest
$settings = New-ScheduledTaskSettingsSet -AllowStartIfOnBatteries -DontStopIfGoingOnBatteries -MultipleInstances IgnoreNew -ExecutionTimeLimit '00:00:00'
$task = New-ScheduledTask -Action $action -Principal $principal -Trigger $trigger -Settings $settings
Register-ScheduledTask 'ColorControl-StartOnLogon' -InputObject $task

Task: ColorControl-AutoRestart Powershell Command:

$action = New-ScheduledTaskAction -Execute "schtasks" -Argument "/run /tn ColorControl-StartOnLogon"
$CIMTriggerClass = Get-CimClass -ClassName MSFT_TaskEventTrigger -Namespace Root/Microsoft/Windows/TaskScheduler:MSFT_TaskEventTrigger
$trigger = New-CimInstance -CimClass $CIMTriggerClass -ClientOnly
$trigger.Subscription = '<QueryList><Query Id="0" Path="Microsoft-Windows-TaskScheduler/Operational"><Select Path="Microsoft-Windows-TaskScheduler/Operational">*[EventData[@Name="ActionSuccess"][Data [@Name="TaskName"]="\ColorControl-StartOnLogon"]]</Select></Query></QueryList>'
$trigger.Enabled = $true
$principal = New-ScheduledTaskPrincipal -UserId "[COMPUTER_NAME]\[USERNAME]" -RunLevel Highest
$settings = New-ScheduledTaskSettingsSet -AllowStartIfOnBatteries -DontStopIfGoingOnBatteries -MultipleInstances IgnoreNew -ExecutionTimeLimit '00:02:00' -Hidden
$task = New-ScheduledTask -Action $action -Principal $principal -Trigger $trigger -Settings $settings
Register-ScheduledTask 'ColorControl-AutoRestart' -InputObject $task

Wow.. those 2 little Powershell commands took way to long to write and test.. I did double check everything and test them out though so that should fix the issues you were having related to the service. Just make sure to set "Elevation-method" to "None" or "Run as admin" in the ColorControl settings (NOT "Use Windows Service" or "Use dedicated elevated process"). Let me know if that helped you out.

Berutoron commented 5 months ago

Hi @lahma0 . Wow, thanks for the help. I haven't tried those tasks yet, but in the meantime I downloaded the LGTV Companion app, which other people suggested I use, and that seems to work fine so far. I'll let you know when I do. Of course, I'd rather use one single program that handles everything and doesn't require me to click a Refresh button after every reboot, but this is good enough for now.

Speaking of the Refresh button, do you think it's possible to create a task that automatically clicks the Refresh button after launching ColorControl? Theoretically anyway, not asking you to make it cause it's such a minor inconvenience. I'm just curious.