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

Wifi support larger patch files #283

Closed
wants to merge 11 commits into from
Closed

Wifi support larger patch files #283

wants to merge 11 commits into from

Conversation

LnnrtS
Copy link
Collaborator

@LnnrtS LnnrtS commented Jun 28, 2024

This removes wifi bridge RAM as the major file size limitation for patch files.

Requests are now streamed from the browser to the metamodule. The other direction is still processing messages as a whole. This can be changed later if required.

On the metamodule side we need a large receive buffer that effectively limits the patch file size for uploading. I defined a new global region for that. This can easily be changed if there is a better way.

Now further problems appear:

  • When uploading large files, all other request are of course blocked, so the presence detection shows "not connected" during that time. That could be solved by fragmenting packets but for now maybe making the presence detection less aggressive is probably enough
  • When a large file is processed, the M4 is busy for a while and doesn't read the uart fifo before it fills up. Proper solution is to have some kind of threading. Quick solution again is to make presence detection less aggressive

Haven't tested the wifi uploading yet. Better to test this once that is resolved.

@LnnrtS
Copy link
Collaborator Author

LnnrtS commented Jun 28, 2024

@danngreen not ready to merge but ready for comment

@danngreen
Copy link
Member

On the metamodule side we need a large receive buffer that effectively limits the patch file size for uploading. I defined a new global region for that. This can easily be changed if there is a better way.

Looks good. 8MB is plenty, we can play with that.

  • When uploading large files, all other request are of course blocked, so the presence detection shows "not connected" during that time. That could be solved by fragmenting packets but for now maybe making the presence detection less aggressive is probably enough

  • When a large file is processed, the M4 is busy for a while and doesn't read the uart fifo before it fills up. Proper solution is to have some kind of threading. Quick solution again is to make presence detection less aggressive

"Less aggressive" means checking for connection less often?

@LnnrtS
Copy link
Collaborator Author

LnnrtS commented Jul 1, 2024

I changed the heartbeat to be sent automatically by the metamodule (instead of as a response from the browser) and increased the timeout after which it would show disconnected.

Now I can easily update files up to ~100kB but something on the metamodule side chokes on them at least in the MB range. Since the serial is at 115200 at the moment, transferring such large files already takes way over one minute.

Should we set a hard limit for patch files which is then is enforced already at the frontend? Or should we first try to push the limit further (which will probably require increasing the link speed)?

@danngreen
Copy link
Member

Should we set a hard limit for patch files which is then is enforced already at the frontend? Or should we first try to push the limit further (which will probably require increasing the link speed)?

If increasing the speed doesn't work (or is unstable with the longer cables), then setting a hard limit on the front end to 100k or something lower to be safe is an OK fallback. I would anticipate that large patches would be rare since you can only make a patch file that big if you have lots of modules with saved state. As transfer time becomes larger, using a physical medium becomes more convenient anyways.

@LnnrtS
Copy link
Collaborator Author

LnnrtS commented Jul 18, 2024

This is ready to be tested.

There is no limit on the upload size yet. Uploading (too) large files will time out because writing to the destination medium on the metamodule can take quite a while and that is not part of estimating the timeout on the frontend yet. I would suggest to leave it like this for now so we can merge all those open changes first and then later see if this actually becomes a problem.

I wouldn't merge all of those wifi branches into main though but combine them all in a separate branch first, consolidate/combine image updates and the merge that into main.

@LnnrtS LnnrtS marked this pull request as ready for review July 18, 2024 10:56
@danngreen
Copy link
Member

Ok, I'll attempt the merge of all wifi branches on a test branch and try it out. But which wifi application.bin has all the features? There are different versions on all three wifi branches. It seems like either the one on this branch or the query_ip branch is the latest. Does a merge need to happen in the wifi repo before generating a new application.bin?

@LnnrtS
Copy link
Collaborator Author

LnnrtS commented Jul 19, 2024 via email

@LnnrtS LnnrtS mentioned this pull request Jul 22, 2024
@LnnrtS
Copy link
Collaborator Author

LnnrtS commented Jul 22, 2024

Closed in favor of #309

@LnnrtS LnnrtS closed this Jul 22, 2024
@danngreen danngreen deleted the wifi_large_patch_files branch July 25, 2024 18:51
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.

2 participants