Skip to content

Commit 270cb04

Browse files
authored
add a config parameter to motion/builtin that sets a default number of threads to use (#5015)
1 parent 63bc09b commit 270cb04

File tree

2 files changed

+86
-14
lines changed

2 files changed

+86
-14
lines changed

services/motion/builtin/builtin.go

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,20 +89,38 @@ type inputEnabledActuator interface {
8989
// Config describes how to configure the service; currently only used for specifying dependency on framesystem service.
9090
type Config struct {
9191
LogFilePath string `json:"log_file_path"`
92+
NumThreads int `json:"num_threads"`
9293
}
9394

9495
// Validate here adds a dependency on the internal framesystem service.
9596
func (c *Config) Validate(path string) ([]string, []string, error) {
97+
if c.NumThreads < 0 {
98+
return nil, nil, fmt.Errorf("cannot configure with %d number of threads, number must be positive", c.NumThreads)
99+
}
96100
return []string{framesystem.InternalServiceName.String()}, nil, nil
97101
}
98102

103+
type builtIn struct {
104+
resource.Named
105+
mu sync.RWMutex
106+
fsService framesystem.Service
107+
movementSensors map[resource.Name]movementsensor.MovementSensor
108+
slamServices map[resource.Name]slam.Service
109+
visionServices map[resource.Name]vision.Service
110+
components map[resource.Name]resource.Resource
111+
logger logging.Logger
112+
state *state.State
113+
configuredDefaultExtras map[string]any
114+
}
115+
99116
// NewBuiltIn returns a new move and grab service for the given robot.
100117
func NewBuiltIn(
101118
ctx context.Context, deps resource.Dependencies, conf resource.Config, logger logging.Logger,
102119
) (motion.Service, error) {
103120
ms := &builtIn{
104-
Named: conf.ResourceName().AsNamed(),
105-
logger: logger,
121+
Named: conf.ResourceName().AsNamed(),
122+
logger: logger,
123+
configuredDefaultExtras: make(map[string]any),
106124
}
107125

108126
if err := ms.Reconfigure(ctx, deps, conf); err != nil {
@@ -128,6 +146,10 @@ func (ms *builtIn) Reconfigure(
128146
fileAppender, _ := logging.NewFileAppender(config.LogFilePath)
129147
ms.logger.AddAppender(fileAppender)
130148
}
149+
if config.NumThreads > 0 {
150+
ms.configuredDefaultExtras["num_threads"] = config.NumThreads
151+
}
152+
131153
movementSensors := make(map[resource.Name]movementsensor.MovementSensor)
132154
slamServices := make(map[resource.Name]slam.Service)
133155
visionServices := make(map[resource.Name]vision.Service)
@@ -162,18 +184,6 @@ func (ms *builtIn) Reconfigure(
162184
return nil
163185
}
164186

165-
type builtIn struct {
166-
resource.Named
167-
mu sync.RWMutex
168-
fsService framesystem.Service
169-
movementSensors map[resource.Name]movementsensor.MovementSensor
170-
slamServices map[resource.Name]slam.Service
171-
visionServices map[resource.Name]vision.Service
172-
components map[resource.Name]resource.Resource
173-
logger logging.Logger
174-
state *state.State
175-
}
176-
177187
func (ms *builtIn) Close(ctx context.Context) error {
178188
ms.mu.Lock()
179189
defer ms.mu.Unlock()
@@ -188,6 +198,7 @@ func (ms *builtIn) Move(ctx context.Context, req motion.MoveReq) (bool, error) {
188198
defer ms.mu.RUnlock()
189199
operation.CancelOtherWithLabel(ctx, builtinOpLabel)
190200

201+
ms.applyDefaultExtras(req.Extra)
191202
plan, err := ms.plan(ctx, req, ms.logger)
192203
if err != nil {
193204
return false, err
@@ -207,6 +218,7 @@ func (ms *builtIn) MoveOnMap(ctx context.Context, req motion.MoveOnMapReq) (moti
207218
// TODO: Deprecated: remove once no motion apis use the opid system
208219
operation.CancelOtherWithLabel(ctx, builtinOpLabel)
209220

221+
ms.applyDefaultExtras(req.Extra)
210222
id, err := state.StartExecution(ctx, ms.state, req.ComponentName, req, ms.newMoveOnMapRequest)
211223
if err != nil {
212224
return uuid.Nil, err
@@ -275,6 +287,7 @@ func (ms *builtIn) MoveOnGlobe(ctx context.Context, req motion.MoveOnGlobeReq) (
275287
// TODO: Deprecated: remove once no motion apis use the opid system
276288
operation.CancelOtherWithLabel(ctx, builtinOpLabel)
277289

290+
ms.applyDefaultExtras(req.Extra)
278291
id, err := state.StartExecution(ctx, ms.state, req.ComponentName, req, ms.newMoveOnGlobeRequest)
279292
if err != nil {
280293
return uuid.Nil, err
@@ -578,6 +591,19 @@ func (ms *builtIn) execute(ctx context.Context, trajectory motionplan.Trajectory
578591
return nil
579592
}
580593

594+
// applyDefaultExtras iterates through the list of default extras configured on the builtIn motion service and adds them to the
595+
// given map of extras if the key does not already exist.
596+
func (ms *builtIn) applyDefaultExtras(extras map[string]any) {
597+
if extras == nil {
598+
extras = make(map[string]any)
599+
}
600+
for key, val := range ms.configuredDefaultExtras {
601+
if _, ok := extras[key]; !ok {
602+
extras[key] = val
603+
}
604+
}
605+
}
606+
581607
func waypointsFromRequest(
582608
req motion.MoveReq,
583609
fsInputs referenceframe.FrameSystemInputs,

services/motion/builtin/builtin_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,3 +1566,49 @@ func TestMultiWaypointPlanning(t *testing.T) {
15661566
test.That(t, finalArmConfig, test.ShouldResemble, referenceframe.FloatsToInputs(goalConfig))
15671567
})
15681568
}
1569+
1570+
func TestConfiguredDefaultExtras(t *testing.T) {
1571+
ctx := context.Background()
1572+
logger := logging.NewTestLogger(t)
1573+
1574+
t.Run("number of threads not configured", func(t *testing.T) {
1575+
// test configuring the number of threads to be zero
1576+
ms, err := NewBuiltIn(ctx, nil, resource.Config{ConvertedAttributes: &Config{}}, logger)
1577+
test.That(t, err, test.ShouldBeNil)
1578+
defer test.That(t, ms.Close(ctx), test.ShouldBeNil)
1579+
1580+
// test that we can override the number of threads with user input
1581+
extras := map[string]any{"num_threads": 1}
1582+
ms.(*builtIn).applyDefaultExtras(extras)
1583+
test.That(t, extras["num_threads"], test.ShouldEqual, 1)
1584+
1585+
// test that if nothing is provided nothing is set
1586+
extras = map[string]any{}
1587+
ms.(*builtIn).applyDefaultExtras(extras)
1588+
test.That(t, extras["num_threads"], test.ShouldBeNil)
1589+
})
1590+
1591+
t.Run("configure number of threads", func(t *testing.T) {
1592+
// test configuring the number of threads to be a nonzero number
1593+
ms, err := NewBuiltIn(ctx, nil, resource.Config{ConvertedAttributes: &Config{NumThreads: 10}}, logger)
1594+
test.That(t, err, test.ShouldBeNil)
1595+
defer test.That(t, ms.Close(ctx), test.ShouldBeNil)
1596+
1597+
// test that we can override the number of threads with user input
1598+
extras := map[string]any{"num_threads": 1}
1599+
ms.(*builtIn).applyDefaultExtras(extras)
1600+
test.That(t, extras["num_threads"], test.ShouldEqual, 1)
1601+
1602+
// test that if nothing is provided we use the default
1603+
extras = map[string]any{}
1604+
ms.(*builtIn).applyDefaultExtras(extras)
1605+
test.That(t, extras["num_threads"], test.ShouldEqual, 10)
1606+
})
1607+
1608+
t.Run("number of threads configured poorly", func(t *testing.T) {
1609+
// test configuring the number of threads to be negative
1610+
cfg := &Config{NumThreads: -1}
1611+
_, _, err := cfg.Validate("")
1612+
test.That(t, err, test.ShouldNotBeNil)
1613+
})
1614+
}

0 commit comments

Comments
 (0)