search

martenjacobs / py-otgw-mqtt

Python OTGW MQTT bridge
MIT License
9 stars 16 forks source link

Tcp #3

Closed jodur closed 5 years ago

jodur commented 6 years ago

Marten,

I have tested the code and made some adjustments to run it proper . Also after a disconnection of the OTGW, the programm will continue to work. I think you could merge it now to the master code when you agree!

martenjacobs commented 5 years ago

Hi Jodur, I'm sorry I never replied to this PR. The reason is that you've made many more changes than necessary for the premise of the PR, which is adding support for TCP connections. This breaks support for Python 2.7 and may have other undesirable side-effects I'd like to test before merging. I don't really have the time to run through everything so it will work on Python 2.7 again and I don't have the necessary testing environment either (i.e. I don't have a TCP-based OTGW). In my experience, one usually tries to minimize the changes in a single PR to only include what's exactly necessary for the proposed feature or bug fix. Other proposed improvements can then be discussed in separate PRs.

jodur commented 5 years ago

Don't feel quilty about it, i didn't spend any time since i posted it. For me this was a learning phyton project. For the TCP/IP, we started in the wrong direction. It seems that pyserial supports TCP/IP socket by default. There is now also a OTGW pyhton library that was devolped for homeassistant and the OTGW is now supported by homeassistant. https://github.com/mvn23/pyotgw

martenjacobs commented 5 years ago

OK, that seems like it would be a better fit for users who'd like to integrate a TCP-enabled OTGW into HomeAssistant. I still feel this app serves a purpose for those of us using a serial-based OTGW (like me) by allowing those to work over a network.

gdschut commented 5 years ago

I tried the HA OTGW implementation with tcp connection, but it made my HA installation very instable, because of connection problems. For me it is important to have a reliable OTGW connection, so I decided to switch to MQTT, and use the py-otgw-mqtt tcp bridge. Of course it would be nice if the HA implementation would work properly, but for now it is not. see also https://github.com/home-assistant/home-assistant/issues/17263 The result of instability was that random things did not work any more, even simple automations were not reliable.