search

kotelnik / plasma-applet-thermal-monitor

Plasma 5 applet for monitoring CPU, GPU and other available temperature sensors.
GNU General Public License v2.0
111 stars 63 forks source link

Added support for Kelvin scale. #33

Closed kscd closed 7 years ago

kscd commented 7 years ago

Hello,

I wanted to see the temperature of my CPU Cores in Kelvin and since this widget didn't support this functionality, I added it. Apart from my personnel preference, I think the Kelvin scale should be added simply because it is the SI unit of the temperature and thusly the 'most correct' unit.

Adding support for more than two temperature units required changes to the way the switching of the unit is handled. Until now there existed a boolean variable 'fahrenheitEnabled' which decided which unit to use. For more than two units this approach fails. As a replacement for 'fahrenheitEnabled' I created the variable 'temperatureUnit', which stores the unit as a string, and adjusted every function to work with 'temperatureUnit' rather than 'fahrenheitEnabled'. This allows for an arbitrarily high number of units which can be introduced now.

Additionally I created the function 'toKelvin(celsia)' to calculate the value in Kelvin from the Celsius value. This is slightly redundant to the function 'toCelsia(kelvin)' which does the opposite, but since it is explicitly stated that Celsius shall be the default scale, I didn't changed that.

Last but not least, I made the appearance of the degree sign '°' dependent on the used temperature unit. For Celsius and Fahrenheit it is displayed, for Kelvin not.

I tested these changes on a machine with Debian Stretch and KDE installed and it works just fine.

kotelnik commented 7 years ago

Hi! Thanks a lot for this pull! Unfortunately it was based on some old revision of the code. The master was not compatible so I incorporated your changes to the master version of this plasmoid. Please check it out if it work OK for you. Thanks again :).

kotelnik commented 7 years ago

Sorry, I mixed this with weather widget, I'm probably tired... please wait. At least this change got even there, funny right? :)

kotelnik commented 7 years ago

OK, now it is finally merged in proper repository, thanks yet again! I alter your code so the temperatureUnit is enum instead of string. It feels more natural to me, I hope you don't mind.

kscd commented 7 years ago

Hi,

thank you for merging my pull request. To choose 'enum' instead of 'string' is indeed the better choice. I somehow didn't thought of this. Thank you for correcting this.