-
Notifications
You must be signed in to change notification settings - Fork 13
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
Suggestions to #70 #85
Conversation
Signed-off-by: Louise Poubel <louise@openrobotics.org>
/// \brief Salinity data to visualize | ||
public: ignition::msgs::Float_V salMsg; | ||
/// \brief Message holding a float vector. | ||
public: ignition::msgs::Float_V floatVMsg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should let the user have the capability of visualizing more than one type of data at a time, because sometimes it's useful to overlap multiple modalities and see how they relate. For example, temperature and salinity together directly imply density, while either one of them alone doesn't.
We might add the capability of adding more topic drop-down lists, rather than restricting to subscribing only to one topic at a time. All that can be future work - we don't need to make this perfect right now, it just needs to be a working prototype for us to debug the sensors for the acceptance mission. For now, I would say we should keep the multiple vectors to be able to subscribe to multiple types rather than replacing them with a single float vector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't want to hardcode them, they can be in an array of vectors that expands and contracts. I can open an issue about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Speaking of not hardcoding, I saw there was negative data in chlorophyll and wasn't sure if it's an error or intended. So I capped off at 0 and visualized everything below 0 at the same color as 0. Looks like in this subset of data we're visualizing, we don't have negatives. Just curious @braanan are those values intended to be negative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I tested it and it works as pictured.
I left one comment about visualizing multiple topics above. Since the current version does not disable anything in #70, as neither have the GUI ability to add multiple topics, I'm going to approve so that I can rebase #85 on these changes.
But we should keep in mind that we should extend floatVMsg
to an array of different topics, and allow subscribing to multiple types, so not necessarily unsubscribing to all other topics right away when a new topic is selected.
For example, one might add salinity as translucent light blue to dark blue, and temperature as translucent light red to dark red. Then by looking at what's dark purple, you'll be able to tell what areas have the highest density. Once we have ocean current data, visualizing multiple topics would be even more relevant, because e.g. 3D current flow is dependent on density, and other such relationships.
The most intuitive way is probably to have a + button to add more rows of topics, and let the user choose different colors. Pretty much like how RViz handles multiple topics of markers.
Let me know if you want to make further (small) changes to this PR. I'll let you merge.
* Science data interpolation using raw matrix for depth slice search * Align lat long in science data visualization with spawned vehicle lat long * resolve conflicts with main Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * replace TODO TEMPORARY HACK comments with Performance trick Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * cleanup Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add mutex for worldOriginSpherical* Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * finish merge Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * better debug msgs and var names Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add BUILD_TESTING to README Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * disable science sensors in empty_environment.sdf Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * fix merge Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Convert sensor lat long to Cartesian. Some cleanup. Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * refactor conversion from lat long to cart into fn Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * manual cherry pick 643c97d from mabelzhang/interpolate_sci_raw_mat Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * remove WorldOriginSphericalService from WorldCommPlugin Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add depth to ConvertLatLonToCart for convenience Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * incorporate updated ConvertLatLonToCart depth Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * clean up FindInterpolators() and SearchInDepthSlice() Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Found 4 NNs in each of 2 z slices holding 1st and 2nd NNs Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add headers to organize the gazillion member fields. reorder fns Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * decrease how often interpolation is called Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * decrease how often interpolation is called, so sci data vis can be shown Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * return 2 sets of 4 points from FindInterpolators() Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * cleanup Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Suggestions to #70 (#85) Signed-off-by: Louise Poubel <louise@openrobotics.org> * PR comments Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * minor PR comments Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * refactor to use pcl PassThrough filter Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * trilinear interpolation math; API WIP; merge from mabelzhang/interpolate_sci_raw_mat Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * fix variable name Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Fix trilinear interpolation. Find ordering in prism for 8 input vertices, match data accordingly; concatenate 2 input arrays into 1; correct inputs to trilinear interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * better printouts for debugging whether points are on prism Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add barycentric interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * remove trilinear interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * remove extra line Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * remove more trilinear stuff; printouts Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * move comment Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * update comment Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * add TODOs Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Fix logic error to interpolate data for ALL sensors; add 1D degenerative case for interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * fix zero row check - abs val needed Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * Check negative lambda to find correct 2D triangle that query point lies within Signed-off-by: Mabel Zhang <mabel@openrobotics.org> * 1D case more accurate, use closest points; fix bug on weights Signed-off-by: Mabel Zhang <mabel@openrobotics.org> Co-authored-by: Louise Poubel <louise@openrobotics.org>
In the interest of eventually upstreaming the visualization plugin to
ign-gazebo
(i.e. gazebosim/gz-sim#1156), I'm making some suggestions to keep the plugin generic. This also means it will automatically scale to other science data sensors that we add in the future.FloatV
topics, populate the dropdown with all topics publishing that message typeThese are just some initial improvements to make sure we're not locking the plugin into a single use case. There are many more usability and functionality fixes that we need to make before actually upstreaming this.