luci
luci copied to clipboard
openwrt
luci-mod-dsl: initial commit
This commit imports the Luci DSL Spectrum graph into LUCI. The spectrum graph shows up as an option in the toplevel status menu. Code works completely standalone, no dependency on other packages (e.g. chart.js)
Signed-off-by: Roland Barenbrug [email protected]
I've had a look at it now. I think the feature is great. But I noticed that it doesn't look good with different themes. I have the darktheme from bootstrap installed. But apart from that, I have always missed such a feature in OpenWrt. I also don't know why I see this yellow bar. Which version stack are you using with the lantiq dsl?
Below a screendump of how it looks like without any theme (or the default theme). Let me have a look how the graph can anticipate the theme being used. Valid comment, never considered this as I only use the defulat one.
@feckert would this be better ?
Btw, running on recent version of OpenWRT (Powered by LuCI Master (git-22.137.71281-d6dbedd) / OpenWrt SNAPSHOT r19781-d55f12cc79). Could you please share the output of sbin/dsl_cpe_pipe.sh with the following parameters:
- g997sansg 0
- g997sansg 1
- g997bansg 0
- g997bansg 1
With that I'll be able to check why you get this long yellow bar.
You need to properly handle scaling for the SNR data, as the group size isn't always 8. There is a maximum of 512 subcarrier groups, and the group size depends on how many subcarriers are actually in use. For ADSL the group size is always 1 (as there is only a maximum of 512 subcarriers), but for VDSL2 the group size can even be different for up- and downstream (for example on a line with profile 35b, the downstream group size is typically 16, while it is 8 for upstream data).
As g997sansg does not report the group size, it makes sense to use g997dsnrg instead (the first parameter of that command is the direction, the second is the data type and needs to be 1 for showtime data). If you implement support for the output of that command, it should also be easy to add support for QLN and Hlog using g997dqlng and g997dhlogg.
Here is data from a VDSL2 profile 35b line (from my own standalone tool): dsl_20220520_195501_raw.txt If it would be helpful for testing, I can also provide data for other VDSL2 profiles (8a/b/c/d, 12a/b, 30a).
You need to properly handle scaling for the SNR data, as the group size isn't always 8. There is a maximum of 512 subcarrier groups, and the group size depends on how many subcarriers are actually in use. For ADSL the group size is always 1 (as there is only a maximum of 512 subcarriers), but for VDSL2 the group size can even be different for up- and downstream (for example on a line with profile 35b, the downstream group size is typically 16, while it is 8 for upstream data).
As
g997sansgdoes not report the group size, it makes sense to useg997dsnrginstead (the first parameter of that command is the direction, the second is the data type and needs to be 1 for showtime data). If you implement support for the output of that command, it should also be easy to add support for QLN and Hlog usingg997dqlngandg997dhlogg.Here is data from a VDSL2 profile 35b line (from my own standalone tool): dsl_20220520_195501_raw.txt If it would be helpful for testing, I can also provide data for other VDSL2 profiles (8a/b/c/d, 12a/b, 30a).
Hi @janh , thnx for your feedback. Let me cook on this for some days and I'll come back.
@janh , @feckert updated the commit to take care of
- better colors in case of dark-mode themes
- handling sub-carriers depending on their number (scaling)
- added graphs covering QLN and HLOG
Please check and share your results.
Scaling in x-direction looks good now: vdsl2-35b.png In some cases, the x value range may not be the same for each graph and the graphs don't align (but it isn't wrong): vdsl2-8b.png
The scaling in y-direction for QLN and Hlog is broken, though. It should be -23 - v/2 for QLN, and 6 - v/10 for Hlog.
Another thing I noticed is that small details may be hidden due to the way the graphs are drawn. In the profile 8b example, there are actually quite a few invalid SNR values around the range of subcarriers 128-256. However, the graph fails to show that. Here is the raw data: dsl_20220729_185621_raw.txt
Scaling in x-direction looks good now: vdsl2-35b.png In some cases, the x value range may not be the same for each graph and the graphs don't align (but it isn't wrong): vdsl2-8b.png
The scaling in y-direction for QLN and Hlog is broken, though. It should be
-23 - v/2for QLN, and6 - v/10for Hlog.Another thing I noticed is that small details may be hidden due to the way the graphs are drawn. In the profile 8b example, there are actually quite a few invalid SNR values around the range of subcarriers 128-256. However, the graph fails to show that. Here is the raw data: dsl_20220729_185621_raw.txt
@janh, are you sure regarding the value translation? Reading the ITU recommendation:
7.10.9.3 Downstream QLN(f) (QLNpsds)
Description: Reports the downstream QLN(f) as one value per sub-carrier group. A first special value
(255) indicates that the QLN(f) for this sub-carrier group is undetermined. A second special value
(254) indicates that no measurement could be done for this sub-carrier group because it is out of the
downstream MEDLEY set. A third special value (0) indicates that the QLN(f) for this sub-carrier
group is greater than or equal to −35dBm/Hz. A fourth special value (251) indicates that the QLN(f)
for this sub-carrier group is less than or equal to −160.5 dBm/Hz. The object is updated only in L0
link state.
Type: Array of QLNdescriptor[0…N–1], where N = index of highest sub-carrier index in the
downstream MEDLEY set, divided by QLNGds, rounded to the higher integer.
QLNdescriptor:
Type: 8-bit unsigned integer.
Unit: −0.5 dBm/Hz.
Offset: −35 dBm/Hz.
Valid values: 1..250 (−35.5 to –160 dBm/Hz), 0, 251, 254, 255 (special values).
HLOG is indeed wrong, had to remove the minus sign:
7.10.7.3 Downstream HLOG(f) (HLOGpsds)
Description: Reports the downstream Hlog(f) as one value per sub-carrier group. A first special value
(1023) indicates that the Hlog(f) for this sub-carrier group is undetermined. A second special value
(1022) indicates that no measurement could be done for this sub-carrier group because it is out of the
MEDLEY set. A third special value (0) indicates that the Hlog(f) for this sub-carrier group is greater
than or equal to +6.0 dB. A fourth special value (1020) indicates that the Hlog(f) for this sub-carrier
group is less than or equal to −96.0 dB. The object is updated only in L0 link state.
Type: Array of HLOGdescriptor[0…N–1], where N = index of highest sub-carrier index in the
MEDLEY set, divided by HLOGG, rounded to the higher integer.
HLOGdescriptor:
Type: 16-bit unsigned integer.
Rec. ITU-T G.997.2 (03/2019) 57
Unit: −0.1 dB.
Offset: 6 dB.
Valid values: 1…1019 (+5.9 to –95.9 dB) and 0,1020,1022,1023 (special values).
Reference: Clause 11.4.1.2.1 of [ITU-T G.9701].
Further, regarding the x value range, I checked that data and with the current code, the result is the expected. The x value range is determined by the product of nGroupSize x nNumData. In the example this is 4 x 1024 = 4096. In fact this is strange because looking at the content, nNumData equal to 512 would have been sufficient.
@janh, are you sure regarding the value translation? Reading the ITU recommendation:
7.10.9.3 Downstream QLN(f) (QLNpsds)
This seems to be from G.997.2, which applies to G.fast. For xDSL, the QLN value is encoded differently. If you look in G.993.2 (VDSL2) or G.997.1 (Physical layer management for DSL transceivers), you'll find that -23 is used as offset (the reserved values are also different for xDSL).
@janh, are you sure regarding the value translation? Reading the ITU recommendation:
7.10.9.3 Downstream QLN(f) (QLNpsds)This seems to be from G.997.2, which applies to G.fast. For xDSL, the QLN value is encoded differently. If you look in G.993.2 (VDSL2) or G.997.1 (Physical layer management for DSL transceivers), you'll find that -23 is used as offset (the reserved values are also different for xDSL).
Clear, wrong recommendation. Fixed.
Just need to decide on a way to represent the out of range values.
One way to represent the error values is by plotting a little red cross as in the example below (below minimum at index 0, above maximum at index 64). The cross is positioned are the defined minimum or maximum value depending on the error. Would that be a sensible way forward ?
In my opinion there is no need to draw missing values in a special way. For xDSL there is only a single special value anyway, which doesn't tell why the value is not valid (and being outside the supported/used carrier set is the most likely reason).
I think fixing the issue I mentioned in my previous comment instead would be enough. It looks to me like the cause is that each data point is drawn using separate overlapping rectangles. Drawing the entire data as a single path should avoid this issse (the drawing code in my own tool does it that way).
Created a new commit with rewritten drawing routing. Rather than drawing bar by bar, it first draws the outline and then fills the graph. This is done block by block. Also decided to replace the original Fritzbox colors by something more decent.
Mmh, guess these are the red lines in the json files. Really need some help how to solve this.
Tested OK @FB7362SL .But to be fair I've placed files directly in live router and restarted rpcd
It doesn't work properly here, there is an exception when drawing downstream SNR for VDSL2 profile 35b (looks like the code assumes a group size of 8 there):
Looking for 65 859
Found 4 -1
Uncaught TypeError: this.dataPoints[endIndex] is undefined
drawBlock …/luci-static/resources/view/status/dsl/dsl-graph-new.js:22
drawBlocks …/luci-static/resources/view/status/dsl/dsl-graph-new.js:18
drawChart …luci-static/resources/view/status/dsl/dsl-graph-new.js:17
<anonymous> …/luci-static/resources/view/status/dsl/dsl-graph-new.js:37
I am not sure if drawing each band separately is a good idea at all. During showtime state, the bbsg command returns values for the MEDLEY set. However the QLN and Hlog data is available for the SUPPRTEDCARRIERS set, which is a superset of the MEDLEY set. So, a part of the data may actually be cut off in some cases. Even for SNR, you would need to use the limit values instead of the border values (the former is based subcarriers that are allowed to be used, the latter is based only on subcarriers that are currently used).
And in any case, the drawing code also needs to be able to handle invalid/zero values within a band, so I don't think drawing bands separately actually adds any value. If performance is a concern, it may help to omit redundant points from the path (i.e. when subsequent values are identical).
Other than that, I can confirm that invalid values are now drawn correctly. (It looks like the code still handles special values as defined for G.fast, though).
All four graphs are now working here.
That's great news. Let's wait for some more review results. Won't touch this for a while until there is some feedback.
@turboproc I'm not sure what's going wrong here, but git seems to suggest there's duplicate files in the PR somehow? I grabbed the plain patch (done by appending .patch to the PR URL). These are the errors git is throwing:
$ git am ../../../patches/luci/0001-luci-mod-dsl.patch
Applying: luci-mod-dsl: initial commit
.git/rebase-apply/patch:57: trailing whitespace.
}
.git/rebase-apply/patch:99: trailing whitespace.
]
warning: 2 lines add whitespace errors.
Applying: luci-mod-dsl: initial commit
.git/rebase-apply/patch:81: trailing whitespace.
//console.time();
.git/rebase-apply/patch:83: trailing whitespace.
const dsSnrData = new DataSet(document.getElementById("dsdB").innerText, mySnrFunction);
.git/rebase-apply/patch:174: trailing whitespace.
]
.git/rebase-apply/patch:367: trailing whitespace.
ctx.fillStyle = "#A0A0A0";
error: modules/luci-mod-dsl/Makefile: already exists in index
error: modules/luci-mod-dsl/htdocs/luci-static/resources/view/status/dsl/dsl-graph-new.js: already exists in index
error: modules/luci-mod-dsl/htdocs/luci-static/resources/view/status/dsl/dsl-spectrum.js: already exists in index
error: modules/luci-mod-dsl/root/usr/share/luci/menu.d/luci-mod-status-dsl.json: already exists in index
error: modules/luci-mod-dsl/root/usr/share/rpcd/acl.d/luci-mod-status-dsl.json: already exists in index
Patch failed at 0002 luci-mod-dsl: initial commit
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
When I inspect the plain patch, I see it creating modules/luci-mod-dsl/Makefile twice, but also other files (the JavaScript files differ in size between patch 1/7 and 2/7 in your patch set though). E.g. dsl-graph-new.js is being added once (patch 1/7, 201 lines of code) and then a second time (patch 2/7, 373 lines of code). I've uploaded the patch here. Result of wget https://patch-diff.githubusercontent.com/raw/openwrt/luci/pull/5895.patch -O 0001-luci-mod-dsl.patch.
@Borromini , not sure what happened. When checking the files in the changed files tab all files look OK.
I have seen in the history, that you have a merge commit. To fix your git issue you could do the following:
-
git reset HEAD~8remove your last commits with out removing our changes -
git add .add your changes -
git push --forceMake a push force to your pullrequest branch
It's a bit late, but shouldn't we consider using the Ubus to get the information we need? I am thinking of
checkmkorgrafana(without time series input) to push this information to these backends as well. Also the advantage would be that we have the whole thing as json. But I am not sure if this works? I have seen that the output of the command is large.
Have been looking into this when starting this topic. Concluded that in that case the ubus connector would require extension as the information required is not exposed yet by any ubus call. For now it works, indeed the data provided is not in JSON format but with some effort it was possible to parse all data.
I have seen in the history, that you have a merge commit. To fix your git issue you could do the following:
git reset HEAD~8remove your last commits with out removing our changesgit add .add your changesgit push --forceMake a push force to your pullrequest branch
Thnx for the suggestion. Will give it a try (just afraid of losing information with this kind of steps).
@turboproc I'm not sure what's going wrong here, but git seems to suggest there's duplicate files in the PR somehow? I grabbed the plain patch (done by appending
.patchto the PR URL). These are the errors git is throwing:$ git am ../../../patches/luci/0001-luci-mod-dsl.patch Applying: luci-mod-dsl: initial commit .git/rebase-apply/patch:57: trailing whitespace. } .git/rebase-apply/patch:99: trailing whitespace. ] warning: 2 lines add whitespace errors. Applying: luci-mod-dsl: initial commit .git/rebase-apply/patch:81: trailing whitespace. //console.time(); .git/rebase-apply/patch:83: trailing whitespace. const dsSnrData = new DataSet(document.getElementById("dsdB").innerText, mySnrFunction); .git/rebase-apply/patch:174: trailing whitespace. ] .git/rebase-apply/patch:367: trailing whitespace. ctx.fillStyle = "#A0A0A0"; error: modules/luci-mod-dsl/Makefile: already exists in index error: modules/luci-mod-dsl/htdocs/luci-static/resources/view/status/dsl/dsl-graph-new.js: already exists in index error: modules/luci-mod-dsl/htdocs/luci-static/resources/view/status/dsl/dsl-spectrum.js: already exists in index error: modules/luci-mod-dsl/root/usr/share/luci/menu.d/luci-mod-status-dsl.json: already exists in index error: modules/luci-mod-dsl/root/usr/share/rpcd/acl.d/luci-mod-status-dsl.json: already exists in index Patch failed at 0002 luci-mod-dsl: initial commit hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead.When I inspect the plain patch, I see it creating
modules/luci-mod-dsl/Makefiletwice, but also other files (the JavaScript files differ in size between patch 1/7 and 2/7 in your patch set though). E.g.dsl-graph-new.jsis being added once (patch 1/7, 201 lines of code) and then a second time (patch 2/7, 373 lines of code). I've uploaded the patch here. Result ofwget https://patch-diff.githubusercontent.com/raw/openwrt/luci/pull/5895.patch -O 0001-luci-mod-dsl.patch.
@Borromini, @feckert , cleaned the chain of commits and did a new commit. Should look better now. Please confirm.
@feckert, on the ubus topic something like
root@dsl-02:/tmp# ubus -v list dsl
'dsl' @88f73b6b
"metrics":{}
"statistics":{}
and
root@dsl-02:/tmp# ubus call dsl statistics
{
"bits": {
"downstream": {
"direction": 1,
"groups": 4096,
"data": [
0,
0,
0,
0,
0,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
15,
0,
0,
0,
0
]
},
"upstream": {
"direction": 0,
"groups": 4096,
"data": [
0,
0,
0,
0,
0,
0,
0,
6,
8,
9,
10,
12,
12,
0,
0,
0,
0
]
}
},
"snr": {
"downstream": {
"direction": 1,
"groupsize": 8,
"groups": 512,
"data": [
255,
255,
255,
255,
255,
255,
255,
255,
255,
149,
153,
155,
157,
.....
255,
255,
255,
255,
255
]
},
"upstream": {
"direction": 0,
"groupsize": 8,
"groups": 512,
"data": [
64,
130,
163,
165,
163,
167,
255,
255,
255,
255,
255,
255,
255,
255,
255
]
}
},
"qln": {
"downstream": {
"direction": 1,
"groupsize": 8,
"groups": 512,
"data": [
255,
255,
255,
255,
255,
255,
....
255,
255,
255,
255,
255,
255,
255
]
},
"upstream": {
"direction": 0,
"groupsize": 8,
"groups": 512,
"data": [
255,
184,
184,
179,
171,
180,
185,
187,
255,
255,
....
255,
255,
255,
255
]
}
},
"hlog": {
"downstream": {
"direction": 1,
"groupsize": 8,
"groups": 512,
"data": [
1023,
1023,
1023,
....
1023,
1023,
1023
]
},
"upstream": {
"direction": 0,
"groupsize": 8,
"groups": 512,
"data": [
1023,
156,
90,
87,
91,
93,
......
1023,
1023,
1023
]
}
}
}
root@dsl-02:/tmp#
Still requires some tweaking but the foundation should be OK.
@turboproc Thanks, applies like expected now. Very interested myself, but I have no time to swap out my main router (which is on 22.03) for one with a master build atm :(.