Detect metered connections for macos when connected to an iOS Personal Hotspot #1902

jramnani · 2024-01-19T00:46:24Z

Description

This change addresses #1884 by adding support for Vorta to detect when macOS is connected to a Wi-Fi network presented by an iOS Personal Hotspot.

I changed how the darwin module gets its information about Wi-Fi networks to use ObjC instead of running command line tools. The benefits of using the ObjC Cocoa APIs is that we get richer information about Wi-Fi networks. There is an explicit API in Cocoa for checking whether a connected Wi-Fi network is an iOS Personal Hotspot.

I left the existing implementation in place, and added a new check for iOS since it seems like the previous implementation was working when tethered with an Android device.

Related Issue

Vorta did not respect metered network exclusion #1884

Motivation and Context

I updated this part of the code base because while I was working through an home Internet outage recently, a Vorta backup running in the background used up all of my allotted data on my mobile phone plan.

How Has This Been Tested?

Automated tests were added and improved in the network_manager.darwin module.
I ran a manual tests against a remote SSH repository. I connected to a Personal Hotspot Wi-Fi, then tried to run a backup. Vorta successfully detected the Hotspot connection and suppressed the backup with a status message for the user.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read the CONTRIBUTING guide.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.

m3nu · 2024-01-19T10:54:42Z

Looks good in principle, but it fails when there is no Wifi at all. This case should be covered.

        wifis = []
        interface: CWInterface = self._get_wifi_interface()
>       interface_name = interface.name()
E       AttributeError: 'NoneType' object has no attribute 'name'

jramnani · 2024-01-19T19:11:00Z

I'm glad to see the test suite caught this.

I appreciate that I could limit my changes to one module. This change was easier to do because of that. Thank you! :)

m3nu · 2024-01-21T09:08:31Z

One minor thing left: It doesn't like the order or imports.

jramnani · 2024-01-21T20:44:55Z

Fixed. Sorry about that. I had assumed PyCharm would add the import for me correctly.

Now I know to run make lint locally to make sure.

real-yfprojects · 2024-01-22T21:02:55Z

src/vorta/network_status/darwin.py

-from typing import Iterator, Optional
+from typing import Iterator, List, Optional
+
+from CoreWLAN import CWInterface, CWNetwork, CWWiFiClient


I was wondering whether this toplevel import is a good practice since the dependency is optional.

Are you suggestion suppressing import errors like what's happening in the notifications module?

https://github.com/borgbase/vorta/blob/master/src/vorta/notifications.py#L8-L11

I have a Linux host I can test with. I'll see if I can do that soon to see if the application throws an ImportError for me.

I can report that on Pop_OS 22.04 I was able to start the app in development mode and do a successful backup to a remote host. Also the full test matrix passed for me via make test-unit on the same host.

I saw an INFO log message: Using NetworkManagerMonitor NetworkStatusMonitor implementation

No INFO or DEBUG logs mentioning the new imports.

I do see some DEBUG log messages from vorta.keyring.abc about No module named 'objc', but they have not affected the manual backup test.

I'm happy to make the change if we want it. But wanted to report back these manual testing results.

An upshot of my local testing was that I also tried a backup while tethered to my iOS device from my Pop_OS laptop and it detected the metered connection. 🎉

The current implementation was tested with Android, but does not work with iOS. Move the existing implementation and include android in the name to make room for adding a new iOS metered connection detection strategy.

Switch from using command line tools to using the Objective-C Cocoa API to get the Wi-Fi status information. Cocoa has an API to specifically check whether a Wi-Fi connection is using a Personal Hotspot on iOS. I'm using a private method to get the Wi-Fi interface object in Cocoa. The reason for this is that cleaning up mocks on PyObjC/ObjC objects is much harder than mocking out methods on objects in our control. Using test doubles also let's me check for different states the Wi-Fi network could be in.

Use the networksetup command on macOS to get the list of the user's Wi-Fi networks. networksetup -listpreferredwirelessnetworks bsd_device It looks like this command and option has existed on macOS since at least 2013. Also add some type annotations around the PyObjC return values to help the reader know what they're dealing with at each step.

The user might have Wi-Fi turned off. Account for that use case.

