Skip to content

Accept and propagate context #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 66 additions & 20 deletions arduino/cli/commander.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@ type commander struct {
func NewCommander() (arduino.Commander, error) {
// Discard arduino-cli log info messages
logrus.SetLevel(logrus.ErrorLevel)
// Initialize arduino-cli configuration

// Initialize arduino-cli configuration.
configuration.Settings = configuration.Init(configuration.FindConfigFileInArgsOrWorkingDirectory(os.Args))
// Create arduino-cli instance, needed to execute arduino-cli commands

// Create and init an arduino-cli instance, needed to execute arduino-cli commands.
inst, err := instance.Create()
if err != nil {
err = fmt.Errorf("creating arduino-cli instance: %w", err)
Expand All @@ -61,34 +63,64 @@ func NewCommander() (arduino.Commander, error) {
return cmd, nil
}

func mergeErrors(err error, errs []error) error {
merr := errors.New("merged errors: ")
empty := true

if err != nil {
merr = fmt.Errorf("%w%v; ", merr, err)
empty = false
}

if len(errs) > 0 {
empty = false
for _, e := range errs {
merr = fmt.Errorf("%w%v; ", merr, e)
}
}

if !empty {
return merr
}
return nil
}

// BoardList executes the 'arduino-cli board list' command
// and returns its result.
func (c *commander) BoardList() ([]*rpc.DetectedPort, error) {
func (c *commander) BoardList(ctx context.Context) ([]*rpc.DetectedPort, error) {
req := &rpc.BoardListRequest{
Instance: c.Instance,
Timeout: time.Second.Milliseconds(),
}

ports, errs, err := board.List(req)
if err != nil {
err = fmt.Errorf("%s: %w", "detecting boards", err)
return nil, err
// There is no obvious way to cancel the execution of this command.
// So, we execute it in a goroutine and leave it running alone if ctx gets cancelled.
Comment on lines +96 to +97
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmaglie any opinion on this?
Is there any alternative?

type resp struct {
err error
ports []*rpc.DetectedPort
}
quit := make(chan resp, 1)
go func() {
ports, errs, err := board.List(req)
quit <- resp{err: mergeErrors(err, errs), ports: ports}
close(quit)
}()

if len(errs) > 0 {
err = errors.New("starting discovery procedure: received errors: ")
for _, e := range errs {
err = fmt.Errorf("%w%v; ", err, e)
// Wait for the command to complete or the context to be terminated.
select {
case <-ctx.Done():
return nil, errors.New("board list command cancelled")
case r := <-quit:
if r.err != nil {
return nil, fmt.Errorf("executing board list command: %w", r.err)
}
return nil, err
return r.ports, nil
}

return ports, nil
}

// UploadBin executes the 'arduino-cli upload -i' command
// and returns its result.
func (c *commander) UploadBin(fqbn, bin, address, protocol string) error {
func (c *commander) UploadBin(ctx context.Context, fqbn, bin, address, protocol string) error {
req := &rpc.UploadRequest{
Instance: c.Instance,
Fqbn: fqbn,
Expand All @@ -97,11 +129,25 @@ func (c *commander) UploadBin(fqbn, bin, address, protocol string) error {
Port: &rpc.Port{Address: address, Protocol: protocol},
Verbose: false,
}

l := logrus.StandardLogger().WithField("source", "arduino-cli").Writer()
if _, err := upload.Upload(context.Background(), req, l, l); err != nil {
err = fmt.Errorf("%s: %w", "uploading binary", err)
return err

// There is no obvious way to cancel the execution of this command.
// So, we execute it in a goroutine and leave it running if ctx gets cancelled.
quit := make(chan error, 1)
go func() {
_, err := upload.Upload(ctx, req, l, l)
quit <- err
close(quit)
}()

// Wait for the upload to complete or the context to be terminated.
select {
case <-ctx.Done():
return errors.New("upload cancelled")
case err := <-quit:
if err != nil {
return fmt.Errorf("uploading binary: %w", err)
}
return nil
}
return nil
}
6 changes: 4 additions & 2 deletions arduino/commander.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@
package arduino

import (
"context"

rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
)

// Commander of arduino package allows to call
// the arduino-cli commands in a programmatic way.
type Commander interface {
BoardList() ([]*rpc.DetectedPort, error)
UploadBin(fqbn, bin, address, protocol string) error
BoardList(ctx context.Context) ([]*rpc.DetectedPort, error)
UploadBin(ctx context.Context, fqbn, bin, address, protocol string) error
//Compile() error
}
2 changes: 1 addition & 1 deletion arduino/grpc/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type boardHandler struct {

// BoardList executes the 'arduino-cli board list' command
// and returns its result.
func (b boardHandler) BoardList() ([]*rpc.DetectedPort, error) {
func (b boardHandler) BoardList(ctx context.Context) ([]*rpc.DetectedPort, error) {
boardListResp, err := b.serviceClient.BoardList(context.Background(),
&rpc.BoardListRequest{Instance: b.instance})

Expand Down
2 changes: 1 addition & 1 deletion arduino/grpc/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (c compileHandler) Compile() error {

// Upload executes the 'arduino-cli upload -i' command
// and returns its result.
func (c compileHandler) UploadBin(fqbn, bin, address, protocol string) error {
func (c compileHandler) UploadBin(ctx context.Context, fqbn, bin, address, protocol string) error {
stream, err := c.serviceClient.Upload(context.Background(),
&rpc.UploadRequest{
Instance: c.instance,
Expand Down
3 changes: 2 additions & 1 deletion cli/dashboard/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package dashboard

import (
"context"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -76,7 +77,7 @@ func runCreateCommand(flags *createFlags) error {
params.Name = &flags.name
}

dashboard, err := dashboard.Create(params, cred)
dashboard, err := dashboard.Create(context.TODO(), params, cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/dashboard/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package dashboard

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -60,7 +61,7 @@ func runDeleteCommand(flags *deleteFlags) error {
}

params := &dashboard.DeleteParams{ID: flags.id}
err = dashboard.Delete(params, cred)
err = dashboard.Delete(context.TODO(), params, cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/dashboard/extract.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package dashboard

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -64,7 +65,7 @@ func runExtractCommand(flags *extractFlags) error {
ID: flags.id,
}

template, err := dashboard.Extract(params, cred)
template, err := dashboard.Extract(context.TODO(), params, cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/dashboard/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package dashboard

import (
"context"
"fmt"
"math"
"os"
Expand Down Expand Up @@ -66,7 +67,7 @@ func runListCommand(flags *listFlags) error {
return fmt.Errorf("retrieving credentials: %w", err)
}

dash, err := dashboard.List(cred)
dash, err := dashboard.List(context.TODO(), cred)
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cli/device/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"

Expand All @@ -27,6 +28,7 @@ import (
"github.com/arduino/arduino-cloud-cli/config"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"go.bug.st/cleanup"
)

type createFlags struct {
Expand Down Expand Up @@ -73,7 +75,10 @@ func runCreateCommand(flags *createFlags) error {
params.FQBN = &flags.fqbn
}

dev, err := device.Create(params, cred)
ctx, cancel := cleanup.InterruptableContext(context.Background())
defer cancel()

dev, err := device.Create(ctx, params, cred)
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cli/device/creategeneric.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"

Expand All @@ -27,6 +28,7 @@ import (
"github.com/arduino/arduino-cloud-cli/config"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"go.bug.st/cleanup"
)

type createGenericFlags struct {
Expand Down Expand Up @@ -66,7 +68,10 @@ func runCreateGenericCommand(flags *createGenericFlags) error {
FQBN: flags.fqbn,
}

dev, err := device.CreateGeneric(params, cred)
ctx, cancel := cleanup.InterruptableContext(context.Background())
defer cancel()

dev, err := device.CreateGeneric(ctx, params, cred)
if err != nil {
return err
}
Expand Down
7 changes: 6 additions & 1 deletion cli/device/createlora.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"

Expand All @@ -27,6 +28,7 @@ import (
"github.com/arduino/arduino-cloud-cli/config"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"go.bug.st/cleanup"
)

type createLoraFlags struct {
Expand Down Expand Up @@ -80,7 +82,10 @@ func runCreateLoraCommand(flags *createLoraFlags) error {
params.FQBN = &flags.fqbn
}

dev, err := device.CreateLora(params, cred)
ctx, cancel := cleanup.InterruptableContext(context.Background())
defer cancel()

dev, err := device.CreateLora(ctx, params, cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/device/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -72,7 +73,7 @@ func runDeleteCommand(flags *deleteFlags) error {
params.ID = &flags.id
}

err = device.Delete(params, cred)
err = device.Delete(context.TODO(), params, cred)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From #112 (comment) @cmaglie

Why TODO? shouldn't this be implemented as the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to intercept ctrl-c only for commands that may need a cleanup.
All the others can die instantly.

if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/device/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"
"strings"
Expand Down Expand Up @@ -67,7 +68,7 @@ func runListCommand(flags *listFlags) error {
}

params := &device.ListParams{Tags: flags.tags}
devs, err := device.List(params, cred)
devs, err := device.List(context.TODO(), params, cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/device/listfqbn.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"os"

"github.com/arduino/arduino-cli/cli/errorcodes"
Expand Down Expand Up @@ -46,7 +47,7 @@ func initListFQBNCommand() *cobra.Command {
func runListFQBNCommand() error {
logrus.Info("Listing supported FQBN")

fqbn, err := device.ListFQBN()
fqbn, err := device.ListFQBN(context.TODO())
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/device/listfrequency.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package device

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -53,7 +54,7 @@ func runListFrequencyPlansCommand() error {
return fmt.Errorf("retrieving credentials: %w", err)
}

freqs, err := device.ListFrequencyPlans(cred)
freqs, err := device.ListFrequencyPlans(context.TODO(), cred)
if err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion cli/device/tag/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package tag

import (
"context"
"fmt"
"os"

Expand Down Expand Up @@ -73,7 +74,7 @@ func runCreateTagsCommand(flags *createTagsFlags) error {
return fmt.Errorf("retrieving credentials: %w", err)
}

if err = tag.CreateTags(params, cred); err != nil {
if err = tag.CreateTags(context.TODO(), params, cred); err != nil {
return err
}

Expand Down
Loading