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

luci-wireless: Add 11be support #7302

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Conversation

rmandrad
Copy link
Contributor

@rmandrad rmandrad commented Oct 3, 2024

Tested with banana rpi-4

In order to complete 11be support - the below pulls will need to be applied

RPCD - openwrt/rpcd#5
iwinfo - openwrt/iwinfo#10

Signed-off-by: Rudy Andram [email protected]
Tested-By: Daniel Pawlik [email protected]

Tested on: arm64 / Mediatek / filogic / banana rpi4 ; QNAP Qualcommax IPQ8074

more details on closed PR - #7279

  • This PR is not from my main or master branch 💩, but a separate branch ✅
  • Each commit has a valid ✒️ Signed-off-by: <[email protected]> row (via git commit --signoff)
  • Each commit and PR title has a valid 📝 <package name>: title first line subject for packages
  • Incremented 🆙 any PKG_VERSION in the Makefile
  • Tested on: (architecture, openwrt version, browser) ✅
  • ( Preferred ) Mention: @ the original code author for feedback
  • ( Preferred ) Screenshot or mp4 of changes:
  • ( Optional ) Closes: e.g. openwrt/luci#issue-number
  • ( Optional ) Depends on: e.g. openwrt/packages#pr-number in sister repo
  • Description: (describe the changes proposed in this PR)

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 3, 2024

created this new pull instead of working on the previous - hopefully has cleared all of the updates I made tabs/spaces etc

@rmandrad rmandrad changed the title Add 11be support luci-wireless - Add 11be support Oct 3, 2024
@rmandrad rmandrad changed the title luci-wireless - Add 11be support luci-wireless: Add 11be support Oct 3, 2024
@systemcrash
Copy link
Contributor

Looks better in any case. What was doing the indentation? Some pre-commit hooks?

@systemcrash
Copy link
Contributor

Those other PRs aren't necessary any longer, are they?

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 3, 2024

Those other PRs aren't necessary any longer, are they?

they are now on snapshot ....

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 3, 2024

Looks better in any case. What was doing the indentation? Some pre-commit hooks?

i was doing the changes on a dev branch ... with code-insiders (beta of vscode) and would promote the code to another vm where I was bundling the changes . The problem was that every change after your feedback was somehow adding more stuff on top ... like i would fix one bit and another would get wrong where was right before ... like a big spaghetti! really don't know i put the blame on the dev vm and code insiders with default settings

gave up and started from scratch ...will keep it simple from now on

Copy link

@N-Storm N-Storm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested LuCI 802.11be support patches from @rmandrad for a few days (compiling from his fork) on my BPI-R4 and it works for me.

Copy link

@bublikOff bublikOff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for me it worked like a charm

Copy link

@Denis2371 Denis2371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are very simple. As I can see, it's just edits to the LuCI interface. Or I'm tired of setting parameters through the files! You can finally do it through the graphical interface, because this patch works perfectly well!

@systemcrash
Copy link
Contributor

I’m happy if you’re happy, @dannil.

looks ready.

Copy link

@Denis2371 Denis2371 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good

@dannil
Copy link
Contributor

dannil commented Oct 4, 2024

I’m happy if you’re happy, @dannil.

looks ready.

I agree, seems ready to merge.

@rmandrad you checked for regressions on a non-WiFi 7-device which I wrote about in the previous PR, that's the QNAP running IPQ8074 right? The most important part is that we avoid regressions, as long as we are just missing some bits and pieces (I can't test this myself unfortunately as I don't have any WiFi 7-gear) that can just be added in future PR:s, the WiFi 7-support is very fresh anyway.

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 4, 2024

I’m happy if you’re happy, @dannil.
looks ready.

I agree, seems ready to merge.

@rmandrad you checked for regressions on a non-WiFi 7-device which I wrote about in the previous PR, that's the QNAP running IPQ8074 right? The most important part is that we avoid regressions, as long as we are just missing some bits and pieces (I can't test this myself unfortunately as I don't have any WiFi 7-gear) that can just be added in future PR:s, the WiFi 7-support is very fresh anyway.

i have tested on a IPQ8074x qualcommax qnap ... only have IPQ874x devices + the banana rpi4

@systemcrash
Copy link
Contributor

@rmandrad BTW don't add merge commits. ( Please remove it ) Just rebase your work on master.

@rmandrad rmandrad force-pushed the 11be branch 2 times, most recently from 4bcfa32 to d12fee9 Compare October 7, 2024 16:23
@systemcrash
Copy link
Contributor

OK. I'm satisfied. Want to take a last look, @dannil ?

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 7, 2024

I can create a separate pull that reformats the entire function like this - let me know

`function format_wifirate(rate) {
if (rate) {
s = '%.1f\xa0%s, %d\xa0%s'.format(rate.rate / 1000, _('Mbit/s'), rate.mhz, _('MHz'));
ht = rate.ht || false;
vht = rate.vht || false;
mhz = rate.mhz || 0;
nss = rate.nss || 0;
mcs = rate.mcs || 0;
sgi = rate.short_gi || false;
he = rate.he || false;
he_gi = rate.he_gi || 0;
he_dcm = rate.he_dcm || 0;
eht = rate.eht || false;
eht_gi = rate.eht_gi || 0;
eht_dcm = rate.eht_dcm || 0;
}

if (ht || vht) {
    if (vht) s += ', VHT-MCS\xa0%d'.format(mcs);
    if (nss) s += ', VHT-NSS\xa0%d'.format(nss);
    if (ht) s += ', MCS\xa0%s'.format(mcs);
    if (sgi) s += ', ' + _('Short GI').replace(/ /g, '\xa0');
}

if (he) {
    s += ', HE-MCS\xa0%d'.format(mcs);
    if (nss) s += ', HE-NSS\xa0%d'.format(nss);
    if (he_gi) s += ', HE-GI\xa0%d'.format(he_gi);
    if (he_dcm) s += ', HE-DCM\xa0%d'.format(he_dcm);
}

if (eht) {
    s += ', EHT-MCS\xa0%d'.format(mcs);
    if (nss) s += ', EHT-NSS\xa0%d'.format(nss);
    if (eht_gi) s += ', EHT-GI\xa0%d'.format(eht_gi);
    if (eht_dcm) s += ', EHT-DCM\xa0%d'.format(eht_dcm);
}

return s;

}`

@systemcrash
Copy link
Contributor

Could you remove to stash from the bottom of your commit message please @rmandrad ?

Tested with filogic banana rpi4 / QNAP qualcommax ipq8074

Signed-off-by: Rudy Andram <[email protected]>
Tested-By: Daniel Pawlik <[email protected]>
Copy link
Contributor

@dannil dannil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@systemcrash let's merge

@systemcrash systemcrash merged commit 383edb4 into openwrt:master Oct 7, 2024
5 checks passed
@systemcrash
Copy link
Contributor

Thanks @rmandrad and @dannil. Merged to master.

@systemcrash
Copy link
Contributor

@rmandrad - is the 11be stuff only in master, including rpcd and iwinfo?

@rmandrad
Copy link
Contributor Author

rmandrad commented Oct 8, 2024

@rmandrad - is the 11be stuff only in master, including rpcd and iwinfo?

yes... of course other folks have their own repos/branches etc

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

Successfully merging this pull request may close these issues.

10 participants