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

Large Overhaul #15

Merged
merged 5 commits into from
Aug 22, 2022
Merged

Conversation

Ebag333
Copy link
Contributor

@Ebag333 Ebag333 commented Jun 6, 2022

This PR does a lot including:

  1. Creates a fully functional code base (operates out of the box, tested on Windows with an Nvidia card).
  2. Moves most of the configuration to a config.yaml file, allowing for creating different configs without having to touch the code base.
  3. Move the bulk of the code out of main.py, making for a much more modular style design.
  4. Easier to expand (AMD GPUs, etc). Additional code that pulls specific information will need to be written, but it can be done so in a modular fashion without impacting existing code.
  5. Variable refresh for pulling/updating data from the system (CPU/GPU info, temps, memory, etc) using queues. Each type can pull data at a different rate as needed (fast for CPU/GPU, slow for disk).
  6. Queue to handle all communication with the display device, this eliminates the errors that you get once you start to try and update a lot of information and it starts stepping on itself. Keeps things serial and done one at a time. I believe this will also fix the "corrupted image" issue referenced in the open issue.
  7. Auto detect comm port. No longer need to hard set it, or if it changes on you then the config is wrong. (Tested for Windows, needs testing for *nix/OSx.)

I'm submitting this with a fully themed "terminal" style design that pulls and displays information for CPU/GPU/Memory/Disk. This uses PSUtil for most of the basic information, and GPUtil for the GPU (Nvidia only unfortunately).

@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 6, 2022

@mathoudebine hope you don't mind such a big update, started off just moving some config out of code and kinda grew from there. :)

@mathoudebine mathoudebine changed the base branch from feature/progress-bar to main June 6, 2022 13:19
@mathoudebine
Copy link
Owner

Hi @Ebag333 and thanks for sharing this amazing work! We are now coming very close to the original software in terms of features 🚀
I think users of this project are mostly people that want to get the original software features in an open-source program so your work will be very much appreciated!

I tested on Linux platform and everything seems to be working fine, except for the signal handling that does not interrupt the program anymore. Do you also have this issue?

I think it could be great to also add an exception handler with an error message for
https://github.com/Ebag333/turing-smart-screen-python/blob/main/library/scheduler.py#L87
in case the GPU isn't supported (like Intel/AMD GPUs) to avoid error stacktrace like:

Exception in thread GPU_Stats:
Traceback (most recent call last):
  File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.8/threading.py", line 870, in run
    self._target(*self._args, **self._kwargs)
  File "/home/mathoudebine/workspace/Ebag333/turing-smart-screen-python/library/scheduler.py", line 50, in wrap
    periodic(scheduler, interval, func)
  File "/home/mathoudebine/workspace/Ebag333/turing-smart-screen-python/library/scheduler.py", line 41, in periodic
    action(*actionargs)
  File "/home/mathoudebine/workspace/Ebag333/turing-smart-screen-python/library/scheduler.py", line 87, in GPUStats
    stats.GPU.stats()
  File "/home/mathoudebine/workspace/Ebag333/turing-smart-screen-python/library/stats.py", line 115, in stats
    gpu_data = GPUtil.getGPUs()
  File "/usr/local/lib/python3.8/dist-packages/GPUtil/GPUtil.py", line 102, in getGPUs
    deviceIds = int(vals[i])
ValueError: invalid literal for int() with base 10: "NVIDIA-SMI has failed because it couldn't communicate with the NVIDIA driver. Make sure that the latest NVIDIA driver is installed and running."

@mathoudebine mathoudebine self-assigned this Jun 6, 2022
@mathoudebine mathoudebine added the enhancement New feature or request label Jun 6, 2022
max_value=CONFIG_DATA['STATS']['CPU']['PERCENTAGE']['GRAPH'].get("MAX_VALUE", 100),
bar_color=CONFIG_DATA['STATS']['CPU']['PERCENTAGE']['GRAPH'].get("BAR_COLOR", (0, 0, 0)),
bar_outline=CONFIG_DATA['STATS']['CPU']['PERCENTAGE']['GRAPH'].get("BAR_OUTLINE", False),
background_image=CONFIG_DATA['STATS']['CPU']['PERCENTAGE']['GRAPH'].get("BACKGROUND_IMAGE", None)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background_color attribute is missing (same for other progress bars in this file)

@Ebag333
Copy link
Contributor Author

