Skip to content

Commit

Permalink
Remove validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@kkufieta
kkufieta committed Nov 10, 2023
1 parent ec4b022 commit 90ce747
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 197 deletions.
29 changes: 14 additions & 15 deletions integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,38 +46,37 @@ func testCartographerPosition(t *testing.T, svc slam.Service, useIMU bool, expec

switch {
case runtime.GOOS == "darwin" && !useIMU:
expectedPos = r3.Vector{X: 3.651426324334424, Y: 1.422454179829863, Z: 0}
expectedPos = r3.Vector{X: 1.9166854207566584, Y: 4.0381299349907644, Z: 0}
expectedOri = &spatialmath.R4AA{
RX: 0,
RY: 0,
RZ: 1,
Theta: 0.0006629744894043836,
}
case runtime.GOOS == "linux" && !useIMU:
expectedPos = r3.Vector{X: 1.2714478890528866, Y: 3.1271067529150076, Z: 0}
expectedPos = r3.Vector{X: 7.507596391989648, Y: 3.193198802065579, Z: 0}
expectedOri = &spatialmath.R4AA{
RX: 0,
RY: 0,
RZ: -1,
Theta: 0.0010751949934010567,
RZ: 1,
Theta: 0.001955831550003536,
}

case runtime.GOOS == "darwin" && useIMU:
// expectedPos = r3.Vector{X: 3.2858556935949954, Y: 2.707920249449911, Z: 0}
expectedPos = r3.Vector{X: 4.4700878707562035, Y: 3.1781587655776358, Z: 0}
expectedPos = r3.Vector{X: 2.0505875736777743, Y: 5.223279102160396, Z: 0}
expectedOri = &spatialmath.R4AA{
RX: 0.9861776038047263,
RY: 0.1637212678758259,
RZ: 0.025477052402116784,
Theta: 0.02399255141454847,
RX: 0.9859881081149332,
RY: 0.16566945009254153,
RZ: 0.019521371928468846,
Theta: 0.024957572547389457,
}
case runtime.GOOS == "linux" && useIMU:
expectedPos = r3.Vector{X: 3.2250269853115867, Y: 5.104006882925285, Z: 0}
expectedPos = r3.Vector{X: 4.034335774857501, Y: 3.4162168550846896, Z: 0}
expectedOri = &spatialmath.R4AA{
RX: 0.9864461301028694,
RY: 0.16360809262540335,
RZ: 0.012506975355798564,
Theta: 0.02398663944371901,
RX: 0.9854659908213061,
RY: 0.16580252152269065,
RZ: 0.036963560316871265,
Theta: 0.02496988133280465,
}
}

Expand Down
43 changes: 0 additions & 43 deletions sensors/lidar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@ package sensors
import (
"bytes"
"context"
"strings"
"time"

"github.com/edaniels/golog"
"github.com/pkg/errors"
"go.opencensus.io/trace"
"go.viam.com/rdk/components/camera"
"go.viam.com/rdk/components/camera/replaypcd"
"go.viam.com/rdk/pointcloud"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/utils/contextutils"
goutils "go.viam.com/utils"
)

// TimedLidar describes a sensor that reports the time the reading is from & whether or not it is
Expand Down Expand Up @@ -108,43 +105,3 @@ func NewLidar(
Lidar: lidar,
}, nil
}

// ValidateGetLidarData checks every sensorValidationIntervalSec if the provided lidar
// returned a valid timed readings every sensorValidationIntervalSec
// until either success or sensorValidationMaxTimeoutSec has elapsed.
// Returns an error no valid reading was returned.
func ValidateGetLidarData(
ctx context.Context,
lidar TimedLidar,
sensorValidationMaxTimeout time.Duration,
sensorValidationInterval time.Duration,
logger golog.Logger,
) error {
ctx, span := trace.StartSpan(ctx, "viamcartographer::sensor::ValidateGetLidarData")
defer span.End()

startTime := time.Now().UTC()

for {
_, err := lidar.TimedLidarReading(ctx)
if err == nil {
break
}

logger.Debugw("ValidateGetLidarData hit error: ", "error", err)
// if the sensor is a replay camera with no data ready, allow validation to pass
// offline mode will stop the mapping session if the sensor still has no data,
// while online mode will continue mapping once data is found by the replay sensor
if strings.Contains(err.Error(), replaypcd.ErrEndOfDataset.Error()) {
break
}
if time.Since(startTime) >= sensorValidationMaxTimeout {
return errors.Wrap(err, "ValidateGetLidarData timeout")
}
if !goutils.SelectContextOrWait(ctx, sensorValidationInterval) {
return ctx.Err()
}
}

return nil
}
42 changes: 0 additions & 42 deletions sensors/lidar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,48 +45,6 @@ func TestNewLidar(t *testing.T) {
})
}

