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

Indicate the progress of flashing for better user experience. #75

Closed
wants to merge 1 commit into from

Conversation

quic-vkraleti
Copy link

Add logs to firehose to show the partition label currently being flashed and progress as a percentage of completion.

firehose.c Outdated
@@ -401,6 +403,9 @@ static int firehose_program(struct qdl_device *qdl, struct program *program, int
lseek(fd, (off_t) program->file_offset * program->sector_size, SEEK_SET);
left = num_sectors;
while (left > 0) {
fprintf(stderr, "[PROGRAM] %d sectors remaining out of %d (%.2f%%)\r",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that we didn't want to update this too often, but with the typical 1MB max_payload_size, and if my math is correct, this will generate about 10kb of logs per second on my machine. So, I don't think we need to make this more fancy at this time.

I'm thinking that we should clean up the logs a bit in general. Until then, I like this patch!

But perhaps move it last in the loop? That way it will end at 100%.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For larger images QDL appear to be stuck without any logs. So updated the patch to limit to 1 log per second. Do check if this is fine.

Add logs to show the partition label currently being flashed and
amount of progress as a percentage of completion.

Signed-off-by: Viswanath Kraleti <[email protected]>
@quic-vkraleti quic-vkraleti changed the title Indicate the progress of flashing for better usability. Indicate the progress of flashing for better user experience. Jun 19, 2024
Copy link
Contributor

@quic-bjorande quic-bjorande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked you to print at the end of the loop, but that obviously doesn't matter, because you have \r there, so the line will typically be overwritten.

But do we not have a problem with "flashed "%s" successfully" being a shorter string for short values of program->label, resulting in "corrupt" output to the log?

ret = firehose_write(qdl, doc);
if (ret < 0) {
fprintf(stderr, "[PROGRAM] failed to write program command\n");
fprintf(stderr, "[ERASE] failed to write program command\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good fix, but unrelated.

while (left > 0) {
t = time(NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting that you just do left % N == 0 for some value of N to avoid printing that often, but I guess this works as well...

@quic-bjorande
Copy link
Contributor

Please see #88 for a more comprehensive implementation.

@andersson
Copy link
Collaborator

Merged #88

@andersson andersson closed this Dec 20, 2024
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.

3 participants