search

betaflight / blackbox-log-viewer

Interactive log viewer for flight logs recorded with blackbox
GNU General Public License v3.0
449 stars 147 forks source link

Added setting of minimum and maximum curves values on the graphs. VITE Web version. #726

Closed demvlad closed 6 months ago

demvlad commented 7 months ago

PR #689 for VITE Web.

netlify[bot] commented 7 months ago

Deploy Preview for origin-blackbox-logviewer ready!

Name Link
Latest commit 0c35172e6e7e088ded32d426122cc5a0419ed2a4
Latest deploy log https://app.netlify.com/sites/origin-blackbox-logviewer/deploys/663b6d766af6290008bfe61e
Deploy Preview https://deploy-preview-726.dev.blackbox.betaflight.com
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

nerdCopter commented 7 months ago

maybe not be properly rebased. image & Conflicting files

demvlad commented 7 months ago

maybe not be properly rebased.

Maybe! I do not understand. I've made rebase and resolved all conflicts. But there were too much conflicts after pushing branch. I've resolved their in the github online interface. Maybe this version will work:) If it will not, then i will repeate rebase.

haslinghuis commented 7 months ago

Probably need to drop only these ones:

image

demvlad commented 7 months ago

I've tried to load this version from github and run it. This is very wunderfull:), but it seems, the program is working! Will study, how to do rebase properly...

demvlad commented 7 months ago

Probably need to drop only these ones:

Yes, i am seeing these. Will try next time :) @haslinghuis @nerdCopter would you write me please, how can i do rebase properly in this case, when both branches have too much commits.

haslinghuis commented 7 months ago

git rebase -i HEAD~153 and drop the 3 commits. Save and force push.

Waiting for @nerdCopter for any alternatives.

nerdCopter commented 7 months ago

i like git rebase -i (interactive) as well. replaceing pick with drop for the suspect commits. when i perform that exact command, my editor shows 4 to maybe drop. image

edit: but then i still get some conflicts :frowning: image

maybe squashing can help. complexity has increased with 1000 commits

demvlad commented 7 months ago

Thank's. Can i do rebase like: rebase -i my_min_max_branch vite_veb_branch or rebase -i vite_veb_branch my_min_max_branch

What rebase direction is better?

haslinghuis commented 7 months ago

Something like that described here: https://git-scm.com/docs/git-rebase

nerdCopter commented 7 months ago

looks good :rocket: image

demvlad commented 7 months ago

looks good 🚀

I've done rebase more accuratly. The test master merge has not any conflicts. I am going to push this version after a small bit time.

nerdCopter commented 7 months ago

i wonder now that it is PWA, are built-in browser based menus still bugged?

demvlad commented 7 months ago

i wonder now that it is PWA, are built-in browser based menus still bugged?

The olde nw.js menu? I've checked nw.js menu in simple html page by using browser, while wrote issue in NW. They had the bugs. I think, it will has the bugs in PWA too.

demvlad commented 7 months ago

looks good 🚀

I've done rebase more accuratly. The test master merge has not any conflicts. I am going to push this version after a small bit time.

Yes! The all rebase for wite web have not merge conflicts.

demvlad commented 7 months ago

Improvements: The menu is simple now. There are no submenues by default. menu

If the stanard menu is not enough for you and you want more features, then click menu item while the Shift key is pressed. And you will see the submenu with extended actions.

The Save item is removed from the menu, because it duplicated the 'Save' checkboxes in Chart setup dialog box. You need to use the 'Save' checkboxes in one case only: if you want to change default min-max values for some curves by using workspace slots or by loading next logs. If 'Save' checkbox is not checked for curve then it will use default presets min-max values by loading next logs or workspace charts preset. I do not like the 'Save' checkboxes name, because the charts configuration will save in any case when you press the Apply button at dialog box or press Shift+num key after that. I want to find other short name for these checkboxes.

demvlad commented 7 months ago

Improved: the right arrow markers are showed at menu items by pressing the Shift key to show the submenu activity. The default menu: 1

The menu with active submenu while Shift key is presed: 2

demvlad commented 7 months ago

