From 7a0850ddae9c20eea2af5b0f1a3fe27aa787849b Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Tue, 25 Jun 2024 15:02:14 +0200 Subject: [PATCH 1/7] Implemented support for percentage state display for ota progress --- command/ota/status.go | 5 +++-- internal/ota-api/dto.go | 39 ++++++++++++++++++++++++++++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/command/ota/status.go b/command/ota/status.go index 6913209d..f13ec5dd 100644 --- a/command/ota/status.go +++ b/command/ota/status.go @@ -27,8 +27,9 @@ func PrintOtaStatus(otaid, otaids, device string, cred *config.Credentials, limi res, err := otapi.GetOtaStatusByOtaID(otaid, limit, order) if err == nil && res != nil { feedback.PrintResult(otaapi.OtaStatusDetail{ - Ota: res.Ota, - Details: res.States, + FirmwareSize: res.FirmwareSize, + Ota: res.Ota, + Details: res.States, }) } else if err != nil { return err diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index eaf09424..2dafeed3 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -18,6 +18,7 @@ package otaapi import ( + "strconv" "strings" "time" @@ -28,8 +29,9 @@ import ( type ( OtaStatusResponse struct { - Ota Ota `json:"ota"` - States []State `json:"states,omitempty"` + FirmwareSize *int64 `json:"firmware_size,omitempty"` + Ota Ota `json:"ota"` + States []State `json:"states,omitempty"` } OtaStatusList struct { @@ -53,8 +55,9 @@ type ( } OtaStatusDetail struct { - Ota Ota `json:"ota"` - Details []State `json:"details,omitempty"` + FirmwareSize *int64 `json:"firmware_size,omitempty"` + Ota Ota `json:"ota"` + Details []State `json:"details,omitempty"` } ) @@ -155,7 +158,8 @@ func (r OtaStatusDetail) String() string { t = table.New() t.SetHeader("Time", "Status", "Detail") for _, s := range r.Details { - t.AddRow(formatHumanReadableTs(s.Timestamp), upperCaseFirst(s.State), s.StateData) + stateData := formatStateData(s.State, s.StateData, r.FirmwareSize, hasReachedFlashState(r.Details)) + t.AddRow(formatHumanReadableTs(s.Timestamp), upperCaseFirst(s.State), stateData) } output += "\nDetails:\n" + t.Render() } @@ -163,6 +167,31 @@ func (r OtaStatusDetail) String() string { return output } +func hasReachedFlashState(states []State) bool { + for _, s := range states { + if s.State == "flash" { + return true + } + } + return false +} + +func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashState bool) string { + if state == "fetch" { + // This is the state 'fetch' of OTA progress. This contains a numer that represents the number of bytes fetched + if hasReceivedFlashState { + return "100%" + } + actualDownloadedData, err := strconv.Atoi(data) + if err != nil || firmware_size == nil || actualDownloadedData <= 0 || *firmware_size <= 0 { // Sanitize and avoid division by zero + return data + } + percentage := (float64(actualDownloadedData) / float64(*firmware_size)) * 100 + return strconv.FormatFloat(percentage, 'f', 2, 64) + "%" + } + return data +} + func upperCaseFirst(s string) string { if len(s) > 0 { s = strings.ReplaceAll(s, "_", " ") From 04888941d1357d69023f3b4d120d406732c9135f Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Tue, 25 Jun 2024 15:11:04 +0200 Subject: [PATCH 2/7] added progress bar --- internal/ota-api/dto.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index 2dafeed3..b763a6be 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -180,18 +180,27 @@ func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashS if state == "fetch" { // This is the state 'fetch' of OTA progress. This contains a numer that represents the number of bytes fetched if hasReceivedFlashState { - return "100%" + return buildSimpleProgressBar(float64(100)) } actualDownloadedData, err := strconv.Atoi(data) if err != nil || firmware_size == nil || actualDownloadedData <= 0 || *firmware_size <= 0 { // Sanitize and avoid division by zero return data } percentage := (float64(actualDownloadedData) / float64(*firmware_size)) * 100 - return strconv.FormatFloat(percentage, 'f', 2, 64) + "%" + return buildSimpleProgressBar(percentage) } return data } +func buildSimpleProgressBar(progress float64) string { + progressInt := int(progress) / 10 + bar := "[" + bar = bar + strings.Repeat("=", progressInt) + bar = bar + strings.Repeat(" ", 10-progressInt) + bar = bar + "] " + strconv.FormatFloat(progress, 'f', 2, 64) + "%" + return bar +} + func upperCaseFirst(s string) string { if len(s) > 0 { s = strings.ReplaceAll(s, "_", " ") From 16b5faba30686d095fee13d497db2ec9fe15e556 Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Tue, 25 Jun 2024 15:23:32 +0200 Subject: [PATCH 3/7] Changed var name --- cli/ota/status.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/cli/ota/status.go b/cli/ota/status.go index 89d67081..4c3ce90e 100644 --- a/cli/ota/status.go +++ b/cli/ota/status.go @@ -38,28 +38,29 @@ type statusFlags struct { func initOtaStatusCommand() *cobra.Command { flags := &statusFlags{} - uploadCommand := &cobra.Command{ + statusCommand := &cobra.Command{ Use: "status", Short: "OTA status", Long: "Get OTA status by OTA or device ID", Run: func(cmd *cobra.Command, args []string) { - if err := runPrintOtaStatusCommand(flags); err != nil { - feedback.Errorf("Error during ota get status: %v", err) + if err := runPrintOtaStatusCommand(flags, cmd); err != nil { + feedback.Errorf("\nError during ota get status: %v", err) os.Exit(errorcodes.ErrGeneric) } }, } - uploadCommand.Flags().StringVarP(&flags.otaID, "ota-id", "o", "", "OTA ID") - uploadCommand.Flags().StringVarP(&flags.otaIDs, "ota-ids", "", "", "OTA IDs (comma separated)") - uploadCommand.Flags().StringVarP(&flags.deviceId, "device-id", "d", "", "Device ID") - uploadCommand.Flags().Int16VarP(&flags.limit, "limit", "l", 10, "Output limit (default: 10)") - uploadCommand.Flags().StringVarP(&flags.sort, "sort", "s", "desc", "Sorting (default: desc)") + statusCommand.Flags().StringVarP(&flags.otaID, "ota-id", "o", "", "OTA ID") + statusCommand.Flags().StringVarP(&flags.otaIDs, "ota-ids", "", "", "OTA IDs (comma separated)") + statusCommand.Flags().StringVarP(&flags.deviceId, "device-id", "d", "", "Device ID") + statusCommand.Flags().Int16VarP(&flags.limit, "limit", "l", 10, "Output limit (default: 10)") + statusCommand.Flags().StringVarP(&flags.sort, "sort", "s", "desc", "Sorting (default: desc)") - return uploadCommand + return statusCommand } -func runPrintOtaStatusCommand(flags *statusFlags) error { +func runPrintOtaStatusCommand(flags *statusFlags, command *cobra.Command) error { if flags.otaID == "" && flags.deviceId == "" && flags.otaIDs == "" { + command.Help() return fmt.Errorf("required flag(s) \"ota-id\" or \"device-id\" or \"ota-ids\" not set") } From 75dbb48d87aa98ad88916cff8081128f7f4da68d Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Wed, 26 Jun 2024 09:46:08 +0200 Subject: [PATCH 4/7] added tests --- internal/ota-api/dto.go | 3 +++ internal/ota-api/dto_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 internal/ota-api/dto_test.go diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index b763a6be..7c24d2e8 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -177,6 +177,9 @@ func hasReachedFlashState(states []State) bool { } func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashState bool) string { + if data == "" || strings.ToLower(data) == "unknown" { + return "" + } if state == "fetch" { // This is the state 'fetch' of OTA progress. This contains a numer that represents the number of bytes fetched if hasReceivedFlashState { diff --git a/internal/ota-api/dto_test.go b/internal/ota-api/dto_test.go new file mode 100644 index 00000000..75f152b3 --- /dev/null +++ b/internal/ota-api/dto_test.go @@ -0,0 +1,19 @@ +package otaapi + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestProgressBar_notCompletePct(t *testing.T) { + firmwareSize := int64(25665 * 2) + bar := formatStateData("fetch", "25665", &firmwareSize, false) + assert.Equal(t, "[===== ] 50.00%", bar) +} + +func TestProgressBar_ifFlashState_goTo100Pct(t *testing.T) { + firmwareSize := int64(25665 * 2) + bar := formatStateData("fetch", "25665", &firmwareSize, true) // If in flash status, go to 100% + assert.Equal(t, "[==========] 100.00%", bar) +} From fa3deeeb0841c5a8d43c2b013c1e29345883abd9 Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Wed, 26 Jun 2024 09:53:03 +0200 Subject: [PATCH 5/7] handling Unknown state data --- internal/ota-api/dto.go | 10 ++++++++-- internal/ota-api/dto_test.go | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index 7c24d2e8..dbda35bf 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -177,14 +177,17 @@ func hasReachedFlashState(states []State) bool { } func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashState bool) string { - if data == "" || strings.ToLower(data) == "unknown" { + if data == "" { return "" } if state == "fetch" { - // This is the state 'fetch' of OTA progress. This contains a numer that represents the number of bytes fetched + // This is the state 'fetch' of OTA progress. This contains a number that represents the number of bytes fetched if hasReceivedFlashState { return buildSimpleProgressBar(float64(100)) } + if strings.ToLower(data) == "unknown" { + return "" + } actualDownloadedData, err := strconv.Atoi(data) if err != nil || firmware_size == nil || actualDownloadedData <= 0 || *firmware_size <= 0 { // Sanitize and avoid division by zero return data @@ -192,6 +195,9 @@ func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashS percentage := (float64(actualDownloadedData) / float64(*firmware_size)) * 100 return buildSimpleProgressBar(percentage) } + if strings.ToLower(data) == "unknown" { + return "" + } return data } diff --git a/internal/ota-api/dto_test.go b/internal/ota-api/dto_test.go index 75f152b3..496f14ca 100644 --- a/internal/ota-api/dto_test.go +++ b/internal/ota-api/dto_test.go @@ -17,3 +17,9 @@ func TestProgressBar_ifFlashState_goTo100Pct(t *testing.T) { bar := formatStateData("fetch", "25665", &firmwareSize, true) // If in flash status, go to 100% assert.Equal(t, "[==========] 100.00%", bar) } + +func TestProgressBar_ifFlashStateAndUnknown_goTo100Pct(t *testing.T) { + firmwareSize := int64(25665 * 2) + bar := formatStateData("fetch", "Unknown", &firmwareSize, true) // If in flash status, go to 100% + assert.Equal(t, "[==========] 100.00%", bar) +} From d02c1c174a7b7a76f67d659af2fec81936841ff6 Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Wed, 26 Jun 2024 11:11:45 +0200 Subject: [PATCH 6/7] addressed comments --- internal/ota-api/dto.go | 41 +++++++++++++++++++++++------------- internal/ota-api/dto_test.go | 14 ++++-------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index dbda35bf..946aee81 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -27,6 +27,8 @@ import ( "github.com/arduino/arduino-cli/table" ) +const progressBarMultiplier = 2 + type ( OtaStatusResponse struct { FirmwareSize *int64 `json:"firmware_size,omitempty"` @@ -157,8 +159,12 @@ func (r OtaStatusDetail) String() string { if len(r.Details) > 0 { t = table.New() t.SetHeader("Time", "Status", "Detail") + fwSize := int64(0) + if r.FirmwareSize != nil { + fwSize = *r.FirmwareSize + } for _, s := range r.Details { - stateData := formatStateData(s.State, s.StateData, r.FirmwareSize, hasReachedFlashState(r.Details)) + stateData := formatStateData(s.State, s.StateData, fwSize, hasReachedFlashState(r.Details)) t.AddRow(formatHumanReadableTs(s.Timestamp), upperCaseFirst(s.State), stateData) } output += "\nDetails:\n" + t.Render() @@ -169,33 +175,33 @@ func (r OtaStatusDetail) String() string { func hasReachedFlashState(states []State) bool { for _, s := range states { - if s.State == "flash" { + if s.State == "flash" || s.State == "reboot" { return true } } return false } -func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashState bool) string { +func formatStateData(state, data string, firmware_size int64, hasReceivedFlashState bool) string { if data == "" { return "" } if state == "fetch" { // This is the state 'fetch' of OTA progress. This contains a number that represents the number of bytes fetched - if hasReceivedFlashState { - return buildSimpleProgressBar(float64(100)) - } - if strings.ToLower(data) == "unknown" { + if data == "Unknown" { return "" } actualDownloadedData, err := strconv.Atoi(data) - if err != nil || firmware_size == nil || actualDownloadedData <= 0 || *firmware_size <= 0 { // Sanitize and avoid division by zero + if err != nil || actualDownloadedData <= 0 || firmware_size <= 0 { // Sanitize and avoid division by zero return data } - percentage := (float64(actualDownloadedData) / float64(*firmware_size)) * 100 + if hasReceivedFlashState { + return buildSimpleProgressBar(float64(100)) + } + percentage := (float64(actualDownloadedData) / float64(firmware_size)) * 100 return buildSimpleProgressBar(percentage) } - if strings.ToLower(data) == "unknown" { + if data == "Unknown" { return "" } return data @@ -203,11 +209,16 @@ func formatStateData(state, data string, firmware_size *int64, hasReceivedFlashS func buildSimpleProgressBar(progress float64) string { progressInt := int(progress) / 10 - bar := "[" - bar = bar + strings.Repeat("=", progressInt) - bar = bar + strings.Repeat(" ", 10-progressInt) - bar = bar + "] " + strconv.FormatFloat(progress, 'f', 2, 64) + "%" - return bar + progressInt = progressInt * progressBarMultiplier + maxProgress := 10 * progressBarMultiplier + var bar strings.Builder + bar.WriteString("[") + bar.WriteString(strings.Repeat("=", progressInt)) + bar.WriteString(strings.Repeat(" ", maxProgress-progressInt)) + bar.WriteString("] ") + bar.WriteString(strconv.FormatFloat(progress, 'f', 2, 64)) + bar.WriteString("%") + return bar.String() } func upperCaseFirst(s string) string { diff --git a/internal/ota-api/dto_test.go b/internal/ota-api/dto_test.go index 496f14ca..bb91225f 100644 --- a/internal/ota-api/dto_test.go +++ b/internal/ota-api/dto_test.go @@ -8,18 +8,12 @@ import ( func TestProgressBar_notCompletePct(t *testing.T) { firmwareSize := int64(25665 * 2) - bar := formatStateData("fetch", "25665", &firmwareSize, false) - assert.Equal(t, "[===== ] 50.00%", bar) + bar := formatStateData("fetch", "25665", firmwareSize, false) + assert.Equal(t, "[========== ] 50.00%", bar) } func TestProgressBar_ifFlashState_goTo100Pct(t *testing.T) { firmwareSize := int64(25665 * 2) - bar := formatStateData("fetch", "25665", &firmwareSize, true) // If in flash status, go to 100% - assert.Equal(t, "[==========] 100.00%", bar) -} - -func TestProgressBar_ifFlashStateAndUnknown_goTo100Pct(t *testing.T) { - firmwareSize := int64(25665 * 2) - bar := formatStateData("fetch", "Unknown", &firmwareSize, true) // If in flash status, go to 100% - assert.Equal(t, "[==========] 100.00%", bar) + bar := formatStateData("fetch", "25665", firmwareSize, true) // If in flash status, go to 100% + assert.Equal(t, "[====================] 100.00%", bar) } From ef4f1db97808e16aec020dd6f7b11722b33d9025 Mon Sep 17 00:00:00 2001 From: Marco Colombo Date: Wed, 26 Jun 2024 14:45:49 +0200 Subject: [PATCH 7/7] simplified code --- internal/ota-api/dto.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/internal/ota-api/dto.go b/internal/ota-api/dto.go index 946aee81..04184ed8 100644 --- a/internal/ota-api/dto.go +++ b/internal/ota-api/dto.go @@ -183,14 +183,11 @@ func hasReachedFlashState(states []State) bool { } func formatStateData(state, data string, firmware_size int64, hasReceivedFlashState bool) string { - if data == "" { + if data == "" || data == "Unknown" { return "" } if state == "fetch" { // This is the state 'fetch' of OTA progress. This contains a number that represents the number of bytes fetched - if data == "Unknown" { - return "" - } actualDownloadedData, err := strconv.Atoi(data) if err != nil || actualDownloadedData <= 0 || firmware_size <= 0 { // Sanitize and avoid division by zero return data @@ -201,9 +198,6 @@ func formatStateData(state, data string, firmware_size int64, hasReceivedFlashSt percentage := (float64(actualDownloadedData) / float64(firmware_size)) * 100 return buildSimpleProgressBar(percentage) } - if data == "Unknown" { - return "" - } return data }