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

Is there a possibility to set a custom timeout time? #25

Open
robsworld opened this issue Oct 2, 2020 · 10 comments
Open

Is there a possibility to set a custom timeout time? #25

robsworld opened this issue Oct 2, 2020 · 10 comments

Comments

@robsworld
Copy link

This library works great, but it takes a long time for my proposal. I need to ping all 256 IPs of a local network. This takes several minutes. Because I am only working local, for me it is fine to set a shorter timeout to save time. Or is there a possibility to ping several IPs at the same time (asynchronously) ?

@adynis
Copy link

adynis commented Oct 2, 2020

Maybe it helps to decrease count parameter...
bool ping(IPAddress dest, byte count = 5);
bool ping(const char *host, byte count = 5);

Otherwise check ping.cpp file where you have some places with a chance of time - optimization...

@marian-craciunescu
Copy link
Owner

there is not easy way to do this async.Part is the limiation of the LWIP stack and so on.
we can expose the retry interval as param , but you are on very limited hardware (the ESP32 chip is great..but it still 4-5 USD chip

@CriPstian
Copy link

I suggest adding the possibility to override the default values via build flags, it might fix part of the issue for @robsworld

#ifndef PING_DEFAULT_COUNT
    #define PING_DEFAULT_COUNT    10
#endif
#ifndef PING_DEFAULT_INTERVAL
    #define PING_DEFAULT_INTERVAL  1
#endif
#ifndef PING_DEFAULT_SIZE
    #define PING_DEFAULT_SIZE     32
#endif
#ifndef PING_DEFAULT_TIMEOUT
    #define PING_DEFAULT_TIMEOUT   1
#endif

@robsworld
Copy link
Author

robsworld commented Oct 10, 2020

Of course getting the possibility to use this libary asynchronously, would be the perfect solution. But even the solution idea of @CriPstian, would be a big step forward. Thanks anybody for paying attention and help :-)
We don't have to forget, because this library is not working asynchronous makes it extremely easy to use. This is also in many cases a great solution.

PS: for everybody who concerns:
I am using this library for a small art project....a frame with neopixels, that represents all the IPs of our network, controls routers, check for free dhcp, ....makes fun :-)
https://imgur.com/a/kCY2eDL

I suggest adding the possibility to override the default values via build flags, it might fix part of the issue for @robsworld

@CriPstian
Copy link

@robsworld You can check the latest version which contains the customizable parameters. Cheers!

@johnerrington
Copy link

johnerrington commented Dec 28, 2021

I've modified the ESP32.cpp to expose more ping stats as follows

float PingClass::averageTime() {
return _avg_time;
}

byte PingClass::expectedCount(){
return _expected_count;
}

byte PingClass::errors(){
return _errors;
}

byte PingClass::success(){
return _success;
}

and in ESP32.h
public:
PingClass();
...
float averageTime();
byte expectedCount();
byte errors();
byte success();

However the 1 second DELAY between pings (in the dancol library) is an issue.
if(ping_seq_num < count){
delay( interval*1000L);

Also the default parameters (in the dancol library) are in whole seconds so there is a minimum of 1 second for timeout and interval. EG #define PING_DEFAULT_TIMEOUT 1

ideally it would be good to have that in milliseconds/microseconds, as would seem to be possible from this code in ESP32Ping-Master/ping.cpp.

#define PING_DEFAULT_TIMEOUT 1
..
timeout = PING_DEFAULT_TIMEOUT;
..
// Setup socket
struct timeval tout;
tout.tv_sec = timeout;
tout.tv_usec = 0;
if (setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tout, sizeof(tout)) < 0)
..

@CriPstian
Copy link

CriPstian commented Jan 4, 2022

Hi @johnerrington this looks like an easy task as we have all the necessary functionality in place.
This discussion can be moved in another issue as a feature request as I believe the question / feature request of @robsworld is already answered / resolved.
Feel free to close this issue if there is nothing else to be done 👍

@johnerrington
Copy link

@CriPstian "this looks like an easy task"

Maybe easy for you Cristian, buta LONG way outside my limited abilities!

"You can check the latest version which contains the customizable parameters"
I believe I'm using the latest version but dont see any way to change the timeout to msec.

@CriPstian
Copy link

Hi @johnerrington, I moved the discussion about your request in #34 so we don't pollute this thread with discussions about converting time units to milliseconds.

#34 is not necessarily something to be implemented by you, or myself, but by anyone who feels capable. If you do not feel capable to contribute, you can open issues, like I did, and hope that the contributors will have time to implement.

On your second point, you must notice that that commentary was directed to someone else, related to the current issue and not your own feature request.

@johnerrington
Copy link

Thanks Cristian, I understand.

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

5 participants