func TestValidateGetLidarData(t *testing.T) {
logger := golog.NewTestLogger(t)
ctx := context.Background()

lidar, imu := s.GoodLidar, s.NoMovementSensor
goodLidar, err := s.NewLidar(ctx, s.SetupDeps(lidar, imu), string(lidar), testDataFrequencyHz, logger)
test.That(t, err, test.ShouldBeNil)

lidar, imu = s.LidarWithInvalidProperties, s.NoMovementSensor
_, err = s.NewLidar(ctx, s.SetupDeps(lidar, imu), string(lidar), testDataFrequencyHz, logger)
test.That(t, err, test.ShouldBeError,
errors.New("configuring lidar camera error: 'camera' must support PCD"))

sensorValidationMaxTimeout := time.Duration(50) * time.Millisecond
sensorValidationInterval := time.Duration(10) * time.Millisecond

t.Run("returns nil if a lidar reading succeeds immediately", func(t *testing.T) {
err := s.ValidateGetLidarData(ctx, goodLidar, sensorValidationMaxTimeout, sensorValidationInterval, logger)
test.That(t, err, test.ShouldBeNil)
})

t.Run("returns nil if a lidar reading succeeds within the timeout", func(t *testing.T) {
lidar, imu = s.WarmingUpLidar, s.NoMovementSensor
warmingUpLidar, err := s.NewLidar(ctx, s.SetupDeps(lidar, imu), string(lidar), testDataFrequencyHz, logger)
test.That(t, err, test.ShouldBeNil)
err = s.ValidateGetLidarData(ctx, warmingUpLidar, sensorValidationMaxTimeout, sensorValidationInterval, logger)
test.That(t, err, test.ShouldBeNil)
})

t.Run("returns error if no lidar reading succeeds by the time the context is cancelled", func(t *testing.T) {
cancelledCtx, cancelFunc := context.WithCancel(ctx)
cancelFunc()

lidar, imu = s.WarmingUpLidar, s.NoMovementSensor
warmingUpLidar, err := s.NewLidar(ctx, s.SetupDeps(lidar, imu), string(lidar), testDataFrequencyHz, logger)
test.That(t, err, test.ShouldBeNil)

err = s.ValidateGetLidarData(cancelledCtx, warmingUpLidar, sensorValidationMaxTimeout, sensorValidationInterval, logger)
test.That(t, err, test.ShouldBeError, context.Canceled)
})
}

