function state fix and mode scene added #36
Conversation
| @@ -293,7 +293,11 @@ def extract_payload(self,data): | |||
| start=data.find(b'{"devId') | |||
| if(start!=-1): | |||
| result = data[start:] #in 2 steps to deal with the case where '}}' is present before {"devId' | |||
| end=result.find(b'}}')+2 | |||
| end=result.find(b'}}') | |||
| if(end==-1): | |||
There was a problem hiding this comment.
👍 nice! I've not encountered this but this is definitely a hole you've plugged.
There was a problem hiding this comment.
I am not surprise that you did not encountered that case. Without the persistent socket this case should not occur like that. You probably observe something related. I suspect that the encoded status payload you got is probably not a complete payload but just the dps with the first key. In my understanding, this is sometimes send by the device (part of the protocol) and you got that message instead of the status.
| @@ -149,25 +149,58 @@ def __init__(self, dev_id, address, local_key=None, dev_type=None, connection_ti | |||
|
|
|||
| self.port = 6668 # default - do not expect caller to pass in | |||
|
|
|||
| self.s = None # persistent socket | |||
There was a problem hiding this comment.
It looks like this change does two things:
- persistent socket
- retry support
These are good additions :-). I would prefer to see them as configurable, possibly optional (I could be persuaded for these to be the new default behavior if we do a MAJOR version bump at the same time, see https://semver.org/ ).
Please add parms to XenonDevice.init(), e.g. something like
persistent_socket=False, retry=0 # requires MINOR version change
persistent_socket=True, retry=10 # requires MAJOR version change
There was a problem hiding this comment.
Sorry, that part was not part of the pull request. You can see the state function of BulbDevice.
I am new to github and my mistake was to commit on the master branch instead of a new branch.
Concerning the persistent socket, in my opinion it is for testing purpose only to see if issue codetheweb/tuyapi#84 can be fix and it seems so.
But this implementation cause some side effects that's why retry is needed.
I can do this change if you think it is good enough or we can close the request. I think a wrapper on top of the core library will be a better choice. I starting something close to that for domoticz: https://github.com/tixi/Domoticz-Tuya-SmartPlug-Plugin
Let me know. In any case, i will not be available the coming week
There was a problem hiding this comment.
I've done the same thing with commits myself, its easily done. I'd prefer we do these separately.
If you are feeling brave you could branch what you have (to save it) and then forcible remove the commits from history you don't want (I suspect but do not know if this will update the PR). A new PR is fine.
RE re-try, I've a preference for doing that outside too but I would be OK if it was added as an option.
I have my own version of the never-closed socket code too. I never had a need for it/benefit so I never ended up committing it. But it could be useful for diagnostics.
| pass | ||
|
|
||
| cpt_connect=0 | ||
| while(cpt_connect<10): # guess 10 is enough |
There was a problem hiding this comment.
hard coded 10 - please replace with parameter or self.retry, etc.
| try: | ||
| self.s.send(payload) | ||
| data = self.s.recv(4096) | ||
| cpt_connect=10 |
There was a problem hiding this comment.
break here instead? If not replace hard coded 10
| cpt_connect=10 | ||
| except (ConnectionResetError, ConnectionRefusedError, BrokenPipeError) as e: | ||
| cpt_connect = cpt_connect+1 | ||
| if(cpt_connect==10): |
There was a problem hiding this comment.
hard coded 10 - please replace with parameter
|
Hi clach04, |
Hi clach04,
The function state() generate a KeyError exception in case the device do not provide colour temperature.
I fix it by putting in the state only the 6 first keys if they are provided by the device. The mapping is done with DPS_2_STATE.
The function set_white() is modified accordingly to take into account the presence/absence of colour temperature.
I have also added the scene mode.