[flashrom] Using libflashrom from fwupd

[Richard Hughes]

On 16 August 2017 at 23:53, Nico Huber <nico.h at gmx.de> wrote:
\> I'm only using it as a static library, kept reading anyway :) The cur-
> rent API is to be considered unstable. We'd like to have it stabilized
> before the next release (whenever that may happen), but it's not a top
> priority.

Right, for fwupd the Purism plugin isn't even in master yet, so I'm
fine for an unstable library for months if required. I think at this
stage keeping it API unstable is probably best, or maybe installing it
but making it clear that the soname is going to be bumped and that
you're not going to feel bad for doing so!

> You already helped to improve our API: I forgot that one might need a
> userdata pointer for the log callback. Will fix that...

Great, thanks already!

> 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,
>     };

That would actually work for me, in fwupd we have something very
similar: https://github.com/hughsie/fwupd/blob/master/libfwupd/fwupd-enums.h#L34

> 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).

Being honest, I don't mind much. For the callback I'd need to setup a
helper struct to pass around the FuPlugin and FuDevice objects to the
callback, so it's really not much more work to include anything else.
Just getting the state and the percentage would be a huuge leap
forward to the screen scraping and guessing we have now.

> do you expect the callback or the userdata pointer to change between
> calls?

In my case, no.

On 17 August 2017 at 00:51, Stefan Tauner <stefan.tauner at gmx.at> wrote:
>>     enum flashrom_progress_stage {
> Is there really a good reason to leak these much details to the UI?

Yes, even non-technical users have been shown to "trust" the update
process more if they know what's happening. Erase, Write, Verify are
used in lots of other places, although I admit I don't show erase in
the GUI and just lump it together with write to avoid the use thinking
that the disk is being erased. In the end-user case I'd probably use
text like "preparing for write".

> However, we can make pretty good guesses that get updated during the
> operation and could simply pass a percentage.

That's actually all I bubble up to the GUI, although be aware a uint8
for the percentage might not be good enough; if the GUI is 2000px
wide, then each 1% increase is 20px wide which makes it look "jerky".
This is doubly true if the update is going to take some time, for
instance a 5 minute update (read, erase, write, read) only gives the
UI an update every 3 seconds, which again looks pretty bad. I can
interpolate this over time, but it's much better if the underlying
library gives us bytes or something like centi-percent instead.

>The progress would not be
> constant in speed (e.g. because reading is usually much faster than
> writing),

I think that's fine.

> 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...

If the percentage does go backwards then we need to give the user a
really good reason of why, otherwise users *will* file issues and bugs
about it.

Thanks to all so far,

Richard