From mboxrd@z Thu Jan  1 00:00:00 1970
From: Leo-Andres Hofmann <hofmann@leo-andres.de>
To: development@lists.ipfire.org
Subject:
 Re: [PATCH] graphs.pl: Make canvas fixed size and limit cpufreq color index
Date: Tue, 14 Mar 2023 08:12:38 +0100
Message-ID: <19aff53d-36de-5d45-1bb7-89d8eae7a0b5@leo-andres.de>
In-Reply-To: <358CBBDE-3C7F-48AF-8994-94316DB07FE7@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============4324034551316193140=="
List-Id: <development.lists.ipfire.org>

--===============4324034551316193140==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

Am 13.03.2023 um 10:23 schrieb Michael Tremer:
> Hello,
>
> Reviewed-by: Michael Tremer <michael.tremer(a)ipfire.org>
>
>> On 13 Mar 2023, at 07:35, Leo-Andres Hofmann <hofmann(a)leo-andres.de> wro=
te:
>>
>> 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(a)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 =3D ('width' =3D> 910, 'height' =3D> 300);
>> +# Approximate size of the final graph image including canvas and labeling=
 (in pixels, mainly used for placeholders)
>> +our %image_size =3D ('width' =3D> 900, 'height' =3D> 300);
>> +
>> +# Size of the actual data area within the image, without labeling (in pix=
els)
>> +our %canvas_size =3D ('width' =3D> 800, 'height' =3D> 190);
>>
>> # List of all available time ranges
>> our @time_ranges =3D ("hour", "day", "week", "month", "year");
>> @@ -48,15 +51,12 @@ my @GRAPH_ARGS =3D (
>> # 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=3DSHADEA".$color{"color19"},
>> "--color=3DSHADEB".$color{"color19"},
>> "--color=3DBACK".$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 =3D 11;
>> for(my $i =3D 0; $i < $cpucount; $i++) {
>> - my $j=3D$i+1;
>> + $j++; $j =3D 1 if $j > 20;
> I think you could have written this slightly shorter by taking the modulus =
of j like so: $j =3D $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."_=3D".$mainsettings{'RRDLOG'}."/collectd/local=
host/cpufreq/cpufreq-".$i.".rrd:value:AVERAGE"
>> ,"CDEF:cpu".$i."=3Dcpu".$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"
>> --=20
>> 2.37.1.windows.1
>>

--===============4324034551316193140==--