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

Add an option to display the whole history of times in the graph #420

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

myarcana
Copy link

@myarcana myarcana commented Jan 8, 2024

Now passing --buffer 0 will scale the graph's x axis so that the entire history is always visible.

I made this option the new default, because I expect most users want a graphical ping plotter when they're dealing with network connection issues, and want to start figuring out why — this is why I came across gping at least. Keeping the entire history in frame lets you see at a glance whether the issues appear to be periodic or not.

@orf
Copy link
Owner

orf commented Feb 17, 2024

I'm off two minds: I think it's useful in some cases, but in others it isn't. After running for a minute this is what I get with your change:

Screenshot 2024-02-17 at 11 18 54

Whereas this is what you get currently:

Screenshot 2024-02-17 at 11 19 50

To me, personally, the second one is much more useful for diagnosing issues, and part of the reason I made it not buffer indefinitely is that large one-off spikes can really make the graph hard to read. I also thought most people would prefer recency over an entire history.

However, we might be able to have it both ways. We could toggle this behaviour on a keypress? i.e pressing the Enter key will start to buffer the history indefinitely, and pressing it again would revert back to a fixed-size buffer? We would do this by keeping a very large fixed buffer but only displaying the most recent portion of it, allowing a you to toggle between "live" and "historic" views?

What do you think? I'd rather go this route than changing the default I think.

@myarcana
Copy link
Author

myarcana commented Feb 17, 2024

I think those are fair points.

My own personal history of wanting to solve sporadic network latency has always been with wifi, and was because of interference or bufferbloat. At those times, and why I wrote this feature, the periods of high-latency vs low-latency were much wider than a few seconds each, so I hadn't considered that a growing buffer would obscure the information like your example shows. My graphs looked more like square waves. For a first-look at network issues in the future, I would still reach for the growing-window, but I've never used ping plotters for reasons other than "what is wrong with my wifi?"

You're dead right that one-off spikes make the whole graph less useful. Maybe just add an option for setting max (and min?) bounds on the graph's y-axis?

Anyway, the point of my request is not to convince anyone that growing-windows are more useful. I just believe gping should have a growing-window feature, so any way you choose to include it would make me happy :)

With the change I wrote, you can still of course pass --buffer 60 to get the 60-second window you found more useful. I just special-cased --buffer 0 to function as a growing-buffer. Surely more useful than a real 0-second buffer! After merging you could change the default back to whatever you want too of course.

I also do think it would be useful idea to have live configuration in gping in general, however I wouldn't have thought about adding live configuration without you mentioning it; last time I used gping, the only "missing feature" I felt was the growing buffer. But it would surely be neat if you could switch from a currently-growing-window to a sliding-window of the same size for example. The most basic—and most useful—feature in this vein would probably simply be a prompt to adjust many of the graph's options including buffer size.

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

Successfully merging this pull request may close these issues.

2 participants