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

[enhancement] respondd.statistics.wireless: transmit radioidentifier #2204

Open
AiyionPrime opened this issue Apr 14, 2021 · 15 comments
Open

Comments

@AiyionPrime
Copy link
Member

AiyionPrime commented Apr 14, 2021

The current implementation of respondd.statistics.wireless does provide a structure much like this:

 "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
    }
  ],

Originally posted by @AiyionPrime in #2201 (comment)

This works okayish for single- or dualband devices, as one can distinguish the radios per frequency.
For devices like the ocedo panda or the potentially the turris mox (thanks @blocktrron and @yanosz )
this approach might cause confusion due to their amounts of (possible) radios.

I'd suggest to add a radio-key to the wireless dictionary,
which would hold the corresponding descriptor used in uci context for the wifi-device.

An entry might then look like this:

    {
      "frequency": 5220,
      "noise": -109,
      "active": 421788737,
      "busy": 12318006,
      "idx": 0,
      "rx": 1747746,
      "tx": 6160088
    },

This is heavily related to #644 and a necessity for a potential enhancement in #2201.

@mweinelt
Copy link
Contributor

That would be the way of least resistance, the proper way would be to make it a map. For backwards compatibility I think your proposal sounds good to me.

@lemoer
Copy link
Member

lemoer commented Apr 15, 2021

I would also prefer keeping backwards compatibility. As long as there is no necessity for breaking changes, let us avoid them.

@blocktrron
Copy link
Member

which would hold the corresponding descriptor used in uci context for the wifi-device.

Not a big fan of that. The radioX sections are enumerated in the orer in which the radios have been detected by the system on first boot. This is not necessarily the case when firmware gets updated (happened on the TL-WDR4900 some time ago).

I'd much rather simply transmit the PHY idx of the radio in the mac80211 subsystem.

@AiyionPrime
Copy link
Member Author

til, they're not the same -.-'
Thanks, that was my intention.

@AiyionPrime
Copy link
Member Author

As they're not the same;
I'd like to rename the proposed field idx for clarity.

@AiyionPrime AiyionPrime changed the title [enhancement] respondd.statistics.wireless: transmit radionames [enhancement] respondd.statistics.wireless: transmit radioidentifier Apr 18, 2021
AiyionPrime added a commit to AiyionPrime/packages that referenced this issue Apr 19, 2021
@AiyionPrime
Copy link
Member Author

PR in packages is open for feedback.

AiyionPrime added a commit to AiyionPrime/packages that referenced this issue Apr 19, 2021
AiyionPrime added a commit to AiyionPrime/packages that referenced this issue Apr 19, 2021
blocktrron pushed a commit to freifunk-gluon/packages that referenced this issue Apr 22, 2021
@AiyionPrime
Copy link
Member Author

b72588a closed this.

@neocturne
Copy link
Member

I don't like this at all. We have added an arbitrary numeric identifier to the respondd data - that's something we usually avoid in favor of stable identifiers like MAC addresses. How is it used, is there some other data structure that provides the same indices for matching? The array entries already contain the radio's frequency, why is it even relevant which radio it belongs to?

@neocturne neocturne reopened this Jul 15, 2021
@blocktrron
Copy link
Member

@NeoRaider The only other identifier providing the same identification would be the PHY path from the radios UCI options (which i like even less). MAC addresses only identify the VIF, but not the PHY (although you can resolve the PHY).

@neocturne
Copy link
Member

So my question remains, what is this data used for, and what is it matched against? If airtime information is most useful for the mesh interfaces, we could use the MAC address of the mesh VIF. Or we could decide that no identifier is necessary at all, because there is nothing it could be matched against anyways.

@mweinelt
Copy link
Contributor

Basically we want to show on the status page which radio is on which frequency, primarily because the frequency is not a per vif thing.

@neocturne
Copy link
Member

Basically we want to show on the status page which radio is on which frequency, primarily because the frequency is not a per vif thing.

Hmm, I see - it seems I'm missing the bigger picture here. What information should be displayed on the status page? All info provided by the airtime module?

@neocturne
Copy link
Member

One option would be to use the PHY MAC address (/sys/class/ieee80211/phy*/macaddress), although we would also need to add that address to other data structures we want to match the PHY against.

@neocturne
Copy link
Member

I'm going to revise the status page code so that the channel display doesn't use respondd data at all (requiring the airtime module for that purpose seems like overkill to me anyways...). The PHY index can then just be removed from the respondd provider again.

@AiyionPrime
Copy link
Member Author

can then just be removed from the respondd provider again.

In that case you might want to engage #644 and update the negative progress of it.

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

5 participants