Ebag333 commented Jun 6, 2022

Yeah definitely need to handle not having an nVidia GPU. I'm not sure yet the best way of handling that, we could add a field to set your GPU type and skip nVidia if you don't have it, that makes the most sense to me. If you disable the GPU stats in the config, it won't try and run that, but that disables all GPU stats (which is kinda fine ATM, since there's no support for AMD/Intel).

I think doing a config setting makes the most sense since most people won't have cases where they actively use a AMD and nVidia GPU at the same time.

I'm also periodically getting an error when running for long periods of time (possibly only when PC is locked):

Exception in thread Queue_Handler:
Traceback (most recent call last):
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\threading.py", line 946, in run
    self._target(*self._args, **self._kwargs)
  File "C:\Users\gabri\PycharmProjects\turing-smart-screen-python-ebag\library\scheduler.py", line 50, in wrap
    scheduler.run()
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.1264.0_x64__qbz5n2kfra8p0\lib\sched.py", line 151, in run
    action(*argument, **kwargs)
  File "C:\Users\gabri\PycharmProjects\turing-smart-screen-python-ebag\library\scheduler.py", line 40, in periodic
    action(*actionargs)
  File "C:\Users\gabri\PycharmProjects\turing-smart-screen-python-ebag\library\scheduler.py", line 109, in QueueHandler
    f(*args)
  File "C:\Users\gabri\PycharmProjects\turing-smart-screen-python-ebag\library\lcd_comm.py", line 33, in WriteData
    ser.write(bytes(byteBuffer))
  File "C:\Users\gabri\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\serial\serialwin32.py", line 317, in write
    raise SerialException("WriteFile failed ({!r})".format(ctypes.WinError()))
serial.serialutil.SerialException: WriteFile failed (PermissionError(13, 'The device does not recognize the command.', None, 22))

I'm thinking the best way of dealing with this is to stop updating if the PC locks, possibly also black out the screen. I'd personally like this so the display will last even longer anyway. I'm leaning toward trying to sneak that in this PR, but it's already pretty big so if you want to push that feature add off I'm okay with that.

Thanks for the code review, I'll check on the rest of your comments.

@davehaglan
Copy link

Hi,

I pulled Ebag33's repo and this is pretty cool. I was able to get CPU temperature working on linux. I added the method below to the cpu class in stats.py:

   @staticmethod
   def temperature():
       cpu_temp = psutil.sensors_temperatures()['coretemp'][0].current

       if CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("SHOW", False):
           lcd.DisplayText(
               ser=config.lcd_comm,
               text=f"Temperature: {cpu_temp:0.0f}* c",
               x=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("X", 0),
               y=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("Y", 0),
               font=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("FONT", "roboto/Roboto-Regular.ttf"),
               font_size=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("FONT_SIZE", 10),
               font_color=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("FONT_COLOR", (0, 0, 0)),
               background_color=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("BACKGROUND_COLOR", (255, 255, 255)),
               background_image=CONFIG_DATA['STATS']['CPU']['TEMPERATURE']['TEXT'].get("BACKGROUND_IMAGE", None)
           )

Code added to scheduler.py:

@async_job("CPU_Temperature")
@schedule(timedelta(seconds=CONFIG_DATA['STATS']['CPU']['TEMPERATURE'].get("INTERVAL", None)).total_seconds())
def CPUTemperature():
    """ Refresh the CPU Temperature """
    # print(f"Refresh CPU Temperature = {stats.CPU.temperature():d}")  
    stats.CPU.temperature()

Add a line to main and the settings to CONFIG.yaml and you get the CPU temperature displayed on linux.

The only thing that would make this cooler is to add the ability to rotate the screen.

Thanks for the awesome code.

@mathoudebine mathoudebine changed the base branch from main to feature/system-monitor August 22, 2022 17:53
@mathoudebine mathoudebine force-pushed the feature/system-monitor branch from 6e18ed1 to 4278f69 Compare August 22, 2022 17:54
@mathoudebine mathoudebine merged commit 50e10f6 into mathoudebine:feature/system-monitor Aug 22, 2022
@mathoudebine
Copy link
Owner

Hi @Ebag333 and @davehaglan
I merged the overhaul branch to a local branch of this project: feature/system-monitor
This way i can also work on how to improve it and write the documentation before merging on master
Thanks

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

Successfully merging this pull request may close these issues.

3 participants