Skip to content

Conversation

@tpikonen
Copy link
Contributor

@tpikonen tpikonen commented Mar 9, 2021

Here's a proposed fix for #974.

I'm not 100% sure on second commit which uses 'ip addr show scope global up' instead of 'ip link' to get a list of candidate interfaces, but reading the man page I think a global interface is what we're after.

BSDs and MacOS do not have 'ip' installed by default, so they should fall back to 'ifconfig'.

@auouymous
Copy link
Member

Doesn't fix the issue, see #974 (comment).

@tpikonen tpikonen force-pushed the fix-connectivity-check branch from 9b4bd2e to a33d620 Compare March 10, 2021 12:58
@tpikonen
Copy link
Contributor Author

You're right @auouymous, it didn't work on busybox. I pushed a new version which checks for interfaces with an inet and broadcast address, which apparently is a sign of active connection. This works at least with 'busybox ip' and 'ip' in Debian.

@elelay elelay merged commit 78e2720 into gpodder:master Mar 13, 2021
@elelay
Copy link
Member

elelay commented Mar 13, 2021

Merged, thanks!

@tpikonen tpikonen deleted the fix-connectivity-check branch March 13, 2021 19:28
@auouymous
Copy link
Member

Little late, but the patch works fine and looks like it should solve the issue.

One problem I see, that existed before this patch, is that all three *_get_active_interfaces methods yield the result. Is there a reason for this? They are only called once and returning the result would be more efficient, right?

@tpikonen
Copy link
Contributor Author

I also don't see the point for using generator functions in this case, but a patch fixing all these functions would have been a lot more invasive (and would have to be tested on all platforms).

@elelay
Copy link
Member

elelay commented Mar 21, 2021

yes, no need for a generator, and no point, really, since all the command output is gathered before analyzing it.

@auouymous
Copy link
Member

I wrote a patch the other day that renamed those three functions and returned a bool. But are those functions usable by extensions? If so, do we care about breaking extensions not in gpodder? They are only called once per manual or automatic update, so performance isn't that important. There are also some functions in util.py not used anywhere in gpodder.

@elelay
Copy link
Member

elelay commented Mar 21, 2021

But are those functions usable by extensions? If so, do we care about breaking extensions not in gpodder?

yes on both counts. Are those used, though? Not likely. An extension would use the connection_available() function.
Personally I wouldn't change them, but if you find it irksome, you may change them.

There are also some functions in util.py not used anywhere in gpodder.

which ones?

@auouymous
Copy link
Member

They are rarely called so I'm fine leaving them alone.

parse_time, isabs, write_m3u_playlist, check_command and website_reachable are unused.

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.

3 participants