RSDK-1059 Add ParallelProjectionOntoXZwithRobotMarker for SLAM visualizations #1762
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
viambot
added
safe to test
added 2 commits
January 12, 2023 00:18
viambot
added
safe to test
ghost
commented
Jan 12, 2023
| sigmaLevel = 7 | ||
| imageHeight = 1080 | ||
| imageWidth = 1080 | ||
| missThreshold = 0.44 |
There was a problem hiding this comment.
Currently these thresholds and the getProbabilityColorFromValue are placeholders with basic logic that mirror to some degree the way the painting is being performed in cartographer. See PR description for more details.
These miss and hit thresholds values are also from cartographer.
There was a problem hiding this comment.
Where are these values in the carto slam lib?
viambot
added
safe to test
ghost
marked this pull request as ready for review
viambot
added
the
safe to test
Co-authored-by: Katharina Xenia Kufieta <katharina.kufieta@gmail.com>
Co-authored-by: Katharina Xenia Kufieta <katharina.kufieta@gmail.com>
Co-authored-by: Katharina Xenia Kufieta <katharina.kufieta@gmail.com>
viambot
added
safe to test
Co-authored-by: Katharina Xenia Kufieta <katharina.kufieta@gmail.com>
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
…allelProjectionOntoXZWithRobotMarker
viambot
added
safe to test
|
nicksanford
requested changes
Jan 23, 2023
| // if needed based on the mean and standard deviation of the X and Z coordinates. | ||
| func calculateScaleFactor(xRange, zRange float64) float64 { | ||
| var scaleFactor float64 | ||
| if xRange != 0 || zRange != 0 { |
There was a problem hiding this comment.
Doesn't this allow for divide by zero errors, which will cause a panic?
e.g. xRange == 10 zRange == 0
|
|
||
| switch { | ||
| case prob < missThreshold: | ||
| b = uint8(255 * ((missThreshold - prob) / (hitThreshold - 0))) |
There was a problem hiding this comment.
-
Why are we subtracting 0 from
hitThreshold? Isn't this a no op? -
Also, are these shades intended to mean anything? I.e. does more green mean something than less green, or are we just trying to make different probabilities in the 3 different buckets distinct from each other? If more green / less green or more glue / less blue is intended to mean something in the current form, can you please add that intent to the comment?
| case prob > hitThreshold: | ||
| g = uint8(255 * ((prob - hitThreshold) / (1 - missThreshold))) | ||
| default: | ||
| b = uint8(255 * ((missThreshold - prob) / (hitThreshold - 0))) |
There was a problem hiding this comment.
Why are we subtracting 0 from hitThreshold? Isn't this a no op?
| g = uint8(255 * ((prob - hitThreshold) / (1 - missThreshold))) | ||
| default: | ||
| b = uint8(255 * ((missThreshold - prob) / (hitThreshold - 0))) | ||
| g = uint8(255 * ((prob - hitThreshold) / (1 - missThreshold))) |
There was a problem hiding this comment.
Won't prob - hitThreshold result in a negative number here? Won't that result in a panic?
|
|
||
| // Adds a point to an image using the value to define the color. If no value is available, | ||
| // the default color of white is used. | ||
| if x >= 0 && x < imageWidth && y >= 0 && y < imageHeight { |
There was a problem hiding this comment.
[nit] could we please call x >= 0 && x < imageWidth && y >= 0 && y < imageHeight a variable like: pointWithinImageStandardDeviation so it is clear that this is where we are filtering out points which fall outside of the image borders as they are not within the standard deviation?
|
|
||
| // Add a red robot marker to the image | ||
| if ppRM.robotPose != nil { | ||
| x := int(math.Round((robotMarker.Point().X - minX) * scaleFactor)) |
There was a problem hiding this comment.
If the robot marker is outside of the standard deviation does this still do the right thing?
| } | ||
|
|
||
| // Calculate the scale factors | ||
| scaleFactor := calculateScaleFactor(maxX-minX, maxZ-minZ) |
There was a problem hiding this comment.
If the scale factor is 0 this will return an image which is black right? Is that ever useful to an end user or would it be better to return an error in this case?
| test.That(t, unusedDepthMap, test.ShouldBeNil) | ||
| }) | ||
|
|
||
| t.Run("Test that projecting two offset pointclouds will produce same image", func(t *testing.T) { |
There was a problem hiding this comment.
Can we please add the following tests:
- Behavior when robot marker falls outside of standard deviation.
- Behavior when either maxX==minX, maxZ==minZ, or both.
- Behavior when any of the point cloud points have nil data
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR creates an image from a pcd by projecting it onto the XZ plane. A red mark is added to the image to denote the current robot position.
Currently, this PR implements the basic logic for converting probability into a color, however, it is waiting on RSDK-1073 to be complete before it can be tested. Note: only cartographer will provide probability data, orbslam will rely on the default (white) color to represent its points.
JIRA Ticket: RSDK-1059
Image:
