glenpp / OrviboS20

Python management script for Orvibo S20 WiFi Plug
GNU General Public License v2.0
13 stars 4 forks source link

Intermittent Exceptions #5

Closed merlin051 closed 6 years ago

merlin051 commented 6 years ago

Hi,

I've been using your script for a while with my home automation setup to control the S20 plugs.

However its always been intermittently failing.

Its hard to capture so i created a batch file to call the script to quickly flash two plugs on/off several times and I've captured three different exceptions as per below, I don't think there is anything wrong with my syntax because the scripts work 80% of the time.

my best guess would be that a subscribe command failed?

I'm no python export so I figured I would ask here whilst I try to learn the syntax and understand the script a little more.

Exception1: C:\Users\media\Desktop>C:\python27\python C:\ha-bridge-master\s20control.py poweron 192.168.0.9 AC:CF:23:93:33:DC Traceback (most recent call last): File "C:\ha-bridge-master\s20control.py", line 346, in control.poweron ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 264, in poweron self._subscribeifneeded ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 260, in _subscribeifneeded self.subscribe ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 243, in subscribe resp['address'], KeyError: 'address'

Exception2:

C:\Users\media\Desktop>C:\python27\python C:\ha-bridge-master\s20control.py poweroff 192.168.0.8 AC:CF:23:8D:2A:10 Traceback (most recent call last): File "C:\ha-bridge-master\s20control.py", line 349, in control.poweroff ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 275, in poweroff self._subscribeifneeded ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 262, in _subscribeifneeded raise # something failed TypeError: exceptions must be old-style classes or derived from BaseException, not NoneType

Exception3:

C:\Users\media\Desktop>C:\python27\python C:\ha-bridge-master\s20control.py poweron 192.168.0.8 AC:CF:23:8D:2A:10 Traceback (most recent call last): File "C:\ha-bridge-master\s20control.py", line 346, in control.poweron ( ip, mac ) File "C:\ha-bridge-master\s20control.py", line 270, in poweron resp = self._listendiscover () File "C:\ha-bridge-master\s20control.py", line 192, in _listendiscover except UnknownPacket, e: # TODO this should be more specific to avoid trapping syntax errors NameError: global name 'UnknownPacket' is not defined test - Copy.bat.txt

s20control.py.txt

glenpp commented 6 years ago

There are limitations with the protocol (eg. it's UDP so vulnerable to packet loss and requires specific ports which there can be contention over) and the plugs themselves seem to not always be listening (eg. during or immediately after an operation) which compounds these factors. Also keep in mind that there's various strange behaviours that can happen on wifi anyway due to power management and other aspects (this is why you find things like phone aps sometimes "warm up" the connection with a burst of dummy traffic before the real traffic). Basically, this means that it will never be 100% reliable no matter what the code does and in particular will be prone to problems on rapid chains of actions.

The particular examples you show here appear to be an early version which was quick & dirty code done while figuring out the protocol. The current version already should handle communications problems more gracefully with clearer use of exceptions. Please try that first.

That said, the reason for the exceptions is that these are situations where the code calling this needs to decide what to do in the context of your particular application. eg. should it try again in a few minutes? should it log the failed request and back off? should it call a human? should it ignore the failure and continue? These are decisions that should not be made in general-purpose "library" code like this.

merlin051 commented 6 years ago

Thanks for the response.

I've updated to the latest script and re-ran the tests and the only exception i get is as follows:

C:\Users\media\Desktop>C:\python27\python C:\ha-bridge-master\s20control.py poweroff 192.168.0.9 AC:CF:23:93:33:DC Traceback (most recent call last): File "C:\ha-bridge-master\s20control.py", line 412, in main() File "C:\ha-bridge-master\s20control.py", line 404, in main control.poweroff(ip, mac) File "C:\ha-bridge-master\s20control.py", line 291, in poweroff self._subscribeifneeded(ip, mac) File "C:\ha-bridge-master\s20control.py", line 275, in _subscribeifneeded self.subscribe(ip, mac) File "C:\ha-bridge-master\s20control.py", line 253, in subscribe resp = self._listendiscover() File "C:\ha-bridge-master\s20control.py", line 194, in _listendiscover except UnknownPacket, e: # TODO this should be more specific to avoid trapping syntax errors NameError: global name 'UnknownPacket' is not defined

I guess what you are saying is that if i wrap this library and then handle and exceptions in the wrapper, i should be able to manage what happens next, rather than it just stopping.

glenpp commented 6 years ago

Yep - that looks like a bug! The exception is defined in the wrong scope, and actually not suited to the way it's ended up being used.

To clarify - yes, this is a library that you can use within your own application, but does have the trick that it can be also used as a command line (or scripted) utility which is also useful for testing it. If you are using it as a utility and there is a communications problem (eg. the device is off the network at the time or just one of the times it's ignoring traffic) then it will exit with an exception. As a library you can catch the exception and take further action that's appropriate to your application.

Commit coming shortly to fix this, though limited testing as I only see errors every few months.