Support for ota download progress #156
Conversation
internal/ota-api/dto.go
Outdated
| for _, s := range states { | ||
| if s.State == "flash" { | ||
| return true | ||
| } | ||
| } |
There was a problem hiding this comment.
sometimes we aren't receiving all the states from the board, so I will change this to just check that there is at least another state after fetch
There was a problem hiding this comment.
I prefer to map possible state after fetch. I've added reboot also.
If fetch fail, error is written in state_data for fetch or is added an additional fail state?
There was a problem hiding this comment.
If fetch fail, error is written in state_data for fetch or is added an additional fail state?
the board will send a fail state
internal/ota-api/dto.go
Outdated
| if hasReceivedFlashState { | ||
| return buildSimpleProgressBar(float64(100)) | ||
| } | ||
| if strings.ToLower(data) == "unknown" { |
There was a problem hiding this comment.
all the errors in the data part always start with the upper case letter, so you could only check against
| if strings.ToLower(data) == "unknown" { | |
| if data == "Unknown" { |
There was a problem hiding this comment.
ok, I was not sure, so I moved to a more flexible approach. BTW, since you have confirmed, I've removed toLower()
internal/ota-api/dto.go
Outdated
| percentage := (float64(actualDownloadedData) / float64(*firmware_size)) * 100 | ||
| return buildSimpleProgressBar(percentage) | ||
| } | ||
| if strings.ToLower(data) == "unknown" { |
There was a problem hiding this comment.
I think you want to check state here not data right?
| if strings.ToLower(data) == "unknown" { | |
| if state == "unknown" { |
There was a problem hiding this comment.
No I want to check data. I received 'Unknown' from api on state_data
{ "state": "start", "state_data": "Unknown", "timestamp": "2024-06-26T07:28:31.000001Z" },
There was a problem hiding this comment.
mhm in that case I think you can add only one unknown check at the top instead of two one inside the fetch branch and another outside
| @@ -154,15 +159,68 @@ 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 { | |||
There was a problem hiding this comment.
I think you can also remove the pointer from FirmwareSize. IMHO there isn't a real reason to use a pointer
There was a problem hiding this comment.
I added a pointer because is doc it is stated that firmware size may not be present. In case it is missing, is it rmeoved from api response or returned as a 0 value?
There was a problem hiding this comment.
It shouldn't be present in the response, but anyway, for Golang that doesn't make any difference you will get a 0 either if the field is not present or if it is an actual 0.
Motivation
Add support for fetch progress for ota download
Reviewer checklist
main.CONTRIBUTING.md) and are well formatted.