Re: PROBLEM: asus_nb_wmi sends KEY_BRIGHTNESSDOWN on pressing CAPS Lock and PrntScrn on Zenbook S 13 UX5304VA

[James John <me@donjajo.com>]

That is noted. I have learnt some things while solving this.

Thank you

On 21/10/2023 09:46, Hans de Goede wrote:
> Hi James,
>
> On 10/20/23 01:22, James John wrote:
>> Hello Hans,
>>
>> Thank you for your support so far. I really appreciate this.
>>
>> I have always wanted to contribute to the kernel, so, this is fun for me! :)
> That is great and thank you for all your help with solving this.
>
>> The 2 evtest logs show that each brightness up/down keypress
>> gets reported twice, once by the "ACPI video bus" device and
>> once bythe "Asus WMI hotkeys" device.
>>
>> I do not think these are multiple events. There are different. One has the value of 0, the other has value of 1.
>> I am not sure what they mean. I initially thought it could be keydown and keyup events, but it is not, because
>> on pressing the keydown, they are still both reported. I also think the desktops handle this, maybe by filtering out
>> 0 values. I use KDE Plasma, and I still have 5% step despite evtest reporting these 2 events.
> The 1 / 0 events are indeed press / release events that is
> not the problem, the problem is that a single keypress reports
> these events on 2 different /dev/input/event# nodes.
>
> Interesting that this is not a problem for KDE, I know it is
> a problem for GNOME. I guess KDE may do some filtering of
> the duplicate events itself.
>
>> I have applied the last 2 patches.
>>
>> 1. Show no output for capslock / printscreen
>>
>> Correct. These keys are no longer captured by Asus WMI hotkeys
>>
>> 2. Show KEY_SELECTIVE_SCREENSHOT events for the
>>     "Screen Capture" hotkey.
>>
>> I am not sure I am getting KEY_SELECTIVE_SCREENSHOT event for the "Screen Capture" hotkey. This is what I get:
>> Event: time 1697757579.588239, type 4 (EV_MSC), code 4 (MSC_SCAN), value 2a
>> Event: time 1697757579.588239, type 1 (EV_KEY), code 634 (?), value 1
>> Event: time 1697757579.588239, -------------- SYN_REPORT ------------
>> Event: time 1697757579.588244, type 1 (EV_KEY), code 634 (?), value 0
>> Event: time 1697757579.588244, -------------- SYN_REPORT ------------
> This is actually the correct output, 634 is 0x27a hexadecimal and:
>
> /usr/include/linux/input-event-codes.h :
>
> /* Select an area of screen to be copied */
> #define KEY_SELECTIVE_SCREENSHOT        0x27a
>
> This is a somewhat (but not really) recent addition to the list
> of KEY_foo defines, so I guess you are just using a somewhat old
> evtest which does not know this code yet.
>
>> And this is what I get for "Screen Capture" hotkey, from the debug you placed
>> [ 1096.691389] asus_wmi: raw event code 0x2a
>> [ 1096.691446] asus_wmi: raw event code 0xffffffffffffffff
>> [ 1097.982976] asus_wmi: raw event code 0x2a
>> [ 1097.983032] asus_wmi: raw event code 0xffffffffffffffff
>>
>>
>> 3. Show no output for brightness up/down,
>>     yet brightness up/down should still work since
>>     these are also reported by the "ACPI video bus"
>>
>> Yes, correct. No output from Asus WMI hotkeys, but there an output from Video bus
> Great, that means that everything works as it should now, thank you.
>
> Regards,
>
> Hans
>
>
>
>
>
>
>> On 18/10/2023 19:35, Hans de Goede wrote:
>>> Hi James,
>>>
>>> On 10/18/23 02:17, me@donjajo.com wrote:
>>>> Hi Hans,
>>>>
>>>> I hope you are feeling better now.
>>>> Thank you so much for your support in resolving this.
>>>>
>>>>> I assume that the first "BACKLIGHT BUTTON" is the backlight DOWN button ?
>>>> Yes. Correct.
>>>>
>>>>
>>>>> 2. Can you please run:
>>>>>
>>>>> sudo evtest and then select the "ACPI video bus" (or something
>>>>> similar) device and see if that reports brightness up/down
>>>>> keypresses?  And then do the same thing for the
>>>>> "Asus WMI hotkeys" device ? I expect the Asus WMI hotkeys
>>>>> device to only report brightness up keypresses (after my
>>>>> hwdb "fix") while I expect brightness-up events to get
>>>>> reported twice, by both the "ACPI video bus" device and
>>>>> the "Asus WMI hotkeys" device.
>>>> Done and attached.
>>>>
>>>>> Can you confirm this? This also means that brightness
>>>>> up will take bigger steps (2 steps per keypress) then
>>>>> brightness down, right ?
>>>> I am not sure I understand what you mean here. But I have attached the output here
>>> The 2 evtest logs show that each brightness up/down keypress
>>> gets reported twice, once by the "ACPI video bus" device and
>>> once bythe "Asus WMI hotkeys" device.
>>>
>>> This means that in e.g. GNOME the brightness will move
>>> up / down by 2 steps for each step, reducing the amount
>>> of steps from 20 to 10, or iow making each step twice
>>> as big. Especially at the low end of the brightness
>>> scale this may be an issue since steeping by 5% there
>>> can already make a big difference and this double
>>> key press reporting now changes this into stepping
>>> by 10% at a time.
>>>
>>>> After applying your patch, it seems to have fixed the issue!
>>> Thank you for all the testing and other then the double
>>> keypress issue + the unknown code messages everything
>>> now looks good!
>>>
>>> I have applied 2 more patches the first one fixes the
>>> unknown code messages and adds a mapping for the
>>> "Screen Capture" hotkey. The second test filters out
>>> the duplicate (duplicate with the "ACPI video bus")
>>> brightness up/down events.
>>>
>>> It would be great if you can add these on top of
>>> the previous 2 patches and then run one last
>>> test for me:
>>>
>>> Run evtest on the "Asus WMI hotkeys" device this should now:
>>>
>>> 1. Show no output for capslock / printscreen
>>>
>>> 2. Show KEY_SELECTIVE_SCREENSHOT events for the
>>>      "Screen Capture" hotkey.
>>>
>>> 3. Show no output for brightness up/down,
>>>      yet brightness up/down should still work since
>>>      these are also reported by the "ACPI video bus"
>>>
>>> It would be great if you can confirm for each of these
>>> that this behaves as expected with the 2 extra patches
>>> applied on top of the previous patches.
>>>
>>> Regards,
>>>
>>> Hans