-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/auto discovery #3
Feature/auto discovery #3
Conversation
Changes to TPLinkManager to better handle discovered devices
Added a configurable timeout setting to device config Exposed getConfig to public to allow easy querying of a devices config Added powerOn, powerOff and powerStatus direct methods
…meout setting Stopped stream_socket_client from generating warnings
…ksmartplug into feature/auto_discovery
…ksmartplug into feature/auto_discovery
…ksmartplug into feature/auto_discovery
…ksmartplug into feature/auto_discovery
This looks good. Give me a chance to have a review and merge. Thank you! |
src/TPLinkDevice.php
Outdated
@@ -62,15 +62,15 @@ public function sendCommand(array $command) | |||
*/ | |||
protected function connectToDevice() | |||
{ | |||
$this->client = stream_socket_client( | |||
$this->client = @stream_socket_client( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we suppressing warnings here? I'm not keen on that. If there's an error generated we should either deal with it or tell the user.
|
||
public function autoDiscoverTPLinkDevices($ipRange, $timeout = 1) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a big fan of Adam Wathan's idea of Refactoring to Collections.
I try to not use foreach loops in the code (I notice there is one actually I forgot to refactor, so I'll do that soon). I'd like to refactor this function and separate it out a little more.
If you're in the mood to have a look at this again that would be great otherwise I'll have a bash at it later.
I haven't tested this yet, but i was thinking something along the lines of this: /**
* Will return a collection of all TPLink devices auto discovered
* on the IP Range given.
*
* These will already have been added to the global config during
* discovery.
*
* @param $ipRange
* @param int $timeout
*
* @return Collection
*/
public function autoDiscoverTPLinkDevices($ipRange, $timeout = 1)
{
return collect(Range::parse($ipRange))
->map(function ($ip) use ($timeout) {
$response = $this->deviceResponse($ip, $timeout);
return is_null($response) ? $response : $this->validTPLinkResponse($response, $ip);
})
->filter();
}
/**
* Try sending systemInfo command to an ip.
* Possible we may get a blank response, if querying another device which uses these ports
*
* @param $ip
* @param $timeout
*
* @return null
*/
protected function deviceResponse($ip, $timeout)
{
try {
$device = new TPLinkDevice(['ip' => $ip, 'port' => 9999, 'timeout' => $timeout], 'autodiscovery');
return $device->sendCommand(TPLinkCommand::systemInfo());
} catch (Exception $exception) {
return null;
}
}
/**
* Check the returned data JSON decodes
* Make sure is not NULL, some devices may return a single character
* LB100 Series seems to respond on port 9999, however return a bad string
* TODO:: investigate LB100 support
*
* @param $response
* @param $ip
*
* @return mixed|TPLinkDevice
*/
protected function validTPLinkResponse($response, $ip)
{
$jsonResponse = json_decode($response);
return is_null($jsonResponse) ? $jsonResponse : $this->discoveredDevice($jsonResponse, $ip);
}
protected function discoveredDevice($jsonResponse, $ip)
{
return $this->newTPLinkDevice([
'ip' => (string)$ip,
'port' => 9999,
'systemInfo' => $jsonResponse->system->get_sysinfo,
], $jsonResponse->system->get_sysinfo->alias);
} |
Removed supression of stream_socker_client errors
# Conflicts: # src/TPLinkDevice.php
Thanks for these. Will review this week if I get time, otherwise it'll be the end of next week. Much appreciated. |
I haven't forgotten about this |
@shanerutter You might join me over here: #6 |
Added ability to auto discover devices on your network and Some PSR-2 clean up