Add tests, and fix some edge cases revealed by tests (yay!)

sucks/__init__.py

@@ -293,7 +293,10 @@ class VacBot():
                 logging.warning("Unknown fan speed: '" + fan + "'")
         self.fan_speed = fan
         self.statusEvents.notify(self.vacuum_status)
+        if self.fan_speed:
             logging.debug("*** clean_status = " + self.clean_status + " fan_speed = " + self.fan_speed)
+        else:
+            logging.debug("*** clean_status = " + self.clean_status + " fan_speed = None")
 
     def _handle_battery_info(self, iq):
         try:
@@ -310,7 +313,7 @@ class VacBot():
             status = CHARGE_MODE_FROM_ECOVACS[status]
         except KeyError:
             logging.warning("Unknown charging status '" + status + "'")
-        else:
+
         self.charge_status = status
         if status != 'idle' or self.vacuum_status == 'charging':
             # We have to ignore the idle messages, because all it means is that it's not
@@ -337,9 +340,14 @@ class VacBot():
         else:
             self._failed_pings = 0
             if self._monitor:
-                # If we don't yet have a vacuum status, request initial statuses again, now that the ping succeeded
+                # If we don't yet have a vacuum status, request initial statuses again now that the ping succeeded
                 if self.vacuum_status == 'offline' or self.vacuum_status is None:
                     self.request_all_statuses()
+            else:
+                # If we're not auto-monitoring the status, then just reset the status to None, which indicates unknown
+                if self.vacuum_status == 'offline':
+                    self.vacuum_status = None
+                    self.statusEvents.notify(self.vacuum_status)
 
     def refresh_components(self):
         try:

tests/test_commands.py

@@ -59,6 +59,18 @@ def test_stop_command():
                   b'<ctl td="Clean"><clean speed="standard" type="stop" /></ctl>')
 
 
+def test_play_sound_command():
+    c = PlaySound()
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl sid="0" td="PlaySound" />')
+
+
+def test_play_sound_command_with_sid():
+    c = PlaySound(sid="1")
+    assert_equals(ElementTree.tostring(c.to_xml()),
+                  b'<ctl sid="1" td="PlaySound" />')
+
+
 def test_get_clean_state_command():
     c = GetCleanState()
     assert_equals(ElementTree.tostring(c.to_xml()),

tests/test_vacbot.py

@@ -2,6 +2,8 @@ from nose.tools import *
 
 from sucks import *
 
+from unittest.mock import Mock
+from sleekxmpp.exceptions import XMPPError
 
 
 def test_handle_clean_report():
@@ -10,6 +12,22 @@ def test_handle_clean_report():
 
     v._handle_ctl({'event': 'clean_report', 'type': 'auto', 'speed': 'strong'})
     assert_equals('auto', v.clean_status)
+    assert_equals('high', v.fan_speed)
+
+    v._handle_ctl({'event': 'clean_report', 'type': 'border', 'speed': 'standard'})
+    assert_equals('edge', v.clean_status)
+    assert_equals('normal', v.fan_speed)
+
+    # Missing fan_speed
+    v = a_vacbot()
+    v._handle_ctl({'event': 'clean_report', 'type': 'border'})
+    assert_equals('edge', v.clean_status)
+    assert_is_none(v.fan_speed)
+
+    # For states not handled by sucks constants, fall back to just using whatever the vacuum said
+    v._handle_ctl({'event': 'clean_report', 'type': 'a_type_not_supported_by_sucks', 'speed': 'a_weird_speed'})
+    assert_equals('a_type_not_supported_by_sucks', v.clean_status)
+    assert_equals('a_weird_speed', v.fan_speed)
 
 
 def test_handle_charge_state():
@@ -25,6 +43,28 @@ def test_handle_charge_state():
     v._handle_ctl({'event': 'charge_state', 'type': 'idle'})
     assert_equals('idle', v.charge_status)
 
+    v._handle_ctl({'event': 'charge_state', 'type': 'a_type_not_supported_by_sucks'})
+    assert_equals('a_type_not_supported_by_sucks', v.charge_status)
+
+
+def test_vacuum_states():
+    # Vacuum state usually mirrors the latest charge or clean report, but there are some edge cases where it doesn't
+    # work that way. This test ensures the edge cases are handled correctly.
+    v = a_vacbot()
+    assert_equals(None, v.vacuum_status)
+
+    v._handle_ctl({'event': 'clean_report', 'type': 'auto', 'speed': 'strong'})
+    assert_equals('auto', v.vacuum_status)
+
+    # Ignore the "idle" charge state in most cases, as it can be reported during a cleaning (such as during initialization)
+    v._handle_ctl({'event': 'clean_report', 'type': 'auto'})
+    v._handle_ctl({'event': 'charge_state', 'type': 'idle'})
+    assert_equals('auto', v.vacuum_status)
+
+    # However, we do honor the idle state when our current state is charging, as that can happen in some certain combination of events
+    v._handle_ctl({'event': 'charge_state', 'type': 'slot_charging'})
+    v._handle_ctl({'event': 'charge_state', 'type': 'idle'})
+    assert_equals('idle', v.vacuum_status)
 
 def test_handle_battery_info():
     v = a_vacbot()
@@ -39,6 +79,143 @@ def test_handle_battery_info():
     v._handle_ctl({'event': 'battery_info', 'power': '000'})
     assert_equals(0.0, v.battery_status)
 
+def test_lifespan_reports():
+    v = a_vacbot()
+    assert_equals({}, v.components)
+
+    v._handle_ctl({'event': 'life_span', 'type': 'side_brush', 'total': '100', 'val': '50'})
+    assert_equals({'side_brush': 0.5}, v.components)
+
+    v._handle_ctl({'event': 'life_span', 'type': 'brush', 'total': '200', 'val': '1'})
+    assert_equals({'side_brush': 0.5, 'main_brush': 0.005}, v.components)
+
+    v._handle_ctl({'event': 'life_span', 'type': 'side_brush', 'total': '100', 'val': '0'})
+    assert_equals({'side_brush': 0, 'main_brush': 0.005}, v.components)
+
+    v._handle_ctl({'event': 'life_span', 'type': 'a_weird_component', 'total': '100', 'val': '87'})
+    assert_equals({'side_brush': 0, 'main_brush': 0.005, 'a_weird_component': 0.87}, v.components)
+
+def test_send_ping_no_monitor():
+    v = a_vacbot()
+
+    mock = v.xmpp.send_ping = Mock()
+    v.send_ping()
+
+    # On four failed pings, vacuum state gets set to 'offline'
+    mock.side_effect = XMPPError()
+    v.send_ping()
+    v.send_ping()
+    v.send_ping()
+    assert_equals(None, v.vacuum_status)
+    v.send_ping()
+    assert_equals('offline', v.vacuum_status)
+
+    # On a successful ping after the offline state, state gets reset to None, indicating that it is unknown
+    mock.side_effect = None
+    v.send_ping()
+    assert_equals(None, v.vacuum_status)
+
+
+def test_send_ping_with_monitor():
+    v = a_vacbot(monitor=True)
+
+    ping_mock = v.xmpp.send_ping = Mock()
+    request_statuses_mock = v.request_all_statuses = Mock()
+
+    # First ping should try to fetch statuses
+    v.send_ping()
+    assert_equals(1, request_statuses_mock.call_count)
+
+    # Nothing blowing up is success
+
+    # On four failed pings, vacuum state gets set to 'offline'
+    ping_mock.side_effect = XMPPError()
+    v.send_ping()
+    v.send_ping()
+    v.send_ping()
+    assert_equals(None, v.vacuum_status)
+    v.send_ping()
+    assert_equals('offline', v.vacuum_status)
+
+    # On a successful ping after the offline state, a request for initial statuses is made
+    ping_mock.side_effect = None
+    request_statuses_mock.reset_mock()
+    v.send_ping()
+    assert_equals(1, request_statuses_mock.call_count)
+
+
+def test_status_event_subscription():
+    v = a_vacbot()
+
+    mock = Mock()
+    v.statusEvents.subscribe(mock)
+    v._handle_ctl({'event': 'clean_report', 'type': 'auto', 'speed': 'strong'})
+    mock.assert_called_once_with('auto')
+
+    mock = Mock()
+    v.statusEvents.subscribe(mock)
+    v._handle_ctl({'event': 'charge_state', 'type': 'going'})
+    mock.assert_called_once_with('returning')
+
+    # Test unsubscribe
+    mock = Mock()
+    subscription = v.statusEvents.subscribe(mock)
+    v._handle_ctl({'event': 'charge_state', 'type': 'going'})
+    assert_equals(1, mock.call_count)
+    subscription.unsubscribe()
+    v._handle_ctl({'event': 'charge_state', 'type': 'slot_charging'})
+    assert_equals(1, mock.call_count)
+
+def test_battery_event_subscription():
+    v = a_vacbot()
+
+    mock = Mock()
+    v.batteryEvents.subscribe(mock)
+    v._handle_ctl({'event': 'battery_info', 'power': '095'})
+    mock.assert_called_once_with(0.95)
+
+    # Test unsubscribe
+    mock = Mock()
+    subscription = v.batteryEvents.subscribe(mock)
+    v._handle_ctl({'event': 'battery_info', 'power': '095'})
+    assert_equals(1, mock.call_count)
+    subscription.unsubscribe()
+    v._handle_ctl({'event': 'battery_info', 'power': '090'})
+    assert_equals(1, mock.call_count)
+
+def test_lifespan_event_subscription():
+    v = a_vacbot()
+
+    mock = Mock()
+    v.lifespanEvents.subscribe(mock)
+    v._handle_ctl({'event': 'life_span', 'type': 'side_brush', 'total': '100', 'val': '50'})
+    mock.assert_called_once_with({'type': 'side_brush', 'lifespan': 0.5})
+
+    # Test unsubscribe
+    mock = Mock()
+    subscription = v.lifespanEvents.subscribe(mock)
+    v._handle_ctl({'event': 'life_span', 'type': 'side_brush', 'total': '100', 'val': '50'})
+    assert_equals(1, mock.call_count)
+    subscription.unsubscribe()
+    v._handle_ctl({'event': 'life_span', 'type': 'side_brush', 'total': '100', 'val': '25'})
+    assert_equals(1, mock.call_count)
+
+def test_error_event_subscription():
+    v = a_vacbot()
+
+    mock = Mock()
+    v.errorEvents.subscribe(mock)
+    v._handle_ctl({'event': 'error', 'error': 'an_error_name'})
+    mock.assert_called_once_with('an_error_name')
+
+    # Test unsubscribe
+    mock = Mock()
+    subscription = v.errorEvents.subscribe(mock)
+    v._handle_ctl({'event': 'error', 'error': 'an_error_name'})
+    assert_equals(1, mock.call_count)
+    subscription.unsubscribe()
+    v._handle_ctl({'event': 'error', 'error': 'an_error_name'})
+    assert_equals(1, mock.call_count)
 
 def test_handle_unknown_ctl():
     v = a_vacbot()
@@ -67,8 +244,8 @@ def test_model_variation():
 
 
 
-def a_vacbot(bot=None):
+def a_vacbot(bot=None, monitor=False):
     if bot is None:
         bot = {"did": "E0000000001234567890", "class": "126", "nick": "bob"}
     return VacBot('20170101abcdefabcdefa', 'ecouser.net', 'abcdef12', 'A1b2C3d4efghijklmNOPQrstuvwxyz12',
-                  bot, 'na')
+                  bot, 'na', monitor=monitor)