Added 2D Charting/Streaming capability to 3D View. Basic functionalit… #158
Conversation
…y is there. Tests: it needs to be tested on different browsers/monitors. I tested with three different log files ranging from 5-30 megabytes. More tests are needed with different log files.
There was a problem hiding this comment.
Really cool... but unfortunately none of the logs I tried it on worked, so I could not test it.
I glanced over it and added some initial remarks inline.
Some more high-level points:
- most important: when trying it on a 28MB high-rate 5min log, the page becomes 38MB. This is becoming too much and we need to send the data more efficiently (i.e. in binary form) and only the data that is needed (ideally with downsampling).
- nan's are not handled correctly: I see 'ReferenceError: nan is not defined' in the browser console.
- the error handling generally needs to be improved. I see errors like
UnboundLocalError: local variable 'vehicle_rates_setpoint' referenced before assignment. We should make sure we have a basic set of required topics (as it is now), and the rest of the topics are all optional, with well-defined bahavior (as for the rest of Flight Review). - when adding a new library (plotly-latest.min.js in that case), please make a separate commit and write the library version into the commit message.
- add a checkbox/button to show/hide all 2D plots?
| } | ||
|
|
||
| #Chart2D_Left { |
There was a problem hiding this comment.
It's worth splitting this off into a separate CSS file now.
|
|
||
| <div id="toolbar"> | ||
| <!-- All HTML-side code for 2D Charting is below. They depend on css classes: DataSelector, Chart2D_Bottom, Chart2D_Right, Chart2D_Left. | ||
| Don't mind the naming. --> |
|
|
||
| var model_scale_factor = {{ model_scale_factor }}; // model-specific scale factor | ||
| var model_uri = "{{ model_uri }}"; | ||
|
|
||
| // Color codes used in static charts | ||
| colors8 = ['#e0212d', '#208900', '#2877a2', '#333333', '#999999', '#e58C33', '#33e5e5', '#e533e5'] |
There was a problem hiding this comment.
Can we get those via jinja arguments from the existing array?
| @@ -386,9 +483,686 @@ | |||
| viewer.timeline.zoomTo(viewer.clock.startTime, viewer.clock.stopTime); | |||
|
|
|||
|
|
|||
| //// 2D CHARTING/STREAMING \\\\ | |||
|
|
|||
| // Helper Functions \\ | |||
There was a problem hiding this comment.
all of this should also go into a separate js file
| } | ||
|
|
||
| #Chart2D_Right { | ||
| z-index: 2; |
There was a problem hiding this comment.
Can you use a separate CSS class for the common configuration?
… data streams are not found default behaviour is to not plot it on the 2D streamer.
|
Thanks! Great comments. I will write a more detailed response later. I added preliminary exception handling Yesterday and I think that should have solved your error Regarding your other points, I'll go over them whenever I have a little more time. I am also collecting feedback from our internal team. |
As long as you push to the same branch it will automatically show up here as well.
Here are 2:
Let me know if you want to discuss in more detail, and more directly e.g. via slack. |
|
@SelimOzel any update? |
|
Not yet Beat. I have been stuck in another project at work. |
This Pull Request adds basic 2D Charting/Streaming to the 3D view for flight_review. Currently, 14 out of 25 plots in the original charts are ported. Data in these charts include attitude (roll/pitch/yaw) and its set-points, attitude rate and its set-points, position and its set-points, actuator outputs, actuator controls, RC manual inputs (which was already there for radio control graphics), velocity, raw gyro and acceleration values.
This PR needs to be tested/reviewed for:
Potential Future Work: