Skip to content
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.

Sign up for GitHub

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

RSDK-2240 Remove Temp Endpoints from SLAM #2141

Merged
merged 5 commits into from
Mar 31, 2023
Merged

RSDK-2240 Remove Temp Endpoints from SLAM #2141

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
afcfdfa
RSDK-2238-add-final-slam-map-res-grpc-methods
nicksanford Mar 28, 2023
9c9c691
wip
nicksanford Mar 29, 2023
ee2bbda
Merge branch 'main' of github.com:viamrobotics/rdk into RSDK-2240_Rem…
jeremyrhyde Mar 30, 2023
3747295
remove temp endpoints;
jeremyrhyde Mar 30, 2023
41b2e86
udpate client comments
jeremyrhyde Mar 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions services/slam/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,15 @@ func TestClientWorkingService(t *testing.T) {
test.That(t, spatial.PoseAlmostEqual(poseSucc, pose), test.ShouldBeTrue)
test.That(t, componentRef, test.ShouldEqual, componentRefSucc)

// test get point cloud map stream
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional removal, I'm in favor to keep pattern with server tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine, we have a ticket to make server tests into individual t.runs, which should probably include these tests too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with john. Lets address client tests not being in t.Runs in the ticket he linked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note to self] we took out the word "stream" everywhere that we aren't specifically testing the streaming aspect of the api

// test get point cloud map
fullBytesPCD, err := slam.GetPointCloudMapFull(context.Background(), workingSLAMClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
// comparing raw bytes to ensure order is correct
test.That(t, fullBytesPCD, test.ShouldResemble, pcd)
// comparing pointclouds to ensure PCDs are correct
testhelper.TestComparePointCloudsFromPCDs(t, fullBytesPCD, pcd)

// test get internal state stream
// test get internal state
fullBytesInternalState, err := slam.GetInternalStateFull(context.Background(), workingSLAMClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
test.That(t, fullBytesInternalState, test.ShouldResemble, internalStateSucc)
Expand All @@ -142,15 +142,15 @@ func TestClientWorkingService(t *testing.T) {
test.That(t, spatial.PoseAlmostEqual(poseSucc, pose), test.ShouldBeTrue)
test.That(t, componentRef, test.ShouldEqual, componentRefSucc)

// test get point cloud map stream
// test get point cloud map
fullBytesPCD, err := slam.GetPointCloudMapFull(context.Background(), workingDialedClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
// comparing raw bytes to ensure order is correct
test.That(t, fullBytesPCD, test.ShouldResemble, pcd)
// comparing pointclouds to ensure PCDs are correct
testhelper.TestComparePointCloudsFromPCDs(t, fullBytesPCD, pcd)

// test get internal state stream
// test get internal state
fullBytesInternalState, err := slam.GetInternalStateFull(context.Background(), workingDialedClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
test.That(t, fullBytesInternalState, test.ShouldResemble, internalStateSucc)
Expand Down Expand Up @@ -178,15 +178,15 @@ func TestClientWorkingService(t *testing.T) {
test.That(t, spatial.PoseAlmostEqual(poseSucc, pose), test.ShouldBeTrue)
test.That(t, componentRef, test.ShouldEqual, componentRefSucc)

// test get point cloud map stream
// test get point cloud map
fullBytesPCD, err := slam.GetPointCloudMapFull(context.Background(), workingDialedClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
// comparing raw bytes to ensure order is correct
test.That(t, fullBytesPCD, test.ShouldResemble, pcd)
// comparing pointclouds to ensure PCDs are correct
testhelper.TestComparePointCloudsFromPCDs(t, fullBytesPCD, pcd)

// test get internal state stream
// test get internal state
fullBytesInternalState, err := slam.GetInternalStateFull(context.Background(), workingDialedClient, nameSucc)
test.That(t, err, test.ShouldBeNil)
test.That(t, fullBytesInternalState, test.ShouldResemble, internalStateSucc)
Expand Down Expand Up @@ -215,11 +215,11 @@ func TestFailingClient(t *testing.T) {
}

failingSLAMService.GetPointCloudMapFunc = func(ctx context.Context, name string) (func() ([]byte, error), error) {
return nil, errors.New("failure during get pointcloud map stream")
return nil, errors.New("failure during get pointcloud map")
}

failingSLAMService.GetInternalStateFunc = func(ctx context.Context, name string) (func() ([]byte, error), error) {
return nil, errors.New("failure during get internal state stream")
return nil, errors.New("failure during get internal state")
}

failingSvc, err := subtype.New(map[resource.Name]interface{}{slam.Named(nameFail): failingSLAMService})
Expand Down Expand Up @@ -252,14 +252,14 @@ func TestFailingClient(t *testing.T) {
test.That(t, pose, test.ShouldBeNil)
test.That(t, componentRef, test.ShouldBeEmpty)

// test get pointcloud map stream
// test get pointcloud map
fullBytesPCD, err := slam.GetPointCloudMapFull(context.Background(), failingSLAMClient, nameFail)
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during get pointcloud map stream")
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during get pointcloud map")
test.That(t, fullBytesPCD, test.ShouldBeNil)

// test get internal state stream
// test get internal state
fullBytesInternalState, err := slam.GetInternalStateFull(context.Background(), failingSLAMClient, nameFail)
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during get internal state stream")
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during get internal state")
test.That(t, fullBytesInternalState, test.ShouldBeNil)

test.That(t, conn.Close(), test.ShouldBeNil)
Expand All @@ -285,12 +285,12 @@ func TestFailingClient(t *testing.T) {

failingSLAMClient := slam.NewClientFromConn(context.Background(), conn, slam.Named(nameFail).String(), logger)

// test get pointcloud map stream
// test get pointcloud map
fullBytesPCD, err := slam.GetPointCloudMapFull(context.Background(), failingSLAMClient, nameFail)
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during callback")
test.That(t, fullBytesPCD, test.ShouldBeNil)

// test get internal state stream
// test get internal state
fullBytesInternalState, err := slam.GetInternalStateFull(context.Background(), failingSLAMClient, nameFail)
test.That(t, err.Error(), test.ShouldContainSubstring, "failure during callback")
test.That(t, fullBytesInternalState, test.ShouldBeNil)
Expand Down
100 changes: 0 additions & 100 deletions services/slam/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,106 +140,6 @@ func (server *subtypeServer) GetInternalState(req *pb.GetInternalStateRequest,
}
}

// GetPositionNew returns a Pose and a component reference string of the robot's current location according to SLAM.
func (server *subtypeServer) GetPositionNew(ctx context.Context, req *pb.GetPositionNewRequest) (
*pb.GetPositionNewResponse, error,
) {
ctx, span := trace.StartSpan(ctx, "slam::server::GetPosition")
defer span.End()

svc, err := server.service(req.Name)
if err != nil {
return nil, err
}

p, componentReference, err := svc.GetPosition(ctx, req.Name)
if err != nil {
return nil, err
}

return &pb.GetPositionNewResponse{
Pose: spatialmath.PoseToProtobuf(p),
ComponentReference: componentReference,
}, nil
}

// GetPointCloudMapStream returns the slam service's slam algo's current map state in PCD format as
// a stream of byte chunks.
func (server *subtypeServer) GetPointCloudMapStream(req *pb.GetPointCloudMapStreamRequest,
stream pb.SLAMService_GetPointCloudMapStreamServer,
) error {
ctx := context.Background()

ctx, span := trace.StartSpan(ctx, "slam::server::GetPointCloudMapStream")
defer span.End()

svc, err := server.service(req.Name)
if err != nil {
return err
}

f, err := svc.GetPointCloudMap(ctx, req.Name)
if err != nil {
return errors.Wrap(err, "getting callback function from GetPointCloudMapStream encountered an issue")
}

// In the future, channel buffer could be used here to optimize for latency
for {
rawChunk, err := f()

if errors.Is(err, io.EOF) {
return nil
}

if err != nil {
return errors.Wrap(err, "getting data from callback function encountered an issue")
}

chunk := &pb.GetPointCloudMapStreamResponse{PointCloudPcdChunk: rawChunk}
if err := stream.Send(chunk); err != nil {
return err
}
}
}

// GetInternalStateStream returns the internal state of the slam service's slam algo in a stream of
// byte chunks.
func (server *subtypeServer) GetInternalStateStream(req *pb.GetInternalStateStreamRequest,
stream pb.SLAMService_GetInternalStateStreamServer,
) error {
ctx := context.Background()
ctx, span := trace.StartSpan(ctx, "slam::server::GetInternalStateStream")
defer span.End()

svc, err := server.service(req.Name)
if err != nil {
return err
}

f, err := svc.GetInternalState(ctx, req.Name)
if err != nil {
return err
}

// In the future, channel buffer could be used here to optimize for latency
for {
rawChunk, err := f()

if errors.Is(err, io.EOF) {
return nil
}

if err != nil {
return errors.Wrap(err, "getting data from callback function encountered an issue")
}

chunk := &pb.GetInternalStateStreamResponse{InternalStateChunk: rawChunk}
if err := stream.Send(chunk); err != nil {
return err
}
}
}

// DoCommand receives arbitrary commands.
func (server *subtypeServer) DoCommand(ctx context.Context,
req *commonpb.DoCommandRequest,
Expand Down
Loading