Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: download and decompress xz images #29

Merged
merged 8 commits into from
Jan 28, 2024

Conversation

samrum
Copy link
Contributor

@samrum samrum commented Jan 23, 2024

For #5

The manifest tests pass and the worker intializes on page load without error, but didn't test whether flashes actually go through.

Wasn't sure if the issue also meant to combine the download and unpacking steps, but that would be possible.

@adeebshihadeh
Copy link
Collaborator

@jnewb1 can you test this and merge?

@jnewb1 jnewb1 linked an issue Jan 23, 2024 that may be closed by this pull request
@jnewb1
Copy link
Contributor

jnewb1 commented Jan 23, 2024

[fastboot] Unpack error Error unpacking archive: TypeError: (intermediate value).getReader is not a function

@jnewb1
Copy link
Contributor

jnewb1 commented Jan 23, 2024

flash_log.txt

attached the console log

@adeebshihadeh
Copy link
Collaborator

@samrum can you update the test to catch this too?

@samrum
Copy link
Contributor Author

samrum commented Jan 23, 2024

Ah, oops, I'm missing a body before the getReader call. I'll add an image worker test for unpackImage which would catch that.

@samrum
Copy link
Contributor Author

samrum commented Jan 24, 2024

Fixed and updated the manifest tests to use the image worker for validating images

@jnewb1
Copy link
Contributor

jnewb1 commented Jan 24, 2024

goes over 100% now
image

edit:
it went up to like 500%, then went back to 100% and got stuck
image

@jnewb1
Copy link
Contributor

jnewb1 commented Jan 24, 2024

flash_log_2.txt

@jnewb1
Copy link
Contributor

jnewb1 commented Jan 24, 2024

blob.js:20 [blob] Download size mismatch {url: 'https://raw.githubusercontent.com/commaai/openpilot/master/system/hardware/tici/agnos.json', expected: 742, actual: 3158}

@samrum
Copy link
Contributor Author

samrum commented Jan 25, 2024

Sorry for the trouble all, I was avoiding testing the actual flashing because I didn't want to mess up my 3x, but actually went through and properly tested it this time.

For the incorrect progress issue:
I pushed a fix for the way progress is calculated when unpacking the xz images since it should be using the actual image size instead of the compressed archive size. It won't be fully fixed until commaai/openpilot#31154 is merged and we have the system alt size available to use for that image. It's fixed now!

For the system alt image getting stuck after unpacking:
It seems like the system alt image's final writable.close() call is dying without throwing an error somehow. Looked into it a bit and not sure why it's happening. It could be related to how long the xz archive takes to decompress, but not sure.

For the agnos.json size mismatch:
This seems like a preexisting issue on https://flash.comma.ai. It's comparing the content length header from the compressed GitHub response to the uncompressed blob.

xz-decompress is a fork of xzwasm that contains a fix for an issue
causing checksum mismatches when awaiting writable writes
@samrum
Copy link
Contributor Author

samrum commented Jan 26, 2024

Should be good now. Tested up to the actual flashing step and verified that the images grabbed for flashing match their expected sizes.

@adeebshihadeh
Copy link
Collaborator

@samrum
Copy link
Contributor Author

samrum commented Jan 28, 2024

Seems like the fetch for the image failed for some reason. It's fine when I run them locally. Does it happen if you re-run the job?

@adeebshihadeh
Copy link
Collaborator

It works, thanks! I'm planning on putting up some more bounties for this once #7 is done if you'd be interested, such as streaming the system image to require less memory (like openpilot does).

@adeebshihadeh adeebshihadeh merged commit 398d72a into commaai:master Jan 28, 2024
2 checks passed
@samrum samrum deleted the use-xz-images branch January 28, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[200$ bounty] download and decompress xz images
3 participants