Skip to content
Open
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
3 changes: 2 additions & 1 deletion cmd/arduino-app-cli/brick/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ func newBricksListCmd() *cobra.Command {
},
}
}

func bricksListHandler() {
res, err := servicelocator.GetBrickService().List()
res, err := servicelocator.GetBrickService().List(nil)
if err != nil {
feedback.Fatal(err.Error(), feedback.ErrGeneric)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/arduino-app-cli/completion/completion.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func BrickIDs() cobra.CompletionFunc {

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()
brickList, err := servicelocator.GetBrickService().List(nil)
if err != nil {
return nil, cobra.ShellCompDirectiveError
}
Expand Down
4 changes: 3 additions & 1 deletion cmd/gendoc/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,9 @@ Contains a JSON object with the details of an error.
OperationId: "getBricks",
Method: http.MethodGet,
Path: "/v1/bricks",
Request: nil,
Request: (*struct {
SupportedOnly bool `query:"supported_only" json:"supported_only" description:"If true, only returns bricks supported by the current board."`
})(nil),
CustomSuccessResponse: &CustomResponseDef{
ContentType: "application/json",
DataStructure: bricks.BrickListResult{},
Expand Down
11 changes: 9 additions & 2 deletions internal/api/docs/openapi.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
openapi: 3.0.3
info:
title: Arduino-App-Cli
Expand Down Expand Up @@ -799,6 +799,13 @@
description: Returns all the existing bricks. Bricks that are ready to use are
marked as installed.
operationId: getBricks
parameters:
- description: If true, only returns bricks supported by the current board.
in: query
name: supported_only
schema:
description: If true, only returns bricks supported by the current board.
type: boolean
Comment on lines +802 to +808
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

duplicated generation of the description.. something wrong with the docs.go??

responses:
"200":
content:
Expand Down Expand Up @@ -1505,8 +1512,6 @@
type: object
AppLocalBrickCreateRequest:
properties:
description:
type: string
name:
type: string
type: object
Expand Down Expand Up @@ -1623,6 +1628,8 @@
type: string
name:
type: string
readme:
type: string
require_model:
type: boolean
status:
Expand Down
14 changes: 13 additions & 1 deletion internal/api/handlers/bricks.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,29 @@ import (
"log"
"log/slog"
"net/http"
"strconv"

"github.com/arduino/arduino-app-cli/internal/api/models"
"github.com/arduino/arduino-app-cli/internal/orchestrator/app"
"github.com/arduino/arduino-app-cli/internal/orchestrator/bricks"
"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
"github.com/arduino/arduino-app-cli/internal/orchestrator/config"
"github.com/arduino/arduino-app-cli/internal/render"
)

func HandleBrickList(brickService *bricks.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
res, err := brickService.List()
var brickFilter bricksindex.BrickFilter = nil

params := r.URL.Query()
filterSupported := params.Get("supported_only")
if filterSupported != "" {
if supported, err := strconv.ParseBool(filterSupported); err == nil && supported {
brickFilter = bricksindex.UnsupportedBrickFilter
}
}

res, err := brickService.List(brickFilter)
if err != nil {
slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()))
render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to retrieve brick list"})
Expand Down
48 changes: 38 additions & 10 deletions internal/e2e/client/client.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/e2e/daemon/brick_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func setupTestBrick(t *testing.T) (*client.CreateAppResp, *client.ClientWithResp
func TestBricksList(t *testing.T) {
httpClient := GetHttpclient(t)

response, err := httpClient.GetBricksWithResponse(t.Context(), func(ctx context.Context, req *http.Request) error { return nil })
response, err := httpClient.GetBricksWithResponse(t.Context(), &client.GetBricksParams{})
require.NoError(t, err)
require.NotEmpty(t, response.JSON200.Bricks)

Expand Down
6 changes: 3 additions & 3 deletions internal/orchestrator/bricks/bricks.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ func NewService(
}
}

func (s *Service) List() (BrickListResult, error) {
res := BrickListResult{Bricks: make([]BrickListItem, len(s.bricksIndex.ListBricks()))}
for i, brick := range s.bricksIndex.ListBricks() {
func (s *Service) List(filter bricksindex.BrickFilter) (BrickListResult, error) {
res := BrickListResult{Bricks: make([]BrickListItem, len(s.bricksIndex.ListBricks(nil)))}
for i, brick := range s.bricksIndex.ListBricks(filter) {
res.Bricks[i] = BrickListItem{
Comment on lines +57 to 59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure, but I guess s.bricksIndex.ListBricks(...) should have the same param?

Suggested change
res := BrickListResult{Bricks: make([]BrickListItem, len(s.bricksIndex.ListBricks(nil)))}
for i, brick := range s.bricksIndex.ListBricks(filter) {
res.Bricks[i] = BrickListItem{
bricks := s.bricksIndex.ListBricks(filter)
res := BrickListResult{Bricks: make([]BrickListItem, len(bricks))}
for i, brick := range bricks {

ID: brick.ID,
Name: brick.Name,
Expand Down
29 changes: 28 additions & 1 deletion internal/orchestrator/bricksindex/bricks_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
yaml "github.com/goccy/go-yaml"

"github.com/arduino/arduino-app-cli/internal/orchestrator/peripherals"
"github.com/arduino/arduino-app-cli/internal/platform"
)

type BricksIndex struct {
Expand All @@ -55,12 +56,19 @@ func (b *BricksIndex) FindBrickByID(id string) (*Brick, bool) {
return nil, false
}

type BrickFilter func(brick Brick) bool

// TODO: use iterator instead of returning a slice
func (b *BricksIndex) ListBricks() []Brick {
func (b *BricksIndex) ListBricks(filter BrickFilter) []Brick {
bricks := slices.Concat(b.AppBricks, b.BuiltInBricks)
slices.SortFunc(bricks, func(a, b Brick) int {
return strings.Compare(a.Name, b.Name)
})

// Filters out bricks where the provided function evaluates to true.
if filter != nil {
bricks = slices.DeleteFunc(bricks, filter)
}
return bricks
}

Expand All @@ -80,6 +88,7 @@ type Brick struct {
ID string `yaml:"id"`
Name string `yaml:"name"`
Description string `yaml:"description"`
SupportedBoards []string `yaml:"supported_boards,omitempty"`
Category string `yaml:"category,omitempty"`
RequiresDisplay string `yaml:"requires_display,omitempty"`
RequireContainer bool `yaml:"require_container"`
Expand Down Expand Up @@ -202,3 +211,21 @@ func parseBrickID(brickID string) (namespace, name string, err error) {
}
return namespace, brickName, nil
}

func GetBoard() string {
fqbn := platform.GetPlatform().FQBN
i := strings.LastIndex(fqbn, ":")
if i != -1 {
return fqbn[i+1:]
}
return ""
}

var boardProvider = GetBoard()

func UnsupportedBrickFilter(b Brick) bool {
if len(b.SupportedBoards) == 0 {
return false
}
return !slices.Contains(b.SupportedBoards, boardProvider)
}
87 changes: 87 additions & 0 deletions internal/orchestrator/bricksindex/bricks_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,90 @@ func TestLoadBrickYamlBrickIndex(t *testing.T) {
})

}

func TestListBricksWithFilter(t *testing.T) {
brick1 := Brick{ID: "1", Name: "brick1", Category: "catA"}
brick2 := Brick{ID: "2", Name: "brick2", Category: "catB"}
brick3 := Brick{ID: "3", Name: "brick3", Category: "catB"}
appBricks := []Brick{brick1, brick2, brick3}

tests := []struct {
name string
filter BrickFilter
wantBricks []Brick
}{
{
name: "Nil filter does not apply any filter",
filter: nil,
wantBricks: appBricks,
},
{
name: "Filters out catB bricks",
filter: func(b Brick) bool { return b.Category == "catB" },
wantBricks: []Brick{brick1},
},
{
name: "Filters out catA bricks",
filter: func(b Brick) bool { return b.Category == "catA" },
wantBricks: []Brick{brick2, brick3},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := &BricksIndex{
AppBricks: appBricks,
}

// act
got := b.ListBricks(tt.filter)
require.Equal(t, len(tt.wantBricks), len(got))
require.Equal(t, tt.wantBricks[0].ID, got[0].ID)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what about the this?

Suggested change
require.Equal(t, tt.wantBricks[0].ID, got[0].ID)
require.Equal(t, tt.wantBricks, got)

})
}
}

func TestUnsupportedBoardFilter(t *testing.T) {
brickEmptySupportedList := Brick{ID: "1", Name: "brick1", SupportedBoards: []string{}}
brickMyBoard := Brick{ID: "2", Name: "brick2", SupportedBoards: []string{"MyBoard"}}
brickOneQ := Brick{ID: "3", Name: "brickOneQ", SupportedBoards: []string{"UnoQ"}}
brickOneQVentunoQ := Brick{ID: "4", Name: "brickOneQVentunoQ", SupportedBoards: []string{"UnoQ", "VentunoQ"}}
brickVentunoQ := Brick{ID: "5", Name: "brickVentunoQ", SupportedBoards: []string{"VentunoQ"}}

boardProvider = "UnoQ"

tests := []struct {
name string
initialBricks []Brick
wantBricks []Brick
}{
{
name: "Empty board list means all supported",
initialBricks: []Brick{brickEmptySupportedList},
wantBricks: []Brick{brickEmptySupportedList},
},
{
name: "Only OneQ board supported bricks with two candidate bricks",
initialBricks: []Brick{brickOneQ, brickOneQVentunoQ, brickVentunoQ},
wantBricks: []Brick{brickOneQ, brickOneQVentunoQ},
},
{
name: "Only OneQ board supported bricks with no candidate bricks",
initialBricks: []Brick{brickMyBoard, brickVentunoQ},
wantBricks: []Brick{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
b := &BricksIndex{
AppBricks: tt.initialBricks,
}

// act
got := b.ListBricks(UnsupportedBrickFilter)
require.Equal(t, len(tt.wantBricks), len(got))
require.Equal(t, tt.wantBricks, got)
})
}
}
Loading