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

Improvements to memory management in fpga_download #159

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sylefeb
Copy link

@sylefeb sylefeb commented Oct 9, 2023

This pull request contains three commits to implement the following changes:

  • Exit the main download loop if an error occurs, such that the badge stays in error.
  • Add a memory allocation failure check on inserting a data block in the req list.
  • Allow the data buffer to be captured in the req list record so as to avoid allocating twice the size of the data block in memory.

@smunaut
Copy link
Collaborator

smunaut commented Oct 9, 2023

Upon looking close, why is exiting the main loop needed ? There is a 1s delay for the user to read the error before resetting, so that should be enough for the user to see something went wrong, that's what's used for the other errors.

The rest looks good to me.

@sylefeb
Copy link
Author

sylefeb commented Oct 9, 2023

The intent is to avoid the message to go unnoticed if not looking at the badge. Typically the error occurs while fpga.py is uploading the data block (in this case it was after a long upload when adding to the req list). The error is shown but disappears as fpga.py keeps going: the bitstream is uploaded next, it uploads fine and it seems all is ok.
The idea is to exit the loop to ensure the badge is stuck on the error regardless of what fpga.py sends next. An alternative would be for fpga.py to stop if it can receive an error code?

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