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

added: additional monitors and status info for cpu freq and vram usage #373

Merged

Conversation

ProPuke
Copy link
Contributor

@ProPuke ProPuke commented Aug 29, 2024

This adds some displays for cpu frequency and vram usage on the indicator (I'm playing with amd-pstate at the moment, and wanted to be able to monitor the cpu freq on the fly)
I've also reordered them a little on the bar and added some separators in settings to hopefully group them a bit clearer.
I improvised a bit on the vram icon. Hopefully somebody else can improve or do something slight better.

I also added some more status bar items to show gpu and vram usage there.
While doing so I noticed that under nvidia the vram monitor wasn't actually showing percentage of vram usage, but fb usage. So I corrected that to show percentage of actual total vram in use, hopefully matching the intent and what happens under amd.

We also weren't ensuring gpu != null in one case (possible crash? :S) so I corrected that.

I've not done anything about i18n. I don't know how that works :D (Do I need to run a script of some kind to update the translation strings?)

added: additional status bar icons for gpu and vram
fixed: nvidia vram indicator was displaying fb usage, not vram
fixed: possible crash case when a gpu was not detected
@stsdc
Copy link
Member

stsdc commented Aug 30, 2024

Thanks for the PR!
I'll try to check it thru the weekend.

@stsdc
Copy link
Member

stsdc commented Sep 6, 2024

After building and installing, and restarting wingpanel I got this error for wingpanel

(io.elementary.wingpanel:128627): GLib-GIO-ERROR **: 23:36:46.129: Settings schema 'com.github.stsdc.monitor.settings' does not contain a key named 'indicator-temperature-state'

@stsdc
Copy link
Member

stsdc commented Sep 6, 2024

Ok, obviously I had indicator build and installation disabled

@ProPuke
Copy link
Contributor Author

ProPuke commented Sep 6, 2024

As a note:

I removed indicator-temperature-state and instead replaced it with indicator-cpu-temperature-state and indicator-gpu-temperature-state
(there is still some commented code making ref to the old indicator-temperature-state, I probably should have tidied that, should you want to enable it for testing)

@stsdc
Copy link
Member

stsdc commented Sep 6, 2024

Codewise and naming is ok. I will try to polish UI a bit and will merge it. Thanks a lot!

@ProPuke
Copy link
Contributor Author

ProPuke commented Sep 6, 2024

Cheers! Thank you

@stsdc
Copy link
Member

stsdc commented Sep 6, 2024

I have removed vram from statusbar. It becomes too clunky IMO.
Need to consider if statusbar needs these cpu, ram etc at all if there is an Indicator

@ProPuke
Copy link
Contributor Author

ProPuke commented Sep 6, 2024

Maybe adding gpu stuff to the status bar was a bit too much.
Just cpu, ram and swap is likely enough if someone wants an at a glance summary on processes running (and I guess maybe gpu usage)
If they want more details they can funnel down into System and view more in-depth info there.
Yeah, I'd over-complicated it.

I'd still keep the status info there if indicators are on, though. If people expect something to be somewhere it should be there, even if they've enabled showing it somewhere else. Don't go against expectations.

@stsdc stsdc merged commit 38d0093 into elementary:dev Sep 7, 2024
2 checks passed
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