Re: [PATCH v5 02/12] HID: pidff: Do not send effect envelope if it's empty

On Tue, 21 Jan 2025 at 10:59, Oliver Neukum [off-list ref] wrote:

I am afraid this is the most convoluted piece of boolean algebra I've seen
in a long time. In particular because it mixes things that do not belong together.

Could you elaborate on that? What here does not belong?

I think the diff is a bit unfortunate and doesn't make it justice, but
this is based on
code that was already there. Instead of just checking if the new
values differ from
old values, we first check if any values are different from 0. If
neither are != 0 OR
the effect didn't contain an envelope previously (NULL here), we
return the value
of the check.

Only if envelope isn't empty and if the effect already had an
envelope, we compare
the new and old values to decide if we need to send an envelope to the device.

if (!needs_new_envelope || !old)
        return needs_new_envelope;

Could be represented as:

if (!needs_new_envelope)
        // empty envelope
        return 0;

if (!old)
        // effect doesn't have an envelope yet
        return needs_new_envelope;

The last part works exactly as it did before.

Just to make sure we're on the same page here, here's the resulting code:

static int pidff_needs_set_envelope(struct ff_envelope *envelope,
                                    struct ff_envelope *old)
{
        bool needs_new_envelope;
        needs_new_envelope = envelope->attack_level  != 0 ||
                             envelope->fade_level    != 0 ||
                             envelope->attack_length != 0 ||
                             envelope->fade_length   != 0;

        if (!needs_new_envelope || !old)
                return needs_new_envelope;

        return envelope->attack_level  != old->attack_level  ||
               envelope->fade_level    != old->fade_level    ||
               envelope->attack_length != old->attack_length ||
               envelope->fade_length   != old->fade_length;
}