ev3: Fix issues with flashing (some?) EV3s.

[jaguilar]

Flash the EV3 all in one go rather than sector-by-sector. This appears to resolve issues we had with flashing (some?) EV3s, and mirrors what we do in pybricksdev.

Fixes https://github.com/pybricks/support/issues/2375.

[laurensvalk]

Fixes https://github.com/pybricks/support/issues/2375.

~~This looks like a useful change, but it's a fix for a different problem :smile: In https://github.com/pybricks/support/issues/2375 it never gets to flashing. It fails right away on getting the bad response to the get version command.~~

EDIT: I'll collect some notes in the issue thread instead.

[dlech]

It seems like this is fixing at least 3 different things. So would be better to split it up into 3 commits.

1. Fix not adding extra padding for non-existent checksum.
2. Change to erase all then flash all instead erase/flash one sector at a time.
3. Verify that the firmware blob size is exactly aligned to sector size.

[jaguilar]

As requested, broke the change into separate commits.

When we try to upload a firmware with the wrong size, we show this alert now, rather than asserting:

[codecov[bot]]

Codecov Report

:x: Patch coverage is 9.09091% with 30 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 67.11%. Comparing base (0aeb53d) to head (c89bc02). :warning: Report is 22 commits behind head on master.

Files with missing lines	Patch %	Lines
src/firmware/sagas.ts	0.00%	30 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2352      +/-   ##
==========================================
- Coverage   72.61%   67.11%   -5.51%     
==========================================
  Files         201      211      +10     
  Lines        5163     5829     +666     
  Branches     1094     1278     +184     
==========================================
+ Hits         3749     3912     +163     
- Misses       1377     1872     +495     
- Partials       37       45       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dlech]

I tried running this (restoring Home firmware) and I am getting a timeout error.

Error: Timeout waiting for EV3 reply
    at sendCommand (http://localhost:36515/static/js/bundle.js:21056:26)
    at sendCommand.next (<anonymous>)
    at handleFlashEV3 (http://localhost:36515/static/js/bundle.js:21070:42)
    at handleFlashEV3.next (<anonymous>)
    at next (http://localhost:36515/static/js/bundle.js:72347:29)
    at currCb (http://localhost:36515/static/js/bundle.js:72431:7)
    at chCbAtKey (http://localhost:36515/static/js/bundle.js:71945:9)
    at currCb (http://localhost:36515/static/js/bundle.js:72431:7)

We need to adjust the timeout for the erase command to be proportional to the size that we are erasing.

Also, the erase dialog does not close when it fails with an error. That needs to be fixed.

Also, also, most of the erasing time depends on the flash memory itself, so we should be able to make the progress bar for erasing actually progress based on time.

[jaguilar]

We need to adjust the timeout for the erase command to be proportional to the size that we are erasing.

I have a fix for this, but the firmware upload will still show as failing because the official firmware is not aligned to the sector boundary and the bootloader returns us a write error if our last write is both

• not max size and,
• does not align to a sector boundary.

Note that the firmware is in fact flashed successfully despite the error.

Also, the erase dialog does not close when it fails with an error. That needs to be fixed.

Do we suspect that this is my fault? I don't see what in my code would have changed this state. If you have any hints as to how to accomplish this -- I am not a react knower.

(The real divergence from the "back to official" flow vs. the usual flow is that in the usual flow, the dialog is closed immediately when firmware flashing starts, whereas in the back to official flow, clicking "restore" does not close the dialogue.)

Also, also, most of the erasing time depends on the flash memory itself, so we should be able to make the progress bar for erasing actually progress based on time.

We could, although I have only tried erasing on my own machine, so I don't know exactly how fast it might go on other machines. Do you want me to assume it's the same speed on my machine as on any other, and just show progress under that assumed speed?

[dlech]

• does not align to a sector boundary.

It sounds like we don't need to bother with making things align to the boundary then.

Do we suspect that this is my fault?

No idea. :smile: I didn't look into it deeply yet.

so I don't know exactly how fast it might go on other machines

My point is that timing doesn't doesn't depend much on the machine, it depends mostly on the flash memory on the EV3, which is going to be the same on all EV3s.

[laurensvalk]

Except it's not the same on all EV3s. For the first generation, the timed indicator in pybricksdev is stuck on 100% for some time before getting ready to flash, even for the 1MB erase size, so it may be quite a lot more for the whole size.

[dlech]

Are you saying that the progress bar is correct for later EV3 hardware? I would calibrate the progress bar for the slowest hardware. I don't see it as a problem if faster hardware finishes before it gets to 100%.

[laurensvalk]

Are you saying that the progress bar is correct for later EV3 hardware? I would calibrate the progress bar for the slowest hardware. I don't see it as a problem if faster hardware finishes before it gets to 100%.

Yeah, we should measure the slowest case and estimate a the time per sector. If we use the full erase command for the restore option we can measure that as well in case it is any different.

does not align to a sector boundary.

What size are you getting? 16384000 ÷ (64 × 1024) = 250 which is a whole number.

$ ls src/firmware/assets/ -al
-rw-rw-r-- 1 laurens laurens 16384000 dec  9 13:27 ev3_firmware_v1.09e.bin
-rw-rw-r-- 1 laurens laurens 16384000 dec  9 13:27 EV3_Firmware_V1.09H.bin
-rw-rw-r-- 1 laurens laurens 16384000 dec  9 13:27 ev3-image-1.10e.bin

[jaguilar]

What size are you getting?

I didn't check the firmware on disk. I'm just looking at what ranges are being flashed in the console logs during the actual firmware flash process. If you flash the 09e firmware using this PR's HEAD commit, and look at the debug logs, the last range to get flashed does not end on a sector boundary. Haven't investigated why.

It sounds like we don't need to bother with making things align to the boundary then.

To be clear, I know that the system starts up. But I do not know that everything works correctly. It may be that the startup process never accesses the last page to be flashed.

packet captures

Corrected packet captures

Let me know if this captures the info you need.

[dlech]

Also, the erase dialog does not close when it fails with an error. That needs to be fixed.

This is fixed by https://github.com/pybricks/pybricks-code/commit/0d0e366cba034ded488aafffb30b50d88657b057 (and for the record, not @jaguilar's fault :wink:)

[dlech]

Let me know if this captures the info you need.

I would also like to see a capture of the problem that requires us to have aligned firmware size.

[jaguilar]

I would also like to see a capture of the problem that requires us to have aligned firmware size.

misaligned_firmware_selected.zip

This packet capture was produced by taking the HEAD of this pull request, and introducing four zero bytes at the end of the firmware. I erased the flash at sector boundaries (including the extra padding -- so, one more sector than would have been erased without the padding), then wrote the whole firmware, including the four padded bytes. I observed a timeout on the pybricks-code end of things.

[dlech]

What happens if we put the actual image size in the 0xf0 command?

[jaguilar]

I'm closing this since I believe all the fixes got subsumed into dlech's changes.