Fix PPP Errors with disconnect and routing (#32)

Fix #30 and #31

Author: DomAmato

Hologram/Network/Cellular.py

@@ -73,6 +73,8 @@ def connect(self, timeout = DEFAULT_CELLULAR_TIMEOUT):
             self.logger.info('Successfully connected to cell network')
             # Disable at sockets mode since we're already establishing PPP.
             # This call is needed in certain modems that have limited interfaces to work with.
+            time.sleep(2)
+            # give the device a little time to enumerate
             self.disable_at_sockets_mode()
             self.__configure_routing()
             self._connection_status = CLOUD_CONNECTED
@@ -85,6 +87,7 @@ def connect(self, timeout = DEFAULT_CELLULAR_TIMEOUT):
 
     def disconnect(self):
         self.logger.info('Disconnecting from cell network')
+        self.__remove_routing()
         success = self.modem.disconnect()
         if success:
             self.logger.info('Successfully disconnected from cell network')
@@ -166,13 +169,23 @@ def __reconnect_after_forced_disconnect(self):
         self.logger.info('Ready to receive data on port %s', self.__receive_port)
 
     def __configure_routing(self):
+        # maybe we don't have to tear down the routes but we probably should
         self.logger.info('Adding routes to Hologram cloud')
         self._route.add('10.176.0.0/16', self.localIPAddress)
         self._route.add('10.254.0.0/16', self.localIPAddress)
         if self.scope == NetworkScope.SYSTEM:
             self.logger.info('Adding system-wide default route to cellular interface')
             self._route.add_default(self.localIPAddress)
 
+    def __remove_routing(self):
+        self.logger.info('Removing routes to Hologram cloud')
+        if self.localIPAddress:
+            self._route.delete('10.176.0.0/16', self.localIPAddress)
+            self._route.delete('10.254.0.0/16', self.localIPAddress)
+            if self.scope == NetworkScope.SYSTEM:
+                self.logger.info('Removing system-wide default route to cellular interface')
+                self._route.delete_default(self.localIPAddress)
+
     def _load_modem_drivers(self):
         dl = DriverLoader.DriverLoader()
         for (modemName, modemHandler) in self._modemHandlers.items():

Hologram/Network/Modem/Modem.py

@@ -13,7 +13,8 @@
 from UtilClasses import ModemResult
 from UtilClasses import SMS
 from Hologram.Event import Event
-from Exceptions.HologramError import SerialError, HologramError, NetworkError
+from Exceptions.HologramError import SerialError, HologramError, NetworkError, PPPError
+
 
 from collections import deque
 import binascii
@@ -100,8 +101,16 @@ def connect(self, timeout):
     def disconnect(self):
 
         if self._mode is not None:
-            return self._mode.disconnect()
-        return None
+            res = self._mode.disconnect()
+            self._mode = None
+            return res
+        else:
+            try:
+                PPP.shut_down_existing_ppp_session(self.logger)
+                return True
+            except PPPError as e:
+                self.logger.info('Got PPPError trying to disconnect open sessions')
+                return None
 
     def _initialize_device_name(self, device_name):
         if device_name is None:
@@ -883,11 +892,17 @@ def modem_mode(self, mode):
 
     @property
     def localIPAddress(self):
-        return self._mode.localIPAddress
+        if self._mode:
+            return self._mode.localIPAddress
+        else:
+            return None
 
     @property
     def remoteIPAddress(self):
-        return self._mode.remoteIPAddress
+        if self._mode:
+            return self._mode.remoteIPAddress
+        else:
+            return None
 
 
     @property

Hologram/Network/Modem/ModemMode/PPP.py

@@ -58,35 +58,37 @@ def connect(self, timeout=DEFAULT_PPP_TIMEOUT):
 
     def disconnect(self):
         self._ppp.disconnect()
-        self.__shut_down_existing_ppp_session()
+        PPP.shut_down_existing_ppp_session(self.logger)
         return True
 
     # EFFECTS: Makes sure that there are no existing PPP instances on the same
     #          device interface.
     def __enforce_no_existing_ppp_session(self):
 
-        pid_list = self.__check_for_existing_ppp_sessions()
+        pid_list = PPP.check_for_existing_ppp_sessions(self.logger)
 
         if len(pid_list) > 0:
             raise PPPError('Existing PPP session(s) are established by pid(s) %s. Please close/kill these processes first'
                            % pid_list)
 
-    def __shut_down_existing_ppp_session(self):
-        pid_list = self.__check_for_existing_ppp_sessions()
+    @staticmethod
+    def shut_down_existing_ppp_session(logger):
+        pid_list = PPP.check_for_existing_ppp_sessions(logger)
 
         # Process this only if it is a valid PID integer.
         for pid in pid_list:
-            self.logger.info('Killing pid %s that currently have an active PPP session',
+            logger.info('Killing pid %s that currently have an active PPP session',
                              pid)
             process = psutil.Process(pid)
             process.terminate()
             # Wait at least 10 seconds for the process to terminate
             process.wait(10)
 
-    def __check_for_existing_ppp_sessions(self):
+    @staticmethod
+    def check_for_existing_ppp_sessions(logger):
 
         existing_ppp_pids = []
-        self.logger.info('Checking for existing PPP sessions')
+        logger.info('Checking for existing PPP sessions')
 
         for proc in psutil.process_iter():
             try:
@@ -95,7 +97,7 @@ def __check_for_existing_ppp_sessions(self):
                 raise PPPError('Failed to check for existing PPP sessions')
 
             if 'pppd' in pinfo['name']:
-                self.logger.info('Found existing PPP session on pid: %s', pinfo['pid'])
+                logger.info('Found existing PPP session on pid: %s', pinfo['pid'])
                 existing_ppp_pids.append(pinfo['pid'])
 
         return existing_ppp_pids

Hologram/Network/Modem/ModemMode/pppd.py

@@ -118,6 +118,10 @@ def disconnect(self):
         if self.proc and self.proc.poll() is None:
             self.proc.send_signal(signal.SIGTERM)
             time.sleep(1)
+            # Reset the values when we disconnect
+            self._laddr = None
+            self._raddr = None
+            self.proc = None
 
 
     # EFFECTS: Returns true if a cellular connection is established.

Hologram/Network/Route.py

@@ -20,7 +20,6 @@
 
 class Route:
     def __init__(self):
-        self.ipr = IPRoute()
         self.logger = logging.getLogger(__name__)
         self.logger.addHandler(NullHandler())
 
@@ -56,13 +55,33 @@ def add_default(self, gateway):
             self.logger.debug('Could not set default route due to NetlinkError: %s', str(e))
 
     def add(self, destination, gateway):
-        self.ipr.route('add',
+        self.logger.debug('Adding Route %s : %s', destination, gateway)
+        with IPRoute() as ipr:
+            ipr.route('add',
                        dst=destination,
                        gateway=gateway)
 
+    def delete_default(self, gateway):
+        try:
+            self.delete(DEFAULT_DESTINATION, gateway)
+        except NetlinkError as e:
+            self.logger.debug('Could not set default route due to NetlinkError: %s', str(e))
+
+    def delete(self, destination, gateway):
+        self.logger.debug('Removing Route %s : %s', destination, gateway)
+        try:
+            with IPRoute() as ipr:
+                ipr.route('del',
+                           dst=destination,
+                           gateway=gateway)
+        except NetlinkError as e:
+            self.logger.debug('Could not delete route due to NetlinkError: %s', str(e))
+
+
     def __interface_index(self, interface):
         index = None
-        indexes = self.ipr.link_lookup(ifname=interface)
+        with IPRoute() as ipr:
+            indexes = ipr.link_lookup(ifname=interface)
         if len(indexes) == 1:
             index = indexes[0]
         return index
@@ -71,7 +90,8 @@ def __get_interface_state(self, interface):
         if self.is_interface_available(interface):
             link_state = None
             ipr_index = self.__interface_index(interface)
-            links = self.ipr.get_links()
+            with IPRoute() as ipr:
+                links = ipr.get_links()
 
             for link in links:
                 if link['index'] == ipr_index: