Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for events to accessory server #160

Merged
merged 3 commits into from
Aug 24, 2019

Conversation

Jc2k
Copy link
Collaborator

@Jc2k Jc2k commented Aug 22, 2019

This is the first step towards #51, but arguably not a complete solution. I have pulled it out of #154 so it can be reviewed in isolation. It doesn't have tests on its own (but there are tests that make use of this in #154).

If you have 2 active connections open to an accessory then a put_characteristic on connection 1 will now trigger events on connection but not its own connection (and vice versa).

This refactors the part of handle_one_request that writes a HTTP response to the wire out into its own function and ensures that only one task can write to the socket at once (otherwise there is a race when there are multiple tasks encrypting and writing to the network).

Then there is a write_event helper on the request handler that writes a single HTTP Event Response to the wire. There is a variant of this on the HTTPServer that can write an event to all active connections.

Right now events are only triggered from put_characteristic HTTP handlers. If you were writing an accessory with homekit_python you could manually call write_event on the HTTPServer when you are recording a change. To fully close #51 we should probably integrate a bit better than this, though.

To see it working I run a demoserver.py in on terminal, pair with it in another then do:

python -m homekit.get_events -f test.json -a test -c 1.10

Then in a third terminal

python -m homekit.put_characteristic -f test.json -a test -c 1.10 true

@jlusiardi
Copy link
Owner

nice one!

@jlusiardi
Copy link
Owner

One little addition:
I get an error if I do the following:

  1. Start demoserver.py
  2. Connect Controller A with get_events
  3. Controller B toggles the light bulb a few times
  4. Kill Controller A with CTRL-C
  5. Toggle one more event with Controller B results in an exception in the demoserver and a hanging Controller B
----------------------------------------
Exception happened during processing of request from ('192.168.178.21', 45992)
Traceback (most recent call last):
  File "/usr/lib/python3.5/socketserver.py", line 625, in process_request_thread
    self.finish_request(request, client_address)
  File "/usr/lib/python3.5/socketserver.py", line 354, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 327, in __init__
    # make connection non blocking so the select can work
  File "/usr/lib/python3.5/socketserver.py", line 681, in __init__
    self.handle()
  File "/usr/lib/python3.5/http/server.py", line 424, in handle
    self.handle_one_request()
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 479, in handle_one_request
    ev = params['ev'] == '1'
  File "/usr/lib/python3.5/http/server.py", line 410, in handle_one_request
    method()
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 1283, in do_PUT
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 677, in _put_characteristics
    # 2) generate shared secret
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 87, in write_event
    Sets the callback function for this accessory server. This will NOT be applied to all accessories registered
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 363, in write_event
    self.log_error(' %r', e)
  File "/home/jlusiardi/Dokumente/src/homekit_python_jc2k/homekit/accessoryserver.py", line 387, in write_encrypted_bytes
    data)
  File "/usr/lib/python3.5/socket.py", line 591, in write
    self._checkClosed()
ValueError: I/O operation on closed file.
----------------------------------------

It is no problem if Controller A quits because enough events were received or due to waiting the given amount of time.

@jlusiardi
Copy link
Owner

This could do the trick:

diff --git a/homekit/accessoryserver.py b/homekit/accessoryserver.py
index 236f3b7..f026965 100644
--- a/homekit/accessoryserver.py
+++ b/homekit/accessoryserver.py
@@ -39,14 +39,13 @@ from homekit.crypto.chacha20poly1305 import chacha20_aead_decrypt, chacha20_aead
 from homekit.crypto.srp import SrpServer
 
 from homekit.exceptions import ConfigurationError, ConfigLoadingError, ConfigSavingError, FormatError, \
-    CharacteristicPermissionError
+    CharacteristicPermissionError, DisconnectedControllerError
 from homekit.http_impl import HttpStatusCodes
 from homekit.model import Accessories, Categories
 from homekit.model.characteristics import CharacteristicsTypes
 from homekit.protocol import TLV 
 from homekit.protocol.statuscodes import HapStatusCodes
 
-
 class AccessoryServer(ThreadingMixIn, HTTPServer):
     """ 
     This server makes accessories accessible via the HomeKit protocol.
@@ -81,10 +80,16 @@ class AccessoryServer(ThreadingMixIn, HTTPServer):
         HTTPServer.__init__(self, (self.data.ip, self.data.port), AccessoryRequestHandler)
 
     def write_event(self, characteristics, source=None):
+        dead_sessions = []
         for session_id, session in self.sessions.items():
             if source and session_id == source:
                 continue
-            session['handler'].write_event(characteristics)
+            try:
+                session['handler'].write_event(characteristics)
+            except DisconnectedControllerError:
+                dead_sessions.append(session_id)
+        for session_id in dead_sessions:
+            del self.sessions[session_id]
 
     def add_accessory(self, accessory):
         self.accessories.add_accessory(accessory)
@@ -384,8 +389,11 @@ class AccessoryRequestHandler(BaseHTTPRequestHandler):
                 self.server.sessions[self.session_id]['accessory_to_controller_count'] += 1
                 out_data += len_bytes + ciper_and_mac[0] + ciper_and_mac[1]
 
-            self.orig_wfile.write(out_data)
-            self.orig_wfile.flush()
+            try:
+                self.orig_wfile.write(out_data)
+                self.orig_wfile.flush()
+            except ValueError as e:
+                raise DisconnectedControllerError()
 
     def handle_one_request(self):
         """
diff --git a/homekit/exceptions.py b/homekit/exceptions.py
index ca4daf9..7a4d0cf 100644
--- a/homekit/exceptions.py
+++ b/homekit/exceptions.py
@@ -254,3 +254,9 @@ class TransportNotSupportedError(HomeKitException):
     def __init__(self, transport):
         Exception.__init__(self,
                            'Transport {t} not supported. See setup.py for required dependencies.'.format(t=transport))
+
+
+class DisconnectedControllerError(HomeKitException):
+    def __init__(self):
+        Exception.__init__(self, 'Controller has passed away')
+

What do you think?

@Jc2k
Copy link
Collaborator Author

Jc2k commented Aug 23, 2019

Looks good - i have applied that change to my branch with a tiny flake8 fix.

@jlusiardi jlusiardi merged commit a66ecab into jlusiardi:master Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement events in the accessory server
2 participants