func TestTimedLidarSensorReading(t *testing.T) {
logger := golog.NewTestLogger(t)
ctx := context.Background()
Expand Down
43 changes: 0 additions & 43 deletions sensors/movementsensor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sensors

import (
"context"
"strings"
"time"

"github.com/edaniels/golog"
Expand All @@ -12,12 +11,10 @@ import (
"github.com/pkg/errors"
"go.opencensus.io/trace"
"go.viam.com/rdk/components/movementsensor"
"go.viam.com/rdk/components/movementsensor/replay"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/spatialmath"
rdkutils "go.viam.com/rdk/utils"
"go.viam.com/rdk/utils/contextutils"
goutils "go.viam.com/utils"
)

const (
Expand Down Expand Up @@ -325,43 +322,3 @@ func NewMovementSensor(
sensor: movementSensor,
}, nil
}

// ValidateGetMovementSensorData checks every sensorValidationIntervalSec if the provided movement sensor
// returned valid timed readings every sensorValidationIntervalSec
// until either success or sensorValidationMaxTimeoutSec has elapsed.
// Returns an error if at least one invalid reading was returned.
func ValidateGetMovementSensorData(
ctx context.Context,
ms TimedMovementSensor,
sensorValidationMaxTimeout time.Duration,
sensorValidationInterval time.Duration,
logger golog.Logger,
) error {
ctx, span := trace.StartSpan(ctx, "viamcartographer::sensor::ValidateGetMovementSensorData")
defer span.End()

startTime := time.Now().UTC()

for {
_, err := ms.TimedMovementSensorReading(ctx)
if err == nil {
break
}

logger.Debugw("ValidateGetMovementSensorData hit error: ", "error", err)
// if the sensor is a replay movement sensor with no data ready, allow validation to pass
// offline mode will stop the mapping session if the sensor still has no data,
// while online mode will continue mapping once data is found by the replay sensor
if strings.Contains(err.Error(), replay.ErrEndOfDataset.Error()) {
break
}
if time.Since(startTime) >= sensorValidationMaxTimeout {
return errors.Wrap(err, "ValidateGetMovementSensorData timeout")
}
if !goutils.SelectContextOrWait(ctx, sensorValidationInterval) {
return ctx.Err()
}
}

return nil
}
12 changes: 0 additions & 12 deletions testhelper/testhelper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ import (
const (
// SlamTimeFormat is the timestamp format used in the dataprocess.
SlamTimeFormat = "2006-01-02T15:04:05.0000Z"
// SensorValidationMaxTimeoutSecForTest is used in the ValidateGetAndSaveData
// function to ensure that the sensor in the GetAndSaveData function
// returns data within an acceptable time.
SensorValidationMaxTimeoutSecForTest = 1
// SensorValidationIntervalSecForTest is used in the ValidateGetAndSaveData
// function for the while loop that attempts to grab data from the
// sensor that is used in the GetAndSaveData function.
SensorValidationIntervalSecForTest = 1
// CartoFacadeTimeoutForTest is the timeout used for capi requests for tests.
CartoFacadeTimeoutForTest = 5 * time.Second
// CartoFacadeInternalTimeoutForTest is the timeout used for internal capi requests for tests.
Expand Down Expand Up @@ -163,8 +155,6 @@ func CreateIntegrationSLAMService(
deps,
cfgService,
logger,
SensorValidationMaxTimeoutSecForTest,
SensorValidationIntervalSecForTest,
CartoFacadeTimeoutForTest,
CartoFacadeInternalTimeoutForTest,
timedLidar,
Expand Down Expand Up @@ -212,8 +202,6 @@ func CreateSLAMService(
deps,
cfgService,
logger,
SensorValidationMaxTimeoutSecForTest,
SensorValidationIntervalSecForTest,
CartoFacadeTimeoutForTest,
CartoFacadeInternalTimeoutForTest,
nil,
Expand Down
26 changes: 0 additions & 26 deletions viam_cartographer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ const (
defaultLidarDataFrequencyHz = 5
defaultMovementSensorDataFrequencyHz = 20
defaultDialMaxTimeoutSec = 30
defaultSensorValidationMaxTimeoutSec = 30
defaultSensorValidationIntervalSec = 1
defaultCartoFacadeTimeout = 5 * time.Minute
defaultCartoFacadeInternalTimeout = 15 * time.Minute
chunkSizeBytes = 1 * 1024 * 1024
Expand Down Expand Up @@ -84,8 +82,6 @@ func init() {
deps,
c,
logger,
defaultSensorValidationMaxTimeoutSec,
defaultSensorValidationIntervalSec,
defaultCartoFacadeTimeout,
defaultCartoFacadeInternalTimeout,
nil,
Expand Down Expand Up @@ -154,8 +150,6 @@ func New(
deps resource.Dependencies,
c resource.Config,
logger golog.Logger,
sensorValidationMaxTimeoutSec int,
sensorValidationIntervalSec int,
cartoFacadeTimeout time.Duration,
cartoFacadeInternalTimeout time.Duration,
testTimedLidarOverride s.TimedLidar,
Expand Down Expand Up @@ -264,26 +258,6 @@ func New(
}
}
}()
if err = s.ValidateGetLidarData(
cancelSensorProcessCtx,
timedLidar,
time.Duration(sensorValidationMaxTimeoutSec)*time.Second,
time.Duration(sensorValidationIntervalSec)*time.Second,
cartoSvc.logger); err != nil {
err = errors.Wrap(err, "failed to get data from lidar")
return nil, err
}
if cartoSvc.movementSensor != nil {
if err = s.ValidateGetMovementSensorData(
cancelSensorProcessCtx,
timedMovementSensor,
time.Duration(sensorValidationMaxTimeoutSec)*time.Second,
time.Duration(sensorValidationIntervalSec)*time.Second,
cartoSvc.logger); err != nil {
err = errors.Wrap(err, "failed to get data from movement sensor")
return nil, err
}
}

if err = initCartoFacade(cancelCartoFacadeCtx, cartoSvc); err != nil {
return nil, err
Expand Down
16 changes: 0 additions & 16 deletions viam_cartographer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,6 @@ func TestNew(t *testing.T) {
test.That(t, svc.Close(context.Background()), test.ShouldBeNil)
})

t.Run("Failed creation of cartographer slam service with invalid lidar sensor "+
"that errors during call to NextPointCloud", func(t *testing.T) {
termFunc := testhelper.InitTestCL(t, logger)
defer termFunc()

attrCfg := &vcConfig.Config{
Camera: map[string]string{"name": string(s.LidarWithErroringFunctions), "data_frequency_hz": testLidarDataFreqHz},
ConfigParams: map[string]string{"mode": "2d"},
EnableMapping: &_true,
}

_, err := testhelper.CreateSLAMService(t, attrCfg, logger)
test.That(t, err, test.ShouldBeError,
errors.New("failed to get data from lidar: ValidateGetLidarData timeout: NextPointCloud error: "+s.InvalidSensorTestErrMsg))
})

t.Run("Successful creation of cartographer slam service in localization mode", func(t *testing.T) {
termFunc := testhelper.InitTestCL(t, logger)
defer termFunc()
Expand Down

0 comments on commit 90ce747

Please sign in to comment.