Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Dual-GPU Monitoring #728

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sanghyunna
Copy link

Sorry for the messy commit log, I accidentally committed the config.yaml file, so I had to revert and push it again 😆

What the PR is about:

I've observed that the monitor can't really handle systems with multiple graphics cards properly. It either averages things out or just picks one. This PR tries to fix that by letting you see stats (like temp, load, memory) for each GPU individually in the themes.


What it does:

  • Getting the data: I've refactored the sensor code (sensors_python.py, sensors_librehardwaremonitor.py) so that it can grab stats and names for all GPUs it can find, not just one. It now returns this data as lists.

  • Storing the data: The main stats.py file (specifically the Gpu class) was reworked quite a bit to hold these lists of data – one list for temps, one for loads, one for names, etc., for all detected GPUs.

  • New Theme functionality: Themes can now use a new format. Inside the STATS -> GPU section, you can add subsections like GPU0:, GPU1:, etc. Underneath those, you define where and how to show the stats (PERCENTAGE, TEMPERATURE, NAME, etc.) for that specific card. I added a theme called MultiGPUTest_3.5 as an example.

  • Keeping old themes working (Backwards Compatibility): If a theme doesn't use the new GPU0/GPU1 format, the code (_render_legacy_gpu in stats.py) will automatically just show the stats for the first GPU it finds, using the old theme definitions. I also updated default.yaml to support both structures gracefully.

  • Handling missing data: Sometimes sensors don't report everything (especially fan speed or FPS). I modified the code in stats.py to handle these NaN values better, showing "N/A" or 0 instead of crashing.

  • GPU Name fetching: GPU names can now be fetched with the right code. They default to vendor-specific names like NVIDIA GPU when the names are unable to fetch.


File Changes Summary:

  • library/sensors/sensors_python.py: Modified GPU methods to return List; added get_gpu_names(); improved fan detection logic.

  • library/sensors/sensors_librehardwaremonitor.py: Added get_all_gpus_and_update(); modified GPU methods to return List; added get_gpu_names().

  • library/sensors/sensors_stub_*.py: Updated GPU methods to return List for dummy GPUs; added get_gpu_names().

  • library/stats.py: Major refactor of Gpu class for per-GPU data; added new rendering (_render_multi_gpu) and legacy rendering (_render_legacy_gpu) logic; improved NaN handling.

  • res/themes/default.yaml: Added GPU0 default structure; kept legacy defaults for compatibility.

  • res/themes/MultiGPUTest_3.5/theme.yaml: Created/refined test theme demonstrating GPU0/GPU1 usage, name display, and background image.


Testing:

I've tested this on my setup:

  • OS: Linux (Ubuntu 22.04)
  • Python: 3.11
  • GPU: 2x NVIDIA RTX 3090
  • Display: 3.5-inch Revision A
  • Sensor Modes Tested: PYTHON, STATIC, STUB

It worked smoothly on my setup, but I haven't had a chance to test it on Windows, on AMD GPUs, on single GPUs or on different displays.
I don't have a Windows setup, so I didn't have a chance to test the LHM sensor.
Further testing would be very much appreciated.
Also, I do know that this is a fairly large bit of code, and this is my first real contribution to an open source project, so feel free to provide any advice or ask me about the code. Thanks.

@mathoudebine
Copy link
Owner

Hi and thank you for your contribution to this project!
I like that you took the time to describe precisely the changes, it helps a lot for the review.
I will review it and test it on Windows with AMD GPU.
Please note that it may not be soon because I haven't had much time to take care of the open pull requests for this project as you may have seen.

@sanghyunna
Copy link
Author

Thanks for the reply!
Give me an update when you have the time 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants