Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

Commit

Permalink
[RSDK-2753] bump rdk post reconfigure refactor (#190)
Browse files Browse the repository at this point in the history 
Co-authored-by: Katharina Xenia Kufieta <[email protected]>
  • Loading branch information
@nicksanford @kkufieta
nicksanford and kkufieta authored Apr 27, 2023
1 parent 6b4eaba commit 4a6f777
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 128 deletions.
8 changes: 4 additions & 4 deletions config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ func DetermineUseLiveData(logger golog.Logger, liveData *bool, sensors []string)
return useLiveData, nil
}

// AttrConfig describes how to configure the SLAM service.
type AttrConfig struct {
// Config describes how to configure the SLAM service.
type Config struct {
Sensors []string `json:"sensors"`
ConfigParams map[string]string `json:"config_params"`
DataDirectory string `json:"data_dir"`
Expand All @@ -55,7 +55,7 @@ type AttrConfig struct {
}

// Validate creates the list of implicit dependencies.
func (config *AttrConfig) Validate(path string) ([]string, error) {
func (config *Config) Validate(path string) ([]string, error) {
if config.ConfigParams["mode"] == "" {
return nil, utils.NewConfigValidationFieldRequiredError(path, "config_params[mode]")
}
Expand Down Expand Up @@ -83,7 +83,7 @@ func (config *AttrConfig) Validate(path string) ([]string, error) {

// GetOptionalParameters sets any unset optional config parameters to the values passed to this function,
// and returns them.
func GetOptionalParameters(config *AttrConfig, defaultPort string,
func GetOptionalParameters(config *Config, defaultPort string,
defaultDataRateMsec, defaultMapRateSec int, logger golog.Logger,
) (string, int, int, bool, bool, error) {
port := config.Port
Expand Down
55 changes: 28 additions & 27 deletions config/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import (
"testing"

"github.com/edaniels/golog"
"go.viam.com/rdk/config"
"go.viam.com/rdk/resource"
"go.viam.com/rdk/services/slam"
"go.viam.com/test"
"go.viam.com/utils"
)
Expand All @@ -20,55 +20,57 @@ func TestValidate(t *testing.T) {
logger := golog.NewTestLogger(t)

t.Run("Empty config", func(t *testing.T) {
model := resource.NewDefaultModel(resource.ModelName("test"))
cfgService := config.Service{Name: "test", Type: "slam", Model: model}
_, err := newAttrConfig(cfgService)
model := resource.DefaultModelFamily.WithModel("test")
cfgService := resource.Config{Name: "test", API: slam.API, Model: model}
_, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeError, newError("error validating \"services.slam.attributes.fake\": \"config_params[mode]\" is required"))
})

t.Run("Simplest valid config", func(t *testing.T) {
cfgService := makeCfgService()
_, err := newAttrConfig(cfgService)
_, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeNil)
})

t.Run("Config without required fields", func(t *testing.T) {
// Test for missing main attribute fields
requiredFields := []string{"data_dir", "use_live_data"}
for _, requiredField := range requiredFields {
logger.Debugf("Testing SLAM config without %s", requiredField)
logger.Debugf("Testing SLAM config without %s\n", requiredField)
cfgService := makeCfgService()
delete(cfgService.Attributes, requiredField)
_, err := newAttrConfig(cfgService)
test.That(t, err, test.ShouldBeError, newError(utils.NewConfigValidationFieldRequiredError(testCfgPath, requiredField).Error()))
_, err := newConfig(cfgService)
expected := newError(utils.NewConfigValidationFieldRequiredError(testCfgPath, requiredField).Error())

test.That(t, err, test.ShouldBeError, expected)
}
// Test for missing config_params attributes
logger.Debug("Testing SLAM config without config_params[mode]")
cfgService := makeCfgService()
delete(cfgService.Attributes["config_params"].(map[string]string), "mode")
_, err := newAttrConfig(cfgService)
_, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeError, newError(utils.NewConfigValidationFieldRequiredError(testCfgPath, "config_params[mode]").Error()))
})

t.Run("Config with invalid parameter type", func(t *testing.T) {
cfgService := makeCfgService()
cfgService.Attributes["use_live_data"] = "true"
_, err := newAttrConfig(cfgService)
_, err := newConfig(cfgService)
expE := newError("1 error(s) decoding:\n\n* 'use_live_data' expected type 'bool', got unconvertible type 'string', value: 'true'")
test.That(t, err, test.ShouldBeError, expE)
cfgService.Attributes["use_live_data"] = true
_, err = newAttrConfig(cfgService)
_, err = newConfig(cfgService)
test.That(t, err, test.ShouldBeNil)
})

t.Run("Config with out of range values", func(t *testing.T) {
cfgService := makeCfgService()
cfgService.Attributes["data_rate_msec"] = -1
_, err := newAttrConfig(cfgService)
_, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeError, newError("cannot specify data_rate_msec less than zero"))
cfgService.Attributes["data_rate_msec"] = 1
cfgService.Attributes["map_rate_sec"] = -1
_, err = newAttrConfig(cfgService)
_, err = newConfig(cfgService)
test.That(t, err, test.ShouldBeError, newError("cannot specify map_rate_sec less than zero"))
})

Expand All @@ -85,7 +87,7 @@ func TestValidate(t *testing.T) {
"value": "0",
"value_2": "test",
}
cfg, err := newAttrConfig(cfgService)
cfg, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeNil)
test.That(t, cfg.DataDirectory, test.ShouldEqual, cfgService.Attributes["data_dir"])
test.That(t, *cfg.UseLiveData, test.ShouldEqual, cfgService.Attributes["use_live_data"])
Expand Down Expand Up @@ -157,9 +159,9 @@ func TestDetermineUseLiveData(t *testing.T) {
}

// makeCfgService creates the simplest possible config that can pass validation.
func makeCfgService() config.Service {
model := resource.NewDefaultModel(resource.ModelName("test"))
cfgService := config.Service{Name: "test", Type: "slam", Model: model}
func makeCfgService() resource.Config {
model := resource.DefaultModelFamily.WithModel("test")
cfgService := resource.Config{Name: "test", API: slam.API, Model: model}
cfgService.Attributes = make(map[string]interface{})
cfgService.Attributes["config_params"] = map[string]string{
"mode": "test mode",
Expand All @@ -176,7 +178,7 @@ func TestGetOptionalParameters(t *testing.T) {
cfgService := makeCfgService()
cfgService.Attributes["sensors"] = []string{"a"}
cfgService.Attributes["use_live_data"] = true
cfg, err := newAttrConfig(cfgService)
cfg, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeNil)
port, dataRateMsec, mapRateSec, useLiveData, deleteProcessedData, err := GetOptionalParameters(cfg, "localhost", 1001, 1002, logger)
test.That(t, err, test.ShouldBeNil)
Expand All @@ -190,23 +192,22 @@ func TestGetOptionalParameters(t *testing.T) {
t.Run("Live data without sensors", func(t *testing.T) {
cfgService := makeCfgService()
cfgService.Attributes["use_live_data"] = true
cfg, err := newAttrConfig(cfgService)
cfg, err := newConfig(cfgService)
test.That(t, err, test.ShouldBeNil)
_, _, _, _, _, err = GetOptionalParameters(cfg, "localhost", 1001, 1002, logger)
test.That(t, err, test.ShouldBeError, newError("sensors field cannot be empty when use_live_data is set to true"))
})
}

func newAttrConfig(cfg config.Service) (*AttrConfig, error) {
attrCfg := &AttrConfig{}

if _, err := config.TransformAttributeMapToStruct(attrCfg, cfg.Attributes); err != nil {
return &AttrConfig{}, newError(err.Error())
func newConfig(conf resource.Config) (*Config, error) {
slamConf, err := resource.TransformAttributeMap[*Config](conf.Attributes)
if err != nil {
return &Config{}, newError(err.Error())
}

if _, err := attrCfg.Validate("services.slam.attributes.fake"); err != nil {
return &AttrConfig{}, newError(err.Error())
if _, err := slamConf.Validate("services.slam.attributes.fake"); err != nil {
return &Config{}, newError(err.Error())
}

return attrCfg, nil
return slamConf, nil
}
36 changes: 18 additions & 18 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ go 1.19
require (
github.com/edaniels/golinters v0.0.5-0.20220906153528-641155550742
github.com/edaniels/golog v0.0.0-20230215213219-28954395e8d0
github.com/edaniels/gostream v0.0.0-20230321201259-9c06d1aeaae5
github.com/edaniels/gostream v0.0.0-20230424213557-e12afcfabcd8
github.com/golang/geo v0.0.0-20210211234256-740aa86cb551
github.com/golangci/golangci-lint v1.51.2
github.com/pkg/errors v0.9.1
github.com/rhysd/actionlint v1.6.23
go.opencensus.io v0.24.0
go.viam.com/api v0.1.111
go.viam.com/rdk v0.2.35
go.viam.com/api v0.1.118
go.viam.com/rdk v0.2.36
go.viam.com/test v1.1.1-0.20220913152726-5da9916c08a2
go.viam.com/utils v0.1.18-0.20230327140716-bfeb34d89117
go.viam.com/utils v0.1.20-0.20230424163529-ce35a14fc60f
google.golang.org/grpc v1.54.0
)

Expand Down Expand Up @@ -42,7 +42,7 @@ require (
github.com/ashanbrown/forbidigo v1.4.0 // indirect
github.com/ashanbrown/makezero v1.1.1 // indirect
github.com/aybabtme/uniplot v0.0.0-20151203143629-039c559e5e7e // indirect
github.com/benbjohnson/clock v1.3.0 // indirect
github.com/benbjohnson/clock v1.3.3 // indirect
github.com/beorn7/perks v1.0.1 // indirect
github.com/bep/debounce v1.2.1 // indirect
github.com/bkielbasa/cyclop v1.2.0 // indirect
Expand All @@ -55,14 +55,14 @@ require (
github.com/butuzov/ireturn v0.1.1 // indirect
github.com/campoy/embedmd v1.0.0 // indirect
github.com/cenkalti/backoff v2.2.1+incompatible // indirect
github.com/cenkalti/backoff/v4 v4.2.0 // indirect
github.com/cenkalti/backoff/v4 v4.2.1 // indirect
github.com/cespare/xxhash/v2 v2.2.0 // indirect
github.com/charithe/durationcheck v0.0.9 // indirect
github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 // indirect
github.com/curioswitch/go-reassign v0.2.0 // indirect
github.com/daixiang0/gci v0.9.1 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 // indirect
github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect
github.com/denis-tingaikin/go-header v0.4.3 // indirect
github.com/desertbit/timer v0.0.0-20180107155436-c41aec40b27f // indirect
github.com/disintegration/imaging v1.6.2 // indirect
Expand Down Expand Up @@ -141,7 +141,7 @@ require (
github.com/kisielk/errcheck v1.6.3 // indirect
github.com/kisielk/gotool v1.0.0 // indirect
github.com/kkHAIKE/contextcheck v1.1.3 // indirect
github.com/klauspost/compress v1.16.3 // indirect
github.com/klauspost/compress v1.16.5 // indirect
github.com/kulti/thelper v0.6.3 // indirect
github.com/kunwardeep/paralleltest v1.0.6 // indirect
github.com/kylelemons/go-gypsy v1.0.0 // indirect
Expand Down Expand Up @@ -169,7 +169,7 @@ require (
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
github.com/mbilski/exhaustivestruct v1.2.0 // indirect
github.com/mgechev/revive v1.2.5 // indirect
github.com/miekg/dns v1.1.52 // indirect
github.com/miekg/dns v1.1.53 // indirect
github.com/mitchellh/copystructure v1.2.0 // indirect
github.com/mitchellh/go-homedir v1.1.0 // indirect
github.com/mitchellh/mapstructure v1.5.0 // indirect
Expand All @@ -192,18 +192,18 @@ require (
github.com/pion/interceptor v0.1.12 // indirect
github.com/pion/logging v0.2.2 // indirect
github.com/pion/mdns v0.0.7 // indirect
github.com/pion/mediadevices v0.4.1-0.20230412180714-14bfaa5dbdd3 // indirect
github.com/pion/mediadevices v0.4.1-0.20230424151458-cadb1557556f // indirect
github.com/pion/randutil v0.1.0 // indirect
github.com/pion/rtcp v1.2.10 // indirect
github.com/pion/rtp v1.7.13 // indirect
github.com/pion/sctp v1.8.6 // indirect
github.com/pion/sdp/v3 v3.0.6 // indirect
github.com/pion/srtp/v2 v2.0.12 // indirect
github.com/pion/stun v0.4.0 // indirect
github.com/pion/transport/v2 v2.1.0 // indirect
github.com/pion/transport/v2 v2.2.0 // indirect
github.com/pion/turn/v2 v2.1.0 // indirect
github.com/pion/udp/v2 v2.0.1 // indirect
github.com/pion/webrtc/v3 v3.1.59 // indirect
github.com/pion/webrtc/v3 v3.1.61 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/polyfloyd/go-errorlint v1.1.0 // indirect
github.com/prometheus/client_golang v1.12.2 // indirect
Expand All @@ -216,7 +216,7 @@ require (
github.com/quasilyte/stdinfo v0.0.0-20220114132959-f7386bf02567 // indirect
github.com/rivo/uniseg v0.4.4 // indirect
github.com/robfig/cron v1.2.0 // indirect
github.com/rs/cors v1.8.3 // indirect
github.com/rs/cors v1.9.0 // indirect
github.com/ryancurrah/gomodguard v1.3.0 // indirect
github.com/ryanrolds/sqlclosecheck v0.4.0 // indirect
github.com/sanposhiho/wastedassign/v2 v2.0.7 // indirect
Expand Down Expand Up @@ -269,26 +269,26 @@ require (
go.mongodb.org/mongo-driver v1.12.0-prerelease.0.20221109213319-d3466eeae7a7 // indirect
go.uber.org/atomic v1.10.0 // indirect
go.uber.org/goleak v1.2.1 // indirect
go.uber.org/multierr v1.10.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.24.0 // indirect
goji.io v2.0.2+incompatible // indirect
golang.org/x/crypto v0.8.0 // indirect
golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect
golang.org/x/exp/typeparams v0.0.0-20230203172020-98cc5a0785f9 // indirect
golang.org/x/image v0.7.0 // indirect
golang.org/x/mod v0.9.0 // indirect
golang.org/x/mod v0.10.0 // indirect
golang.org/x/net v0.9.0 // indirect
golang.org/x/oauth2 v0.6.0 // indirect
golang.org/x/oauth2 v0.7.0 // indirect
golang.org/x/sync v0.1.0 // indirect
golang.org/x/sys v0.7.0 // indirect
golang.org/x/text v0.9.0 // indirect
golang.org/x/tools v0.7.0 // indirect
golang.org/x/tools v0.8.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
gonum.org/v1/gonum v0.12.0 // indirect
gonum.org/v1/plot v0.12.0 // indirect
google.golang.org/api v0.114.0 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230322174352-cde4c949918d // indirect
google.golang.org/genproto v0.0.0-20230410155749-daa745c078e1 // indirect
google.golang.org/protobuf v1.30.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/square/go-jose.v2 v2.6.0 // indirect
Expand Down
Loading

0 comments on commit 4a6f777

Please sign in to comment.