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

waittime is not deadtime #31

Closed
ChristofKaufmann opened this issue Feb 11, 2019 · 4 comments
Closed

waittime is not deadtime #31

ChristofKaufmann opened this issue Feb 11, 2019 · 4 comments
Assignees

Comments

@ChristofKaufmann
Copy link

Summerizing this answer deadtime (-w) waits for the whole application and waittime (-W) for an individual ping. However, there is an important difference, when combined with count (-c). Look at these examples, trying to ping a non-existing IP address, and compare the numbers of transmitted packets:

$ ping -c 1 -w 4 2.3.4.5
PING 2.3.4.5 (2.3.4.5) 56(84) bytes of data.

--- 2.3.4.5 ping statistics ---
4 packets transmitted, 0 received, 100% packet loss, time 3052ms

and

$ ping -c 1 -W 4 2.3.4.5
PING 2.3.4.5 (2.3.4.5) 56(84) bytes of data.

--- 2.3.4.5 ping statistics ---
1 packets transmitted, 0 received, 100% packet loss, time 0ms

So in my application case I need the latter one. However, in pingparsing currently only the the first one (deadtime) is implemented, but this option is also called waittime (deprecated). I think both might be useful, depending on the application.

@thombashi thombashi self-assigned this Feb 12, 2019
@thombashi
Copy link
Owner

@ChristofKaufmann
Thank you for your feedback.

I had added timeout support for both the library and the CLI at pingparsing 0.14.0 to support the latter case that described in your comment.
Please try the version.

note: I think timeout is more suitable for the option name rather than waittime. because both Linux(-W) and Windows(-w) described this kind of option as timeout.

@ChristofKaufmann
Copy link
Author

OK, it is really called timeout. I don't know why I got confused with waittime. You're right, timeout is the better name.

I tried the new version with this code:

import time
import pingparsing

transmitter = pingparsing.PingTransmitter()
transmitter.destination_host = "2.3.4.5"
transmitter.count = 1
transmitter.timeout = 3000

start = time.time()
result = transmitter.ping()
stop = time.time()

ping_parser = pingparsing.PingParsing()
parsed = ping_parser.parse(result)

fstr = "time used: {} s, transmitted: {}, received: {}"
print(fstr.format(stop-start, parsed.packet_transmit, parsed.packet_receive))

and the result is:

time used: 3.0096986293792725 s, transmitted: 1, received: 0

So it works! Thanks!

However, I find it confusing that deadtime is in seconds and timeout is in milliseconds. You could maybe accept a float in seconds and round for linux and multiply by 1000 (and round) for windows. Then integer could still be used and it were more consistent in my opinion. Apart from this the issue can be closed from my side.

@thombashi
Copy link
Owner

Thank you for your confirmation and feedback.

I find it confusing that deadtime is in seconds and timeout is in milliseconds. You could maybe accept a float in seconds and round for linux and multiply by 1000 (and round) for windows. Then integer could still be used and it were more consistent in my opinion.

I agree with you this would be confusing, but I think that specifying milliseconds timeouts with float in seconds would be a little bit pain.
I have a plan to improve usability by supporting human readable time specifications like
transmitter.timeout = "3000msec" or transmitter.timeout = "3sec"
(also still support specify by int for backward compatibility)

@ChristofKaufmann
Copy link
Author

Sounds reasonable. Thanks for the discussion and implementation!

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

No branches or pull requests

2 participants