happyleavesaoc / python-orvibo

MIT License
36 stars 20 forks source link

Discover configured devices #3

Closed rasathus closed 8 years ago

rasathus commented 8 years ago

Hello.

Im in the process of looking at the module with a view to implementing discovery of devices on the network.

After looking through the code, I realised that the hard work had already been done, in that it can send discovery requests, and receive the responses.

At the moment the majority of the code is structured under the S20 object, which requires the host details to initialise. I thought I would open this ticket to discuss breaking some of this out, and restructuring it to make it possible to implement discover as a standalone feature.

Does anyone have any thoughts on this ?

happyleavesaoc commented 8 years ago

That's the reason I didn't initially add discovery.

I think it may be worthwhile to have an S20Manager object that would handle the following:

This change would move the send/receive functions into the manager, and all S20 objects would share the same manager/socket with a command queue. (which should fix some issues that manifest when you have multiple sockets being controlled at the same time). It would also be a convenient place to do discovery.

The downside is a bit more complexity when using this module.

rasathus commented 8 years ago

Thats along the lines of my initial thoughts.

Do you think the S20Manager should decode the messages to retrieve the host and then hand it off the the relevant S20 object, or should it decode the messages and call the relevant method on the registered S20 object to deal with that type of event ?

sergiookey commented 8 years ago

Thank for your help. For ORVIBO ALLONE R1U? I am interesting a control total with Python.

happyleavesaoc commented 8 years ago

I don't feel strongly a particular way with respect to which class should handle message decode. Intuitively, I'd think that S20Manager would handle decode, since it has to handle at least some decode already, specifically the discovery messages.

happyleavesaoc commented 8 years ago

I'm considering moving _socket to be a module-level variable, accessed with locks. Then extracting the necessary parts of discover to a module-level function. I think this could keep the design simpler.

happyleavesaoc commented 8 years ago

I added discover. It returns a set of host addresses.

seoyoungjin commented 8 years ago

I'm using orvibo switch with homeassistant. I made a fork and commited some changes. Please review. https://github.com/seoyoungjin/python-orvibo Thanks.

happyleavesaoc commented 8 years ago

I took a look at your commits - nice changes. Can you do a pull request? Then we can merge the commits.

seoyoungjin commented 8 years ago

I created new pull request but I don't know this is right because I'm not good at git. :-) Anyway I'm writing more code.

happyleavesaoc commented 8 years ago

Merged your PR, thanks!