[Nut-upsdev] nutdrv_qx interface change proposal item_t::preprocess

[Nick Ma.]

Hi and thank you for your effort.

You are very close to what I want to achieve, but I was already aware of
the impossibility of querying direct from a command. I wanted the
following (both would be covered by your changes):
1. preprocess an item_t in the direction: NUT->DEVICE
    in order to be able to support queries like ups.generated.yesterday
("QED<date-1><Checksum>\r") and ups.generated.today
    (note: today's generated power is even more complicated because it
can't be queried due to QED<date>, but has to be queried hour by hour
QEH<date><time>)
2. requesting data from a specific day requested from higher layers
(such as the client).

On 19/05/15 02:11, hyouko at gmail.com wrote:
> mmh..
>
> At the moment there are 3 distinct preprocess functions that operate
> on 2 different types:
> - info_rw_t - preprocess();
> - item_t - preprocess();
> - item_t - preprocess_answer().
>
> For info_rw_t: range/enum values are preprocessed just to see if that
> specific value is to be used or not depending on the conditions the
> driver meets at runtime (e.g.: low voltage devices -> low voltage
> ranges). So, in this case, directions don't seem to make sense to me.
>
> For item_t:
> - preprocess_answer() is always (and only) executed everytime the
> driver reads a new 'answer' (be it as a consequence of a query or of
> an instant command/setvar, no difference) from the device. So, in this
> case, too, I can't see the ambiguity.
> - preprocess(): this function is executed
>   * for queries: to translate a value from device-form to NUT-form (so
> direction is DEVICE -> NUT, i.e.: read from device the 'answer' to
> that query);
>   * for instant commands/setvars (QX_FLAG_CMD/QX_FLAG_SETVAR): to fill
> the command to be sent to device with data from NUT-form to
> device-form (so direction is NUT -> DEVICE, i.e.: write to device;
> answer is expected to be either 'command accepted' or 'command
> rejected').
>   So direction already depends on item_t type (i.e. qxflags).
Allright, so I had a misunderstanding there. What goes in the
preprocess(..) method and what is supposed to go out grinded my gears a
couple of time, even after going through the code and whose
documentations.  More verbose documentation of the preprocess(..) [1]
would be nice though.
> Now, I know that there can be queries that need to be preprocessed
> (QED<date> and the like, right?), 
Correct.
> the closest thing you can do is to:
> 1. set a var (let's say "zig.zag") to a value (so there should be both
> a bare "zig.zag" item_t with info_flags ST_FLAG_RW and a
> QX_FLAG_SETVAR "zig.zag" one in nutdrv_qx) or use an internal var that
> gets its value from something other;
> 2. use that var (either the user-provided one, "zig.zag" , or the
> internal one) to fill the query (preprocess from NUT-form to
> device-form and write to device) and then proceed as per other queries
> (read from device the 'answer' and get the values, say "ding.dong.1",
> "ding.dong.2", ..., which may also need to be preprocessed from
> device-form to NUT-form).
Thank you, thats probably the way I'm going to take here.
> Thus, in the end, you may need both the NUT->device and device->NUT
> preprocess functions and I don't feel enthusiastic about having big
> functions that switch on direction - I already see myself lost trying
> to get out of it.
I would prefer one single preprocess(..) method which handles all the
preprocessing (similar to what proposed [2]), rather than expanding the
item_t structure any further. But thats just me.
There's a lot of potential beautifying in the item_t structure (e.g.
moving all the dynamic data a sub structure leaving a single dynamic
field, building preprocessor wrappers).
> What I'd do instead is to add a 3rd item_t preprocess function (be it
> 'preprocess_command()') that works like the preprocess_answer() one
> (but in the opposite direction): this function would be called just
> before the command (any command, both queries and instant
> commands/setvars, no difference) would be sent to the device so that:
> - queries that need to be preprocessed, can be;
> - devices that require a CRC or any other weird transliteration, can
> be satisfied (at this time instant commands/setvars are already filled
> with user-provided data, and the same would be true for those queries
> that need to be preprocessed: you'd only have to call the CRC/whatever
> function before quitting the query-preprocess one).
>
> Here's a first try at it:
> https://github.com/zykh/nut/tree/nutdrv_qx_preprocess_command
That would work for me too. I'm looking forward to the merge.

[1]
https://github.com/networkupstools/nut/blob/master/drivers/nutdrv_qx.h#L97-L101
[2]
https://github.com/nickma82/nut/commit/ce330a3873fd0a9243a61258db243a6eee=327e65