add back --retries flag

[umbynos]

Add --retries flag just like in #24.
We removed this behavior during the refactoring.
Basically, the FirmwareUploader tries to update the firmware 9 times in a row (by default). Sometimes there could be some failings because of programmer out of sync, serial port closing etc… With this flag, the process should be more reliable

[per1234]

Is this here only as a "belt and suspenders" measure in case there was some missed error handling in the code?

[umbynos]

Actually, this condition is here because it allows breaking from the retry loop. Not sure if this can be handled better...
After every error in the loop there's a continue statement that allows to start from the next iteration of the loop in case something is wrong

[per1234]

After every error in the loop there's a continue statement that allows to start from the next iteration of the loop in case something is wrong

And for that reason, err should never have any value other than nil if this condition is reached, right?

[umbynos]

Correct

[umbynos]

🙈 I forgot the continue statement here https://github.com/arduino/FirmwareUploader/pull/61/files#diff-9bcc47c1909291c8177f70e16def7a94afe0d6c3fb6c937a8f2ccc91432b8aecR233
Thanks a lot @per1234

[per1234]

OK, I'm mostly just trying to make sure I understand the code. Since it should never be other than nil if the code is working correctly, I was wondering if a better "belt and suspenders" approach wouldn't be to panic:

if err !=nil{
  panic(fmt.Sprintf("Unhandled error: %s", err))
}
break

I generally just throw in a panic to handle situations that should never occur unless something is broken in the code. It's not clear to me whether it would be beneficial to make the code resilient to this condition, since it could mask a bug.

[umbynos]

Well that behavior is present here.
At the end of the specified numbers of retries the program exits

[per1234]

Yes, but that's a different situation from an unhandled error.

Anyway, I am happy with it as is. I think the retries are a very nice improvement. I always wish AVRDUDE had this option.

[silvanocerza]

This solution certainly works and I like it, but I feel like it could be done in a clearer way if you extract the logic into a function so you can do something like this:

var err error
for retry := 0; retry <= int(retries); retry++ {
    err = doTheThingsYouGottaDo(params)
    if err == nil {
        break
    }
}

if err != nil {
    // Tell the user and exit with error code
}

// Success