diff --git a/etc/golangci.yaml b/etc/golangci.yaml index 7c20ea276..8702a0809 100644 --- a/etc/golangci.yaml +++ b/etc/golangci.yaml @@ -12,12 +12,12 @@ linters: - contextcheck - cyclop - deadcode + - depguard - exhaustivestruct - exhaustruct - forcetypeassert - funlen - gocognit - - gocritic - godox - goerr113 - gochecknoglobals @@ -36,6 +36,7 @@ linters: - maligned - makezero - musttag + - nakedret - nestif - nlreturn - nosnakecase @@ -60,8 +61,12 @@ linters-settings: - standard - default - prefix(github.com/viamrobotics/viam-cartographer) + gocritic: + enabled-tags: + disabled-checks: + - dupSubExpr gofumpt: - lang-version: "1.19" + lang-version: "1.21" extra-rules: true gosec: excludes: @@ -71,6 +76,39 @@ linters-settings: disable: - fieldalignment - shadow + - composites + revive: + # Unfortunately configuring a single rules disables all other rules, even + # if we set `enable-all: true` + # + # To get around this, we include default rules: + # https://github.com/mgechev/revive/blob/master/defaults.toml + rules: + - name: blank-imports + - name: context-as-argument + disabled: false + arguments: + - allowTypesBefore: "testing.TB,*testing.T,*testing.B,*testing.F" + - name: context-keys-type + - name: dot-imports + - name: empty-block + - name: error-naming + - name: error-return + - name: error-strings + - name: errorf + - name: exported + - name: increment-decrement + - name: indent-error-flow + - name: package-comments + - name: range + - name: receiver-naming + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: unreachable-code + - name: var-declaration + - name: var-naming lll: line-length: 140 issues: diff --git a/sensorprocess/testhelper.go b/sensorprocess/testhelper.go index 43ea2ab97..3eb7d0e01 100644 --- a/sensorprocess/testhelper.go +++ b/sensorprocess/testhelper.go @@ -111,14 +111,15 @@ func validAddLidarReadingInOnlineTestHelper( test.That(t, call.timeout, test.ShouldEqual, config.Timeout) } - if testLidar == s.GoodLidar { + switch { + case testLidar == s.GoodLidar: test.That(t, calls[0].currentReading.ReadingTime.Before(calls[1].currentReading.ReadingTime), test.ShouldBeTrue) test.That(t, calls[1].currentReading.ReadingTime.Before(calls[2].currentReading.ReadingTime), test.ShouldBeTrue) - } else if testLidar == s.ReplayLidar { + case testLidar == s.ReplayLidar: readingTime, err := time.Parse(time.RFC3339Nano, s.TestTimestamp) test.That(t, err, test.ShouldBeNil) test.That(t, calls[0].currentReading.ReadingTime.Equal(readingTime), test.ShouldBeTrue) - } else { + default: t.Errorf("no timestamp tests provided for %v", string(testLidar)) } } diff --git a/sensors/movementsensor.go b/sensors/movementsensor.go index 39aee5873..769d9561a 100644 --- a/sensors/movementsensor.go +++ b/sensors/movementsensor.go @@ -324,11 +324,12 @@ func NewMovementSensor( } func averageReadingTimes(a, b time.Time) time.Time { - if b.Equal(a) { + switch { + case b.Equal(a): return a - } else if b.After(a) { + case b.After(a): return a.Add(b.Sub(a) / 2) - } else { + default: return b.Add(a.Sub(b) / 2) } } diff --git a/sensors/test_deps.go b/sensors/test_deps.go index 6948da8d6..6797cee1d 100644 --- a/sensors/test_deps.go +++ b/sensors/test_deps.go @@ -138,12 +138,12 @@ var ( IMUWithErroringFunctions: getIMUWithErroringFunctions, ReplayIMU: func() *inject.MovementSensor { return getReplayIMU(TestTimestamp) }, InvalidReplayIMU: func() *inject.MovementSensor { return getReplayIMU(BadTime) }, - FinishedReplayIMU: func() *inject.MovementSensor { return getFinishedReplayIMU() }, + FinishedReplayIMU: getFinishedReplayIMU, GoodOdometer: getGoodOdometer, OdometerWithErroringFunctions: getOdometerWithErroringFunctions, ReplayOdometer: func() *inject.MovementSensor { return getReplayOdometer(TestTimestamp) }, InvalidReplayOdometer: func() *inject.MovementSensor { return getReplayOdometer(BadTime) }, - FinishedReplayOdometer: func() *inject.MovementSensor { return getFinishedReplayOdometer() }, + FinishedReplayOdometer: getFinishedReplayOdometer, MovementSensorNotIMUNotOdometer: getMovementSensorNotIMUAndNotOdometer, GoodMovementSensorBothIMUAndOdometer: getGoodMovementSensorBothIMUAndOdometer, MovementSensorBothIMUAndOdometerWithErroringFunctions: getMovementSensorBothIMUAndOdometerWithErroringFunctions, @@ -151,7 +151,7 @@ var ( MovementSensorWithInvalidProperties: getMovementSensorWithInvalidProperties, ReplayMovementSensorBothIMUAndOdometer: func() *inject.MovementSensor { return getReplayMovementSensor(TestTimestamp) }, InvalidReplayMovementSensorBothIMUAndOdometer: func() *inject.MovementSensor { return getReplayMovementSensor(BadTime) }, - FinishedReplayMovementSensor: func() *inject.MovementSensor { return getFinishedReplayMovementSensor() }, + FinishedReplayMovementSensor: getFinishedReplayMovementSensor, } ) diff --git a/viam-cartographer/src/carto_facade/carto_facade.cc b/viam-cartographer/src/carto_facade/carto_facade.cc index 8b463d63b..e78bf2910 100644 --- a/viam-cartographer/src/carto_facade/carto_facade.cc +++ b/viam-cartographer/src/carto_facade/carto_facade.cc @@ -178,7 +178,7 @@ CartoFacade::CartoFacade(viam_carto_lib *pVCL, const viam_carto_config c, path_to_internal_state_file = config.existing_map; }; -CartoFacade::~CartoFacade() { } +CartoFacade::~CartoFacade() {} void CartoFacade::IOInit() { if (state != CartoFacadeState::INITIALIZED) { @@ -644,12 +644,12 @@ void CartoFacade::AddLidarReading(const viam_carto_lidar_reading *sr) { bstring camera_sensor = to_bstring(config.camera); bool known_sensor = biseq(camera_sensor, sr->lidar); bdestroy(camera_sensor); - if (!known_sensor) { + if (!known_sensor) { VLOG(1) << "expected sensor: " << to_std_string(sr->lidar) << " to be " << config.camera; throw VIAM_CARTO_UNKNOWN_SENSOR_NAME; } - + std::string lidar_reading = to_std_string(sr->lidar_reading); if (lidar_reading.length() == 0) { throw VIAM_CARTO_LIDAR_READING_EMPTY; diff --git a/viam_cartographer.go b/viam_cartographer.go index ed9707afe..625fc0262 100644 --- a/viam_cartographer.go +++ b/viam_cartographer.go @@ -695,13 +695,14 @@ func (cartoSvc *CartographerService) Properties(ctx context.Context) (slam.Prope props.SensorInfo = append(props.SensorInfo, slam.SensorInfo{Name: cartoSvc.movementSensor.Name(), Type: slam.SensorTypeMovementSensor}) } - if cartoSvc.enableMapping && cartoSvc.existingMap == "" { + switch { + case cartoSvc.enableMapping && cartoSvc.existingMap == "": props.MappingMode = slam.MappingModeNewMap - } else if cartoSvc.enableMapping && cartoSvc.existingMap != "" { + case cartoSvc.enableMapping && cartoSvc.existingMap != "": props.MappingMode = slam.MappingModeUpdateExistingMap - } else if !cartoSvc.enableMapping && cartoSvc.existingMap != "" { + case !cartoSvc.enableMapping && cartoSvc.existingMap != "": props.MappingMode = slam.MappingModeLocalizationOnly - } else { + default: return slam.Properties{}, errors.New("invalid mode: localizing requires an existing map") }