RSDK-6179 - use docommand to postprocess #311
Conversation
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
| return nil | ||
| } | ||
|
|
||
| func updatePointCloudWithRemovedPoints(updatedData *[]byte, points []r3.Vector) error { |
There was a problem hiding this comment.
Not sure what the best way to do this is - the way it is currently implemented the user would have to input the exact point they want to be removed. Maybe we should instead take in a point and radius and remove any points within that circle?
There was a problem hiding this comment.
I think having a default sized radius would be fine for now.
If we wanted to cover more options then I’m wondering if there’s a good way to let users define a radius or a rectangle as options, depending on what gets sent in the do command
There was a problem hiding this comment.
updated to have a default radius of 1 cm for now
viambot
added
safe to test
| return Task{}, ErrXNotFloat64 | ||
| } | ||
|
|
||
| task.Points = append(task.Points, r3.Vector{X: xFloat, Y: yFloat}) |
There was a problem hiding this comment.
Should we handle Z/make sure a user did not provide it or is this sufficient?
There was a problem hiding this comment.
I think not handling z is sufficient
| ) | ||
|
|
||
| const ( | ||
| fullConfidence = 100 |
There was a problem hiding this comment.
Right now i set all the added points to have 100% confidence. I did this because I figured if a user added it that number made sense. I could see an argument for having this be configurable but opted for this to simplify the DoCommand. What to others think?
There was a problem hiding this comment.
I think using full confidence makes sense. Cannot think of a use case where we would want it to be different
viambot
added
safe to test
kim-mishra
force-pushed
the
RSDK-6179-use-docommand-to-postprocess
branch
from
8943a02
to
5401553
Compare
viambot
added
safe to test
| confidence score to be encoded in the blue parameter of the RGB value, on a | ||
| scale from 1-100. | ||
| */ | ||
| err := pc.Set(point, pointcloud.NewColoredData(color.NRGBA{B: fullConfidence})) |
There was a problem hiding this comment.
[q]Would we be interested in adding another color here to mark the pointcloud as an added obstacle?
the prime component currently overwrites all colors to be grayscale based on the blue channel, so actually visualizing the obstacles would require a change in the prime component to handle the new channel but I think it would be a cool visual
There was a problem hiding this comment.
@randhid do you think this would be worth doing for the delivery bot?
There was a problem hiding this comment.
Will it be more than a day's work? If so yep. If it's complicated, we'll do in a future date when we more formally do this. Save this idea, it's good.
viambot
added
safe to test
| } | ||
|
|
||
| // Overwrite updatedData with new points | ||
| *updatedData = make([]byte, buf.Len()) |
There was a problem hiding this comment.
should we check if nil? What happens if it is?
There was a problem hiding this comment.
*updatedData, which we pass into Read. Come to think fo it can buf.Len() ever be zero?
There was a problem hiding this comment.
I tested this fmt.Print(make([]byte, 0) == nil) and it it is false, I will add a nil check!
There was a problem hiding this comment.
I think there's another one of these higher up, don't forget that one too!
viambot
added
safe to test
viambot
added
safe to test
postprocess/postprocess.go
Outdated
|
|
||
| func updatePointCloudWithRemovedPoints(updatedData *[]byte, points []r3.Vector) error { | ||
| if updatedData == nil { | ||
| return errors.New("cannot provide nil udpated data") |
There was a problem hiding this comment.
This can also be a common error in this package for now.
viambot
added
safe to test
viambot
added
safe to test
This PR adds postprocessing functionality via the
DoCommand. This is achieved by storing 2 additional parameters on the cartoSvc struct.In
PointCloudMapif postprocessing has been "turned on" (cartoSvc.postprocessed == true) all of the postprocessing tasks are iterated through & a new pointcloud map is returned with the added/removed points.The following functionality was added to the
DoCommand:postprocess_toggle: turns on and off postprocessingpostprocess_add: adds points to pointcloud map. Value must be specified as[{X: x, Y: y}, ...]postprocess_add: adds points to pointcloud map. Value must be specified as[{X: x, Y: y}, ...]postprocess_undo: undoes last postprocessing steps.