Improve documentation and coverage (#3)

* Working on coverage * Add a debugging log statement * Documentation * Document tcp_handle.py (not finished) * Documentation and code cleanup * Documentation * More documentation * Add some documentation in tcp_handle.py * Add module docstrings for constants.py and exceptions.py * More shell tests * More work on tests * Linting * Working on testing AdbDevice._open exceptions * Working on test_shell_error_ids * Remove print statement * More coverage * Add checksum test * Working on authentication tests * 94% coverage * Remove print command * Add link to documentation to the README * Python 2 compatibility * Cleanup * Remove Doxyfile
JeffLIrion · Sep 22, 2019 · b3a0ae9 · b3a0ae9
1 parent f8f70dc
commit b3a0ae9
Show file tree

Hide file tree

Showing 10 changed files with 544 additions and 2,625 deletions.
diff --git a/Doxyfile b/Doxyfile
diff --git a/README.rst b/README.rst
@@ -8,4 +8,6 @@ adb\_shell
    :target: https://coveralls.io/github/JeffLIrion/adb_shell?branch=master
 
 
+Documentation for this package can be found at https://adb-shell.readthedocs.io/.
+
 This Python package implements ADB shell functionality.  It originated from `python-adb <https://github.com/google/python-adb>`_.
diff --git a/adb_shell/adb_device.py b/adb_shell/adb_device.py
diff --git a/adb_shell/adb_message.py b/adb_shell/adb_message.py
@@ -1,4 +1,14 @@
-"""TODO
+"""Functions and an :class:`AdbMessage` class for packing and unpacking ADB messages.
+
+.. rubric:: Contents
+
+* :class:`AdbMessage`
+
+    * :attr:`AdbMessage.checksum`
+    * :meth:`AdbMessage.pack`
+
+* :func:`checksum`
+* :func:`unpack`
 
 """
 
@@ -9,17 +19,17 @@
 
 
 def checksum(data):
-    """TODO
+    """Calculate the checksum of the provided data.
 
     Parameters
     ----------
-    data : TODO
-        TODO
+    data : bytearray, bytes, str
+        The data
 
     Returns
     -------
-    TODO
-        TODO
+    int
+        The checksum
 
     """
     # The checksum is just a sum of all the bytes. I swear.
@@ -42,27 +52,25 @@ def checksum(data):
 
 
 def unpack(message):
-    """TODO
+    """Unpack a received ADB message.
 
     Parameters
     ----------
-    message : TODO
-        TODO
+    message : bytes
+        The received message
 
     Returns
     -------
-    cmd : TODO
-        TODO
-    arg0 : TODO
+    cmd : int
+        The ADB command
+    arg0 : int
         TODO
-    arg1 : TODO
-        TODO
-    data_length : TODO
-        TODO
-    data_checksum : TODO
-        TODO
-    unused_magic : TODO
+    arg1 : int
         TODO
+    data_length : int
+        The length of the data sent by the device (used by :meth:`adb_shell.adb_device._read`)
+    data_checksum : int
+        The checksum of the data sent by the device
 
     Raises
     ------
@@ -71,18 +79,42 @@ def unpack(message):
 
     """
     try:
-        cmd, arg0, arg1, data_length, data_checksum, unused_magic = struct.unpack(constants.MESSAGE_FORMAT, message)
+        cmd, arg0, arg1, data_length, data_checksum, _ = struct.unpack(constants.MESSAGE_FORMAT, message)
     except struct.error as e:
         raise ValueError('Unable to unpack ADB command. ({})'.format(len(message)), constants.MESSAGE_FORMAT, message, e)
 
     return cmd, arg0, arg1, data_length, data_checksum
 
 
 class AdbMessage(object):
-    """TODO
+    """A helper class for packing ADB messages.
+
+    Parameters
+    ----------
+    command : bytes
+        A command; examples used in this package include :const:`adb_shell.constants.AUTH`, :const:`adb_shell.constants.CNXN`, :const:`adb_shell.constants.CLSE`, :const:`adb_shell.constants.OPEN`, and :const:`adb_shell.constants.OKAY`
+    arg0 : int
+        Usually the local ID, but :meth:`~adb_shell.adb_device.AdbDevice.connect` provides :const:`adb_shell.constants.VERSION`, :const:`adb_shell.constants.AUTH_SIGNATURE`, and :const:`adb_shell.constants.AUTH_RSAPUBLICKEY`
+    arg1 : int
+        Usually the remote ID, but :meth:`~adb_shell.adb_device.AdbDevice.connect` provides :const:`adb_shell.constants.MAX_ADB_DATA`
+    data : bytes
+        The data that will be sent
+
+    Attributes
+    ----------
+    arg0 : int
+        Usually the local ID, but :meth:`~adb_shell.adb_device.AdbDevice.connect` provides :const:`adb_shell.constants.VERSION`, :const:`adb_shell.constants.AUTH_SIGNATURE`, and :const:`adb_shell.constants.AUTH_RSAPUBLICKEY`
+    arg1 : int
+        Usually the remote ID, but :meth:`~adb_shell.adb_device.AdbDevice.connect` provides :const:`adb_shell.constants.MAX_ADB_DATA`
+    command : int
+        The input parameter ``command`` converted to an integer via :const:`adb_shell.constants.ID_TO_WIRE`
+    data : bytes
+        The data that will be sent
+    magic : int
+        ``self.command`` with its bits flipped; in other words, ``self.command + self.magic == 2**32 - 1``
 
     """
-    def __init__(self, command=None, arg0=None, arg1=None, data=b''):
+    def __init__(self, command, arg0=None, arg1=None, data=b''):
         self.command = constants.ID_TO_WIRE[command]
         self.magic = self.command ^ 0xFFFFFFFF
         self.arg0 = arg0
@@ -95,19 +127,19 @@ def pack(self):
         Returns
         -------
         bytes
-            TODO
+            The message packed into the format required by ADB
 
         """
         return struct.pack(constants.MESSAGE_FORMAT, self.command, self.arg0, self.arg1, len(self.data), self.checksum, self.magic)
 
     @property
     def checksum(self):
-        """TODO
+        """Return ``checksum(self.data)``
 
         Returns
         -------
-        TODO
-            TODO
+        int
+            The checksum of ``self.data``
 
         """
         return checksum(self.data)
diff --git a/adb_shell/constants.py b/adb_shell/constants.py
@@ -1,4 +1,4 @@
-"""TODO
+"""Constants used throughout the code.
 
 """
 
@@ -21,13 +21,13 @@
 #: ADB protocol version.
 VERSION = 0x01000000
 
-#: AUTH constants for arg0.
+#: AUTH constant for arg0.
 AUTH_TOKEN = 1
 
-#: AUTH constants for arg0.
+#: AUTH constant for arg0.
 AUTH_SIGNATURE = 2
 
-#: AUTH constants for arg0.
+#: AUTH constant for arg0.
 AUTH_RSAPUBLICKEY = 3
 
 AUTH = b'AUTH'
@@ -39,9 +39,13 @@
 SYNC = b'SYNC'
 WRTE = b'WRTE'
 
+#: Commands that are recognized by :meth:`~adb_shell.adb_device.AdbDevice._read`
 IDS = (AUTH, CLSE, CNXN, OKAY, OPEN, SYNC, WRTE)
 
+#: A dictionary where the keys are the commands in :const:`IDS` and the values are the keys converted to integers
 ID_TO_WIRE = {cmd_id: sum(c << (i * 8) for i, c in enumerate(bytearray(cmd_id))) for cmd_id in IDS}
+
+#: A dictionary where the keys are integers and the values are their corresponding commands (type: bytes) from :const:`IDS`
 WIRE_TO_ID = {wire: cmd_id for cmd_id, wire in ID_TO_WIRE.items()}
 
 #: An ADB message is 6 words in little-endian.

diff --git a/adb_shell/exceptions.py b/adb_shell/exceptions.py
@@ -1,4 +1,4 @@
-"""TODO
+"""ADB-related exceptions.
 
 """
 

diff --git a/adb_shell/tcp_handle.py b/adb_shell/tcp_handle.py
@@ -1,14 +1,12 @@
-"""TODO
+"""A class for creating a socket connection with the device and sending and receiving data.
 
 * :class:`TcpHandle`
 
-    * :meth:`TcpHandle._connect`
-    * :meth:`TcpHandle.BulkRead`
-    * :meth:`TcpHandle.BulkWrite`
-    * :meth:`TcpHandle.Close`
-    * :meth:`TcpHandle.serial_number`
-    * :meth:`TcpHandle.Timeout`
-    * :meth:`TcpHandle.TimeoutSeconds`
+    * :attr:`TcpHandle.available`
+    * :attr:`TcpHandle.bulk_read`
+    * :attr:`TcpHandle.bulk_write`
+    * :meth:`TcpHandle.close`
+    * :meth:`TcpHandle.connect`
 
 """
 
@@ -23,23 +21,20 @@
 class TcpHandle(object):
     """TCP connection object.
 
-    Provides same interface as `UsbHandle`.
-
     Parameters
     ----------
     serial : str, bytes, bytearray
-        Android device serial of the form "host" or "host:port". (Host may be an IP address or a host name.)
-    timeout_s : TODO, None
+        Android device serial of the form "host" or "host:port".  (Host may be an IP address or a host name.)
         TODO
 
     Attributes
     ----------
-    _connection : TODO, None
-        TODO
+    _connection : socket.socket, None
+        A socket connection to the device
     host : str
-        TODO
+        The address of the device
     port : str
-        TODO
+        The device port to which we are connecting (default is 5555)
     serial_number : str
         ``<host>:<port>``
 
@@ -57,13 +52,18 @@ def __init__(self, serial):
 
     @property
     def available(self):
-        """TODO
+        """Whether the socket connection has been created.
+
+        Returns
+        -------
+        bool
+            Whether the connection has been created
 
         """
         return bool(self._connection)
 
     def close(self):
-        """TODO
+        """Close the socket connection.
 
         """
         if self._connection:
@@ -72,7 +72,12 @@ def close(self):
             self._connection = None
 
     def connect(self, auth_timeout_s=None):
-        """TODO
+        """Create a socket connection to the device.
+
+        Parameters
+        ----------
+        auth_timeout_s : TODO
+            TODO
 
         """
         timeout = constants.DEFAULT_AUTH_TIMEOUT_S if auth_timeout_s is None else auth_timeout_s
@@ -87,13 +92,13 @@ def bulk_read(self, numbytes, timeout_s=None):
         ----------
         numbytes : int
             TODO
-        timeout_s : TODO, None
-            TODO
+        timeout_s : int, None
+            When the timeout argument is omitted, ``select.select`` blocks until at least one file descriptor is ready. A time-out value of zero specifies a poll and never blocks.
 
         Returns
         -------
-        TODO
-            TODO
+        bytes
+            The received data
 
         Raises
         ------
@@ -117,12 +122,12 @@ def bulk_write(self, data, timeout_s=None):
         data : TODO
             TODO
         timeout_s : TODO, None
-            TODO
+            When the timeout argument is omitted, ``select.select`` blocks until at least one file descriptor is ready. A time-out value of zero specifies a poll and never blocks.
 
         Returns
         -------
-        TODO
-            TODO
+        int
+            The number of bytes sent
 
         Raises
         ------

diff --git a/docs/source/index.rst b/docs/source/index.rst
@@ -14,7 +14,7 @@ adb\_shell Documentation
 
 
 .. include:: ../../README.rst
-   :start-line: 10
+   :start-line: 12
 
 
 Indices and tables

diff --git a/tests/patchers.py b/tests/patchers.py
@@ -5,9 +5,17 @@
 from adb_shell.tcp_handle import TcpHandle
 
 
-MSG_CONNECT = AdbMessage(command=constants.CNXN, arg0=constants.VERSION, arg1=constants.MAX_ADB_DATA, data=b'host::unknown1234567890\0')
+MSG_CONNECT = AdbMessage(command=constants.CNXN, arg0=0, arg1=0, data=b'host::unknown\0')
+MSG_CONNECT_WITH_AUTH_INVALID = AdbMessage(command=constants.AUTH, arg0=0, arg1=0, data=b'host::unknown\0')
+MSG_CONNECT_WITH_AUTH1 = AdbMessage(command=constants.AUTH, arg0=constants.AUTH_TOKEN, arg1=0, data=b'host::unknown\0')
+MSG_CONNECT_WITH_AUTH2 = AdbMessage(command=constants.CNXN, arg0=0, arg1=0, data=b'host::unknown\0')
+MSG_CONNECT_WITH_AUTH_NEW_KEY2 = AdbMessage(command=constants.AUTH, arg0=0, arg1=0, data=b'host::unknown\0')
+MSG_CONNECT_WITH_AUTH_NEW_KEY3 = AdbMessage(command=constants.CNXN, arg0=0, arg1=0, data=b'host::unknown\0')
 
-BULK_READ_LIST0 = [b'CNXN\x00\x00\x00\x00\x00\x00\x00\x00\t\x00\x00\x00\xe4\x02\x00\x00\xbc\xb1\xa7\xb1', bytearray(b'device::\x00')]
+BULK_READ_LIST = [MSG_CONNECT.pack(), MSG_CONNECT.data]
+BULK_READ_LIST_WITH_AUTH_INVALID = [MSG_CONNECT_WITH_AUTH_INVALID.pack(), MSG_CONNECT_WITH_AUTH_INVALID.data]
+BULK_READ_LIST_WITH_AUTH = [MSG_CONNECT_WITH_AUTH1.pack(), MSG_CONNECT_WITH_AUTH1.data, MSG_CONNECT_WITH_AUTH2.pack(), MSG_CONNECT_WITH_AUTH2.data]
+BULK_READ_LIST_WITH_AUTH_NEW_KEY = [MSG_CONNECT_WITH_AUTH1.pack(), MSG_CONNECT_WITH_AUTH1.data, MSG_CONNECT_WITH_AUTH_NEW_KEY2.pack(), MSG_CONNECT_WITH_AUTH_NEW_KEY2.data, MSG_CONNECT_WITH_AUTH_NEW_KEY3.pack(), MSG_CONNECT_WITH_AUTH_NEW_KEY3.data]
 
 
 class FakeSocket(object):
@@ -36,7 +44,6 @@ def close(self):
 
     def connect(self, auth_timeout_s=None):
         self._connection = True
-        self.bulk_read_list = [MSG_CONNECT.pack(), MSG_CONNECT.data]
 
     def bulk_read(self, numbytes, timeout_s=None):
         return self.bulk_read_list.pop(0)
@@ -45,6 +52,12 @@ def bulk_write(self, data, timeout_s=None):
         return len(data)
 
 
+class FakeTcpHandleWithAuth(FakeTcpHandle):
+    def connect(self, auth_timeout_s=None):
+        self._connection = True
+        self.bulk_read_list = [MSG_CONNECT_WITH_AUTH.pack(), MSG_CONNECT_WITH_AUTH.data]
+
+
 # `socket` patches
 patch_create_connection = patch('socket.create_connection', return_value=FakeSocket())
 
@@ -57,3 +70,5 @@ def bulk_write(self, data, timeout_s=None):
 
 # `TcpHandle` patches
 patch_tcp_handle = patch('adb_shell.adb_device.TcpHandle', FakeTcpHandle)
+
+patch_tcp_handle_with_auth = patch('adb_shell.adb_device.TcpHandle', FakeTcpHandleWithAuth)