feat(net): interface discovery #2025

devsnek · 2020-02-29T22:08:27Z

Allows you to specify interface-type=wired/wireless instead of interface=ifname

Closes #339

codecov · 2020-02-29T22:14:09Z

Codecov Report

Merging #2025 (4b6b690) into master (eb302f9) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master   #2025      +/-   ##
=========================================
- Coverage    7.83%   7.80%   -0.04%     
=========================================
  Files         142     142              
  Lines       10299   10340      +41     
=========================================
  Hits          807     807              
- Misses       9492    9533      +41

Flag	Coverage Δ
unittests	`7.80% <0.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
include/adapters/net.hpp	`0.00% <ø> (ø)`
src/adapters/net.cpp	`0.00% <0.00%> (ø)`
src/modules/network.cpp	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb302f9...0af0364. Read the comment docs.

patrick96

I have often thought what it would take to make the network module automatically detect interfaces. And after reviewing this, I came to the conclusion that it is really hard.

You did a good job.

I think we should also check that interfaces aren't virtual when we search for wired or wireless interfaces to not accidentally select the loopback or tunnel interfaces.

As I noted in my comment below, this doesn't have to be perfect and take every edge case into account, it just has to be good enough to work for most people with "normal" setups and your code largely does that. We can always refine the code in the future if people report it not behaving as expected for them.

src/adapters/net.cpp

patrick96

Code looks good. Still need to actually test it though ;)

src/adapters/net.cpp

devsnek · 2020-03-04T03:41:55Z

@patrick96 how would i go about testing this?

patrick96 · 2020-03-04T08:35:44Z

Oh sorry. I meant I still need to test it.

devsnek · 2020-04-16T00:58:32Z

@patrick96 did you ever get around to testing this?

devsnek · 2020-06-23T03:23:33Z

@patrick96 bump

devsnek · 2020-08-05T14:35:36Z

@patrick96 👀

zampierilucas · 2020-08-05T14:39:11Z

@patrick96 I would really love to see this go through, there's some way that I can help with testing it.

KivalM · 2020-09-24T21:48:50Z

@patrick96 Really looking forward to this addition.^^

zampierilucas · 2020-09-24T22:08:13Z

While we wait for this PR to go through, what a ended up doing was setting up my config file like

[module/network]
type = internal/network
interface = ${env:DEFAULT_NETWORK_INTERFACE}

and on .profile added
export DEFAULT_NETWORK_INTERFACE=`ip route | grep '^default' | awk '{print $5}' | head -n1`
So every time my .profile is sourced, the default interface is updated to the current one, If it changed.

devsnek · 2020-12-06T00:26:00Z

@patrick96 ping again

joaquingx · 2020-12-07T17:28:02Z

@joaquingx While we wait for this PR to go through, what a ended up doing was setting up my config file like
[module/network]
type = internal/network
interface = ${env:DEFAULT_NETWORK_INTERFACE}
and on .profile added
export DEFAULT_NETWORK_INTERFACE=`ip route | grep '^default' | awk '{print $5}' | head -n1`
So every time my .profile is sourced, the default interface is updated to the current one, If it changed.

Hey guys :), I suppose to tagging me here is a typo 😅, if possible please change it 🙏

patrick96

Sorry for the delay

I tested the changes and everything seems to work fine.
I also added some small suggestions for error handling.

We recently started tracking the changelog directly in the repo (https://github.com/polybar/polybar/blob/master/CONTRIBUTING.md#changelog). Please add these changes to the changelog (you may need to rebase and force push for this).

I do have some thoughts about the default behavior of the module. But this doesn't have to happen in this PR.
Right now interface-type is required if interface is not specified.
I think, the module should work even if neither is specified. In that case it should just pick a wired or wireless interface.
If you want to work on this, let me know, otherwise I'll open an issue and let someone else take a stab at it.

src/modules/network.cpp

devsnek · 2021-01-03T02:30:08Z

I think it would be good to leave the no-options config to a separate PR. imo that should try to find whichever interface is "active" which seems out of scope of this.

patrick96 · 2021-01-03T10:37:47Z

I think it would be good to leave the no-options config to a separate PR. imo that should try to find whichever interface is "active" which seems out of scope of this.

You're right. I also didn't really think about the "active" part. I would have just taken the first available non-virtual interface. I am going to open a new issue for this.

patrick96

Everything looks good now.

Thanks! 🚀

patrick96 · 2021-01-03T11:27:23Z

I have decided against adding active interface detection. Most use cases for that can be covered with three modules:

[module/net-base]
format-disconnected = 
...

[module/net-wired]
inherit = module/net-base
interface-type = wired

[module/net-wireless]
inherit = module/net-base
interface-type = wireless

This will only show active interfaces as well.

vertisan · 2021-02-01T17:59:33Z

When it will be released?

patrick96 · 2021-02-01T20:26:54Z

Don't know yet

Currently (v3.5.6) need the patch from polybar/polybar#2025

patrick96 requested changes Mar 2, 2020

View reviewed changes

src/adapters/net.cpp Outdated Show resolved Hide resolved

src/adapters/net.cpp Outdated Show resolved Hide resolved

src/adapters/net.cpp Outdated Show resolved Hide resolved

devsnek force-pushed the autodetect-network-interface branch 2 times, most recently from e0af364 to 7465e6b Compare March 3, 2020 04:04

devsnek commented Mar 3, 2020

View reviewed changes

src/adapters/net.cpp Outdated Show resolved Hide resolved

patrick96 requested changes Mar 3, 2020

View reviewed changes

src/adapters/net.cpp Outdated Show resolved Hide resolved

devsnek force-pushed the autodetect-network-interface branch from 7465e6b to f33df1c Compare March 4, 2020 03:41

This comment has been minimized.

Sign in to view

patrick96 mentioned this pull request May 3, 2020

Creaded dynamic interface string #2084

Closed

zampierilucas approved these changes May 16, 2020

View reviewed changes

patrick96 self-requested a review December 14, 2020 09:35

patrick96 requested changes Jan 3, 2021

View reviewed changes

src/modules/network.cpp Outdated Show resolved Hide resolved

src/modules/network.cpp Outdated Show resolved Hide resolved

src/modules/network.cpp Show resolved Hide resolved

devsnek force-pushed the autodetect-network-interface branch from f33df1c to b91d67b Compare January 3, 2021 02:28

devsnek force-pushed the autodetect-network-interface branch from b91d67b to 4b6b690 Compare January 3, 2021 02:34

feat(net): interface discovery

0af0364

devsnek force-pushed the autodetect-network-interface branch from 4b6b690 to 0af0364 Compare January 3, 2021 02:34

patrick96 approved these changes Jan 3, 2021

View reviewed changes

patrick96 merged commit f7c2d83 into polybar:master Jan 3, 2021

devsnek deleted the autodetect-network-interface branch January 3, 2021 15:08

patrick96 mentioned this pull request Mar 29, 2021

[feature request] provide generic configuration for first time users #2405

Closed

weirane added a commit to weirane/dotfiles that referenced this pull request Aug 10, 2021

Use interface-type instead of hard coding the name

49d47bb

Currently (v3.5.6) need the patch from polybar/polybar#2025

This was referenced Sep 9, 2021

VPN module not shown #2496

Closed

fix(net): Check if interface is valid first. #2497

Merged

hantianjz mentioned this pull request Dec 1, 2021

feat(net): improve interface discovery #2557

Merged

1 task

Uh oh!

feat(net): interface discovery #2025

feat(net): interface discovery #2025

Uh oh!

Conversation

devsnek commented Feb 29, 2020 • edited by patrick96 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

devsnek commented Mar 4, 2020

Uh oh!

patrick96 commented Mar 4, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

devsnek commented Apr 16, 2020

Uh oh!

devsnek commented Jun 23, 2020

Uh oh!

devsnek commented Aug 5, 2020

Uh oh!

zampierilucas commented Aug 5, 2020

Uh oh!

KivalM commented Sep 24, 2020

Uh oh!

zampierilucas commented Sep 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devsnek commented Dec 6, 2020

Uh oh!

joaquingx commented Dec 7, 2020

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devsnek commented Jan 3, 2021

Uh oh!

patrick96 commented Jan 3, 2021

Uh oh!

patrick96 left a comment

Choose a reason for hiding this comment

Uh oh!

patrick96 commented Jan 3, 2021

Uh oh!

vertisan commented Feb 1, 2021

Uh oh!

patrick96 commented Feb 1, 2021

Uh oh!

Uh oh!

devsnek commented Feb 29, 2020 •

edited by patrick96

Loading

codecov bot commented Feb 29, 2020 •

edited

Loading

zampierilucas commented Sep 24, 2020 •

edited

Loading