I think, the "Default" tables caption in Chart setup dialog box is better then 'Save'.

demvlad commented 7 months ago

The workspace save strategy is changed #689.

I think, that are all till. I invite all participants for testing.

demvlad commented 7 months ago

The 'This curves' actions replaced into submenu. Finaly we have next variants menu:

Main menu default

1

Main menu with active submenu while the Shift key is pressed

2

'This curves' actions submenu

3

Main menu if the chart has one curve only

4

demvlad commented 7 months ago

Improvements: The Apply and Cancel buttons are enabled while menu is open. You can use it to close the dialog box without pressing of Back and Return menu items. The Alt (optionKey on Mac) key is used instead of Shift to use hidden submenu. The 'Full range' item of 'This curve' menu has submenu while Alt key is pressed. The 'Default' item on main menu has hidden submenu too. The menu source code structure is improved.

The issue of this version what i know, and their resolving at moment:

The 'Full range' submenu on 'This curve' menu while Alt key is pressed. sub3

sub_single

demvlad commented 7 months ago

Resolved issue of Esc and Alt keys actions. I've found that in Vite Web version the Alt-key activate the browsers menu in Windows. Therefore in the Vite Web version i am using Shift-key.

demvlad commented 7 months ago

@ctzsnooze Please, check this update.

  1. I've disable input autocompletion
  2. The active top level 'Return' or 'Back' menu items have more smart design

I notice, that in WEB version the menu modify key is 'Shift', in no WEB - 'Alt'

This is link to current NO WEB version. https://github.com/demvlad/blackbox-log-viewer/tree/min_max_chart_settings I will continue to support this version. The all new updates in future will be on this link.

demvlad commented 7 months ago

@ctzsnooze I've reverted the all autocomplete preventions commits. Please check this version.

demvlad commented 6 months ago

Improvements:

demvlad commented 6 months ago

@ctzsnooze , the last issues are resolved. Updated:

demvlad commented 6 months ago

Resolved error of data storage by using Expo, Grid, Smooth buttons

demvlad commented 6 months ago

THE MANUAL The MinMax PR can control curves scale and placement quickly and full. The current MinMax curves settings are showed at 'Configure graphs' dialog box in the "Minimum" and "Maximum" columns. 1

The MinMax values can be changed:

To show context menu you must do right mouse click on Minimum or Maximum values field what you want to edit. The main context menu: 2

The main menu include following parts:

  1. The menu actions to edit all curves
  2. The menu actions to edit one curves what you selected by right mouse click (The 'Gyro pitch' for example at the picture)

The actions for all curves:

The single curve submenu has same actions: 3

The main menu has extended mode. You need press the 'Shift' key to activate it and select the submenu, what you need. 4

The 'Like this one' extended submenu. You can change MinMax values and select curves what you need by using the checkboxes to apply these values. Click 'SET MIN-MAX VALUES' item for apply. Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 5

The 'Zoom in', 'Zoom out' extended submenu. You can set the zoom procent value and select curves what you need by using the checkboxes to apply zoom. Click 'ZOOM IN', 'ZOOM OUT' items for apply zoom Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 6

The 'Default' extended submenu. You can select curves what you need by using the checkboxes to apply default values. Click 'SET CURVES TO DEFAULT' item for apply Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 7

The 'Full range' extended submenu. You can select curves what you need by using the checkboxes to apply values. Click 'At all global log time' menu item to set MinMax values from log data during all time. Click 'At local window time' menu item to set MinMax values from current time interval at the chart window. Click 'At markere time range' menu item to set MinMax values from markered time interval what you select by using "I", "O" keys. If it is not select then will apply all log time interval. Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 8

The 'One scale' extended submenu. You can select curves what you need by using the checkboxes to set the same scale. Click 'SET SET CURVES TO SAME SCALE' item for apply Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 9

The 'Centered' extended submenu. You can select curves what you need by using the checkboxes to centered. Click 'SET CURVES TO ZERO OFFSET' item for apply Click 'Back' menu item to go back to main menu. Click 'Apply change' or 'Cancel' button on the main 'Configure graphs' dialog box to close the menu and dialog box immediately 10

