fix: Fix GPU temperature conversion in the popup chart

[maxd]

This PR fixes GPU temperature conversion in the popup chart after an incorrect fix in the fb63ece commit related to the #1809 issue.

Before this fix the GPU chart show incorrect temperature value:

As you can see it shows 0C instead of 48C and 3300F instead of 118F.

After this fix it should show the correct values:

[exelban]

hi. Please use temperature function to convert the temperature.

[BenTalagan]

Hi, any update on this PR ? It adresses an annoying bug, and it'd be awesome to get it finalized ! Thx for the hard work.

[maxd]

@exelban The temperature function is responsible for converting and formatting. I can't use it here because I only need the converting part. I suppose there are no ways to solve described problem without splitting this function into two.

So, I decided to update my PR and refactor this function. Now, the temperature function will be responsible for converting to a selected temperature unit, and the format extension function will be responsible for formatting.

Is this an acceptable solution, or can you provide a better one?

[exelban]

hi. Sorry for not responding for a long time.

Could you please provide the value that are in value for a temperature? There is no this value on the Apple silicon. So I cannot check that by myself.

I want to understand why you make these changes in the temperature function.

[maxd]

Here are the values of value variable from Modules/GPU/popup.swift#update method (in master branch pointing to fc4ed263c576df3dc7c8dd184c026982ec66d205 commit):

and values in GPU charts at this moment:

As you can see the charts shows invalid values.

With my fixes they show the right values:

P.S. These values were collected on my MackBook Pro 2017:

[exelban]

hmm, looks similar to #1984 please check the master, maybe it's fixes that problem too.

[maxd]

hmm, looks similar to #1984

Yes, this is the same problem.

please check the master, maybe it's fixes that problem too.

My previous tests were on the master (fc4ed263c576df3dc7c8dd184c026982ec66d205) branch in my forked repository. I updated the master branch from the upstream repository (now it points to f2858f80b8fd6833238d9202ef1dd85ad9536396 commit) and repeated tests. They show the same problem with the charts described in the previous comment.

P.S. Rebased this PR to the upstream master.

[exelban]

1984 is already fixed in the master by f2858f8. Could you check if it fix your problem?

[maxd]

As I said in my previous comment the current master branch pointed to f2858f8 commit has the same problem.

Could you please point to the commit where #1984 was fixed? This issue is open and doesn't have related PR.

[exelban]

oh, sorry. I thought I had pushed that issue already. Will add that tomorrow.

[exelban]

@maxd hi, please check this commit.

[maxd]

It works but in this case, you make a lot of useless operations:

1) This code converts source value in Celsius degrees to units specified in settings 2) Then it formats the converted value into a string 3) Then this code removes the C and F sub-strings 4) Then the digits function extracts digits from a string 5) Then the string of digits converts to Double type

In my first commit, I showed how to fix the problem using the operation from the first item and avoid the rest of the useless operations (from 2 to 5 items).

Are you sure you want to use such an unoptimal solution to this problem?

[exelban]

Ok, since the problem solved I'm going to close that issue.