diff --git a/config/config.go b/config/config.go index d000dfe1..28efc2cb 100644 --- a/config/config.go +++ b/config/config.go @@ -17,19 +17,21 @@ func newError(configError string) error { // Config describes how to configure the SLAM service. type Config struct { - Camera map[string]string `json:"camera"` - MovementSensor map[string]string `json:"movement_sensor"` - ConfigParams map[string]string `json:"config_params"` - DataDirectory string `json:"data_dir"` - MapRateSec *int `json:"map_rate_sec"` - IMUIntegrationEnabled bool `json:"imu_integration_enabled"` - Sensors []string `json:"sensors"` - DataRateMsec *int `json:"data_rate_msec"` + Camera map[string]string `json:"camera"` + MovementSensor map[string]string `json:"movement_sensor"` + ConfigParams map[string]string `json:"config_params"` + DataDirectory string `json:"data_dir"` + MapRateSec *int `json:"map_rate_sec"` + Sensors []string `json:"sensors"` + DataRateMsec *int `json:"data_rate_msec"` CloudStoryEnabled bool `json:"cloud_story_enabled"` ExistingMap string `json:"existing_map"` EnableMapping *bool `json:"enable_mapping"` UseCloudSlam *bool `json:"use_cloud_slam"` + // NewConfigFlag is used to access the config changes from imu and cloud story work while providing backwards compatibility + // To be removed during JIRA Ticket: RSDK-4742 https://viam.atlassian.net/browse/RSDK-4742 + NewConfigFlag bool `json:"new_config_flag"` } // OptionalConfigParams holds the optional config parameters of SLAM. @@ -54,7 +56,7 @@ func (config *Config) Validate(path string) ([]string, error) { cameraName := "" imuName := "" imuExists := false - if config.IMUIntegrationEnabled { + if config.NewConfigFlag { var ok bool cameraName, ok = config.Camera["name"] if !ok { @@ -111,7 +113,7 @@ func GetOptionalParameters(config *Config, defaultLidarDataRateMsec, defaultIMUD ) (OptionalConfigParams, error) { var optionalConfigParams OptionalConfigParams // feature flag for new config - if config.IMUIntegrationEnabled { + if config.NewConfigFlag { strCameraDataFreqHz, exists := config.Camera["data_frequency_hz"] if !exists { optionalConfigParams.LidarDataRateMsec = defaultLidarDataRateMsec @@ -174,7 +176,7 @@ func GetOptionalParameters(config *Config, defaultLidarDataRateMsec, defaultIMUD optionalConfigParams.EnableMapping = *config.EnableMapping } - if err := validateModes(optionalConfigParams, config.IMUIntegrationEnabled); err != nil { + if err := validateModes(optionalConfigParams, config.NewConfigFlag); err != nil { return OptionalConfigParams{}, err } diff --git a/config/config_test.go b/config/config_test.go index 35d783e8..6d4ec1d5 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -14,7 +14,7 @@ import ( func testValidateTesthelper( t *testing.T, - imuIntegrationEnabled bool, + newConfigFlag bool, cloudStoryEnabled bool, suffix string, ) { @@ -24,12 +24,12 @@ func testValidateTesthelper( t.Run(fmt.Sprintf("Empty config %s", suffix), func(t *testing.T) { model := resource.DefaultModelFamily.WithModel("test") cfgService := resource.Config{Name: "test", API: slam.API, Model: model} - if imuIntegrationEnabled || cloudStoryEnabled { + if newConfigFlag || cloudStoryEnabled { cfgService.Attributes = make(map[string]interface{}) } - if imuIntegrationEnabled { - cfgService.Attributes["imu_integration_enabled"] = true + if newConfigFlag { + cfgService.Attributes["new_config_flag"] = true } if cloudStoryEnabled { @@ -37,7 +37,7 @@ func testValidateTesthelper( } _, err := newConfig(cfgService) - if imuIntegrationEnabled { + if newConfigFlag { test.That(t, err, test.ShouldBeError, newError("error validating \"services.slam.attributes.fake\": \"camera[name]\" is required")) } else { test.That(t, err, test.ShouldBeError, newError("error validating \"services.slam.attributes.fake\": \"sensors\" must not be empty")) @@ -45,14 +45,13 @@ func testValidateTesthelper( }) t.Run(fmt.Sprintf("Simplest valid config %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) _, err := newConfig(cfgService) test.That(t, err, test.ShouldBeNil) }) t.Run(fmt.Sprintf("Config without required fields %s", suffix), func(t *testing.T) { - requiredFields := getRequiredFields(imuIntegrationEnabled, cloudStoryEnabled) - + requiredFields := getRequiredFields(newConfigFlag, cloudStoryEnabled) dataDirErr := utils.NewConfigValidationFieldRequiredError(testCfgPath, "data_dir") sensorErr := utils.NewConfigValidationError(testCfgPath, errSensorsMustNotBeEmpty) cameraErr := utils.NewConfigValidationError(testCfgPath, errCameraMustHaveName) @@ -64,7 +63,7 @@ func testValidateTesthelper( } for _, requiredField := range requiredFields { logger.Debugf("Testing SLAM config without %s\n", requiredField) - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) delete(cfgService.Attributes, requiredField) _, err := newConfig(cfgService) @@ -72,7 +71,7 @@ func testValidateTesthelper( } // Test for missing config_params attributes logger.Debug("Testing SLAM config without config_params[mode]") - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) delete(cfgService.Attributes["config_params"].(map[string]string), "mode") _, err := newConfig(cfgService) test.That(t, err, test.ShouldBeError, newError(utils.NewConfigValidationFieldRequiredError(testCfgPath, "config_params[mode]").Error())) @@ -81,7 +80,7 @@ func testValidateTesthelper( t.Run(fmt.Sprintf("Config with invalid parameter type %s", suffix), func(t *testing.T) { key := "data_dir" - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes[key] = true _, err := newConfig(cfgService) @@ -96,8 +95,8 @@ func testValidateTesthelper( t.Run(fmt.Sprintf("Config with out of range values %s", suffix), func(t *testing.T) { var mapRateSecError error - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) - if imuIntegrationEnabled { + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) + if newConfigFlag { cfgService.Attributes["camera"] = map[string]string{ "name": "a", "data_frequency_hz": "-1", @@ -130,7 +129,7 @@ func testValidateTesthelper( }) t.Run(fmt.Sprintf("All parameters e2e %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["sensors"] = []string{"a", "b"} cfgService.Attributes["data_rate_msec"] = 1001 cfgService.Attributes["map_rate_sec"] = 1002 @@ -150,10 +149,10 @@ func testValidateTesthelper( }) } -func getRequiredFields(imuIntegrationEnabled, cloudStoryEnabled bool) []string { +func getRequiredFields(newConfigFlag, cloudStoryEnabled bool) []string { requiredFields := []string{} - if imuIntegrationEnabled { + if newConfigFlag { requiredFields = append(requiredFields, "camera") } else { requiredFields = append(requiredFields, "sensors") @@ -166,16 +165,16 @@ func getRequiredFields(imuIntegrationEnabled, cloudStoryEnabled bool) []string { } func TestValidate(t *testing.T) { - for _, imuEnabled := range []bool{true, false} { + for _, newConfigFlag := range []bool{true, false} { for _, cloudStoryEnabled := range []bool{true, false} { - suffix := fmt.Sprintf("with imuIntegrationEnabled = %t & cloudStoryEnabled = %t", imuEnabled, cloudStoryEnabled) - testValidateTesthelper(t, imuEnabled, cloudStoryEnabled, suffix) + suffix := fmt.Sprintf("with newConfigFlag = %t & cloudStoryEnabled = %t", newConfigFlag, cloudStoryEnabled) + testValidateTesthelper(t, newConfigFlag, cloudStoryEnabled, suffix) } } } // makeCfgService creates the simplest possible config that can pass validation. -func makeCfgService(IMUIntegrationEnabled, cloudStoryEnabled bool) resource.Config { +func makeCfgService(NewConfigFlag, cloudStoryEnabled bool) resource.Config { model := resource.DefaultModelFamily.WithModel("test") cfgService := resource.Config{Name: "test", API: slam.API, Model: model} cfgService.Attributes = map[string]interface{}{ @@ -183,8 +182,8 @@ func makeCfgService(IMUIntegrationEnabled, cloudStoryEnabled bool) resource.Conf "data_dir": "path", } - if IMUIntegrationEnabled { - cfgService.Attributes["imu_integration_enabled"] = true + if NewConfigFlag { + cfgService.Attributes["new_config_flag"] = true cfgService.Attributes["camera"] = map[string]string{ "name": "a", } @@ -201,14 +200,14 @@ func makeCfgService(IMUIntegrationEnabled, cloudStoryEnabled bool) resource.Conf func getOptionalParametersTestHelper( t *testing.T, - imuIntegrationEnabled bool, + newConfigFlag bool, cloudStoryEnabled bool, suffix string, ) { logger := golog.NewTestLogger(t) t.Run(fmt.Sprintf("Pass default parameters %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["sensors"] = []string{"a"} cfg, err := newConfig(cfgService) test.That(t, err, test.ShouldBeNil) @@ -230,8 +229,8 @@ func getOptionalParametersTestHelper( }) t.Run(fmt.Sprintf("Return overrides %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) - if imuIntegrationEnabled { + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) + if newConfigFlag { cfgService.Attributes["camera"] = map[string]string{ "name": "testNameCamera", "data_frequency_hz": "2", @@ -259,7 +258,7 @@ func getOptionalParametersTestHelper( 1002, logger) test.That(t, err, test.ShouldBeNil) - if imuIntegrationEnabled { + if newConfigFlag { test.That(t, optionalConfigParams.ImuName, test.ShouldEqual, "testNameSensor") test.That(t, optionalConfigParams.ImuDataRateMsec, test.ShouldEqual, 500) test.That(t, optionalConfigParams.LidarDataRateMsec, test.ShouldEqual, 500) @@ -277,7 +276,7 @@ func getOptionalParametersTestHelper( }) t.Run(fmt.Sprintf("Pass invalid existing map %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["existing_map"] = "test-file" cfg, err := newConfig(cfgService) test.That(t, err, test.ShouldBeNil) @@ -298,10 +297,10 @@ func getOptionalParametersTestHelper( }) t.Run(fmt.Sprintf("config that puts cartographer in offline mode and in localization mode %s", suffix), func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["existing_map"] = "test-file.pbstream" cfgService.Attributes["enable_mapping"] = false - if imuIntegrationEnabled { + if newConfigFlag { cfgService.Attributes["camera"] = map[string]string{ "name": "testcam", "data_frequency_hz": "0", @@ -318,7 +317,7 @@ func getOptionalParametersTestHelper( 1002, logger) if cloudStoryEnabled { - if imuIntegrationEnabled { + if newConfigFlag { test.That(t, err, test.ShouldBeError, errLocalizationInOfflineModeIMU) } else { test.That(t, err, test.ShouldBeError, errLocalizationInOfflineMode) @@ -331,19 +330,19 @@ func getOptionalParametersTestHelper( } }) - if imuIntegrationEnabled { - sensorAttributeTestHelper(t, logger, imuIntegrationEnabled, cloudStoryEnabled) + if newConfigFlag { + sensorAttributeTestHelper(t, logger, newConfigFlag, cloudStoryEnabled) } } func sensorAttributeTestHelper( t *testing.T, logger *zap.SugaredLogger, - imuIntegrationEnabled bool, + newConfigFlag bool, cloudStoryEnabled bool, ) { t.Run("Unit test return error if lidar data frequency is invalid", func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["camera"] = map[string]string{ "name": "a", "data_frequency_hz": "b", @@ -360,7 +359,7 @@ func sensorAttributeTestHelper( }) t.Run("Unit test return error if imu data frequency is invalid", func(t *testing.T) { - cfgService := makeCfgService(imuIntegrationEnabled, cloudStoryEnabled) + cfgService := makeCfgService(newConfigFlag, cloudStoryEnabled) cfgService.Attributes["camera"] = map[string]string{ "name": "a", "data_frequency_hz": "1", @@ -382,10 +381,10 @@ func sensorAttributeTestHelper( } func TestGetOptionalParameters(t *testing.T) { - for _, imuEnabled := range []bool{true, false} { + for _, newConfigFlag := range []bool{true, false} { for _, cloudStoryEnabled := range []bool{true, false} { - suffix := fmt.Sprintf("with imuIntegrationEnabled = %t & cloudStoryEnabled = %t", imuEnabled, cloudStoryEnabled) - getOptionalParametersTestHelper(t, imuEnabled, cloudStoryEnabled, suffix) + suffix := fmt.Sprintf("with newConfigFlag = %t & cloudStoryEnabled = %t", newConfigFlag, cloudStoryEnabled) + getOptionalParametersTestHelper(t, newConfigFlag, cloudStoryEnabled, suffix) } } } diff --git a/integration_test.go b/integration_test.go index d96d6c68..ba54767c 100644 --- a/integration_test.go +++ b/integration_test.go @@ -158,9 +158,9 @@ func testHelperCartographer( ConfigParams: map[string]string{ "mode": reflect.ValueOf(subAlgo).String(), }, - MapRateSec: &mapRateSec, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, + MapRateSec: &mapRateSec, + DataDirectory: dataDirectory, + NewConfigFlag: true, } // Add lidar component to config (required) diff --git a/testhelper/testhelper.go b/testhelper/testhelper.go index 063e28b0..eb3549c6 100644 --- a/testhelper/testhelper.go +++ b/testhelper/testhelper.go @@ -182,7 +182,7 @@ func CreateSLAMService( // feature flag for IMU Integration sets whether to use the dictionary or list format for configuring sensors cameraName := "" imuName := "" - if cfg.IMUIntegrationEnabled { + if cfg.NewConfigFlag { cameraName = cfg.Camera["name"] imuName = cfg.MovementSensor["name"] } else { diff --git a/viam_cartographer.go b/viam_cartographer.go index 76831b43..27d94aa7 100644 --- a/viam_cartographer.go +++ b/viam_cartographer.go @@ -202,7 +202,7 @@ func New( // feature flag for new config lidarName := "" - if svcConfig.IMUIntegrationEnabled { + if svcConfig.NewConfigFlag { lidarName = svcConfig.Camera["name"] } else { lidarName = svcConfig.Sensors[0] diff --git a/viam_cartographer_test.go b/viam_cartographer_test.go index f623105c..5998e4cc 100644 --- a/viam_cartographer_test.go +++ b/viam_cartographer_test.go @@ -358,10 +358,10 @@ func TestNewFeatureFlag(t *testing.T) { }() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -382,11 +382,11 @@ func TestNewFeatureFlag(t *testing.T) { }() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, - MovementSensor: map[string]string{"name": "good_imu", "data_frequency_hz": testIMUDataFreqHz}, + Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, + MovementSensor: map[string]string{"name": "good_imu", "data_frequency_hz": testIMUDataFreqHz}, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -408,10 +408,10 @@ func TestNewFeatureFlag(t *testing.T) { }() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "lidar_with_erroring_functions", "data_frequency_hz": testLidarDataFreqHz}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "lidar_with_erroring_functions", "data_frequency_hz": testLidarDataFreqHz}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, } _, err = testhelper.CreateSLAMService(t, attrCfg, logger) @@ -433,11 +433,11 @@ func TestNewFeatureFlag(t *testing.T) { }() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, - MovementSensor: map[string]string{"name": "imu_with_invalid_properties", "data_frequency_hz": testIMUDataFreqHz}, + Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, + MovementSensor: map[string]string{"name": "imu_with_invalid_properties", "data_frequency_hz": testIMUDataFreqHz}, } _, err = testhelper.CreateSLAMService(t, attrCfg, logger) @@ -454,11 +454,11 @@ func TestNewFeatureFlag(t *testing.T) { defer fsCleanupFunc() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar"}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - MapRateSec: &_zeroInt, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "good_lidar"}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + MapRateSec: &_zeroInt, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -507,10 +507,10 @@ func TestNewFeatureFlag(t *testing.T) { defer fsCleanupFunc() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar"}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "good_lidar"}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -560,10 +560,10 @@ func TestNewFeatureFlag(t *testing.T) { defer fsCleanupFunc() attrCfg := &vcConfig.Config{ - Camera: map[string]string{}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - IMUIntegrationEnabled: true, + Camera: map[string]string{}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -588,11 +588,11 @@ func TestClose(t *testing.T) { }() attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "replay_lidar"}, - ConfigParams: map[string]string{"mode": "2d"}, - DataDirectory: dataDirectory, - MapRateSec: &testMapRateSec, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "replay_lidar"}, + ConfigParams: map[string]string{"mode": "2d"}, + DataDirectory: dataDirectory, + MapRateSec: &testMapRateSec, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) @@ -641,11 +641,11 @@ func TestDoCommand(t *testing.T) { test.That(t, err, test.ShouldBeNil) attrCfg := &vcConfig.Config{ - Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, - ConfigParams: map[string]string{"mode": "2d", "test_param": "viam"}, - DataDirectory: dataDirectory, - MapRateSec: &testMapRateSec, - IMUIntegrationEnabled: true, + Camera: map[string]string{"name": "good_lidar", "data_frequency_hz": testLidarDataFreqHz}, + ConfigParams: map[string]string{"mode": "2d", "test_param": "viam"}, + DataDirectory: dataDirectory, + MapRateSec: &testMapRateSec, + NewConfigFlag: true, } svc, err := testhelper.CreateSLAMService(t, attrCfg, logger) test.That(t, err, test.ShouldBeNil)