The single curve submenu has one 'Full range' extended item: It is like 'Full range' extended submenu, but for one selected curve only. 11

If you open context menu for one curves chart, then you see the short menu: 12

When you store the chart configuration in the workspace, the MinMax values stored for next using too.

This is non WEB version the PR: https://github.com/betaflight/blackbox-log-viewer/pull/689

ctzsnooze commented 6 months ago

Working REALLY WELL!! Absolutely love this PR. I can now quickly and easily set up the display exactly how I need it. Particularly helpful for debugs that don't have sensible scaling info. Ready to merge? I think so. Definitely ready for review.

ctzsnooze commented 6 months ago

May require a new workspaces file. Here's my current file. workspaces-ctzsnooze.json.zip

demvlad commented 6 months ago

@haslinghuis Yes, we have two big improvements at moment: the VITE WEB and my MinMax. The all are need testing. Therefore i will continue to support my non Web PR version to different some bugs, what will possible.

demvlad commented 6 months ago

@ctzsnooze workspace is used by default. His workspace uses curves min-max values, what this PR can control. Other improvements:

haslinghuis commented 6 months ago

:thinking: we should add default workspace in separate PR as there is some discussion about having multiple presets as default.

Anything that could reduce the size of this PR :stuck_out_tongue:

See also: #726

demvlad commented 6 months ago

🤔 we should add default workspace in separate PR as there is some discussion about having multiple presets as default.

These are a small different workspaces. This workspace PR stored any min-max values what you can set to scale and place curves at the chart. If the full web version can not save local data and need some multy-presets, probably i will prefer developers local variant :)

demvlad commented 6 months ago

The small improvement: The mouse click at curves on the legend works like toggle button (make the analyser chart as visible/unvisible)

ctzsnooze commented 6 months ago

@haslinghuis @nerdCopter @SupaflyFPV This PR is ready to merge now

nerdCopter commented 6 months ago

i feel like Default should be first in menu -- thoughts? image

demvlad commented 6 months ago

i feel like Default should be first in menu -- thoughts?

Yes, maybe.

demvlad commented 6 months ago

Updated:

sonarcloud[bot] commented 6 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

nerdCopter commented 6 months ago

The Sonar Cloud shows that many switch/case just return the same thing return value; so it seems the switch/case are not even needed. please verify or reply otherwise.

demvlad commented 6 months ago

The Sonar Cloud shows that many switch/case just return the same thing return value; so it seems the switch/case are not even needed. please verify or reply otherwise.

switch/case ??? Yes, there are in default curves and units converters code. I've used the previouse source code as the ground. There are huge switch/case code really. And this code "return the same thing return value". Ok, I will check it.

demvlad commented 6 months ago

@nerdCopter The example of code: case 'FFT_TIME': switch (fieldName) { case 'debug[0]': return value; case 'debug[1]': return value; // debug 2 = not used // debug 3 = not used default: return value; } The all cases return back the same input value today. But maybe someone will edit this or similar DEBUGs next time, i do not know that.

haslinghuis commented 6 months ago

Will need to be refactored in another PR some time.

nerdCopter commented 6 months ago

okay, i approved. if refactoring for a future date, no problem.

demvlad commented 6 months ago

Main issue with this PR is that the user's workspaces have to be rebuilt.

@ctzsnooze It is possible to use olde workspaces in this PR. It has default curves MinMax values as start after loading.
I've checked this on supafly.json file for example.

nerdCopter commented 6 months ago

Although useful, this file should not have been committed: src/workspaces-ctzsnooze.json

demvlad commented 6 months ago

The link to manual:

https://github.com/betaflight/blackbox-log-viewer/pull/726#issuecomment-2079546532

demvlad commented 6 months ago

Although useful, this file should not have been committed: src/workspaces-ctzsnooze.json

This is workspace by default, it is used in main.js. This workspace showes using of MinMax.

haslinghuis commented 6 months ago

@demvlad huge thank you for all improvements. Very much appreciated. Would love to see the documentation included in https://github.com/betaflight/betaflight.com