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

info-container cpu-core temperature colouring not working #867

Closed
Abasz opened this issue Oct 23, 2017 · 19 comments
Closed

info-container cpu-core temperature colouring not working #867

Abasz opened this issue Oct 23, 2017 · 19 comments

Comments

@Abasz
Copy link

Abasz commented Oct 23, 2017

Hi,

I noticed that on the right info bar the cpu temperatures are not colered properly (I discovered this because I wanted to add Fan speed and System temp to the Cpu temp info).

I think the above is do to the following:
In the bundle.js the bg-info, bg-success has a class of --bg-info and --bg-success so these classes are not recognised. Furthermore, in the CSS bg-info and bg-success needs an !important tag otherwise the general background overrides the colouring.

I do not know whether this is intentional or not (although I remember that in earlier versions of the theme cpu and hdd temp were only visible on the info-container if these were overheating).

image

@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

I'm not sure how to add CPU fan to data output?

Yes, there is a bug. Thanks a lot for reporting. Fixing now.

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

do not worry about CPU fan staff :) (actually webmin system-status-lib.pl needed to be edited for that)

@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

Can you give me the RPM speed for low, normal, warning and danger?

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

That depends a lot on the configuration.

For my setting low is below 1600 normal is 1600 to 2100, warning is 2100-2800; danger is above 2800

iliajie pushed a commit that referenced this issue Oct 23, 2017
…mation in Side Slider #867

By default, sensor information is hidden in Side Slider, unless warning or danger levels are reached.
Mentioned behaviour can be bypassed and labels forced to be shown constantly, by adding following variables to Theme Extensions/JavaScript:
`config_custom_force_display_cpu_sensors` and/or `config_custom_force_display_drive_sensors`.
@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

I made a patch for fan speed as well.

You can also now force to show sensors data in Side Slider.

Please give it a try.

By default, sensor information is hidden in Side Slider, unless warning or danger levels are reached.
Mentioned behaviour can be bypassed and forced to be shown constantly, by adding variables to Theme Extensions/JavaScript:

`config_custom_force_display_cpu_sensors` and/or `config_custom_force_display_drive_sensors`.

@iliajie iliajie closed this as completed Oct 23, 2017
@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

Tested and it almost works:

For unknown reasons the hdd temp is not loaded properly (only every second span is loaded)

image

@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

What it's shown on System Information page? Can you make screenshot out of it?

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

image

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

One additional though: Would it make sense to implement the same colouring on the main dashboard as on the right info container?

@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

Yes, makes senses.

@iliajie
Copy link
Collaborator

iliajie commented Oct 23, 2017

Looking at your screenshot I have no idea yet why drive temperature not shown in the Right Slider.

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

Ok I think I konw where the problem comes from:

When you assemble the spans from the main dash board data I think the   is copied instead of the temp data:

<span class="badge-custom badge-drivestatus" data-stats="drive" style="margin-right:3px; margin-bottom: 3px">sda: 38°C </span>&nbsp;
<span class="badge-custom badge-drivestatus" data-stats="drive" style="margin-right:3px; margin-bottom: 3px">sdb: 37°C </span>&nbsp;

<span class="badge-custom badge-drivestatus" data-stats="drive" style="margin-right:3px; margin-bottom: 3px">&nbsp;</span>

This would explain why on the screen shot 37 degrees is shown on as the temp for the 3rd hdd (that is actually the temp for the second)

@Abasz
Copy link
Author

Abasz commented Oct 23, 2017

Hi, I think I found the solution (and the bug) for the incorrect render of the HDD temp:

In boundle js there is a line:


if ("hdd_temperature" == e) {
                            var _ = 0;
                            $.each($(i), function....

I was suspecting that the iteration does not go through correctly. So in case we specify the selector as below, the spans are rendered correctly:

$.each($(".badge-drivestatus:not(.badge-cpustatus)"), function(e, t) {

by the way I was wondering whether the uniminify js and css is available somewhere (or it is not made public on purpose :))

@iliajie
Copy link
Collaborator

iliajie commented Oct 25, 2017

Hi,

How about latest fix 9bdcff1, give it a try please. Does it work perfect now?

@Abasz
Copy link
Author

Abasz commented Oct 26, 2017

Hi,
Thanks for the update,

I difined the RPM limit in themeextention (really nice touch!)

The colouring logic on the main page is not 100% working on the RPM (the other parts I think working):
image

function page_sysinfo_update_temperature_labels() {
    var e = $("#system-status .badge-drivestatus");
    e.length && $.each(e, function(e, t) {
        var i = $(this).text()
          , a = parseInt(i.split(":")[1])
          , n = i.indexOf("°C") > -1
          , s = HTML.label.temperature(a, n);
        Test.string(s) && $(this).removeClass(function(e, t) {
            return (t.match(/(^|\s)bg-\S+/g) || []).join(" ")
        }).addClass(s)
    })

I think you might have missed the RPM check from the above code like you included in the information_update. Change the s variable to the following would solve the issue:

, s = i.indexOf("RPM") > -1 ? HTML.label.rpm(a) : HTML.label.temperature(a, n);

In addition one small note: from a styling perspective I kind of liked the old oval forms for the temperatures:

image

But of course I can change this in theme extension for if I wan to :)

@iliajie
Copy link
Collaborator

iliajie commented Oct 26, 2017

Okay, right.

How about now?

cf75446

@Abasz
Copy link
Author

Abasz commented Oct 26, 2017

Seems to be working nicely! Thanks for your continuous support

iliajie pushed a commit that referenced this issue Nov 3, 2017
…mation in Side Slider #867

By default, sensor information is hidden in Side Slider, unless warning or danger levels are reached.
Mentioned behaviour can be bypassed and labels forced to be shown constantly, by adding following variables to Theme Extensions/JavaScript:
`config_custom_force_display_cpu_sensors` and/or `config_custom_force_display_drive_sensors`.
@iliajie
Copy link
Collaborator

iliajie commented Nov 17, 2021

.. very surprising and confusing at the same time, that you had RPM output back then .. perhaps, Jamie changed something in the past .. but there should had been no fan speed until now ..

@jcameron
Copy link
Collaborator

I have accepted some commits to this code a few months ago.

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

No branches or pull requests

3 participants