Hi Dmitry,
we had discussed earlier this point with Hans:
Hans:
>>> 3) In goodix_ts_report_key you do:
>>>
>>> for (i = 0; i < GOODIX_MAX_KEYS; ++i)
>>> if (key_value & (1 << i))
>>> input_report_key(ts->input_dev,
ts->keymap[i], 1
>>>
>>> But if the user then jumps his finger from say touch_key 0 to
touch_key 1
>>> without us receiving a "packet" in between with GOODIX_HAVE_KEY set,
>>> then we never release touch_key 0. So instead this really should be:
>>>
>>> for (i = 0; i < GOODIX_MAX_KEYS; ++i)
>>> input_report_key(ts->input_dev,
ts->keymap[i],
>>> key_value & (1 << i));
>>>
Me:
>> It seems, that this problem never happens. When user moves finger from
>> button to button, we stably receive 2-3 packets without
>> GOODIX_HAVE_KEY in between, that release all previous touches. From
>> other hand, your change will not work when the same keycode is
>> assigned to several buttons - it will be immediately released.
Hans:
>
> Hmm, interesting point I did not think of that. That would be a bit
> weird thing to do, but it is not impossible...
>
> I'm afraid Dmitry (the input maintainer) will likely make the same
> remark as I do though (when you submit this upstream). But we'll see.
>
> Keeping this as is is fine with me.
So I'm impressed about your mutual understanding) And waiting for your
decision.
Thank you!
Kind regards
Dmitry Mastykin
On 3/24/20 9:51 PM, Dmitry Torokhov wrote:
Hi Dmitry,
On Mon, Mar 16, 2020 at 10:53:03AM +0300, Dmitry Mastykin wrote:
+static void goodix_ts_report_key(struct goodix_ts_data *ts, u8 *data)
+{
+ int touch_num;
+ u8 key_value;
+ int i;
+
+ if (data[0] & GOODIX_HAVE_KEY) {
+ touch_num = data[0] & 0x0f;
+ key_value = data[1 + ts->contact_size * touch_num];
+ for (i = 0; i < GOODIX_MAX_KEYS; ++i)
+ if (key_value & (1 << i))
+ input_report_key(ts->input_dev, ts->keymap[i], 1);
+ } else
+ for (i = 0; i < GOODIX_MAX_KEYS; ++i)
+ input_report_key(ts->input_dev, ts->keymap[i], 0);
Should this be written as:
if (data[0] & GOODIX_HAVE_KEY) {
touch_num = data[0] & 0x0f;
key_value = data[1 + ts->contact_size * touch_num];
} else {
/* Release all keys */
key_value = 0;
}
for (i = 0; i < GOODIX_MAX_KEYS; i++)
input_report_key(ts->input_dev, ts->keymap[i],
key_value & BIT(i);
or the device may send incremental updates to the keys pressed without
resending currently pressed keys (sounds unlikely)?
Thanks.
