search

helloSystem / Utilities

Utilities written in PyQt5, meant for use with helloSystem
BSD 2-Clause "Simplified" License
28 stars 29 forks source link

Fix "Network.app" issues #150

Open CocoCR300 opened 1 year ago

CocoCR300 commented 1 year ago
CocoCR300 commented 1 year ago

I used a different set of icons because the ones being used were very hard to see on the menu, but the new ones are color blue... Maybe that's an issue, and also the way of determining the signal level was taken from wifimgr and I'm not totally sure if it works well here.

CocoCR300 commented 1 year ago

For some reason the fix made on ifconfig down and up is messing with my current configuration, after bringing the interface down and up it can't connect to any network, and scanning for networks gives nothing. Also, adding up to the line ifconfig_wlan0 in /etc/rc.conf produces the same behavior.

CocoCR300 commented 1 year ago

@probonopd, I think this is ready for review, there are still some things to fix and I want to do, but I want your opinion on the current work, specially the part about watching /var/log/messages (to check authentication failures), if that is something you want to avoid. About the icons, we need a new set, the current one only includes some for the different signal levels, nothing for "acquiring connection" or "no signal", and mixing different icons looks a bit bad. There's also a new password dialog!

probonopd commented 1 year ago

@CocoCR300 in a quick test, this has been working very well for me. I really like how you placed the signal strength and encryption icons to the right-hand side. Very welcome improvements. I hope the QFileSystemWatcher will prove robust enough. I guess the only way to find this out is to try it on a larger scale, so I am inclined to merge as soon as you think it's ready for merging.

As for the color icons, there is a way in Qt to render them as grayscale. I am using the same for the icons in the global menu bar at the right-hand corner (left to the time):

https://github.com/helloSystem/Menu/commit/c13548d3866c3896d728d2388bd0d4cd636a3a91

I don't have the PyQt5 equivalent handy, but I am sure you will figure it out (if not, feel free to ping me anytime).

Great stuff! 👍

probonopd commented 1 year ago

Do you think you could add in wired, too? Keep in mind that wired network devices (e.g., USB) can come and go, too.

CocoCR300 commented 1 year ago

I can try with USB tethering from my cellphone, that's the only thing I have available since my laptop doesn't have an ethernet connector, I hope that's enough to implement something.

probonopd commented 1 year ago

helloSystem 0.8.1 should have USB Tethering work out of the box, at least for Android phones.

CocoCR300 commented 1 year ago

@probonopd I added a pretty simple support for wired interfaces, I didn't found a way to get a "friendly name" for the interface and the current code just checks for "ue" in the interface name to say if it's "USB Ethernet", everything else is named as "Ethernet (interface_name)". Also, there's still an issue with the menu item selected background color that I couldn't fix.

probonopd commented 1 year ago

Great work.

It seems that we should check the return code of wpa_cli scan_results a bit more carefully, since on a system without a wireless network card, it currently crashes when you click on the Wired Ethernet icon with:

"QSystemTrayIcon::setVisible: No Icon set\nTraceback (most recent call last):\n File \"/tmp/Utilities/./Under Construction/Network.app/Network\", line 199, in show_menu\n self.updateMenu()\n File \"/tmp/Utilities/./Under Construction/Network.app/Network\", line 377, in updateMenu\n if \"wpa_state=COMPLETED\" in self.status_lines:\nAttributeError: 'NetworkMenu' object has no attribute 'status_lines'\n"

Can you please try on a system that has no wireless NIC or has it disabled?

probonopd commented 1 year ago

I have 2 USB Ethernet adapters in my system, but only one is plugged into a cable, and hence only one is active - which one?

image

% dmesg | grep Ethernet
ugen1.6: <Realtek USB-C Dock Ethernet> at usbus1
ure0: <Realtek USB-C Dock Ethernet, class 0/0, rev 2.10/31.03, addr 5> on usbus1
ue0: <USB Ethernet> on ure0
ue0: Ethernet address: ...
ue1: <USB Ethernet> on axe0
ue1: Ethernet address: ...

% ifconfig
ue0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500        options=68009b<RXCSUM,TXCSUM,VLAN_MTU,VLAN_HWTAGGING,VLAN_HWCSUM,LINKSTATE,RXCSUM_IPV6,TXCSUM_IPV6>
        ether ...
        inet6 ...%ue0 prefixlen 64 scopeid 0x2
        media: Ethernet autoselect (none)
        status: no carrier
        nd6 options=23<PERFORMNUD,ACCEPT_RTADV,AUTO_LINKLOCAL>
ue1: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
        options=8000b<RXCSUM,TXCSUM,VLAN_MTU,LINKSTATE>
        ether ...
        inet6 ...%ue1 prefixlen 64 scopeid 0x3
        inet6 ... prefixlen 64 autoconf
        inet 192.168.0... netmask 0xffffff00 broadcast 192.168.0.255
        media: Ethernet autoselect (100baseTX <full-duplex>)
        status: active
        nd6 options=23<PERFORMNUD,ACCEPT_RTADV,AUTO_LINKLOCAL>
probonopd commented 1 year ago

Great work!

Instead of launching lots of ifconfig processes which is quite some overhead, maybe we could use something like https://www.freshports.org/net/py-netifaces (sudo pkg install net/py-netifaces)? Looking at the description, it looks easy to use: https://pypi.org/project/netifaces/.

Also we don't need to do the polling all the time; only while the menu is open. Like we are doing in Volume.app,

https://github.com/helloSystem/Utilities/blob/e46cf70055e16726bc4c125df99ee7418171862b/System/Volume.app/Volume#L143-L145

CocoCR300 commented 1 year ago

Thanks! Great, I thought you didn't wanted to add dependencies, using a library will be a big relief for this. I'll make those changes, but something is bugging me out, I don't really know how to handle more than one network device being active, it didn't worked past days, but now I'm connected to Wi-Fi and using USB tethering to connect to internet (Not really useful for me, but it can be for some people). And yes, seems like there can be only one default route in the routing table, so one of the interfaces should have like an "internet" icon (which doesn't exist in the current icon set internet-web-browser-symbolic can be the one).

probonopd commented 1 year ago

In general you are right, I am trying to avoid dependencies. But all those launched processes for the polling... are not really helping for great performance; so I think it's an OK tradeoff here to introduce this dependency if you agree.

CocoCR300 commented 1 year ago

I couldn't do much use of the library you suggested, it doesn't give all the details needed for the application (interface status, interfaces being up/down). It would also be beneficial to use a library to interact with wpa_supplicant. About switching between network interfaces, only the root user can alter the routing table :/