Skip to content
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

Closed

Conversation

shanerutter
Copy link

Added ability to auto discover devices on your network and Some PSR-2 clean up

@jonnywilliamson
Copy link
Owner

jonnywilliamson commented Dec 7, 2017

This looks good. Give me a chance to have a review and merge.

Thank you!

@@ -62,15 +62,15 @@ public function sendCommand(array $command)
*/
protected function connectToDevice()
{
$this->client = stream_socket_client(
$this->client = @stream_socket_client(
Copy link
Owner

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)
{
Copy link
Owner

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.

@jonnywilliamson
Copy link
Owner

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);
    }

@jonnywilliamson
Copy link
Owner

Thanks for these. Will review this week if I get time, otherwise it'll be the end of next week.

Much appreciated.

@jonnywilliamson
Copy link
Owner

I haven't forgotten about this

@jonnywilliamson
Copy link
Owner

@shanerutter You might join me over here: #6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants