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

fix bridge device section confusion #1061

Merged
merged 1 commit into from
Oct 22, 2023
Merged

Conversation

pony1k
Copy link
Contributor

@pony1k pony1k commented Oct 18, 2023

Bridge ports are added to the first device section, which may be the wrong one. This changes the behaviour follwing @ilario s suggestion. Now, the first device section that has type bridge and the name br-lan will be used.
After reading the /bin/config_generate script in OpenWrt base-files I am confident that there will always be such a device section on first boot.

fixes #1060

@codecov-commenter
Copy link

Codecov Report

Merging #1061 (96590e9) into master (4c51c7e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 96590e9 differs from pull request most recent head 0bf768e. Consider uploading reports for the commit 0bf768e to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #1061      +/-   ##
==========================================
+ Coverage   79.67%   79.69%   +0.01%     
==========================================
  Files          53       53              
  Lines        4542     4550       +8     
==========================================
+ Hits         3619     3626       +7     
- Misses        923      924       +1     
Files Coverage Δ
...s/lime-system/files/usr/lib/lua/lime/proto/lan.lua 80.00% <100.00%> (+3.40%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ilario
Copy link
Member

ilario commented Oct 19, 2023

Wow this looks veeerrrry nice!!!!!

And it also solves, in a more elegant way, the problem patched in 6b72b3b

Did you test on OpenWrt 22.03 also?

@spiccinini @G10h4ck check this out

@G10h4ck
Copy link
Member

G10h4ck commented Oct 19, 2023

Seems good to me, thanks @pony1k

@pony1k
Copy link
Contributor Author

pony1k commented Oct 22, 2023

I tested it on

  • fritzbox 4020, OpenWrt 23.05.0, non-dsa
  • fritzbox 4040, OpenWrt 22.03.5, non-dsa
  • TP-Link Archer C6 v3, OpenWrt 22.03.5, dsa

By tested I mean I removed all ports from the bridge device section and see if lime-config brings them back. For me is fine to be merged.

@spiccinini spiccinini merged commit d0c498f into libremesh:master Oct 22, 2023
1 check passed
@pony1k pony1k changed the title draft: fix bridge device section confusion fix bridge device section confusion Oct 23, 2023
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.

lime-proto-lan confuses bridge device section
5 participants