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

Troubles with foreach #2

Closed
temandroid opened this issue Feb 13, 2019 · 10 comments
Closed

Troubles with foreach #2

temandroid opened this issue Feb 13, 2019 · 10 comments

Comments

@temandroid
Copy link

In

foreach ($arr as $ip){
 if (filter_var($ip, FILTER_VALIDATE_IP)) {
  $query=new Query('/queue/simple/print');
  $query->add('?target='.$ip.'/32');
  $responses=$ros->write($query)->read();
  print_r($responses);
  echo "<hr>";
 }

as result have answers like Array ( [after] => Array ( [] => ) )
So new query was exec so fast as I understand...
For now I move $ros=new Client($con); inside foreach and it work, but it's not good, of course, and so slow...

@EvilFreelancer
Copy link
Owner

EvilFreelancer commented Feb 13, 2019

Hello and thanks for your report!
Probably faster solution for right now will be a simple inserting sleep(1); at the end of loop. Something like this:

foreach ($arr as $ip) {
    if (false === filter_var($ip, FILTER_VALIDATE_IP)) {
        continue;
    }

    $query = new Query('/queue/simple/print');
    $query->add('?target=' . $ip . '/32');
    $responses = $ros->write($query)->read();
    print_r($responses);
    echo "<hr>";
    sleep(1);
}

Today (after work) I planned to release a new version of the library. which contains a few of changes that improve the readability of the code. If solution with sleep(1) is will be okay I can add this to new version of library by default.

@temandroid
Copy link
Author

Hello, thank u for fast reaction :)
It's too slow, if I have more than 100 records in queue with 1s sleep, i tried with usleep(1000000), it work here but also very slow. Is it possible to wait answer from api as end of query?
For now I change logic of my sript for function call one time not 100 times.

@EvilFreelancer
Copy link
Owner

Hi @temandroid!

Is it possible to wait answer from api as end of query?

Yep, I think you need to play with 'timeout' parameter of API client, look at this example.

Unfortunately I couldn't reproduce your problem on my router (I've tried to run 100 parallel scripts in same time), maybe you can send me your queue table as an example?

Btw, what is version of RouterOS which you use on your device?

@temandroid
Copy link
Author

temandroid commented Feb 14, 2019

Hmm, in your example i receiving an array of all queues, not filtered by target...
I make an example on working ROS (6.43.11) this:

$ips=array ();
for ($i=2; $i <= 128; $i++) {
 array_push($ips, '10.50.0.'.$i);
}
$conn=(new Config())->set('timeout', 1)->set('host', '127.0.0.1')->set('user', 'admin')->set('pass', 'admin');
$ros=new Client($conn);
foreach ($ips as $ip) {
 echo $ip."<br>";
 $query=new Query('/queue/simple/print');
 $query->add('?target='.$ip.'/32');
 $response=$ros->write($query)->read();
 print_r($response);
 echo "<hr>";
}

image

@EvilFreelancer
Copy link
Owner

EvilFreelancer commented Feb 14, 2019

Wow, that's a very strange! Looks like a bug with API socket in RouterOS.
It seems that the socket does not have time to form an answer to your request in time and simply adds each next query one to the query queue.

Probaly just need add yet another call of read method, try this:

$ips=array ();
for ($i=2; $i <= 128; $i++) {
 array_push($ips, '10.50.0.'.$i);
}
$conn=(new Config())->set('timeout', 1)->set('host', '127.0.0.1')->set('user', 'admin')->set('pass', 'admin');
$ros=new Client($conn);
foreach ($ips as $ip) {
 echo $ip."<br>";
 $query=new Query('/queue/simple/print');
 $query->add('?target='.$ip.'/32');
 $response=$ros->write($query)->read();
 print_r($response);
 echo "<hr>";
 // Add this
 $ros->read();
}

@temandroid
Copy link
Author

I tried, and as a result I was stuck on an IP which is not in the queue...
With class pear2/Net_RouterOS, similar function working well..

 $request=new PEAR2\Net\RouterOS\Request('/queue/simple/print');
 $query=PEAR2\Net\RouterOS\Query::where('target', $ip.'/32');
 $request->setQuery($query);
 $responses=$ros->sendSync($request);
 print_r($responses);

So, I don't think that is a bug

@matracine
Copy link
Collaborator

Found it !
You will have a pull request with explanations this evening I hope.

matracine pushed a commit to matracine/routeros-api-php that referenced this issue Mar 9, 2019
@temandroid
Copy link
Author

Sounds good. nice work!
Waiting new release :)

@matracine
Copy link
Collaborator

matracine commented Mar 10, 2019 via email

@temandroid
Copy link
Author

No)) On release page, like 0.8.2 :)

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

No branches or pull requests

3 participants