The DarwinNetworkManager can now determine if the user is connected to a Personal Hotspot Wi-Fi network from iOS. Account for whether the user has Wi-Fi turned on and off.

According to Apple's developer documentation, creating CWInterface objects directly are discouraged. Instead, they prefer to use CWInterface objects created by CWWiFiClient. This also happens to be more compliant with Apple's application sandbox. Creating CWInterface objects directly accesses raw BSD sockets which is not allowed in the sandbox. More details here: https://developer.apple.com/documentation/corewlan/cwinterface

I have one of these in my list of networks in Vorta. And this also covers a missing branch in get_known_wifis.

This is to provide a little more clarity. Especially since this class is subclassing another one.

When a Mac does not have a Wi-Fi interface, CWWiFiClient.interface() can return None. Update the type annotation to mark it as Optional, and account for the null condition in the other methods.

The CI tests failed on python 3.8. I used the wrong type annotation to describe a list of SystemWifiInfo's. The tests now pass for me when I run 'make test-unit' using a python 3.8 interpreter.

m3nu force-pushed the metered-connections-for-macos branch from 216b75b to bf4a2df Compare January 20, 2024 10:26

real-yfprojects reviewed Jan 22, 2024

View reviewed changes

jramnani and others added 12 commits February 2, 2024 11:52

Add dependency for pyobjc-CoreWLAN on darwin

06034ec

Rename existing implementation with Android

d68c696

The current implementation was tested with Android, but does not work with iOS. Move the existing implementation and include android in the name to make room for adding a new iOS metered connection detection strategy.

Add test for get_current_wifi when wifi is off

de9e724

The user might have Wi-Fi turned off. Account for that use case.

Add iOS Personal Hotspot support to is_network_metered

a8acf4d

The DarwinNetworkManager can now determine if the user is connected to a Personal Hotspot Wi-Fi network from iOS. Account for whether the user has Wi-Fi turned on and off.

Add test case for blank Wi-Fi network name

61d554b

I have one of these in my list of networks in Vorta. And this also covers a missing branch in get_known_wifis.

Move private method below public methods

fbe52f1

This is to provide a little more clarity. Especially since this class is subclassing another one.

Account for when there is no wifi interface

59184fe

When a Mac does not have a Wi-Fi interface, CWWiFiClient.interface() can return None. Update the type annotation to mark it as Optional, and account for the null condition in the other methods.

Fix type annotation error

7fcfbe5

The CI tests failed on python 3.8. I used the wrong type annotation to describe a list of SystemWifiInfo's. The tests now pass for me when I run 'make test-unit' using a python 3.8 interpreter.

Fix linter issue with imports

f174bcb

m3nu force-pushed the metered-connections-for-macos branch from 2dfd298 to f174bcb Compare February 2, 2024 11:52

m3nu approved these changes Feb 2, 2024

View reviewed changes

m3nu merged commit 634f984 into borgbase:master Feb 2, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Detect metered connections for macos when connected to an iOS Personal Hotspot #1902

Detect metered connections for macos when connected to an iOS Personal Hotspot #1902

jramnani commented Jan 19, 2024

m3nu commented Jan 19, 2024

jramnani commented Jan 19, 2024

m3nu commented Jan 21, 2024

jramnani commented Jan 21, 2024

real-yfprojects Jan 22, 2024

jramnani Jan 23, 2024

jramnani Jan 23, 2024

jramnani Jan 23, 2024 •

edited

Loading

Detect metered connections for macos when connected to an iOS Personal Hotspot #1902

Detect metered connections for macos when connected to an iOS Personal Hotspot #1902

Conversation

jramnani commented Jan 19, 2024

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

m3nu commented Jan 19, 2024

jramnani commented Jan 19, 2024

m3nu commented Jan 21, 2024

jramnani commented Jan 21, 2024

real-yfprojects Jan 22, 2024

Choose a reason for hiding this comment

jramnani Jan 23, 2024

Choose a reason for hiding this comment

jramnani Jan 23, 2024

Choose a reason for hiding this comment

jramnani Jan 23, 2024 • edited Loading

Choose a reason for hiding this comment

jramnani Jan 23, 2024 •

edited

Loading