Skip to content

Commit

Permalink
Remove GetLatestMapInfo from SLAM (#313)
Browse files Browse the repository at this point in the history
Co-authored-by: kim-mishra <121991867+kim-mishra@users.noreply.github.com>
  • Loading branch information
jeremyrhyde and kim-mishra authored Feb 7, 2024
1 parent 4964c56 commit 014d13d
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 75 deletions.
8 changes: 3 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ require (
go.opencensus.io v0.24.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.26.0
go.viam.com/api v0.1.245
go.viam.com/rdk v0.18.0
go.viam.com/api v0.1.257
go.viam.com/rdk v0.20.1-0.20240207164326-b493060b7606
go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2
go.viam.com/utils v0.1.54
go.viam.com/utils v0.1.60
)

require (
Expand All @@ -37,7 +37,6 @@ require (
github.com/OpenPeeDeeP/depguard v1.1.1 // indirect
github.com/a8m/envsubst v1.4.2 // indirect
github.com/ajstarks/svgo v0.0.0-20211024235047-1546f124cd8b // indirect
github.com/alecthomas/participle/v2 v2.1.1 // indirect
github.com/alexkohler/prealloc v1.0.0 // indirect
github.com/alingse/asasalint v0.0.11 // indirect
github.com/apache/arrow/go/arrow v0.0.0-20211112161151-bc219186db40 // indirect
Expand Down Expand Up @@ -74,7 +73,6 @@ require (
github.com/edaniels/golog v0.0.0-20230215213219-28954395e8d0 // indirect
github.com/edaniels/lidario v0.0.0-20220607182921-5879aa7b96dd // indirect
github.com/edaniels/zeroconf v1.0.10 // indirect
github.com/erh/scheme v0.0.0-20230727180956-91bc1abd571e // indirect
github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5 // indirect
github.com/esimonov/ifshort v1.0.4 // indirect
github.com/ettle/strcase v0.1.1 // indirect
Expand Down
19 changes: 7 additions & 12 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ github.com/ajstarks/deck/generate v0.0.0-20210309230005-c3f852c02e19/go.mod h1:T
github.com/ajstarks/svgo v0.0.0-20180226025133-644b8db467af/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw=
github.com/ajstarks/svgo v0.0.0-20211024235047-1546f124cd8b h1:slYM766cy2nI3BwyRiyQj/Ud48djTMtMebDqepE95rw=
github.com/ajstarks/svgo v0.0.0-20211024235047-1546f124cd8b/go.mod h1:1KcenG0jGWcpt8ov532z81sp/kMMUG485J2InIOyADM=
github.com/alecthomas/assert/v2 v2.3.0 h1:mAsH2wmvjsuvyBvAmCtm7zFsBlb8mIHx5ySLVdDZXL0=
github.com/alecthomas/participle/v2 v2.1.1 h1:hrjKESvSqGHzRb4yW1ciisFJ4p3MGYih6icjJvbsmV8=
github.com/alecthomas/participle/v2 v2.1.1/go.mod h1:Y1+hAs8DHPmc3YUFzqllV+eSQ9ljPTk0ZkPMtEdAx2c=
github.com/alecthomas/repr v0.2.0 h1:HAzS41CIzNW5syS8Mf9UwXhNH1J9aix/BvDRf1Ml2Yk=
github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
Expand Down Expand Up @@ -287,8 +283,6 @@ github.com/envoyproxy/go-control-plane v0.9.9-0.20210512163311-63b5d3c536b0/go.m
github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.mod h1:AFq3mo9L8Lqqiid3OhADV3RfLJnjiw63cSpi+fDTRC0=
github.com/envoyproxy/protoc-gen-validate v0.0.14/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/erh/scheme v0.0.0-20230727180956-91bc1abd571e h1:QYPfobvLZcrOizm3tEPP1RiXFn5i0RpGJfslxPisieY=
github.com/erh/scheme v0.0.0-20230727180956-91bc1abd571e/go.mod h1:I5jlaysFLpVU0rMv7m2T5ut73mqBn/+F42LuPk88Zes=
github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5 h1:Yzb9+7DPaBjB8zlTR87/ElzFsnQfuHnVUVqpZZIcV5Y=
github.com/erikstmartin/go-testdb v0.0.0-20160219214506-8d10e4a1bae5/go.mod h1:a2zkGnVExMxdzMo3M0Hi/3sEU+cWnZpSni0O6/Yb/P0=
github.com/esimonov/ifshort v1.0.1/go.mod h1:yZqNJUrNn20K8Q9n2CrjTKYyVEmX209Hgu+M1LBpeZE=
Expand Down Expand Up @@ -425,6 +419,7 @@ github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJA
github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee/go.mod h1:L0fX3K22YWvt/FAX9NnzrNzcI4wNYi9Yku4O0LKYflo=
github.com/gobwas/pool v0.2.0/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
github.com/gobwas/ws v1.0.2/go.mod h1:szmBTxLgaFppYjEmNtny/v3w89xOydFnnZMcgRRu/EM=
github.com/goccy/go-graphviz v0.1.2 h1:sWSJ6w13BCm/ZOUTHDVrdvbsxqN8yyzaFcHrH/hQ9Yg=
github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU=
github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I=
github.com/gofrs/flock v0.8.0/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14jxHU=
Expand Down Expand Up @@ -1390,14 +1385,14 @@ go.uber.org/zap v1.18.1/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI=
go.uber.org/zap v1.23.0/go.mod h1:D+nX8jyLsMHMYrln8A0rJjFt/T/9/bGgIhAqxv5URuY=
go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo=
go.uber.org/zap v1.26.0/go.mod h1:dtElttAiwGvoJ/vj4IwHBS/gXsEu/pZ50mUIRWuG0so=
go.viam.com/api v0.1.245 h1:DL5x1fxg0VUwPJRI1KaSwXeCZR/PnknTpw7AU0Mmu18=
go.viam.com/api v0.1.245/go.mod h1:msa4TPrMVeRDcG4YzKA/S6wLEUC7GyHQE973JklrQ10=
go.viam.com/rdk v0.18.0 h1:PiqAdBrE+4xr6X43AD5qbELQIJ0jMWgj49uY6x1aJXA=
go.viam.com/rdk v0.18.0/go.mod h1:zOfpk0LP2QKkdWmNwvSUTZldZ2KjrXCIo8KzF+eDfnk=
go.viam.com/api v0.1.257 h1:t1AJN99Us+H9p8iVftY4a/QLPejrzy+ypr6kYGpAd8w=
go.viam.com/api v0.1.257/go.mod h1:msa4TPrMVeRDcG4YzKA/S6wLEUC7GyHQE973JklrQ10=
go.viam.com/rdk v0.20.1-0.20240207164326-b493060b7606 h1:SH/TPZhralm5myCePGkPjPnmxxGJF7pSQUXqEEsEU8U=
go.viam.com/rdk v0.20.1-0.20240207164326-b493060b7606/go.mod h1:C30jq8oLol7SQCYROrzbwKtunWWAL4wBYd340+DmOO8=
go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2 h1:oBiK580EnEIzgFLU4lHOXmGAE3MxnVbeR7s1wp/F3Ps=
go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2/go.mod h1:XM0tej6riszsiNLT16uoyq1YjuYPWlRBweTPRDanIts=
go.viam.com/utils v0.1.54 h1:1ENNj0atwDngCVKAOr4gXSJnoQdVd88qm38tEnqghaE=
go.viam.com/utils v0.1.54/go.mod h1:tjPInze4C0UYFRqL/FU96yqhJpHR1zjiNZ7qChTN/b8=
go.viam.com/utils v0.1.60 h1:Q1EtxJbTbdvFwDBj4M7uZ/3lLjY63JBrCLlZlYL9VBQ=
go.viam.com/utils v0.1.60/go.mod h1:O6CsAqjd7xMfx7Ip6GWTLGsg3WTJ/wr3JySho4D55qY=
go4.org/unsafe/assume-no-moving-gc v0.0.0-20231121144256-b99613f794b6 h1:lGdhQUN/cnWdSH3291CUuxSEqc+AsGTiDxPP3r2J0l4=
go4.org/unsafe/assume-no-moving-gc v0.0.0-20231121144256-b99613f794b6/go.mod h1:FftLjUGFEDu5k8lt0ddY+HcrH/qU/0qk+H8j9/nTl3E=
goji.io v2.0.2+incompatible h1:uIssv/elbKRLznFUy3Xj4+2Mz/qKhek/9aZQDUMae7c=
Expand Down
13 changes: 4 additions & 9 deletions testhelper/integrationtesthelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,19 +113,14 @@ func testCartographerPosition(t *testing.T, svc slam.Service, useIMU bool, expec

// Checks the cartographer map and confirms there at least 100 map points.
func testCartographerMap(t *testing.T, svc slam.Service, localizationMode bool) {
timestamp1, err := svc.LatestMapInfo(context.Background())
props, err := svc.Properties(context.Background())
test.That(t, err, test.ShouldBeNil)
test.That(t, props.CloudSlam, test.ShouldBeFalse)
test.That(t, props.MappingMode == slam.MappingModeLocalizationOnly, test.ShouldEqual, localizationMode)

pcd, err := slam.PointCloudMapFull(context.Background(), svc)
test.That(t, err, test.ShouldBeNil)
test.That(t, pcd, test.ShouldNotBeNil)
timestamp2, err := svc.LatestMapInfo(context.Background())
test.That(t, err, test.ShouldBeNil)

if localizationMode {
test.That(t, timestamp1, test.ShouldResemble, timestamp2)
} else {
test.That(t, timestamp2.After(timestamp1), test.ShouldBeTrue)
}

pointcloud, err := pointcloud.ReadPCD(bytes.NewReader(pcd))
test.That(t, err, test.ShouldBeNil)
Expand Down
21 changes: 1 addition & 20 deletions viam_cartographer.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,6 @@ func New(
logger: logger,
cartoFacadeTimeout: cartoFacadeTimeout,
cartoFacadeInternalTimeout: cartoFacadeInternalTimeout,
mapTimestamp: time.Now().UTC(),
enableMapping: optionalConfigParams.EnableMapping,
existingMap: optionalConfigParams.ExistingMap,
}
Expand Down Expand Up @@ -527,8 +526,7 @@ type CartographerService struct {
sensorProcessWorkers sync.WaitGroup
cartoFacadeWorkers sync.WaitGroup

mapTimestamp time.Time
jobDone atomic.Bool
jobDone atomic.Bool

postprocessed atomic.Bool
postprocessingTasks []postprocess.Task
Expand Down Expand Up @@ -663,23 +661,6 @@ func (cartoSvc *CartographerService) Properties(ctx context.Context) (slam.Prope
return props, nil
}

// LatestMapInfo returns a new timestamp every time it is called when in mapping mode, to signal
// that the map should be updated. In localizing, the timestamp returned is the timestamp of the session.
func (cartoSvc *CartographerService) LatestMapInfo(ctx context.Context) (time.Time, error) {
_, span := trace.StartSpan(ctx, "viamcartographer::CartographerService::LatestMapInfo")
defer span.End()

if err := cartoSvc.isOpenAndRunningLocally("LatestMapInfo"); err != nil {
return time.Time{}, err
}

if cartoSvc.SlamMode != cartofacade.LocalizingMode {
cartoSvc.mapTimestamp = time.Now().UTC()
}

return cartoSvc.mapTimestamp, nil
}

// DoCommand receives arbitrary commands.
func (cartoSvc *CartographerService) DoCommand(ctx context.Context, req map[string]interface{}) (map[string]interface{}, error) {
_, span := trace.StartSpan(ctx, "viamcartographer::CartographerService::DoCommand")
Expand Down
33 changes: 4 additions & 29 deletions viam_cartographer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
// certain exported functions which we do not want to make available to the user. It also runs integration tests
// that test the interaction with the core C++ viam-cartographer code and the Golang implementation of the
// cartographer slam service.
//
//nolint:dupl
package viamcartographer_test

import (
"context"
"os"
"testing"
"time"

"github.com/pkg/errors"
viamgrpc "go.viam.com/rdk/grpc"
Expand All @@ -34,9 +35,8 @@ const (
)

var (
_zeroTime = time.Time{}
_true = true
_false = false
_true = true
_false = false
)

func TestNew(t *testing.T) {
Expand Down Expand Up @@ -70,10 +70,6 @@ func TestNew(t *testing.T) {
test.That(t, gisF, test.ShouldBeNil)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrUseCloudSlamEnabled)

mapTime, err := svc.LatestMapInfo(ctx)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrUseCloudSlamEnabled)
test.That(t, mapTime, test.ShouldResemble, time.Time{})

prop, err := svc.Properties(ctx)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrUseCloudSlamEnabled)
test.That(t, prop, test.ShouldResemble, slam.Properties{})
Expand Down Expand Up @@ -162,9 +158,6 @@ func TestNew(t *testing.T) {
test.That(t, ok, test.ShouldBeTrue)
test.That(t, cs.SlamMode, test.ShouldEqual, cartofacade.LocalizingMode)

timestamp1, err := svc.LatestMapInfo(context.Background())
test.That(t, err, test.ShouldBeNil)

// Test position
pose, componentReference, err := svc.Position(context.Background())
test.That(t, pose, test.ShouldBeNil)
Expand All @@ -188,11 +181,6 @@ func TestNew(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
test.That(t, is, test.ShouldNotBeNil)

timestamp2, err := svc.LatestMapInfo(context.Background())
test.That(t, err, test.ShouldBeNil)
test.That(t, timestamp1.After(_zeroTime), test.ShouldBeTrue)
test.That(t, timestamp1, test.ShouldResemble, timestamp2)

// Test properties
prop, err := svc.Properties(context.Background())
test.That(t, err, test.ShouldBeNil)
Expand Down Expand Up @@ -230,9 +218,6 @@ func TestNew(t *testing.T) {
test.That(t, err, test.ShouldNotBeNil)
test.That(t, err.Error(), test.ShouldContainSubstring, "VIAM_CARTO_GET_POSITION_NOT_INITIALIZED")

timestamp1, err := svc.LatestMapInfo(context.Background())
test.That(t, err, test.ShouldBeNil)

// Test pointcloud map
pcmFunc, err := svc.PointCloudMap(context.Background())
test.That(t, err, test.ShouldBeNil)
Expand All @@ -249,12 +234,6 @@ func TestNew(t *testing.T) {
test.That(t, err, test.ShouldBeNil)
test.That(t, is, test.ShouldNotBeNil)

timestamp2, err := svc.LatestMapInfo(context.Background())
test.That(t, err, test.ShouldBeNil)

test.That(t, timestamp1.After(_zeroTime), test.ShouldBeTrue)
test.That(t, timestamp2.After(timestamp1), test.ShouldBeTrue)

// Test properties
prop, err := svc.Properties(context.Background())
test.That(t, err, test.ShouldBeNil)
Expand Down Expand Up @@ -341,10 +320,6 @@ func TestClose(t *testing.T) {
test.That(t, gisF, test.ShouldBeNil)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrClosed)

mapTime, err := svc.LatestMapInfo(ctx)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrClosed)
test.That(t, mapTime, test.ShouldResemble, time.Time{})

prop, err := svc.Properties(ctx)
test.That(t, err, test.ShouldBeError, viamcartographer.ErrClosed)
test.That(t, prop, test.ShouldResemble, slam.Properties{})
Expand Down

0 comments on commit 014d13d

Please sign in to comment.