From 7a63d5fd2c783fc47b481e98b8a4a1f8d20c8a82 Mon Sep 17 00:00:00 2001 From: Davide Date: Thu, 6 Nov 2025 11:08:38 +0100 Subject: [PATCH 1/6] fix(bricks): overwrite the brick variables of an app (#44) --------- Co-authored-by: Luca Rinaldi --- internal/orchestrator/bricks/bricks.go | 14 +-- internal/orchestrator/bricks/bricks_test.go | 112 ++++++++++++++++++ .../orchestrator/bricks/testdata/.gitignore | 1 + .../bricks/testdata/bricks-list.yaml | 25 ++++ .../bricks/testdata/dummy-app/app.yaml | 6 + .../bricks/testdata/dummy-app/python/main.py | 2 + 6 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 internal/orchestrator/bricks/bricks_test.go create mode 100644 internal/orchestrator/bricks/testdata/.gitignore create mode 100644 internal/orchestrator/bricks/testdata/bricks-list.yaml create mode 100644 internal/orchestrator/bricks/testdata/dummy-app/app.yaml create mode 100644 internal/orchestrator/bricks/testdata/dummy-app/python/main.py diff --git a/internal/orchestrator/bricks/bricks.go b/internal/orchestrator/bricks/bricks.go index 759b5cc6..20dd9948 100644 --- a/internal/orchestrator/bricks/bricks.go +++ b/internal/orchestrator/bricks/bricks.go @@ -248,25 +248,24 @@ func (s *Service) BrickCreate( ) error { brick, present := s.bricksIndex.FindBrickByID(req.ID) if !present { - return fmt.Errorf("brick not found with id %s", req.ID) + return fmt.Errorf("brick %q not found", req.ID) } for name, reqValue := range req.Variables { value, exist := brick.GetVariable(name) if !exist { - return errors.New("variable does not exist") + return fmt.Errorf("variable %q does not exist on brick %q", name, brick.ID) } if value.DefaultValue == "" && reqValue == "" { - return errors.New("variable default value cannot be empty") + return fmt.Errorf("variable %q cannot be empty", name) } } for _, brickVar := range brick.Variables { if brickVar.DefaultValue == "" { if _, exist := req.Variables[brickVar.Name]; !exist { - return errors.New("variable does not exist") + return fmt.Errorf("required variable %q is mandatory", brickVar.Name) } - return errors.New("variable default value cannot be empty") } } @@ -289,25 +288,20 @@ func (s *Service) BrickCreate( if idx == -1 { return fmt.Errorf("model %s does not exsist", *req.Model) } - brickInstance.Model = models[idx].ID } brickInstance.Variables = req.Variables if brickIndex == -1 { - appCurrent.Descriptor.Bricks = append(appCurrent.Descriptor.Bricks, brickInstance) - } else { appCurrent.Descriptor.Bricks[brickIndex] = brickInstance - } err := appCurrent.Save() if err != nil { return fmt.Errorf("cannot save brick instance with id %s", req.ID) } - return nil } diff --git a/internal/orchestrator/bricks/bricks_test.go b/internal/orchestrator/bricks/bricks_test.go new file mode 100644 index 00000000..b5a93173 --- /dev/null +++ b/internal/orchestrator/bricks/bricks_test.go @@ -0,0 +1,112 @@ +// This file is part of arduino-app-cli. +// +// Copyright 2025 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-app-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package bricks + +import ( + "testing" + + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" + "go.bug.st/f" + + "github.com/arduino/arduino-app-cli/internal/orchestrator/app" + "github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex" +) + +func TestBrickCreate(t *testing.T) { + bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + require.Nil(t, err) + brickService := NewService(nil, bricksIndex, nil) + + t.Run("fails if brick id does not exist", func(t *testing.T) { + err = brickService.BrickCreate(BrickCreateUpdateRequest{ID: "not-existing-id"}, f.Must(app.Load("testdata/dummy-app"))) + require.Error(t, err) + require.Equal(t, "brick \"not-existing-id\" not found", err.Error()) + }) + + t.Run("fails if the requestes variable is not present in the brick definition", func(t *testing.T) { + req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ + "NON_EXISTING_VARIABLE": "some-value", + }} + err = brickService.BrickCreate(req, f.Must(app.Load("testdata/dummy-app"))) + require.Error(t, err) + require.Equal(t, "variable \"NON_EXISTING_VARIABLE\" does not exist on brick \"arduino:arduino_cloud\"", err.Error()) + }) + + t.Run("fails if a required variable is set empty", func(t *testing.T) { + req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ + "ARDUINO_DEVICE_ID": "", + "ARDUINO_SECRET": "a-secret-a", + }} + err = brickService.BrickCreate(req, f.Must(app.Load("testdata/dummy-app"))) + require.Error(t, err) + require.Equal(t, "variable \"ARDUINO_DEVICE_ID\" cannot be empty", err.Error()) + }) + + t.Run("fails if a mandatory variable is not present in the request", func(t *testing.T) { + req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ + "ARDUINO_SECRET": "a-secret-a", + }} + err = brickService.BrickCreate(req, f.Must(app.Load("testdata/dummy-app"))) + require.Error(t, err) + require.Equal(t, "required variable \"ARDUINO_DEVICE_ID\" is mandatory", err.Error()) + }) + + t.Run("the brick is added if it does not exist in the app", func(t *testing.T) { + tempDummyApp := paths.New("testdata/dummy-app.temp") + err := tempDummyApp.RemoveAll() + require.Nil(t, err) + require.Nil(t, paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp)) + + req := BrickCreateUpdateRequest{ID: "arduino:dbstorage_sqlstore"} + err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp.String()))) + require.Nil(t, err) + after, err := app.Load(tempDummyApp.String()) + require.Nil(t, err) + require.Len(t, after.Descriptor.Bricks, 2) + require.Equal(t, "arduino:dbstorage_sqlstore", after.Descriptor.Bricks[1].ID) + }) + t.Run("the variables of a brick are updated", func(t *testing.T) { + tempDummyApp := paths.New("testdata/dummy-app.brick-override.temp") + err := tempDummyApp.RemoveAll() + require.Nil(t, err) + err = paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp) + require.Nil(t, err) + bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + require.Nil(t, err) + brickService := NewService(nil, bricksIndex, nil) + + deviceID := "this-is-a-device-id" + secret := "this-is-a-secret" + req := BrickCreateUpdateRequest{ + ID: "arduino:arduino_cloud", + Variables: map[string]string{ + "ARDUINO_DEVICE_ID": deviceID, + "ARDUINO_SECRET": secret, + }, + } + + err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp.String()))) + require.Nil(t, err) + + after, err := app.Load(tempDummyApp.String()) + require.Nil(t, err) + require.Len(t, after.Descriptor.Bricks, 1) + require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) + require.Equal(t, deviceID, after.Descriptor.Bricks[0].Variables["ARDUINO_DEVICE_ID"]) + require.Equal(t, secret, after.Descriptor.Bricks[0].Variables["ARDUINO_SECRET"]) + }) +} diff --git a/internal/orchestrator/bricks/testdata/.gitignore b/internal/orchestrator/bricks/testdata/.gitignore new file mode 100644 index 00000000..d4685d62 --- /dev/null +++ b/internal/orchestrator/bricks/testdata/.gitignore @@ -0,0 +1 @@ +*.temp \ No newline at end of file diff --git a/internal/orchestrator/bricks/testdata/bricks-list.yaml b/internal/orchestrator/bricks/testdata/bricks-list.yaml new file mode 100644 index 00000000..8e3114d6 --- /dev/null +++ b/internal/orchestrator/bricks/testdata/bricks-list.yaml @@ -0,0 +1,25 @@ +bricks: +- id: arduino:arduino_cloud + name: Arduino Cloud + description: Connects to Arduino Cloud + require_container: false + require_model: false + require_devices: false + mount_devices_into_container: false + ports: [] + category: null + variables: + - name: ARDUINO_DEVICE_ID + description: Arduino Cloud Device ID + - name: ARDUINO_SECRET + description: Arduino Cloud Secret +- id: arduino:dbstorage_sqlstore + name: Database - SQL + description: Simplified database storage layer for Arduino sensor data using SQLite + local database. + require_container: false + require_model: false + require_devices: false + mount_devices_into_container: false + ports: [] + category: storage diff --git a/internal/orchestrator/bricks/testdata/dummy-app/app.yaml b/internal/orchestrator/bricks/testdata/dummy-app/app.yaml new file mode 100644 index 00000000..281821c6 --- /dev/null +++ b/internal/orchestrator/bricks/testdata/dummy-app/app.yaml @@ -0,0 +1,6 @@ +name: Copy of Blinking LED from Arduino Cloud +description: Control the LED from the Arduino IoT Cloud using RPC calls +icon: ☁️ +ports: [] +bricks: +- arduino:arduino_cloud: \ No newline at end of file diff --git a/internal/orchestrator/bricks/testdata/dummy-app/python/main.py b/internal/orchestrator/bricks/testdata/dummy-app/python/main.py new file mode 100644 index 00000000..336e825c --- /dev/null +++ b/internal/orchestrator/bricks/testdata/dummy-app/python/main.py @@ -0,0 +1,2 @@ +def main(): + pass From cdbb28c05135f7f75930700a38447d6972648088 Mon Sep 17 00:00:00 2001 From: Luca Rinaldi Date: Thu, 6 Nov 2025 14:44:34 +0100 Subject: [PATCH 2/6] feat: flash sketch in ram (#12) * feat: flash sketch in ram * workaround file check * a better workaround * fix linter --- internal/orchestrator/orchestrator.go | 71 +++++++++++++++++++++++---- 1 file changed, 61 insertions(+), 10 deletions(-) diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 884515be..19181998 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -1124,8 +1124,6 @@ func compileUploadSketch( arduinoApp *app.ArduinoApp, w io.Writer, ) error { - const fqbn = "arduino:zephyr:unoq" - logrus.SetLevel(logrus.ErrorLevel) // Reduce the log level of arduino-cli srv := commands.NewArduinoCoreServer() @@ -1147,6 +1145,9 @@ func compileUploadSketch( } sketch := sketchResp.GetSketch() profile := sketch.GetDefaultProfile().GetName() + if profile == "" { + return fmt.Errorf("sketch %q has no default profile", sketchPath) + } initReq := &rpc.InitRequest{ Instance: inst, SketchPath: sketchPath, @@ -1186,18 +1187,13 @@ func compileUploadSketch( // build the sketch server, getCompileResult := commands.CompilerServerToStreams(ctx, w, w, nil) - - // TODO: add build cache compileReq := rpc.CompileRequest{ Instance: inst, - Fqbn: fqbn, + Fqbn: "arduino:zephyr:unoq", SketchPath: sketchPath, BuildPath: buildPath, Jobs: 2, } - if profile == "" { - compileReq.Libraries = []string{sketchPath + "/../../sketch-libraries"} - } err = srv.Compile(&compileReq, server) if err != nil { @@ -1220,12 +1216,67 @@ func compileUploadSketch( slog.Info("Used library " + lib.GetName() + " (" + lib.GetVersion() + ") in " + lib.GetInstallDir()) } + if err := uploadSketchInRam(ctx, w, srv, inst, sketchPath, buildPath); err != nil { + slog.Warn("failed to upload in ram mode, trying to configure the board in ram mode, and retry", slog.String("error", err.Error())) + if err := configureMicroInRamMode(ctx, w, srv, inst); err != nil { + return err + } + return uploadSketchInRam(ctx, w, srv, inst, sketchPath, buildPath) + } + return nil +} + +func uploadSketchInRam(ctx context.Context, + w io.Writer, + srv rpc.ArduinoCoreServiceServer, + inst *rpc.Instance, + sketchPath string, + buildPath string, +) error { stream, _ := commands.UploadToServerStreams(ctx, w, w) - return srv.Upload(&rpc.UploadRequest{ + if err := srv.Upload(&rpc.UploadRequest{ Instance: inst, - Fqbn: fqbn, + Fqbn: "arduino:zephyr:unoq:flash_mode=ram", SketchPath: sketchPath, ImportDir: buildPath, + }, stream); err != nil { + return err + } + return nil +} + +// configureMicroInRamMode uploads an empty binary overing any sketch previously uploaded in flash. +// This is required to be able to upload sketches in ram mode after if there is already a sketch in flash. +func configureMicroInRamMode( + ctx context.Context, + w io.Writer, + srv rpc.ArduinoCoreServiceServer, + inst *rpc.Instance, +) error { + emptyBinDir := paths.New("/tmp/empty") + _ = emptyBinDir.MkdirAll() + defer func() { _ = emptyBinDir.RemoveAll() }() + + zeros, err := os.Open("/dev/zero") + if err != nil { + return err + } + defer zeros.Close() + + empty, err := emptyBinDir.Join("empty.ino.elf-zsk.bin").Create() + if err != nil { + return err + } + defer empty.Close() + if _, err := io.CopyN(empty, zeros, 50); err != nil { + return err + } + + stream, _ := commands.UploadToServerStreams(ctx, w, w) + return srv.Upload(&rpc.UploadRequest{ + Instance: inst, + Fqbn: "arduino:zephyr:unoq:flash_mode=flash", + ImportDir: emptyBinDir.String(), }, stream) } From cd53babbe83bbf7fc0ecc0f48f3074e23e044f1e Mon Sep 17 00:00:00 2001 From: Davide Date: Thu, 6 Nov 2025 15:01:36 +0100 Subject: [PATCH 3/6] ci: update repository references in Taskfile.yml (#10) Co-authored-by: lucarin91 --- Taskfile.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index c30e0036..d11bae23 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -114,7 +114,7 @@ tasks: echo "Runner version set as: {{ .EXAMPLE_VERSION }}" TMP_PATH="$(mktemp -d)" DEST_PATH="debian/arduino-app-cli/home/arduino/.local/share/arduino-app-cli/" - echo "Cloning arduino/app-bricks-example into temporary directory ${TMP_PATH}..." + echo "Cloning arduino/app-bricks-examples into temporary directory ${TMP_PATH}..." git clone --depth 1 --branch "{{ .EXAMPLE_VERSION }}" https://github.com/arduino/app-bricks-examples "${TMP_PATH}" rm -rf "${DEST_PATH}/examples" mkdir -p "${DEST_PATH}" @@ -190,7 +190,7 @@ tasks: TMP_PATH="$(mktemp -d)" echo "Cloning examples into temporary directory ${TMP_PATH}..." - git clone --depth 1 https://github.com/arduino/app-bricks-example.git "${TMP_PATH}" + git clone --depth 1 https://github.com/arduino/app-bricks-examples.git "${TMP_PATH}" echo "Installing examples to ${DEST_PATH}examples" rm -rf "${DEST_PATH}examples" From c64fbf65409772f99ff471dd17a6d74cffa25d5d Mon Sep 17 00:00:00 2001 From: mirkoCrobu <214636120+mirkoCrobu@users.noreply.github.com> Date: Thu, 6 Nov 2025 16:55:54 +0100 Subject: [PATCH 4/6] feat(api): return variable configurations in `/apps/:id/bricks` and `apps/:id/bricks/:id` (#18) * add variables_details * add unit test * update openapi * add test e2e * fix bug * add deprecated field description for openapi * refactoring test constants * restore license header * rename variables_details to config_variables * rename variable name * udate docs and tests e2e * refactoring variables --- internal/api/docs/openapi.yaml | 17 +++++ internal/e2e/client/client.gen.go | 23 ++++-- internal/e2e/daemon/app_test.go | 4 +- internal/e2e/daemon/const.go | 10 +-- internal/e2e/daemon/instance_bricks_test.go | 24 +++++++ internal/orchestrator/bricks/bricks.go | 68 ++++++++++++------ internal/orchestrator/bricks/bricks_test.go | 80 +++++++++++++++++++++ internal/orchestrator/bricks/types.go | 22 ++++-- 8 files changed, 205 insertions(+), 43 deletions(-) diff --git a/internal/api/docs/openapi.yaml b/internal/api/docs/openapi.yaml index 577d77d1..f2b3a999 100644 --- a/internal/api/docs/openapi.yaml +++ b/internal/api/docs/openapi.yaml @@ -1276,6 +1276,17 @@ components: name: type: string type: object + BrickConfigVariable: + properties: + description: + type: string + name: + type: string + required: + type: boolean + value: + type: string + type: object BrickCreateUpdateRequest: properties: model: @@ -1325,6 +1336,10 @@ components: type: string category: type: string + config_variables: + items: + $ref: '#/components/schemas/BrickConfigVariable' + type: array id: type: string model: @@ -1336,6 +1351,8 @@ components: variables: additionalProperties: type: string + description: 'Deprecated: use config_variables instead. This field is kept + for backward compatibility.' type: object type: object BrickListItem: diff --git a/internal/e2e/client/client.gen.go b/internal/e2e/client/client.gen.go index 9e12c9ec..f6094430 100644 --- a/internal/e2e/client/client.gen.go +++ b/internal/e2e/client/client.gen.go @@ -119,6 +119,14 @@ type AppReference struct { Name *string `json:"name,omitempty"` } +// BrickConfigVariable defines model for BrickConfigVariable. +type BrickConfigVariable struct { + Description *string `json:"description,omitempty"` + Name *string `json:"name,omitempty"` + Required *bool `json:"required,omitempty"` + Value *string `json:"value,omitempty"` +} + // BrickCreateUpdateRequest defines model for BrickCreateUpdateRequest. type BrickCreateUpdateRequest struct { Model *string `json:"model"` @@ -142,12 +150,15 @@ type BrickDetailsResult struct { // BrickInstance defines model for BrickInstance. type BrickInstance struct { - Author *string `json:"author,omitempty"` - Category *string `json:"category,omitempty"` - Id *string `json:"id,omitempty"` - Model *string `json:"model,omitempty"` - Name *string `json:"name,omitempty"` - Status *string `json:"status,omitempty"` + Author *string `json:"author,omitempty"` + Category *string `json:"category,omitempty"` + ConfigVariables *[]BrickConfigVariable `json:"config_variables,omitempty"` + Id *string `json:"id,omitempty"` + Model *string `json:"model,omitempty"` + Name *string `json:"name,omitempty"` + Status *string `json:"status,omitempty"` + + // Variables Deprecated: use config_variables instead. This field is kept for backward compatibility. Variables *map[string]string `json:"variables,omitempty"` } diff --git a/internal/e2e/daemon/app_test.go b/internal/e2e/daemon/app_test.go index bc6556e6..1de8ff64 100644 --- a/internal/e2e/daemon/app_test.go +++ b/internal/e2e/daemon/app_test.go @@ -470,7 +470,7 @@ func TestDeleteApp(t *testing.T) { t.Run("DeletingExampleApp_Fail", func(t *testing.T) { var actualResponseBody models.ErrorResponse - deleteResp, err := httpClient.DeleteApp(t.Context(), noExisitingExample) + deleteResp, err := httpClient.DeleteApp(t.Context(), "ZXhhbXBsZXM6anVzdGJsaW5f") require.NoError(t, err) defer deleteResp.Body.Close() @@ -818,7 +818,7 @@ func TestAppPorts(t *testing.T) { respBrick, err := httpClient.UpsertAppBrickInstanceWithResponse( t.Context(), *createResp.JSON201.Id, - StreamLitUi, + "arduino:streamlit_ui", client.BrickCreateUpdateRequest{}, func(ctx context.Context, req *http.Request) error { return nil }, ) diff --git a/internal/e2e/daemon/const.go b/internal/e2e/daemon/const.go index f656fd0f..3bb35899 100644 --- a/internal/e2e/daemon/const.go +++ b/internal/e2e/daemon/const.go @@ -16,11 +16,7 @@ package daemon const ( - ImageClassifactionBrickID = "arduino:image_classification" - StreamLitUi = "arduino:streamlit_ui" - expectedDetailsAppNotfound = "unable to find the app" - expectedDetailsAppInvalidAppId = "invalid app id" - noExistingApp = "dXNlcjp0ZXN0LWFwcAw" - malformedAppId = "this-is-definitely-not-base64" - noExisitingExample = "ZXhhbXBsZXM6anVzdGJsaW5f" + ImageClassifactionBrickID = "arduino:image_classification" + noExistingApp = "dXNlcjp0ZXN0LWFwcAw" + malformedAppId = "this-is-definitely-not-base64" ) diff --git a/internal/e2e/daemon/instance_bricks_test.go b/internal/e2e/daemon/instance_bricks_test.go index 3399476c..c210a9e6 100644 --- a/internal/e2e/daemon/instance_bricks_test.go +++ b/internal/e2e/daemon/instance_bricks_test.go @@ -31,6 +31,28 @@ import ( "github.com/arduino/arduino-app-cli/internal/e2e/client" ) +const ( + expectedDetailsAppInvalidAppId = "invalid app id" + expectedDetailsAppNotfound = "unable to find the app" +) + +var ( + expectedConfigVariables = []client.BrickConfigVariable{ + { + Description: f.Ptr("path to the custom model directory"), + Name: f.Ptr("CUSTOM_MODEL_PATH"), + Required: f.Ptr(false), + Value: f.Ptr("/home/arduino/.arduino-bricks/ei-models"), + }, + { + Description: f.Ptr("path to the model file"), + Name: f.Ptr("EI_CLASSIFICATION_MODEL"), + Required: f.Ptr(false), + Value: f.Ptr("/models/ootb/ei/mobilenet-v2-224px.eim"), + }, + } +) + func setupTestApp(t *testing.T) (*client.CreateAppResp, *client.ClientWithResponses) { httpClient := GetHttpclient(t) createResp, err := httpClient.CreateAppWithResponse( @@ -68,6 +90,7 @@ func TestGetAppBrickInstances(t *testing.T) { require.NoError(t, err) require.Len(t, *brickInstances.JSON200.Bricks, 1) require.Equal(t, ImageClassifactionBrickID, *(*brickInstances.JSON200.Bricks)[0].Id) + require.Equal(t, expectedConfigVariables, *(*brickInstances.JSON200.Bricks)[0].ConfigVariables) }) @@ -111,6 +134,7 @@ func TestGetAppBrickInstanceById(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, brickInstance.JSON200) require.Equal(t, ImageClassifactionBrickID, *brickInstance.JSON200.Id) + require.Equal(t, expectedConfigVariables, (*brickInstance.JSON200.ConfigVariables)) }) t.Run("GetAppBrickInstanceByBrickID_InvalidAppID_Fails", func(t *testing.T) { diff --git a/internal/orchestrator/bricks/bricks.go b/internal/orchestrator/bricks/bricks.go index 20dd9948..e8dea9da 100644 --- a/internal/orchestrator/bricks/bricks.go +++ b/internal/orchestrator/bricks/bricks.go @@ -19,7 +19,6 @@ import ( "errors" "fmt" "log/slog" - "maps" "slices" "github.com/arduino/go-paths-helper" @@ -80,15 +79,20 @@ func (s *Service) AppBrickInstancesList(a *app.ArduinoApp) (AppBrickInstancesRes if !found { return AppBrickInstancesResult{}, fmt.Errorf("brick not found with id %s", brickInstance.ID) } + + variablesMap, configVariables := getBrickConfigDetails(brick, brickInstance.Variables) + res.BrickInstances[i] = BrickInstance{ - ID: brick.ID, - Name: brick.Name, - Author: "Arduino", // TODO: for now we only support our bricks - Category: brick.Category, - Status: "installed", - ModelID: brickInstance.Model, // TODO: in case is not set by the user, should we return the default model? - Variables: brickInstance.Variables, // TODO: do we want to show also the default value of not explicitly set variables? + ID: brick.ID, + Name: brick.Name, + Author: "Arduino", // TODO: for now we only support our bricks + Category: brick.Category, + Status: "installed", + ModelID: brickInstance.Model, // TODO: in case is not set by the user, should we return the default model? + Variables: variablesMap, // TODO: do we want to show also the default value of not explicitly set variables? + ConfigVariables: configVariables, } + } return res, nil } @@ -104,12 +108,7 @@ func (s *Service) AppBrickInstanceDetails(a *app.ArduinoApp, brickID string) (Br return BrickInstance{}, fmt.Errorf("brick %s not added in the app", brickID) } - variables := make(map[string]string, len(brick.Variables)) - for _, v := range brick.Variables { - variables[v.Name] = v.DefaultValue - } - // Add/Update the variables with the ones from the app descriptor - maps.Copy(variables, a.Descriptor.Bricks[brickIndex].Variables) + variables, configVariables := getBrickConfigDetails(brick, a.Descriptor.Bricks[brickIndex].Variables) modelID := a.Descriptor.Bricks[brickIndex].Model if modelID == "" { @@ -117,16 +116,43 @@ func (s *Service) AppBrickInstanceDetails(a *app.ArduinoApp, brickID string) (Br } return BrickInstance{ - ID: brickID, - Name: brick.Name, - Author: "Arduino", // TODO: for now we only support our bricks - Category: brick.Category, - Status: "installed", // For now every Arduino brick are installed - Variables: variables, - ModelID: modelID, + ID: brickID, + Name: brick.Name, + Author: "Arduino", // TODO: for now we only support our bricks + Category: brick.Category, + Status: "installed", // For now every Arduino brick are installed + Variables: variables, + ConfigVariables: configVariables, + ModelID: modelID, }, nil } +func getBrickConfigDetails( + brick *bricksindex.Brick, userVariables map[string]string, +) (map[string]string, []BrickConfigVariable) { + variablesMap := make(map[string]string, len(brick.Variables)) + variableDetails := make([]BrickConfigVariable, 0, len(brick.Variables)) + + for _, v := range brick.Variables { + finalValue := v.DefaultValue + + userValue, ok := userVariables[v.Name] + if ok { + finalValue = userValue + } + variablesMap[v.Name] = finalValue + + variableDetails = append(variableDetails, BrickConfigVariable{ + Name: v.Name, + Value: finalValue, + Description: v.Description, + Required: v.IsRequired(), + }) + } + + return variablesMap, variableDetails +} + func (s *Service) BricksDetails(id string, idProvider *app.IDProvider, cfg config.Configuration) (BrickDetailsResult, error) { brick, found := s.bricksIndex.FindBrickByID(id) diff --git a/internal/orchestrator/bricks/bricks_test.go b/internal/orchestrator/bricks/bricks_test.go index b5a93173..2f03d96b 100644 --- a/internal/orchestrator/bricks/bricks_test.go +++ b/internal/orchestrator/bricks/bricks_test.go @@ -110,3 +110,83 @@ func TestBrickCreate(t *testing.T) { require.Equal(t, secret, after.Descriptor.Bricks[0].Variables["ARDUINO_SECRET"]) }) } + +func TestGetBrickInstanceVariableDetails(t *testing.T) { + tests := []struct { + name string + brick *bricksindex.Brick + userVariables map[string]string + expectedConfigVariables []BrickConfigVariable + expectedVariableMap map[string]string + }{ + { + name: "variable is present in the map", + brick: &bricksindex.Brick{ + Variables: []bricksindex.BrickVariable{ + {Name: "VAR1", Description: "desc"}, + }, + }, + userVariables: map[string]string{"VAR1": "value1"}, + expectedConfigVariables: []BrickConfigVariable{ + {Name: "VAR1", Value: "value1", Description: "desc", Required: true}, + }, + expectedVariableMap: map[string]string{"VAR1": "value1"}, + }, + { + name: "variable not present in the map", + brick: &bricksindex.Brick{ + Variables: []bricksindex.BrickVariable{ + {Name: "VAR1", Description: "desc"}, + }, + }, + userVariables: map[string]string{}, + expectedConfigVariables: []BrickConfigVariable{ + {Name: "VAR1", Value: "", Description: "desc", Required: true}, + }, + expectedVariableMap: map[string]string{"VAR1": ""}, + }, + { + name: "variable with default value", + brick: &bricksindex.Brick{ + Variables: []bricksindex.BrickVariable{ + {Name: "VAR1", DefaultValue: "default", Description: "desc"}, + }, + }, + userVariables: map[string]string{}, + expectedConfigVariables: []BrickConfigVariable{ + {Name: "VAR1", Value: "default", Description: "desc", Required: false}, + }, + expectedVariableMap: map[string]string{"VAR1": "default"}, + }, + { + name: "multiple variables", + brick: &bricksindex.Brick{ + Variables: []bricksindex.BrickVariable{ + {Name: "VAR1", Description: "desc1"}, + {Name: "VAR2", DefaultValue: "def2", Description: "desc2"}, + }, + }, + userVariables: map[string]string{"VAR1": "v1"}, + expectedConfigVariables: []BrickConfigVariable{ + {Name: "VAR1", Value: "v1", Description: "desc1", Required: true}, + {Name: "VAR2", Value: "def2", Description: "desc2", Required: false}, + }, + expectedVariableMap: map[string]string{"VAR1": "v1", "VAR2": "def2"}, + }, + { + name: "no variables", + brick: &bricksindex.Brick{Variables: []bricksindex.BrickVariable{}}, + userVariables: map[string]string{}, + expectedConfigVariables: []BrickConfigVariable{}, + expectedVariableMap: map[string]string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualVariableMap, actualConfigVariables := getBrickConfigDetails(tt.brick, tt.userVariables) + require.Equal(t, tt.expectedVariableMap, actualVariableMap) + require.Equal(t, tt.expectedConfigVariables, actualConfigVariables) + }) + } +} diff --git a/internal/orchestrator/bricks/types.go b/internal/orchestrator/bricks/types.go index ae803745..868c563a 100644 --- a/internal/orchestrator/bricks/types.go +++ b/internal/orchestrator/bricks/types.go @@ -34,13 +34,21 @@ type AppBrickInstancesResult struct { } type BrickInstance struct { - ID string `json:"id"` - Name string `json:"name"` - Author string `json:"author"` - Category string `json:"category"` - Status string `json:"status"` - Variables map[string]string `json:"variables,omitempty"` - ModelID string `json:"model,omitempty"` + ID string `json:"id"` + Name string `json:"name"` + Author string `json:"author"` + Category string `json:"category"` + Status string `json:"status"` + Variables map[string]string `json:"variables,omitempty" description:"Deprecated: use config_variables instead. This field is kept for backward compatibility."` + ConfigVariables []BrickConfigVariable `json:"config_variables,omitempty"` + ModelID string `json:"model,omitempty"` +} + +type BrickConfigVariable struct { + Name string `json:"name"` + Value string `json:"value"` + Description string `json:"description"` + Required bool `json:"required"` } type BrickVariable struct { From 5a33829763f80f52261eaa4d3cdedf3f7e1e569f Mon Sep 17 00:00:00 2001 From: Davide Date: Fri, 7 Nov 2025 17:30:53 +0100 Subject: [PATCH 5/6] fix: install a library missing from local library-index (#50) * feat: enhance AddSketchLibrary to update library index and log progress * fix: improve library index update handling in AddSketchLibrary * fix: improve error handling in AddSketchLibrary by logging warnings instead of failing on library index update * fix: extend library index update interval to 60 seconds for improved performance * fix: update library index refresh interval to 10 minutes for improved performance * fix: improve error handling in AddSketchLibrary by logging warnings for library index update failures * Update internal/orchestrator/sketch_libs.go Co-authored-by: Luca Rinaldi * Update internal/orchestrator/sketch_libs.go Co-authored-by: Luca Rinaldi --------- Co-authored-by: Luca Rinaldi --- internal/orchestrator/sketch_libs.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/internal/orchestrator/sketch_libs.go b/internal/orchestrator/sketch_libs.go index d28c9ba8..3ef3b7ff 100644 --- a/internal/orchestrator/sketch_libs.go +++ b/internal/orchestrator/sketch_libs.go @@ -17,6 +17,8 @@ package orchestrator import ( "context" + "log/slog" + "time" "github.com/arduino/arduino-cli/commands" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -25,6 +27,8 @@ import ( "github.com/arduino/arduino-app-cli/internal/orchestrator/app" ) +const indexUpdateInterval = 10 * time.Minute + func AddSketchLibrary(ctx context.Context, app app.ArduinoApp, libRef LibraryReleaseID, addDeps bool) ([]LibraryReleaseID, error) { srv := commands.NewArduinoCoreServer() var inst *rpc.Instance @@ -43,6 +47,15 @@ func AddSketchLibrary(ctx context.Context, app app.ArduinoApp, libRef LibraryRel return nil, err } + stream, _ := commands.UpdateLibrariesIndexStreamResponseToCallbackFunction(ctx, func(curr *rpc.DownloadProgress) { + slog.Debug("downloading library index", "progress", curr.GetMessage()) + }) + // update the local library index after a certain time, to avoid if a library is added to the sketch but the local library index is not update, the compile can fail (because the lib is not found) + req := &rpc.UpdateLibrariesIndexRequest{Instance: inst, UpdateIfOlderThanSecs: int64(indexUpdateInterval.Seconds())} + if err := srv.UpdateLibrariesIndex(req, stream); err != nil { + slog.Warn("error updating library index, skipping", slog.String("error", err.Error())) + } + resp, err := srv.ProfileLibAdd(ctx, &rpc.ProfileLibAddRequest{ Instance: inst, SketchPath: app.MainSketchPath.String(), @@ -59,6 +72,7 @@ func AddSketchLibrary(ctx context.Context, app app.ArduinoApp, libRef LibraryRel if err != nil { return nil, err } + return f.Map(resp.GetAddedLibraries(), rpcProfileLibReferenceToLibReleaseID), nil } From c2b5eda9d9cad6c47b44e658d660ec8cded41edc Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 10 Nov 2025 11:33:08 +0100 Subject: [PATCH 6/6] feat: add brick details completions (#54) This add completions from brick details while tabbing in cli mode. --- cmd/arduino-app-cli/brick/details.go | 2 ++ cmd/arduino-app-cli/completion/completion.go | 22 ++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/cmd/arduino-app-cli/brick/details.go b/cmd/arduino-app-cli/brick/details.go index a1ba72f1..e898c9a0 100644 --- a/cmd/arduino-app-cli/brick/details.go +++ b/cmd/arduino-app-cli/brick/details.go @@ -22,6 +22,7 @@ import ( "github.com/spf13/cobra" + "github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/completion" "github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/internal/servicelocator" "github.com/arduino/arduino-app-cli/cmd/feedback" "github.com/arduino/arduino-app-cli/internal/orchestrator/bricks" @@ -36,6 +37,7 @@ func newBricksDetailsCmd(cfg config.Configuration) *cobra.Command { Run: func(cmd *cobra.Command, args []string) { bricksDetailsHandler(args[0], cfg) }, + ValidArgsFunction: completion.BrickIDs(), } } diff --git a/cmd/arduino-app-cli/completion/completion.go b/cmd/arduino-app-cli/completion/completion.go index 418f9a3e..ae222201 100644 --- a/cmd/arduino-app-cli/completion/completion.go +++ b/cmd/arduino-app-cli/completion/completion.go @@ -24,6 +24,7 @@ import ( "github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/internal/servicelocator" "github.com/arduino/arduino-app-cli/cmd/feedback" "github.com/arduino/arduino-app-cli/internal/orchestrator" + "github.com/arduino/arduino-app-cli/internal/orchestrator/bricks" "github.com/arduino/arduino-app-cli/internal/orchestrator/config" ) @@ -97,3 +98,24 @@ func ApplicationNamesWithFilterFunc(cfg config.Configuration, filter func(apps o return res, cobra.ShellCompDirectiveNoFileComp } } + +func BrickIDs() cobra.CompletionFunc { + return BrickIDsWithFilterFunc(func(_ bricks.BrickListItem) bool { return true }) +} + +func BrickIDsWithFilterFunc(filter func(apps bricks.BrickListItem) bool) cobra.CompletionFunc { + return func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + brickList, err := servicelocator.GetBrickService().List() + if err != nil { + return nil, cobra.ShellCompDirectiveError + } + + var res []string + for _, brick := range brickList.Bricks { + if filter(brick) { + res = append(res, brick.ID) + } + } + return res, cobra.ShellCompDirectiveNoFileComp + } +}