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

gluon-status-page: add channels to status-page #2201

Merged

Conversation

AiyionPrime
Copy link
Member

Implements wifi-channels on status-page

originally #1727, now part of #1415

@AiyionPrime
Copy link
Member Author

This device is testing this until the PR gets merged:

http://lilienstra-e.n.ffh.zone

and in case rdns does not work:
http://[2001:678:978:114:b64b:d6ff:fe20:2778]/cgi-bin/status

@mweinelt
Copy link
Contributor

Thanks for working on this.

I don't think this feels right next to the client count. How about adding a new "Radio" section and iterate over all radios in there. Maybe also make "channel" part of the key, not the value, and if you are highly motivated maybe also add the frequency in MHz.

@mweinelt mweinelt added 0. type: enhancement The changeset is an enhancement 3. topic: status-page labels Apr 12, 2021
@AiyionPrime
Copy link
Member Author

Adding such a section appears to become tricky with the current data.
This is what wireless looks like on my router:

 "wireless": [
    {
      "frequency": 2412,
      "noise": -95,
      "active": 438652660,
      "busy": 149880419,
      "rx": 136320927,
      "tx": 5144566
    },
    {
      "frequency": 5220,
      "noise": -109,
      "active": 421788737,
      "busy": 12318006,
      "rx": 1747746,
      "tx": 6160088
    }
  ],

These two are neither named nor in order of radio0 and radio1. Likely they come out of a lua map and are not ordered at all; just a wild guess, I haven't looked into that yet.
Info I can

Therefore one thing we could do is this:

Radio

2.4 GHz   Channel 1 (2412 MHz)
5 GHz     Channel 44 (5220 MHz)

If you've got an idea @mweinelt or want it like that, let me know in the next hours and I might even get this done before noon.

@AiyionPrime
Copy link
Member Author

For reference: This is what my draft looked like:
img

And http://lilienstra-e.n.ffh.zone does now present the draft with the dedicated radio-section of @mweinelt.

Careful, this PR is not updated yet.

@Adorfer
Copy link
Contributor

Adorfer commented Apr 13, 2021

this "channel-number next to client-count" may look a bit misleading if client-AP disbled for a radio and just enabled for wifimesh.
Or is there a separate value for "mesh-links per channel", in case wifimes is enabled for that radio?

@AiyionPrime
Copy link
Member Author

@Adorfer I'm not sure if I understood you.
Have you taken a look at the implementation of hexas suggestion? The image was just posted for reference.

"channel-number next to client-count"

is not going to be in the final PR.

I do not know about a separate value mesh-links per channel I expect it to not exist.

@mweinelt
Copy link
Contributor

mweinelt commented Apr 13, 2021

Uh, if I remember correctly this is the same nodeinfo that we use in respondd, so we would need a proper design for how to modify the format. This makes it somewhat more complicated.

Mock-Up of what I think would be a big improvement:

Radios

radio0  2400 MHz
        Channel 1
radio1  5220 MHz
        Channel 44
radio2  5700 MHz
        Channel 144

@AiyionPrime AiyionPrime force-pushed the status_page_wifi_channel branch from 6e2b7dd to cd3a5c4 Compare April 14, 2021 06:45
@AiyionPrime
Copy link
Member Author

This is what we currently can do:

Radios

radio    2412 MHz
         Channel 1
radio    5220 MHz
         Channel 44
radio    5700 MHz
         Channel 140

@AiyionPrime
Copy link
Member Author

AiyionPrime commented Apr 14, 2021

@mweinelt I implemented the changes according to my last answer;
The code now works with any amount of interfaces instead of one per band like it did before.

http://lilienstra-e.n.ffh.zone runs a firmware similar to this PR, the difference is the place the js is loaded from in order for me to rewrite the js part faster.

I marked in code where one would improve after someone tackles #2204.

@AiyionPrime
Copy link
Member Author

While http://lilienstra-e.n.ffh.zone does still contain master and this PR,
http://idx-test.n.ffh.zone/cgi-bin/status does contain a draft with this, as well as freifunk-gluon/packages#244.
Depending on how fast merging progresses, and whether the above remark is meant to be blocking,

Mock-Up of what I think would be a big improvement:

Radios

radio0  2400 MHz
        Channel 1
radio1  5220 MHz
        Channel 44
radio2  5700 MHz
        Channel 144

Originally posted by @mweinelt in #2201 (comment)

it might not make a difference to mark this as blocked until the latter is merged.

Else, this would be ready for review and merge afterwords.

@AiyionPrime AiyionPrime force-pushed the status_page_wifi_channel branch from a8fa65c to aaaf02b Compare April 22, 2021 20:19
@AiyionPrime AiyionPrime force-pushed the status_page_wifi_channel branch from aaaf02b to f14707c Compare April 22, 2021 20:43
@AiyionPrime
Copy link
Member Author

As @blocktrron merged the package earlier, I just rebased this on master and added everything @mweinelt requested earlier.

The code compiles and still runs here:
http://lilienstra-e.n.ffh.zone/cgi-bin/status

@blocktrron
Copy link
Member

Sorry to be the party pooper here - The respondd-airtime module is an optional dependency (hence it being in the package repo).

I like this idea (and would love to see more wireless stats on the status page). So i would propose to move the airtime module to core Gluon. Any objections against that?

@neocturne
Copy link
Member

Sorry to be the party pooper here - The respondd-airtime module is an optional dependency (hence it being in the package repo).

I like this idea (and would love to see more wireless stats on the status page). So i would propose to move the airtime module to core Gluon. Any objections against that?

The "packages" repo is for packages that don't depend on the Gluon core, we don't need to move anything. respondd itself is in that repo as well.

What we need to decide on is whether we want to make the respondd-airtime module a hard dependency of the status page, or if the page should just omit the missing information when the module is not available. In my opinion making it optional is the better approach.

@AiyionPrime
Copy link
Member Author

What we need to decide on is whether we want to make the respondd-airtime module a hard dependency of the status page, or if the page should just omit the missing information when the module is not available. In my opinion making it optional is the better approach.

I think the current implementation does suffice;
If the module is not installed, the status page does still render properly, like it did before I started this PR.

Let me know, if you still need changes?

A node without the module but with code is this one:
http://[2001:678:978:114:b64b:d6ff:fe22:4ebc]/cgi-bin/status

@blocktrron blocktrron merged commit 0ce961e into freifunk-gluon:master Apr 27, 2021
@AiyionPrime AiyionPrime deleted the status_page_wifi_channel branch April 28, 2021 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. type: enhancement The changeset is an enhancement 3. topic: status-page
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants