ArduPilot / apm_planner

APM Planner Ground Control Station (Qt)
https://ardupilot.org
Other
507 stars 466 forks source link

Graph: Ideas for even better log analysis. #1005

Closed AndKe closed 7 years ago

AndKe commented 7 years ago

AP2 is (by far) the greatest log analysis tool I know. It's speed, smooth navigation is great.

please bear with me for a feature request that would make it even better: During tuning , writing of POH, gathering performance data or general log analysis, we often need to know an average, or difference during a certain maneuver or time.

Please see mockup. If a checkbutton is selected, two red lines appear, they can be dragged to select area of interest. the mouseover displays the time between those lines, and we can sea delta (difference) for value during that time frame, and it's average.

Usage examples: Delta:

Average: -easily quantify how much an axis is changing during apparently "flat" logline, select 10s , see how much variation we have in pitch. -get proper average throut , used for documenting multirotors, 1kg payload = throut xx , 2kg=throut xx - then we know it's maximum capacity vs flight times m vs redundancy reserves. -Also, during very smooth hover in still air, properly tuned multi has low variations in throut, - all rcout.chX should have similar average (and delta) - otherwise it's a indication for some problem.

The tools, as illustrated, will aid proper log analysis a lot.

screenshot from 2017-02-06 22-17-41

Arne-W commented 7 years ago

Cool idea - should be done! :+1:

Arne-W commented 7 years ago

Started to work on this feature, but the classes which are responsible for the graph view need some refactoring cause they get bigger and bigger. So far I have updated to a new QCustomPlot version and split the the Graph view code in 2 classes - one for online viewing and one for offline log analysis. Next thing will be the cursor... I thought about a context menu when clicking into the graph to add one or two cursors. I will tell you when the feature is testable...

AndKe commented 7 years ago

Great :)

On 4 Mar 2017 19:58, "Arne Wischmann" notifications@github.com wrote:

Started to work on this feature, but the classes which are responsible for the graph view need some refactoring cause they get bigger and bigger. So far I have updated to a new QCustomPlot version and split the the Graph view code in 2 classes - one for online viewing and one for offline log analysis. Next thing will be the cursor... I thought about a context menu when clicking into the graph to add one or two cursors. I will tell you when the feature is testable...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ArduPilot/apm_planner/issues/1005#issuecomment-284172948, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZpqjo-GflOO4Pzh5UY9VH7YjaQ2x6Lks5ribROgaJpZM4L4x14 .

Arne-W commented 7 years ago

Ok its nearly done :smile: If somebody has some time to waste he could check THIS branch. New Features:

I still have to do some cleanup - the PR will come in the next days.

AndKe commented 7 years ago

no time to waste, but then, I do not consider this a waste of time :)

Fantastic - almost there ! -please add value delta. (difference between min-max you are already displaying) don't know what "Add simple cursor" is supposed to do. -finally when displaying data tooltip: please rearrage so values are in order :
min , max, delta, avg , cursorposition
This way, the tooltip will be more pleasant to watch when moving mouse along the graph, as only the last The distance between value name and the final numbers on the long tooltip , can be solved by adding a horizontal bar for each two values, or using alternating colors.

Great work! edit: found how to drag the cursors, click first, then drag. :) I tried to click and drag..

BTW: delta between line numbers in range is displayed with three decimals :)

Arne-W commented 7 years ago

Thank you! The cursors should be selectable by clicking directly on them. If a cursor is selected you can click on it an move it as long as its clicked. To deselect you have to click somewhere else. Works like the Y-Axis scaling feature. The simple cursor is the old double click feature. It acts as a link between the Graph and the table view. Rearranging the values is a good idea - and perhaps I should rename the simple cursor...

AndKe commented 7 years ago

I see you did not get the edited version, most likely using mail to read it. My final addition was also: "BTW: delta between line numbers in range is displayed with three decimals :)" Have a nice weekend.

AndKe commented 7 years ago

On second thought: The doubleclick to log feature could be a green checkbox below (like in master), or just permanently on, AFAIK, we don't need doubleclick to be available in graph for any other purpose, so there is most likely no reason to have an option to toggle it.

On 17 March 2017 at 20:39, Arne Wischmann notifications@github.com wrote:

Thank you! The cursors should be selectable by clicking directly on them. If a cursor is selected you can click on it an move it as long as its clicked. To deselect you have to click somewhere else. Works like the Y-Axis scaling feature. The simple cursor is the old double click feature. It acts as a link between the Graph and the table view. Rearranging the values is a good idea - and perhaps I should rename the simple cursor...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ArduPilot/apm_planner/issues/1005#issuecomment-287452042, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZpqk-CG7zQVPbilxYIEOFJdFJgnqw_ks5rmuFpgaJpZM4L4x14 .

AndKe commented 7 years ago

Final thought: by using a green checkbox to toggle the range selection ,the UI would have fewer "different places to do things" , and people would easier discover this great feature. So I say the green checkboxes are the way to go.

On 17 March 2017 at 21:59, André Kjellstrup andre.kjellstrup@gmail.com wrote:

On second thought: The doubleclick to log feature could be a green checkbox below (like in master), or just permanently on, AFAIK, we don't need doubleclick to be available in graph for any other purpose, so there is most likely no reason to have an option to toggle it.

On 17 March 2017 at 20:39, Arne Wischmann notifications@github.com wrote:

Thank you! The cursors should be selectable by clicking directly on them. If a cursor is selected you can click on it an move it as long as its clicked. To deselect you have to click somewhere else. Works like the Y-Axis scaling feature. The simple cursor is the old double click feature. It acts as a link between the Graph and the table view. Rearranging the values is a good idea - and perhaps I should rename the simple cursor...

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ArduPilot/apm_planner/issues/1005#issuecomment-287452042, or mute the thread https://github.com/notifications/unsubscribe-auth/ABZpqk-CG7zQVPbilxYIEOFJdFJgnqw_ks5rmuFpgaJpZM4L4x14 .

billbonney commented 7 years ago

Is there a PR for these changes or is it a WIP?

Arne-W commented 7 years ago

@billbonney at the moment its a WIP - we are still discussing some behaviour. I think it will become a PR in the next days. If you want to test it THIS is my WIP branch. @AndKe I made some changes would be nice if you could have a look.

AndKe commented 7 years ago

@Arne-W Very nice ! works great.
One more thing while you are at those files, the default loaded log does not display the grid with data, a new user would not be aware of it's existence. Please change the GUI so it reveals at least 3-5 lines if data.

Arne-W commented 7 years ago

Done :smile: - I will create a PR

AndKe commented 7 years ago

OOPS ! : please see below:

Altitude minimum 174.11 ,max 174.75 (both correct), and the arerage is 261 ? :)

screenshot from 2017-03-19 16-05-14

Arne-W commented 7 years ago

Found and fixed it - thanks :+1: - a typical off by one! Anything else :smile:

AndKe commented 7 years ago

unrelated; are you able to reproduce this https://github.com/ArduPilot/apm_planner/issues/646#issuecomment-221203527 ? (just getting a bit desperate on that one)

Arne-W commented 7 years ago

As feature is completed and merged I will close this.