pear2 / Net_RouterOS

This package allows you to read and write information from a RouterOS host using the MikroTik RouterOS API protocol.
http://pear2.php.net/PEAR2_Net_RouterOS
242 stars 116 forks source link

Client->loop() returns callback without properties #22

Closed FezzFest closed 8 years ago

FezzFest commented 8 years ago

I have a piece of code that grabs some device information and saves it into a database. I have two classes, a RequestHandler class:

$client = new RouterOS\Client($ip, 'admin', 'password', null, false, 10);

$request = new RouterOS\Request('/system identity print');
$request->setTag("deviceIdentity");
$callback = array($responseHandler, 'notifyDeviceInfo');
$client->sendAsync($request, $callback);

$request = new RouterOS\Request('/system license print');
$request->setTag("deviceLicense");
$callback = array($responseHandler, 'notifyDeviceInfo');
$client->sendAsync($request, $callback);

$request = new RouterOS\Request('/system routerboard print');
$request->setTag("deviceRouterboard");
$callback = array($responseHandler, 'notifyDeviceInfo');
$client->sendAsync($request, $callback);

$request = new RouterOS\Request('/system resource print');
$request->setTag("deviceResource");
$callback = array($responseHandler, 'notifyDeviceInfo');
$client->sendAsync($request, $callback);

$client->loop();

And a ResponseHandler class which defines a callback function:

public function notifyDeviceInfo(RouterOS\Response $response) {
    switch ($response->getTag()) {
            case "deviceIdentity":
                $this->identityResponse = $response;
                break;
            case "deviceLicense":
                $this->licenseResponse = $response;
                break;
            case "deviceRouterboard":
                $this->routerboardResponse = $response;
                break;
            case "deviceResource":
                $this->resourceResponse = $response;
                break;
    }

    if ($this->identityResponse && $this->licenseResponse && $this->routerboardResponse && $this->resourceResponse) {
        var_dump($this->identityResponse);
        var_dump($this->licenseResponse);
        var_dump($this->routerboardResponse);
        var_dump($this->resourceResponse);
    }
}

Even though the commands complete successfully and the resulting variables are of the correct RouterOS\Response type, the response does not contain any results:

object(PEAR2\Net\RouterOS\Response)#23 (4) {
  ["unrecognizedWords":protected]=>
  array(0) {
  }
  ["_type":"PEAR2\Net\RouterOS\Response":private]=>
  string(5) "!done"
  ["attributes":protected]=>
  array(0) {
  }
  ["_tag":"PEAR2\Net\RouterOS\Message":private]=>
  string(13) "deviceLicense"
}

I expected the response to look like this:

object(PEAR2\Net\RouterOS\Response)#23 (4) {
  ["unrecognizedWords":protected]=>
  array(0) {
  }
  ["_type":"PEAR2\Net\RouterOS\Response":private]=>
  string(3) "!re"
  ["attributes":protected]=>
  array(3) {
    ["software-id"]=>
    string(9) "4YYU-NQ7X"
    ["nlevel"]=>
    string(1) "6"
    ["features"]=>
    string(0) ""
  }
  ["_tag":"PEAR2\Net\RouterOS\Message":private]=>
  string(13) "deviceLicense"
}

Do you have an idea of what could be going wrong?

boenrobot commented 8 years ago

All of those requests return responses that contain one !re reply, followed by one !done reply. The !re reply is the one containing the data you want, while the !done is empty.

In your response callback, you're only checking the tag, but never the response type. Combine that with overwriting the previous response entirely (rather than appending it to a collection/array of some sort), and the result now looks obvious - you get 3 !done replies, which don't contain any properties, because all the stuff you actually wanted was at the previous !re reply.

To do what you want, you should either be appending stuff to the respective variables OR (in the case of these menus) you could simply detect the response type, and only replace the old one if it's a !re reply, e.g.

public function notifyDeviceInfo(RouterOS\Response $response) {
    if ($response->getType() === Response::TYPE_DATA) {
        switch ($response->getTag()) {
                case "deviceIdentity":
                    $this->identityResponse = $response;
                    break;
                case "deviceLicense":
                    $this->licenseResponse = $response;
                    break;
                case "deviceRouterboard":
                    $this->routerboardResponse = $response;
                    break;
                case "deviceResource":
                    $this->resourceResponse = $response;
                    break;
        }
    }

or quite frankly, you should also be able to use:

public function notifyDeviceInfo(RouterOS\Response $response) {
    if ($response->getType() === Response::TYPE_DATA) {
        $this->{$response->getTag()} = $response;
    }

if you just adjust your tag names to match the property names or vice versa.

FezzFest commented 8 years ago

Thank you! The code works a treat and thanks to your explanation I see where I've gone wrong :)