Seeing if I can split the XMPP responsibilities from the vacuum responsibilities.

sucks/__init__.py

@@ -126,57 +126,37 @@ class EcoVacsAPI:
         return str(b64encode(result), 'utf8')
 
 
-class VacBot(ClientXMPP):
+class VacBot():
     def __init__(self, user, domain, resource, secret, vacuum, continent):
-        ClientXMPP.__init__(self, user + '@' + domain, '0/' + resource + '/' + secret)
 
-        self.user = user
-        self.domain = domain
-        self.resource = resource
         self.vacuum = vacuum
-        self.continent = continent
-        self.credentials['authzid'] = user
-        self.add_event_handler("session_start", self.session_start)
-
-        self.ready_flag = Event()
         self.clean_status = None
         self.charge_status = None
         self.battery_status = None
 
-    def wait_until_ready(self):
+        self.xmpp = EcoVacsXMPP(user, domain, resource, secret, continent)
-        self.ready_flag.wait()
+        self.xmpp.register_callback("CleanReport", self._handle_clean_report)
+        self.xmpp.register_callback("ChargeState", self._handle_charge_report)
+        self.xmpp.register_callback("BatteryInfo", self._handle_battery_report)
+        self.xmpp.register_callback("error", self._handle_error)
 
-    def session_start(self, event):
+    def connect_and_wait_until_ready(self):
-        logging.debug("----------------- starting session ----------------")
+        self.xmpp.connect_and_wait_until_ready()
-        logging.debug("event = {}".format(event))
-        self.ready_flag.set()
 
-        self.__register_callback("CleanReport", self.handle_clean_report)
+        self.xmpp.schedule('Ping', 30, lambda: self.xmpp.send_ping(self._vacuum_adress()), repeat=True)
-        self.__register_callback("ChargeState", self.handle_charge_report)
-        self.__register_callback("BatteryInfo", self.handle_battery_report)
-        self.__register_callback("error", self.handle_error)
 
-        self.schedule('Ping', 30, self.send_ping, repeat=True)
+    def _handle_clean_report(self, iq):
-
-
-    def __register_callback(self, kind, function):
-        self.register_handler(Callback(kind,
-                                       MatchXPath('{jabber:client}iq/{com:ctl}query/{com:ctl}ctl[@td="' + kind + '"]'),
-                                       function))
-
-
-    def handle_clean_report(self, iq):
         self.clean_status = iq.find('{com:ctl}query/{com:ctl}ctl/{com:ctl}clean').get('type')
         logging.debug("*** clean_status = " + self.clean_status)
 
-    def handle_battery_report(self, iq):
+    def _handle_battery_report(self, iq):
         try:
             self.battery_status = float(iq.find('{com:ctl}query/{com:ctl}ctl/{com:ctl}battery').get('power')) / 100
             logging.debug("*** battery_status = {:.0%}".format(self.battery_status))
         except ValueError:
             logging.warning("couldn't parse battery status " + ET.tostring(iq))
 
-    def handle_charge_report(self, iq):
+    def _handle_charge_report(self, iq):
         report = iq.find('{com:ctl}query/{com:ctl}ctl/{com:ctl}charge').get('type')
         if report.lower() == 'going':
             self.charge_status = 'returning'
@@ -188,45 +168,78 @@ class VacBot(ClientXMPP):
             logging.warning("Unknown charging status '" + report + "'")
         logging.debug("*** charge_status = " + self.charge_status)
 
-    def handle_error(self, iq):
+    def _handle_error(self, iq):
         error = iq.find('{com:ctl}query/{com:ctl}ctl').get('error')
         error_no = iq.find('{com:ctl}query/{com:ctl}ctl').get('errno')
         logging.debug("*** error = " + error_no + " " + error)
 
+    def _vacuum_adress(self):
+        return self.vacuum['did'] + '@' + self.vacuum['class'] + '.ecorobot.net/atom'
+
     def send_command(self, xml):
