From 989f01f60938225e8c9606f2965b1caa574e211f Mon Sep 17 00:00:00 2001 From: Sindre Hansen Date: Wed, 1 Mar 2023 15:26:29 +0100 Subject: [PATCH] Remove tcp_client and slave_mode --- blueye/sdk/drone.py | 143 +++++++------------------------------------- tests/conftest.py | 20 ------- tests/test_sdk.py | 25 +------- 3 files changed, 23 insertions(+), 165 deletions(-) diff --git a/blueye/sdk/drone.py b/blueye/sdk/drone.py index d1181226..799666d4 100755 --- a/blueye/sdk/drone.py +++ b/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,8 +120,8 @@ 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) @@ -203,42 +129,17 @@ def connect(self, timeout: float = None): 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: diff --git a/tests/conftest.py b/tests/conftest.py index 034d1d94..7963ed01 100644 --- a/tests/conftest.py +++ b/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 diff --git a/tests/test_sdk.py b/tests/test_sdk.py index 782b7471..f0729a25 100644 --- a/tests/test_sdk.py +++ b/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)