thombashi / pingparsing

pingparsing is a CLI-tool/Python-library parser and transmitter for ping command :arrow_right_hook:
https://pingparsing.rtfd.io/
MIT License
78 stars 10 forks source link

waittime is not deadtime #31

Closed ChristofKaufmann closed 5 years ago

ChristofKaufmann commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Sounds reasonable. Thanks for the discussion and implementation!