bluerobotics / cockpit

An intuitive and customizable cross-platform ground control station for remote vehicles of all types.
https://docs.bluerobotics.com/ardusub-zola/software/control-station/Cockpit-1.0/overview/
Other
46 stars 19 forks source link

Allow logging data from very generic indicator #913

Closed ArturoManzoli closed 2 months ago

ArturoManzoli commented 3 months ago

Closes #600

rafaellehmkuhl commented 3 months ago

I think the situation where the user is already logging from a VGI will be very common, as there are default VGIs that the user will probably add to several views.

What if we put a "log that" checkbox in the VGI, and if the user clicks it and there's another one already, it offers to ovewrite the previous or change the name of this VGI?

ArturoManzoli commented 3 months ago

I think the situation where the user is already logging from a VGI will be very common, as there are default VGIs that the user will probably add to several views.

What if we put a "log that" checkbox in the VGI, and if the user clicks it and there's another one already, it offers to ovewrite the previous or change the name of this VGI?

I agree that we must differentiate VGIs from different views. The log configuration page already have the ability to switch them on and off. Maybe a cleaner idea would be to add, only on the logs configurations page, the view that the VGI is from. This way the user can decide over there to log them or not (check image below). This solution anyway is kind of temporary since we are rethinking the way the user set the telemetry logs (#621 #875).

image

rafaellehmkuhl commented 3 months ago

Will be discussing more about how to do it next week.

rafaellehmkuhl commented 2 months ago

From our discussion today:

Problem: The mini-widgets are being loaded/unloaded from the screen when the view is changed, which would make the logs specific to each view (not desirable).

Solution: We agreed to change the current code on the top/bottom bar to stop reloading then when views are changed, and instead render them all, showing only the one for the current view, as we are doing with the widgets themselves.

This makes the log configuration Profile specific, as desirable. This also fixes the undesirable behavior of having all the mini-widgets being visually reloaded when the view is changed, which differs from the widgets' behavior, which are pre-rendered and cause a much better experience for the user.

ArturoManzoli commented 2 months ago

As discussed outside here, the bug of changing a mini-widget name causing the others with the same name to disappear persists.

We agreed to use the widget hash instead of the displayName for tracking of each one, which will fix the bugs and UX problem.

The problems with multiple instances of mini-widgets were solved using an instance count. Since we don't need unique instances of the mini-widgets, and there is a 'refreshWidgetsHashs' method inside the 'miniWidgetContainer' which makes the VGI hashes mutable, the solution that treats all mini-widget instances as generic instances is simpler and more reliable.