Hi,

Am 13.03.2023 um 10:23 schrieb Michael Tremer:

Hello,

Reviewed-by: Michael Tremer michael.tremer@ipfire.org

...

On 13 Mar 2023, at 07:35, Leo-Andres Hofmann hofmann@leo-andres.de wrote:

This fixes an issue where systems with many CPU cores run out of usable graph colors and canvas area. It also unifies the canvas size for all graphs.

Discussion: https://community.ipfire.org/t/cpu-freq-graph-not-working-with-i5-1235u/9396

Fixes: #12890

Signed-off-by: Leo-Andres Hofmann hofmann@leo-andres.de

config/cfgroot/graphs.pl | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/config/cfgroot/graphs.pl b/config/cfgroot/graphs.pl index 3368e5aad..cefe0b90d 100644 --- a/config/cfgroot/graphs.pl +++ b/config/cfgroot/graphs.pl @@ -30,8 +30,11 @@ require '/var/ipfire/general-functions.pl'; require "${General::swroot}/lang.pl"; require "${General::swroot}/header.pl";

-# Graph image size in pixel -our %image_size = ('width' => 910, 'height' => 300); +# Approximate size of the final graph image including canvas and labeling (in pixels, mainly used for placeholders) +our %image_size = ('width' => 900, 'height' => 300);

+# Size of the actual data area within the image, without labeling (in pixels) +our %canvas_size = ('width' => 800, 'height' => 190);

# List of all available time ranges our @time_ranges = ("hour", "day", "week", "month", "year"); @@ -48,15 +51,12 @@ my @GRAPH_ARGS = ( # For a more 'organic' look "--slope-mode",

• # HxW define the size of the output image
• "--full-size-mode",

# Watermark "-W www.ipfire.org",

• # Default size
• "-w $image_size{'width'}",
• "-h $image_size{'height'}",

• # Canvas width/height
• "-w $canvas_size{'width'}",
• "-h $canvas_size{'height'}",

# Use alternative grid "--alt-y-grid", @@ -1090,18 +1090,19 @@ sub updatecpufreqgraph { "--color=SHADEA".$color{"color19"}, "--color=SHADEB".$color{"color19"}, "--color=BACK".$color{"color21"},

• "COMMENT:".sprintf("%-10s",$Lang::tr{'caption'}),

• "COMMENT:".sprintf("%-15s",$Lang::tr{'caption'}),

"COMMENT:".sprintf("%15s",$Lang::tr{'maximal'}), "COMMENT:".sprintf("%15s",$Lang::tr{'average'}), "COMMENT:".sprintf("%15s",$Lang::tr{'minimal'}), "COMMENT:".sprintf("%15s",$Lang::tr{'current'})."\j" );

• my $j = 11;

for(my $i = 0; $i < $cpucount; $i++) {

• my $j=$i+1;

• $j++; $j = 1 if $j > 20;

I think you could have written this slightly shorter by taking the modulus of j like so: $j = $i % 20;

But since this is not changing the behaviour, I am happy to see this merged.

Thanks for the review. Yes of course, no idea why I didn't think of that...

Best, Leo

-Michael

...

push(@command,"DEF:cpu".$i."_=".$mainsettings{'RRDLOG'}."/collectd/localhost/cpufreq/cpufreq-".$i.".rrd:value:AVERAGE" ,"CDEF:cpu".$i."=cpu".$i."_,1000000,/"

• ,"LINE1:cpu".$i.$color{"color1$j"}."A0:cpu ".$i." "

• ,"LINE1:cpu".$i.$color{"color$j"}."A0:cpu ".$i." "

,"GPRINT:cpu".$i.":MAX:%3.0lf Mhz" ,"GPRINT:cpu".$i.":AVERAGE:%3.0lf Mhz" ,"GPRINT:cpu".$i.":MIN:%3.0lf Mhz" -- 2.37.1.windows.1