Don't use keepalive for connections
Swamp-Ig opened this issue · 13 comments
So as discovered by @gcondon1 the connection doesn't like keepalives
It should really just be a matter of disabling keepalive on the connection, update around here
uncomment the async IO use
The aiohttp client session also needs to be set up with a TCP Connector, see docs here, add flag:
force_close=False
The connection is created here so realistically we'll need to create a ClientSession object internally rather than use the one supplied by hass. Might need to rearrange the constructor to get rid of the client session argument.
Need to test this out too, but this is a lot less work than putting in a command queue.
I spent a while debugging this last night, trying to workout why aiohttp packets weren't working, but other clients (like curl were).
After multiple attempts to clone the headers and body it just wouldn't work.
But I have realised that packets being sent through aiohttp, the iZone controller is responding before its even received the body of the post request.
I am not sure if its because aiohttp is breaking the head and body in to two frames, or the line ends are throwing off the controller. But still investigating.
Packet trace from aiohttp:
POST /SystemON HTTP/1.1
User-Agent: HA
Connection: close
Host: 10.0.0.2
Content-Length: 18
Content-Type: application/json
HTTP/1.1 200 OK
Connection: close
Content-Type: application/json
Cache-Control: no-cache
{"SystemON": "on"}
Alright, this is crazy.
Exact same script, exact same packet sent from my Mac and from my Pi (HA).
Mac works, Pi doesn't.
This is either a bug in aiohttp, link level, or both. Either way its above the application level, so not much more we can do.
I'll investigate moving back to Requests and using seperate requests for each command. Or just write a basic socket connection class for sending messages.
Yep, it's pretty weird. I spent ages trying to get it to work, including submitting bugs to aiohttp, but to not much effect. I think it must be some kind of bug in the izone controller, but the iZone people don't seem too interested in fixing it.
The async IO thread is a single thread, it will block awaiting IO response from the server which isn't entirely ideal, your whole HA app is then blocked waiting for input. I think the way to go might be to have a worker thread doing the actual requests, queue the request in the aio thread, then do a non-blocking wait for response while the other thread does the actual request. I think the iZone controller will only handle one request at a time, so this will work. Put the request object in the queue, and then message back to the main thread when it's ready. It's messy, but hopefully should work.
I can see if I can get some time to do some coding, but life is pretty busy at the moment, I will have lots of time from the 1st of Feb.
Just had a thought, is there some way of getting aiohttp to not fragment the request?
What’s Im testing at the moment is creating a session for the requests and setting keep alive to false.
So far so good, if it works I’ll push it up to the master
Seems to be working more reliably now, could you give me push access and ill push up the fix
I had a look at your fix, and tried running it while using wireshark to look at the traffic, but it still didn't work right. It seems the POST request was getting fragmented into more than one TCP packet, and that was what was causing the issue.
The beauty of this fix is that it works perfectly with aio, so won't block waiting for a response from the server, which is much better.
I put in the small code changes in setup.py that you had in your fork.
I've just committed some updates to the repo that should fix the problems. I need to do a bit more debugging to be sure but you can have a go with that if you like.
Sorry, I like my fix better. 😄
- It uses aio so it's not going to lock up the HA core waiting on the response from the server.
- I did test something like you suggested there, it still fragmented the request which meant that the server sent back a spume of bad TCP messages (you could see these in wireshark).
The only real concern is that it just uses raw sockets rather than an HTTP library, so there is the potential that it might not work in some edge cases, but I think it's good enough for purpose.
Looks good, with a controlled device like this, I don’t think we’ll experience too many http edge cases
Oh you might need to update the version at Pypi
I’m having trouble importing 1.0.4
I think there's a slight lag between uploading and it being available. I just had to restart my HA because it didn't download properly the first time and it's working.