-        c = self.wrap_command(xml)
+        self.xmpp.send_command(xml, self._vacuum_adress())
+
+    def run(self, action):
+        self.send_command(action.to_xml())
+        action.wait_for_completion(self)
+
+    def disconnect(self, wait=False):
+        self.xmpp.disconnect(wait=wait)
+
+
+class EcoVacsXMPP(ClientXMPP):
+    def __init__(self, user, domain, resource, secret, continent):
+        ClientXMPP.__init__(self, user + '@' + domain, '0/' + resource + '/' + secret)
+
+        self.user = user
+        self.domain = domain
+        self.resource = resource
+        self.continent = continent
+        self.credentials['authzid'] = user
+        self.add_event_handler("session_start", self.session_start)
+
+        self.ready_flag = Event()
+
+    def wait_until_ready(self):
+        self.ready_flag.wait()
+
+    def session_start(self, event):
+        logging.debug("----------------- starting session ----------------")
+        logging.debug("event = {}".format(event))
+        self.ready_flag.set()
+
+    def register_callback(self, kind, function):
+        self.register_handler(Callback(kind,
+                                       MatchXPath('{jabber:client}iq/{com:ctl}query/{com:ctl}ctl[@td="' + kind + '"]'),
+                                       function))
+
+    def send_command(self, xml, recipient):
+        c = self._wrap_command(xml, recipient)
         logging.debug('Sending command {0}'.format(c))
         c.send()
 
-    def wrap_command(self, ctl):
+    def _wrap_command(self, ctl, recipient):
-        q = self.make_iq_query(xmlns=u'com:ctl', ito=self.__vacuum_adress(), ifrom=self.__my_address())
+        q = self.make_iq_query(xmlns=u'com:ctl', ito=recipient, ifrom=self._my_address())
         q['type'] = 'set'
         for child in q.xml:
             if child.tag.endswith('query'):
                 child.append(ctl)
                 return q
 
-    def send_ping(self):
+    def _my_address(self):
-        q = self.make_iq_get(ito=self.__vacuum_adress(), ifrom=self.__my_address())
+        return self.user + '@' + self.domain + '/' + self.resource
+
+    def send_ping(self, to):
+        q = self.make_iq_get(ito=to, ifrom=self._my_address())
         q.xml.append(ET.Element('ping', {'xmlns': 'urn:xmpp:ping'}))
         logging.debug("*** sending ping ***")
         q.send()
 
-    def __my_address(self):
-        return self.user + '@' + self.domain + '/' + self.resource
-
-    def __vacuum_adress(self):
-        return self.vacuum['did'] + '@' + self.vacuum['class'] + '.ecorobot.net/atom'
-
     def connect_and_wait_until_ready(self):
         self.connect(('msg-{}.ecouser.net'.format(self.continent), '5223'))
         self.process()
         self.wait_until_ready()
 
-    def run(self, action):
-        self.send_command(action.to_xml())
-        action.wait_for_completion(self)
-
 
 class VacBotCommand:
     def __init__(self, name, args=None, wait=None, terminal=False):

tests/test_cli.py

@@ -61,20 +61,6 @@ def test_continent_for_country():
     assert_equal(continent_for_country('fr'), 'eu')
 
 
-def test_wrap_command():
-    v = VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
-               {"did": "E0000000001234567890", "class": "126", "nick": "bob"}, 'na')
-    c = str(v.wrap_command(Clean(1).to_xml()))
-    assert_true(re.search(r'from="20170101abcdefabcdefa@ecouser.net/abcdef12"', c))
-    assert_true(re.search(r'to="E0000000001234567890@126.ecorobot.net/atom"', c))
-
-
-def test_model_variation():
-    v = VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
-               {"did": "E0000000001234567890", "class": "141", "nick": "bob"}, 'na')
-    c = str(v.wrap_command(Clean(1).to_xml()))
-    assert_true(re.search(r'to="E0000000001234567890@141.ecorobot.net/atom"', c))
-
 
 def test_main_api_setup():
     with requests_mock.mock() as m:

tests/test_commands.py

@@ -0,0 +1,51 @@
+from xml.etree import ElementTree
+
+from nose.tools import *
+
+from sucks import *
+
+
+def test_custom_command():
+    # Ensure a custom-built command generates the expected XML payload
+    c = VacBotCommand('CustomCommand', {'type': 'customtype'})
+    assert_equals(ElementTree.tostring(c.to_xml()),
+
+                  b'<ctl td="CustomCommand"><customcommand type="customtype" /></ctl>')
+
+
+def test_custom_command_noargs():
+    # Ensure a custom-built command with no args generates XML without an args element
+    c = VacBotCommand('CustomCommand')
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl td="CustomCommand" />')
+
+
+def test_clean_command():
+    c = Clean(10)
+    assert_equals(c.terminal, False)
+    assert_equals(c.wait, 10)
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl td="Clean"><clean speed="standard" type="auto" /></ctl>')  # protocol has attribs in other order
+
+
+def test_edge_command():
+    # called Edge because that's what the UI uses, even though the protocol is different
+    c = Edge(10)
+    assert_equals(c.terminal, False)
+    assert_equals(c.wait, 10)
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl td="Clean"><clean speed="strong" type="border" /></ctl>')  # protocol has attribs in other order
+
+
+def test_charge_command():
+    c = Charge()
+    assert_equals(c.terminal, True)
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl td="Charge"><charge type="go" /></ctl>')
+
+
+def test_stop_command():
+    c = Stop()
+    assert_equals(c.terminal, True)
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl td="Clean"><clean speed="standard" type="stop" /></ctl>')

