On Thu, 17 Aug 2017 00:53:36 +0200 Nico Huber nico.h@gmx.de wrote:
I guess this would suffice for read and verify, which are simple, continuous operations. Write, however, is comprised of one, two or three steps (depending on options and the data): Read (needed to know which bytes have to change), (comparing to the data to be written) write (if necessary) and optionally verify. So there is no straight forward way to tell during read, for instance, if we are at 100% after the read or not. We could compare the data early, but that might result in an awk- ward implementation. My idea (not yet convinced if it's any good) would be to add a `stage` parameter to the callback:
enum flashrom_progress_stage { FLASHROM_PROGRESS_READ, FLASHROM_PROGRESS_WRITE, FLASHROM_PROGRESS_VERIFY, };
Is there really a good reason to leak these much details to the UI? Of course it would allow it to draw a very detailed and realistic picture (and a dedicated flashrom UI might want to do that (possibly just for the lols though)) but for a generic UI where libflashrom is just one of multiple targets I don't think this is helpful at all but complicates things.
Richard, we cannot easily determine how many net (as in netto) bytes we have to transfer (in and out) because it very much depends on the circumstances as Nico explained. However, we can make pretty good guesses that get updated during the operation and could simply pass a percentage. The progress would not be constant in speed (e.g. because reading is usually much faster than writing), and it would sometimes even be non-monotonic (if a write fails, we re-read the whole chip to make absolutely sure we get everything correct thus increasing the total number of bytes to transfer... or after the first read we might notice that we can skip writing 90% of the blocks because only a small part is updated) but this would drastically reduce the complexity of the interface and would still update often (spinner-ish) and usually relatively consistently and monotonic. I think this is the best compromise between doing nothing and getting insane by trying to get all information to the caller that it needs to get a complete picture (because the latter is by definition impossible since we don't know it either).
I personally don't like to lie to users even if they beg for it. If flashrom does not exit it does useful stuff (of course there are some corner cases, e.g., if hardware is broken but these cases should a) be fixed by timeouts within flashrom, and b) be covered by timeouts in the callers and NOT by the user manually killing anything...). So IMHO a message to not worry + a spinner as long as your UI is working correctly (+ a timeout that really is the last resort) is way better and more honest than a made up progress indicator that can never tell the full story... but that's my subjective opinion. flashrom as of yet does not even have the spinner (and that's bad!) and to add that we need the same infrastructure as a full progressbar anyway so we of course have considered implementing this a few times but certainly we will add some way to the API eventually and your input is very welcome...:
The short answer is yes. Nico also briefly mentioned an idea on IRC to use a mechanism similar to flashrom_set_log_callback() to avoid complicating function signatures for the read, write, and verify functions.
Correct, I would prefer a method to set the current callback and private pointer over adding more parameters. Though, we could have another set of functions ofc, with more parameters.
Richard, please let us know, what you'd prefer. After all, the interface should be designed to be most usable (and not driven by internals). e.g. do you expect the callback or the userdata pointer to change between calls?