diff --git a/cmd/arduino-app-cli/system/system.go b/cmd/arduino-app-cli/system/system.go index c23d5317e..a419a06be 100644 --- a/cmd/arduino-app-cli/system/system.go +++ b/cmd/arduino-app-cli/system/system.go @@ -113,11 +113,14 @@ func newUpdateCmd() *cobra.Command { events := updater.Subscribe() for event := range events { - if event.Type == update.ErrorEvent { + switch event.Type { + case update.ErrorEvent: // TODO: add colors to error messages err := event.GetError() feedback.Printf("Error: %s [%s]", err.Error(), update.GetUpdateErrorCode(err)) - } else { + case update.ProgressEvent: + feedback.Printf("[%s] %s %.2f", event.Type.String(), event.GetProgress().Name, event.GetProgress().Progress) + default: feedback.Printf("[%s] %s", event.Type.String(), event.GetData()) } diff --git a/internal/api/handlers/update.go b/internal/api/handlers/update.go index 49ec1b294..91e7e6f6c 100644 --- a/internal/api/handlers/update.go +++ b/internal/api/handlers/update.go @@ -152,7 +152,8 @@ func HandleUpdateEvents(updater *update.Manager) http.HandlerFunc { slog.Info("APT event channel closed, stopping SSE stream") return } - if event.Type == update.ErrorEvent { + switch event.Type { + case update.ErrorEvent: err := event.GetError() code := render.InternalServiceErr if c := update.GetUpdateErrorCode(err); c != update.UnknownErrorCode { @@ -162,7 +163,12 @@ func HandleUpdateEvents(updater *update.Manager) http.HandlerFunc { Code: code, Message: err.Error(), }) - } else { + case update.ProgressEvent: + sseStream.Send(render.SSEEvent{ + Type: event.Type.String(), + Data: event.GetProgress(), + }) + default: sseStream.Send(render.SSEEvent{ Type: event.Type.String(), Data: event.GetData(), diff --git a/internal/helpers/helper_test.go b/internal/helpers/helper_test.go new file mode 100644 index 000000000..9cf599d9e --- /dev/null +++ b/internal/helpers/helper_test.go @@ -0,0 +1,130 @@ +// 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 helpers + +import ( + "testing" + + rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" +) + +func TestArduinoCLIDownloadProgressToString(t *testing.T) { + tests := []struct { + name string + input *rpc.DownloadProgress + expected string + }{ + { + name: "Download Start", + input: &rpc.DownloadProgress{ + Message: &rpc.DownloadProgress_Start{ + Start: &rpc.DownloadProgressStart{ + Url: "http://example.com/index.json", + }, + }, + }, + expected: "Download started: http://example.com/index.json", + }, + { + name: "Download Update", + input: &rpc.DownloadProgress{ + Message: &rpc.DownloadProgress_Update{ + Update: &rpc.DownloadProgressUpdate{ + Downloaded: 1048576, + TotalSize: 5242880, + }, + }, + }, + expected: "Downloading: 1.00MiB / 5.00MiB", + }, + { + name: "Download End Success", + input: &rpc.DownloadProgress{ + Message: &rpc.DownloadProgress_End{ + End: &rpc.DownloadProgressEnd{ + Success: true, + Message: "Done", + }, + }, + }, + expected: "Download completed", + }, + { + name: "Download End Failure", + input: &rpc.DownloadProgress{ + Message: &rpc.DownloadProgress_End{ + End: &rpc.DownloadProgressEnd{ + Success: false, + Message: "Network error", + }, + }, + }, + expected: "Download failed: Network error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := ArduinoCLIDownloadProgressToString(tt.input) + if res != tt.expected { + t.Errorf("expected '%s', got '%s'", tt.expected, res) + } + }) + } +} + +func TestArduinoCLITaskProgressToString(t *testing.T) { + tests := []struct { + name string + input *rpc.TaskProgress + expected string + }{ + { + name: "Task Running", + input: &rpc.TaskProgress{ + Name: "Install", + Percent: 50.0, + }, + expected: "Install: 50%", + }, + { + name: "Task With Message", + input: &rpc.TaskProgress{ + Name: "Unpacking", + Message: "Extracting...", + Percent: 10.0, + }, + expected: "Unpacking (Extracting...): 10%", + }, + { + name: "Task Completed", + input: &rpc.TaskProgress{ + Name: "Install", + Completed: true, + }, + expected: "Install: completed", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := ArduinoCLITaskProgressToString(tt.input) + if res != tt.expected { + t.Errorf("expected '%s', got '%s'", tt.expected, res) + } + }) + } +} diff --git a/internal/helpers/helpers.go b/internal/helpers/helpers.go index 2bb15ea70..8addcef87 100644 --- a/internal/helpers/helpers.go +++ b/internal/helpers/helpers.go @@ -24,28 +24,33 @@ import ( ) func ArduinoCLIDownloadProgressToString(progress *rpc.DownloadProgress) string { - switch { - case progress.GetStart() != nil: - return fmt.Sprintf("Download started: %s", progress.GetStart().GetUrl()) - case progress.GetUpdate() != nil: - return fmt.Sprintf("Download progress: %s", progress.GetUpdate()) - case progress.GetEnd() != nil: - return fmt.Sprintf("Download completed: %s", progress.GetEnd()) + if start := progress.GetStart(); start != nil { + return fmt.Sprintf("Download started: %s", start.GetUrl()) } - return progress.String() + if update := progress.GetUpdate(); update != nil { + downloaded := ToHumanMiB(update.GetDownloaded()) + total := ToHumanMiB(update.GetTotalSize()) + return fmt.Sprintf("Downloading: %s / %s", downloaded, total) + } + if end := progress.GetEnd(); end != nil { + if !end.GetSuccess() { + return fmt.Sprintf("Download failed: %s", end.GetMessage()) + } + return "Download completed" + } + return "" } func ArduinoCLITaskProgressToString(progress *rpc.TaskProgress) string { - data := fmt.Sprintf("Task %s:", progress.GetName()) - if progress.GetMessage() != "" { - data += fmt.Sprintf(" (%s)", progress.GetMessage()) + name := progress.GetName() + if msg := progress.GetMessage(); msg != "" { + name += fmt.Sprintf(" (%s)", msg) } + if progress.GetCompleted() { - data += " completed" - } else { - data += fmt.Sprintf(" %.2f%%", progress.GetPercent()) + return fmt.Sprintf("%s: completed", name) } - return data + return fmt.Sprintf("%s: %.0f%%", name, progress.GetPercent()) } func GetHostIP() (string, error) { diff --git a/internal/update/apt/service.go b/internal/update/apt/service.go index b4675add6..98ee9b6c2 100644 --- a/internal/update/apt/service.go +++ b/internal/update/apt/service.go @@ -77,7 +77,6 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string, eventCB f if !s.lock.TryLock() { return update.ErrOperationAlreadyInProgress } - defer s.lock.Unlock() // At the end of the upgrade, always try to restart the services (that need it). @@ -93,6 +92,7 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string, eventCB f }() eventCB(update.NewDataEvent(update.StartEvent, "Upgrade is starting")) + eventCB(update.NewProgressEvent("apt upgrade", 0.0)) stream := runUpgradeCommand(ctx, names) for line, err := range stream { if err != nil { @@ -100,15 +100,16 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string, eventCB f } eventCB(update.NewDataEvent(update.UpgradeLineEvent, line)) } - eventCB(update.NewDataEvent(update.StartEvent, "apt cleaning cache is starting")) + eventCB(update.NewProgressEvent("apt cleanup", 80.0)) for line, err := range runAptCleanCommand(ctx) { if err != nil { return fmt.Errorf("error running apt clean command: %w", err) } + eventCB(update.NewDataEvent(update.UpgradeLineEvent, line)) } - + eventCB(update.NewProgressEvent("remove old bricks", 85.0)) eventCB(update.NewDataEvent(update.UpgradeLineEvent, "Stop and destroy docker containers and images ....")) streamCleanup := cleanupDockerContainers(ctx) for line, err := range streamCleanup { @@ -120,6 +121,7 @@ func (s *Service) UpgradePackages(ctx context.Context, names []string, eventCB f eventCB(update.NewDataEvent(update.UpgradeLineEvent, line)) } } + eventCB(update.NewProgressEvent("pull new bricks", 90.0)) // TODO: Remove this workaround once docker image versions are no longer hardcoded in arduino-app-cli. // Tracking issue: https://github.com/arduino/arduino-app-cli/issues/600 diff --git a/internal/update/arduino/arduino.go b/internal/update/arduino/arduino.go index 0905e40e6..1912ed2e7 100644 --- a/internal/update/arduino/arduino.go +++ b/internal/update/arduino/arduino.go @@ -139,19 +139,41 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st return update.ErrOperationAlreadyInProgress } - downloadProgressCB := func(curr *rpc.DownloadProgress) { - data := helpers.ArduinoCLIDownloadProgressToString(curr) - slog.Debug("Download progress", slog.String("download_progress", data)) - eventCB(update.NewDataEvent(update.UpgradeLineEvent, data)) + defer a.lock.Unlock() + + const indexBase float32 = 0.0 + const indexWeight float32 = 30.0 + const upgradeBase float32 = 30.0 + const upgradeWeight float32 = 60.0 + + makeDownloadProgressCallback := func(name string, basePercentage, phaseWeight float32) func(*rpc.DownloadProgress) { + return func(curr *rpc.DownloadProgress) { + // FIXME: update this helper function + data := helpers.ArduinoCLIDownloadProgressToString(curr) + eventCB(update.NewDataEvent(update.UpgradeLineEvent, data)) + if updateInfo := curr.GetUpdate(); updateInfo != nil { + if updateInfo.GetTotalSize() <= 0 { + return + } + localProgress := (float32(updateInfo.GetDownloaded()) / float32(updateInfo.GetTotalSize())) * 100.0 + totalArduinoProgress := basePercentage + (localProgress/100.0)*phaseWeight + eventCB(update.NewProgressEvent(name, totalArduinoProgress)) + } + } } - taskProgressCB := func(msg *rpc.TaskProgress) { - data := helpers.ArduinoCLITaskProgressToString(msg) - slog.Debug("Task progress", slog.String("task_progress", data)) - eventCB(update.NewDataEvent(update.UpgradeLineEvent, data)) + makeTaskProgressCallback := func(name string, basePercentage, phaseWeight float32) func(*rpc.TaskProgress) { + return func(msg *rpc.TaskProgress) { + // FIXME: update this helper function + data := helpers.ArduinoCLITaskProgressToString(msg) + eventCB(update.NewDataEvent(update.UpgradeLineEvent, data)) + if !msg.GetCompleted() { + localProgress := msg.GetPercent() + totalArduinoProgress := basePercentage + (localProgress/100.0)*phaseWeight + eventCB(update.NewProgressEvent(name, totalArduinoProgress)) + } + } } - defer a.lock.Unlock() - eventCB(update.NewDataEvent(update.StartEvent, "Upgrade is starting")) logrus.SetLevel(logrus.ErrorLevel) // Reduce the log level of arduino-cli @@ -176,19 +198,26 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st }() { - stream, _ := commands.UpdateIndexStreamResponseToCallbackFunction(ctx, downloadProgressCB) + updateIndexProgressCB := makeDownloadProgressCallback("update index", indexBase, indexWeight) + stream, _ := commands.UpdateIndexStreamResponseToCallbackFunction(ctx, updateIndexProgressCB) if err := srv.UpdateIndex(&rpc.UpdateIndexRequest{Instance: inst}, stream); err != nil { return fmt.Errorf("error updating index: %w", err) } + + eventCB(update.NewProgressEvent("update index", indexBase+indexWeight)) + if err := srv.Init(&rpc.InitRequest{Instance: inst}, commands.InitStreamResponseToCallbackFunction(ctx, nil)); err != nil { return fmt.Errorf("error initializing instance: %w", err) } } + platformDownloadCB := makeDownloadProgressCallback("platform download", upgradeBase, upgradeWeight) + platformTaskCB := makeTaskProgressCallback("platform upgrade", upgradeBase, upgradeWeight) + stream, respCB := commands.PlatformUpgradeStreamResponseToCallbackFunction( ctx, - downloadProgressCB, - taskProgressCB, + platformDownloadCB, + platformTaskCB, ) if err := srv.PlatformUpgrade( &rpc.PlatformUpgradeRequest{ @@ -219,8 +248,8 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st }, commands.PlatformInstallStreamResponseToCallbackFunction( ctx, - downloadProgressCB, - taskProgressCB, + platformDownloadCB, + platformTaskCB, ), ) if err != nil { @@ -234,15 +263,14 @@ func (a *ArduinoPlatformUpdater) UpgradePackages(ctx context.Context, names []st eventCB(update.NewDataEvent(update.UpgradeLineEvent, line)) }) - err := srv.BurnBootloader( + if err := srv.BurnBootloader( &rpc.BurnBootloaderRequest{ Instance: inst, Fqbn: "arduino:zephyr:unoq", Programmer: "jlink", }, commands.BurnBootloaderToServerStreams(ctx, cbw, cbw), - ) - if err != nil { + ); err != nil { return fmt.Errorf("error burning bootloader: %w", err) } diff --git a/internal/update/event.go b/internal/update/event.go index 2aac04ee9..8cf0644c4 100644 --- a/internal/update/event.go +++ b/internal/update/event.go @@ -15,7 +15,9 @@ package update -import "go.bug.st/f" +import ( + "go.bug.st/f" +) // EventType defines the type of upgrade event. type EventType int @@ -24,16 +26,22 @@ const ( UpgradeLineEvent EventType = iota StartEvent RestartEvent + ProgressEvent DoneEvent ErrorEvent ) // Event represents a single event in the upgrade process. type Event struct { - Type EventType + Type EventType + progress *ProgressInfo + data string + err error // error field for error events +} - data string - err error // error field for error events +type ProgressInfo struct { + Name string + Progress float32 } func (t EventType) String() string { @@ -44,6 +52,8 @@ func (t EventType) String() string { return "restarting" case StartEvent: return "starting" + case ProgressEvent: + return "progress" case DoneEvent: return "done" case ErrorEvent: @@ -60,6 +70,13 @@ func NewDataEvent(t EventType, data string) Event { } } +func NewProgressEvent(name string, progress float32) Event { + return Event{ + Type: ProgressEvent, + progress: &ProgressInfo{Name: name, Progress: progress}, + } +} + func NewErrorEvent(err error) Event { return Event{ Type: ErrorEvent, @@ -77,6 +94,11 @@ func (e Event) GetError() error { return e.err } +func (e Event) GetProgress() ProgressInfo { + f.Assert(e.Type == ProgressEvent, "not a progress event") + return *e.progress +} + type PackageType string const ( diff --git a/internal/update/update.go b/internal/update/update.go index e2a17d8c9..1c882b7eb 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -137,17 +137,39 @@ func (m *Manager) UpgradePackages(ctx context.Context, pkgs []UpgradablePackage) // update of the cores we will end up with inconsistent state, or // we need to re run the upgrade because the orchestrator interrupted // in the middle the upgrade of the cores. - if err := m.arduinoPlatformUpdateService.UpgradePackages(ctx, arduinoPlatform, m.broadcast); err != nil { + + const arduinoWeight float32 = 20.0 + const aptWeight float32 = 80.0 + + if err := m.arduinoPlatformUpdateService.UpgradePackages(ctx, arduinoPlatform, func(e Event) { + if e.Type == ProgressEvent { + progress := e.GetProgress() + globalProgress := (progress.Progress / 100.0) * arduinoWeight + m.broadcast(NewProgressEvent(progress.Name, globalProgress)) + } else { + m.broadcast(e) + } + }); err != nil { m.broadcast(NewErrorEvent(fmt.Errorf("failed to upgrade Arduino packages: %w", err))) // continue with deb packages upgrade. } - if err := m.debUpdateService.UpgradePackages(ctx, debPkgs, m.broadcast); err != nil { + if err := m.debUpdateService.UpgradePackages(ctx, debPkgs, func(e Event) { + if e.Type == ProgressEvent { + progress := e.GetProgress() + globalProgress := arduinoWeight + (progress.Progress/100.0)*aptWeight + m.broadcast(NewProgressEvent(progress.Name, globalProgress)) + } else { + m.broadcast(e) + } + }); err != nil { m.broadcast(NewErrorEvent(fmt.Errorf("failed to upgrade APT packages: %w", err))) return } + m.broadcast(NewProgressEvent("upgrade", 100.0)) + m.broadcast(NewDataEvent(DoneEvent, "Update completed")) }() return nil