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

Jump to bottom

Remove GetLatestMapInfo from SLAM #313

Merged
merged 13 commits into from
Feb 7, 2024
Merged

Remove GetLatestMapInfo from SLAM #313

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a90de10
RSDK-6040 Add Properties endpoint
jeremyrhyde Dec 26, 2023
d0f2bf2
update comment for Properties
jeremyrhyde Dec 26, 2023
6eb5e0b
add bad case
jeremyrhyde Dec 26, 2023
9d217f1
Update viam_cartographer.go
jeremyrhyde Dec 27, 2023
e3c915b
pr fixes
jeremyrhyde Dec 27, 2023
5845cea
RSDK-6133 REmove GEtLatestMapInfo
jeremyrhyde Dec 29, 2023
a2d528c
merge
jeremyrhyde Jan 17, 2024
f3634f3
properties
jeremyrhyde Jan 22, 2024
77d7144
go mod update
jeremyrhyde Feb 6, 2024
0c352af
replace
jeremyrhyde Feb 6, 2024
f897c56
make lint issues
jeremyrhyde Feb 6, 2024
002a536
nolint of gomoddirectives
jeremyrhyde Feb 6, 2024
a9fda2f
remove replace and pin to latest
jeremyrhyde Feb 7, 2024
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
8 changes: 3 additions & 5 deletions go.mod
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,19 +116,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ func New(
logger: logger,
cartoFacadeTimeout: cartoFacadeTimeout,
cartoFacadeInternalTimeout: cartoFacadeInternalTimeout,
mapTimestamp: time.Now().UTC(),
enableMapping: optionalConfigParams.EnableMapping,
existingMap: optionalConfigParams.ExistingMap,
}
Expand Down Expand Up @@ -501,8 +500,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 @@ -637,23 +635,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
View file
Open in desktop
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
Loading