I have a windows gaming handheld tool program I am creating that has several windows (like main window, a quick access menu window, etc). Many of these windows display several system values such as battery %, charging status, and the current thermal design power (TDP) of the CPU. Because some of the values require using external programs like RyzenAdj.exe and RW.exe that read the computer MMIO and cpu MSR, I don't want each window to manage this. Accidentally calling RyzenAdj.exe from two threads causes the exe to lock up.

To make everything easier on the UI thread, I have a dedicated background thread that loops through and gets these values every few seconds and posts them to a public class of variables. I then have dispatch timers in each window that checks the public class every few seconds for changes, but this seems like overkill and an event handler would be better suited.

Here is an example of how my background thread collecting values does

while (programRunning == true)
{
PublicVariables.networkStatus = getNetworkStatus();
PublicVariables.currentTDP = readCurrentTDP();
PublicVariables.batteryPercentage = 
readCurrentBatteryPercentage():
Thread.Sleep(1000);
}

In the different windows I have a dispatch timer that on tick says

labelTDP = PublicVariables.currentTDP;
labelNetworkStatus = PublicVariables.networkStatus;

This feels like i'm using unnecessary timers. I think I want an event handler, but i'm also concerned that using an event handler and having multiple subscribers could initiate multiple instances of the class that reads tdp, and could cause the RyzenAdj.exe program to stop.

Do you have any advice on how to handle this?

CodePudding user response：

Well, first of all, don't lock thread using Thread.Sleep - Task.Delay is wonderfull for that and enables the thread to be reused.

And you're right, i would define event to which classes would subscribe and raise it every time we read values. Also, I would resign from static class PublicVariables and put them in the class doing reading :)

Also, I can se potential asynchrony here, so I would suggest something like this

// Put awaits where there is possibility to do so
// Below should be properties with private setter
networkStatus = await getNetworkStatus();
currentTDP = await readCurrentTDP();
batteryPercentage = await readCurrentBatteryPercentage();
// Raise event
ReadUpdate();

await Task.Delay(1000);

Notes

1. You can also run tasks in parallel
2. You can start tasks and make async getters, and interested class just would await on its own
3. Instead of Task.Delay it could be timer trigger.

CodePudding user response：

There is no need for raising an event, either: You can just write the newly fetched values to properties in a shared instance of a class that implements INotifyPropertyChanged, and have the various windows bind directly to those properties. WPF will automatically marshal changes to the UI thread.