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

Last ping shouldn't wait 1s #29

Open
TomasKonir opened this issue Jan 7, 2021 · 5 comments
Open

Last ping shouldn't wait 1s #29

TomasKonir opened this issue Jan 7, 2021 · 5 comments

Comments

@TomasKonir
Copy link

ping_start() forces to wait 1s after each ping. I suggest to don't wait in last cycle and return immediately.

@adynis
Copy link

adynis commented Jan 7, 2021

I find it like a good proposal, but I think it should be tested before.
Also an enhancement could be to add the time interval as an option to PingClass::ping (I see that this

_options.coarse_time = 1;
is useless )

@TomasKonir
Copy link
Author

I updated my local sources like below and it works perfectly for me.

--- ping.old.cpp 2020-06-05 09:46:00.000000000 +0200
+++ ping.cpp 2021-01-07 18:26:01.457435408 +0100
@@ -322,7 +322,9 @@
if (ping_send(s, &ping_target, size) == ERR_OK) {
ping_recv(s);
}
- delay( interval*1000L);
+ if(ping_seq_num < count){
+ delay( interval*1000L);
+ }
}

closesocket(s);

@marian-craciunescu
Copy link
Owner

please raise a pull request

@MikeyMoMo
Copy link

Only if you have some other way to wait. If you don't wait, you might not get the answer and declare it bad.

@TomasKonir
Copy link
Author

It's not true.
Current behavior is wait 1s between pings and wait 1s after last ping too.
For example: If i only want to check, if host is alive and have code like this:

bool hostAlive = false;
for(int i=0;i<5;i++){
  if(ping(host,1)){
    hostAlive = true;
    break;
  } else {
    delay(1000);
  }
}

With current code, each ping() takes at least 1s due to wait after last ping, which is not needed.
With my simple change, last wait is removed and ping returns immediately after succes or failure.
Success check than takes few ms instead of 1s.

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

4 participants