Remove tcp_client and slave_mode

blueye/sdk/drone.py

@@ -8,12 +8,6 @@
 
 import blueye.protocol
 import requests
-from blueye.protocol import TcpClient
-from blueye.protocol.exceptions import (
-    MismatchedReply,
-    NoConnectionToDrone,
-    ResponseTimeout,
-)
 from packaging import version
 
 from .camera import Camera
@@ -23,37 +17,6 @@
 from .motion import Motion
 
 
-class SlaveModeWarning(UserWarning):
-    """Raised when trying to perform action not possible in slave mode"""
-
-
-class _SlaveTcpClient:
-    """A dummy TCP client that warns you if you use any of its functions"""
-
-    def __getattr__(self, name):
-        def method(*args):
-            warnings.warn(
-                f"Unable to call {name}{args} with client in slave mode",
-                SlaveModeWarning,
-                stacklevel=2,
-            )
-
-        return method
-
-
-class _NoConnectionTcpClient:
-    """A TCP client that raises a ConnectionError if you use any of its functions"""
-
-    def __getattr__(self, name):
-        def method(*args, **kwargs):
-            raise ConnectionError(
-                "The connection to the drone is not established, "
-                "try calling the connect method before retrying"
-            )
-
-        return method
-
-
 class Config:
     def __init__(self, parent_drone: "Drone"):
         self._parent_drone = parent_drone
@@ -87,34 +50,24 @@ def set_drone_time(self, time: int):
 class Drone:
     """A class providing an interface to a Blueye drone's functions
 
-    Automatically connects to the drone using the default ip and port when instantiated, this
-    behaviour can be disabled by setting `autoConnect=False`.
-
-    The drone only supports one client controlling it at a time, but if you pass
-    `slaveModeEnabled=True` you will still be able to receive data from the drone.
+    Automatically connects to the drone using the default ip when instantiated, this behaviour can
+    be disabled by setting `autoConnect=False`.
     """
 
     def __init__(
         self,
         ip="192.168.1.101",
-        tcpPort=2011,
         autoConnect=True,
-        slaveModeEnabled=False,
+        timeout=3,
     ):
         self._ip = ip
-        self._port = tcpPort
-        self._slave_mode_enabled = slaveModeEnabled
-        if slaveModeEnabled:
-            self._tcp_client = _SlaveTcpClient()
-        else:
-            self._tcp_client = _NoConnectionTcpClient()
         self.camera = Camera(self, is_guestport_camera=False)
         self.motion = Motion(self)
         self.logs = Logs(self)
         self.config = Config(self)
-
+        self.connected = False
         if autoConnect is True:
-            self.connect(timeout=3)
+            self.connect(timeout=timeout)
 
     def _verify_required_blunux_version(self, requirement: str):
         """Verify that Blunux version is higher than requirement
@@ -125,7 +78,7 @@ def _verify_required_blunux_version(self, requirement: str):
         the requirement.
         """
 
-        if not self.connection_established:
+        if not self.connected:
             raise ConnectionError(
                 "The connection to the drone is not established, try calling the connect method "
                 "before retrying"
@@ -136,17 +89,12 @@ def _verify_required_blunux_version(self, requirement: str):
                 f"{requirement} or higher is required."
             )
 
-    @property
-    def connection_established(self):
-        if isinstance(self._tcp_client, _NoConnectionTcpClient):
-            return False
-        else:
-            return True
-
-    def _update_drone_info(self):
+    def _update_drone_info(self, timeout: float = 3):
         """Request and store information about the connected drone"""
         try:
