[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Patch Set 5: (4 comments) File cli_output.c: https://review.coreboot.org/c/flashrom/+/84102/comment/43390002_53a3d3a8?usp... : PS4, Line 119: 16 * FLASHROM_PROGRESS_NR For 100% it will be 13 chars which I rounded to 16. `\b \b` is: 1. Move cursor to the left 2. Print space to overwrite what is under the cursor (or to its right depending on cursor's shape) 3. Move cursor to the left again
Does this relate to the part in commit message:
No, that's a separate thing.
But the concern is still valid right?
Yes, but I just saw that it's not a real problem. A flag indicating that something was printed can be set in `flashrom_print_cb()` below and reset after printing a progress here. If the flag is set when this function is called, need to move to a new line (ideally would know whether last print included a new line or not), otherwise can just erase previous progress. File flashrom.c: https://review.coreboot.org/c/flashrom/+/84102/comment/b195d54f_ff781022?usp... : PS5, Line 1235: /* const size_t flash_size = flashctx->chip->total_size * 1024; */ : /* const size_t page_size = flashctx->chip->page_size; */
I think I can remove these lines Sure.
https://review.coreboot.org/c/flashrom/+/84102/comment/8574887a_14b1a599?usp... : PS5, Line 1958: init_progress(flashctx, FLASHROM_PROGRESS_READ, flash_size); // WTF?
What was your concern on this line? Was something not working? (see line comment) I think it was before addressing progress reporting on SPI reads, so adding this line caused progress to reach 200%.
File include/flash.h: https://review.coreboot.org/c/flashrom/+/84102/comment/02956254_20130dd4?usp... : PS1, Line 587: struct stage_progress stage_progress[FLASHROM_PROGRESS_NR];
I think (I think) I understand. […] From the client side, yes. Also, this change switches from programmers reporting total and current position to them reporting amount of processed data, so the total and current values need to be stored somewhere and this is where it's stored.
-- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 5 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk@3mdeb.com> Gerrit-Reviewer: Aarya <aarya.chaumal@gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev@google.com> Gerrit-Reviewer: Peter Marheine <pmarheine@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal@gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm@chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev@google.com> Gerrit-Comment-Date: Sun, 29 Sep 2024 12:07:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm@chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
participants (1)
-
Sergii Dmytruk (Code Review)