-
Notifications
You must be signed in to change notification settings - Fork 37
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
Multi ping #109
base: master
Are you sure you want to change the base?
Multi ping #109
Conversation
Nice, I had the same use case a few times and thought about that. Screenshot looks very cool I don't currently have time to look closer at the code. If you think this variable is shady/wrong I'd guess you're right. 😉 |
This looks pretty cool. It's going to be some work to make this convenient across multiple platforms. But, not insurmountable. I will give up on trying to keep the cnping.exe at 32kB. |
I think there is something off with the makros, this looks like you are including linux headers in a windows build. Haven't looked at the code yet... (I first thought it would be an error in the pipeline and #110 came out of that.)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can try this locally with make cnping.exe
but it needs some cross platform libraries installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import is resolved but there seems to be a missmatch in agruments and an undefined "psaddr" now. I can reproduce this comiling on Fedora.
psaddr was a global variable. But it can not be anymore, because it is different for each host. Thats why i moved it into the |
What surprises me is that it would compile for you but not for me or the ci. Note that our makefile is not be that reliable. Try make clean or even a manual clean of the build files in order to conpile everything from scratch. |
I just tried the executeable on Windows11. It created an error message, but the ping was running. This is the "No Admin privs" Version. I belive the error is related to the variable i just re-launched the With the "Admin privs" version i get: "Ping send failed: No error (0)". |
I belive i had some manual untracked changes in the Makefile for debugging purposes. I restored the original Makefile now and fixed all compile problems i found. |
I have now just removed that code block, that just blocked repeated execution of Now the "No Admins privs" version works on my win11 machine with multiple ping hosts. But i belive there is still some problems with |
…fer length; deduplicate code to send ping packet
As you can see i have run a test with 17 different hosts (even tho most of them where unreachable). So in this screenshot i have a window height of 593px.
So we have a height of 34px per host.
I have 3 plans on how this could be fixed:
What do you guys think? Anyways it might be best to not always calculate that during the draw. Is there an callback from rawdraw, when the window got resized, so we can recalculate those things then? |
I think 2 is just fine. If someone really is unhappy with it and uses it a lot we or they can always iterate on that. Being not perfect at times can also be a feature to draw in new interested contributors. Especially if it is not breaking anything. (I came on board because the background used to be all blue on startup. I made it black.) In general I'd lean strongly to the KISS principle and prefer stuff breaking in an obvious way or looking unpolished at times very much over making it complex and harder to understand, debug and maintain. BUT as you are the one investing the time here: Do whatever gives you the most joy, I'm fine with all options you listed. 😉 |
Off topic: I heard that blurring is not safe because it can be reversed. Not sure if the blurred data is super sensitive but you may want to keep that in mind. |
Thanks :) I am aware of that, but i believe that only works on slightly blurred images. But anyway the host names are not super secret as they are not available to the internet anyways. Just not anybody's concern. |
I did implement 2 now. Quite simple and easy and works as intended. Even with many hosts you cant really tell, that the last one might have a different size. I also inverted the order of the hosts, so the first host in the command line is displayed at the top.
Hehe. true. My first contribution here was the interface |
Just gave it a quick test and I think this is a great feature! Really makes sense for debugging questions like: "Is my wifi bad, is my router buggy, is my isp not performing, is the destination host just slow?" This answers it all in a glance! And without fiddling around with multiple windows, having to close them again etc etc. For better comparison between ping times I just used the Be sure to remove the "draft" flag when you think its ready! |
i have done some tests now:
I have attached my builds (amd64) if someone wants to try for themself. Even tho the version with admin privileges is broken i believe this PR is "completed" as on the current master this doesn't even compile. So i consider that feature "not made worse" 😄 |
bump |
I often require to ping multiple hosts. managing 1 or 2 cnping windows is okay, but after that it gets annoying. So i thought why not allow cnping to ping to more than one host at a given time?
So i implemented a simple "multiPing"
there are for sure some problems:
i haven't tested windows at all. Would be pure luck if it still compiles for winhistogram mode is brokenhistorical min/max/los is mixed between all viewsI just wanted to show you what i have got so far and ask for your opinion on that.
You think its worth to continue with this branch?
What is up with the
using_regular_ping
variable?Looks like is either uninitilized or 1. Seems wrong / shady to me.