tests/test_ecovacs_xmpp.py

@@ -0,0 +1,15 @@
+from re import search
+
+from nose.tools import *
+
+from sucks import *
+
+
+# There are few tests for the XMPP stuff here because it's relatively complicated to test given
+# the library's design and its multithreaded nature and lack of explicit testing support.
+
+def test_wrap_command():
+    x = EcoVacsXMPP('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12', 'na')
+    c = str(x._wrap_command(Clean(1).to_xml(), 'E0000000001234567890@126.ecorobot.net/atom'))
+    assert_true(search(r'from="20170101abcdefabcdefa@ecouser.net/abcdef12"', c))
+    assert_true(search(r'to="E0000000001234567890@126.ecorobot.net/atom"', c))

tests/test_vacbot.py

@@ -1,70 +1,18 @@
-from xml.etree import ElementTree
-
-from re import search
 from nose.tools import *
 
 from sucks import *
 
 
-# There are few tests for the XMPP stuff here because a) it's relatively complicated to test given
+def test_bot_address():
-# the library's design and its multithreaded nature, and b) I'm manually testing every change anyhow,
+    v = vacbot_for_bot({"did": "E0000000001234567890", "class": "126", "nick": "bob"})
-# as it's not clear how the robot really behaves.
+    assert_equals('E0000000001234567890@126.ecorobot.net/atom', v._vacuum_adress())
-
-def test_custom_command():
-    # Ensure a custom-built command generates the expected XML payload
-    c = VacBotCommand('CustomCommand', {'type': 'customtype'})
-    assert_equals(ElementTree.tostring(c.to_xml()),
-
-                  b'<ctl td="CustomCommand"><customcommand type="customtype" /></ctl>')
-
-def test_custom_command_noargs():
-    # Ensure a custom-built command with no args generates XML without an args element
-    c = VacBotCommand('CustomCommand')
-    assert_equals(ElementTree.tostring(c.to_xml()),
-                  b'<ctl td="CustomCommand" />')
-
-
-def test_clean_command():
-    c = Clean(10)
-    assert_equals(c.terminal, False)
-    assert_equals(c.wait, 10)
-    assert_equals(ElementTree.tostring(c.to_xml()),
-                  b'<ctl td="Clean"><clean speed="standard" type="auto" /></ctl>')  # protocol has attribs in other order
-
-
-def test_edge_command():
-    # called Edge because that's what the UI uses, even though the protocol is different
-    c = Edge(10)
-    assert_equals(c.terminal, False)
-    assert_equals(c.wait, 10)
-    assert_equals(ElementTree.tostring(c.to_xml()),
-                  b'<ctl td="Clean"><clean speed="strong" type="border" /></ctl>')  # protocol has attribs in other order
-
-
-def test_charge_command():
-    c = Charge()
-    assert_equals(c.terminal, True)
-    assert_equals(ElementTree.tostring(c.to_xml()),
-                  b'<ctl td="Charge"><charge type="go" /></ctl>')
-
-
-def test_stop_command():
-    c = Stop()
-    assert_equals(c.terminal, True)
-    assert_equals(ElementTree.tostring(c.to_xml()),
-                  b'<ctl td="Clean"><clean speed="standard" type="stop" /></ctl>')
-
-
-def test_wrap_command():
-    v = VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
-               {"did": "E0000000001234567890", "class": "126", "nick": "bob"}, 'na')
-    c = str(v.wrap_command(Clean(1).to_xml()))
-    assert_true(search(r'from="20170101abcdefabcdefa@ecouser.net/abcdef12"', c))
-    assert_true(search(r'to="E0000000001234567890@126.ecorobot.net/atom"', c))
 
 
 def test_model_variation():
-    v = VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
+    v = vacbot_for_bot({"did": "E0000000001234567890", "class": "141", "nick": "bob"})
-               {"did": "E0000000001234567890", "class": "141", "nick": "bob"}, 'na')
+    assert_equals('E0000000001234567890@141.ecorobot.net/atom', v._vacuum_adress())
-    c = str(v.wrap_command(Clean(1).to_xml()))
+
-    assert_true(search(r'to="E0000000001234567890@141.ecorobot.net/atom"', c))
+
+def vacbot_for_bot(bot):
+    return VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
+                  bot, 'na')