-            response = requests.get(f"http://{self._ip}/diagnostics/drone_info", timeout=3).json()
+            response = requests.get(
+                f"http://{self._ip}/diagnostics/drone_info", timeout=timeout
+            ).json()
         except (
             requests.ConnectTimeout,
             requests.ReadTimeout,
@@ -164,28 +112,6 @@ def _update_drone_info(self):
         self.serial_number = response["serial_number"]
         self.uuid = response["hardware_id"]
 
-    def _connect_to_tcp_socket(self):
-        try:
-            self._tcp_client.connect()
-        except NoConnectionToDrone:
-            raise ConnectionError("Could not establish connection with drone")
-
-    def _start_watchdog(self):
-        """Starts the thread for petting the watchdog
-
-        _connect_to_tcp_socket() must be called first"""
-        try:
-            self._tcp_client.start()
-        except RuntimeError:
-            # Ignore multiple starts
-            pass
-
-    def _clean_up_tcp_client(self):
-        """Stops the watchdog thread and closes the TCP socket"""
-        self._tcp_client.stop()
-        self._tcp_client._sock.close()
-        self._tcp_client = _NoConnectionTcpClient()
-
     def connect(self, timeout: float = None):
         """Start receiving telemetry info from the drone, and publishing watchdog messages
 
@@ -194,51 +120,26 @@ def connect(self, timeout: float = None):
 
         - *timeout* (float): Seconds to wait for connection
         """
-
-        self._update_drone_info()
+        # TODO: Deal with exceptions
+        self._update_drone_info(timeout=timeout)
         self._telemetry_watcher = TelemetryClient(self)
         self._ctrl_client = CtrlClient(self)
         self._watchdog_publisher = WatchdogPublisher(self)
         self._req_rep_client = ReqRepClient(self)
         self._telemetry_watcher.start()
         self._req_rep_client.start()
         self._ctrl_client.start()
-        if self._slave_mode_enabled:
-            # No need to touch the TCP stuff if we're in slave mode so we return early
-            return
-
-        if not self.connection_established:
-            self._tcp_client = TcpClient(ip=self._ip, port=self._port, autoConnect=False)
-            self._connect_to_tcp_socket()
-
-        try:
-            # The drone runs from a read-only filesystem, and as such does not keep any state,
-            # therefore when we connect to it we should send the current time
-            self.config.set_drone_time(int(time.time()))
+        self._watchdog_publisher.start()
 
-            self.ping()
-            self.motion.send_thruster_setpoint(0, 0, 0, 0)
-
-            self._start_watchdog()
-        except ResponseTimeout as e:
-            self._clean_up_tcp_client()
-            raise ConnectionError(
-                f"Found drone at {self._ip} but was unable to take control of it. "
-                "Is there another client connected?"
-            ) from e
-        except MismatchedReply:
-            # The connection is out of sync, likely due to a previous connection being
-            # disconnected mid-transfer. Re-instantiating the connection should solve the issue
-            self._clean_up_tcp_client()
-            self.connect(timeout)
-        except BrokenPipeError:
-            # Have lost connection to drone, need to reestablish TCP client
-            self._clean_up_tcp_client()
-            self.connect(timeout)
+        # The drone runs from a read-only filesystem, and as such does not keep any state,
+        # therefore when we connect to it we should send the current time
+        self.config.set_drone_time(int(time.time()))
+        self.ping()
+        self.motion.send_thruster_setpoint(0, 0, 0, 0)
+        self.connected = True
 
     def disconnect(self):
-        """Disconnects the TCP connection, allowing another client to take control of the drone"""
-
+        """Disconnects the connection, allowing another client to take control of the drone"""
         self._watchdog_publisher.stop()
         self._telemetry_watcher.stop()
         self._req_rep_client.stop()
@@ -247,9 +148,7 @@ def disconnect(self):
         self._telemetry_watcher = None
         self._req_rep_client = None
         self._ctrl_client = None
-
-        if self.connection_established and not self._slave_mode_enabled:
-            self._clean_up_tcp_client()
+        self.connected = False
 
     @property
     def lights(self) -> float:

tests/conftest.py

@@ -54,18 +54,6 @@ def mocked_requests(requests_mock):
     requests_mock.get(f"http://192.168.1.101/logcsv", content=str.encode(dummy_logs))
 
 
-@pytest.fixture
-def mocked_TcpClient(mocker):
-    """Fixture for mocking the TcpClient from blueye.protocol
-
-    Note: This mock is passed create=True, which has the potential to be dangerous since it would
-    allow you to test against methods that don't exist on the actual class. Due to the way methods
-    are added to TcpClient (they are instantiated on runtime, depending on which version of the
-    protocol is requested) mocking the class in the usual way would be quite cumbersome.
-    """
-    return mocker.patch("blueye.sdk.drone.TcpClient", create=True)
-
-
 @pytest.fixture
 def mocked_ctrl_client(mocker):
     return mocker.patch("blueye.sdk.drone.CtrlClient", autospec=True)
@@ -90,7 +78,6 @@ def mocked_req_rep_client(mocker):
 def mocked_drone(
     request,
     mocker,
-    mocked_TcpClient,
     mocked_requests,
     mocked_ctrl_client,
     mocked_watchdog_publisher,
@@ -124,10 +111,3 @@ def mocked_drone(
     if hasattr(request, "param"):
         drone.software_version_short = request.param
     return drone
-
-
-@pytest.fixture
-def mocked_slave_drone(mocker, mocked_TcpClient, mocked_requests):
-    drone = blueye.sdk.Drone(autoConnect=False, slaveModeEnabled=True)
-    drone.connect()
-    return drone

tests/test_sdk.py

@@ -35,17 +35,6 @@ def test_angle_conversion(self, mocked_drone, old_angle, new_angle):
         assert pose["yaw"] == new_angle
 
 
-@pytest.mark.skip(reason="Will fix later")
-class TestSlaveMode:
-    def test_warning_is_raised(self, mocker, mocked_slave_drone):
-        mocked_warn = mocker.patch("warnings.warn", autospec=True)
-
-        # Call function that requires tcp connection
-        mocked_slave_drone.lights = 0
-
-        mocked_warn.assert_called_once()
-
-
 def test_documentation_opener(mocker):
     mocked_webbrowser_open = mocker.patch("webbrowser.open", autospec=True)
     import os
@@ -90,10 +79,8 @@ def test_software_version(mocked_drone):
     assert mocked_drone.software_version_short == "1.4.7"
 
 
-def test_verify_sw_version_raises_connection_error_when_not_connected(mocked_drone: Drone, mocker):
-    mocker.patch(
-        "blueye.sdk.Drone.connection_established", new_callable=PropertyMock, return_value=False
-    )
+def test_verify_sw_version_raises_connection_error_when_not_connected(mocked_drone: Drone):
+    mocked_drone.connected = False
     with pytest.raises(ConnectionError):
         mocked_drone._verify_required_blunux_version("1.4.7")
 
@@ -142,14 +129,6 @@ def test_update_drone_info_raises_ConnectionError_when_not_connected(
         mocked_drone._update_drone_info()
 
 
-def test_connect_ignores_repeated_starts_on_watchdog_thread(mocked_drone):
-    mocked_drone.disconnect()
-    assert mocked_drone.connection_established is False
-    mocked_drone._tcp_client.start.side_effect = RuntimeError
-    mocked_drone.connect(1)
-    assert mocked_drone.connection_established is True
-
-
 def test_active_video_streams_return_correct_number(mocked_drone: Drone):
     NStreamersTel = bp.NStreamersTel(n_streamers={"main": 1, "guestport": 2})
     NStreamersTel_serialized = NStreamersTel.__class__.serialize(NStreamersTel)