[PATCH v2] adb-mouse: convert to use QemuInputHandler

[PATCH v2] adb-mouse: convert to use QemuInputHandler

[Mark Cave-Ayland]

Update the ADB mouse implementation to use QemuInputHandler instead of the
legacy qemu_add_mouse_event_handler() function.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 10 deletions(-)

v2:
- Rebase onto master

- Replace (DeviceState *)s with dev in adb_mouse_realize() as suggested by
  Phil


diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index 144a0ccce7..c0e0282fee 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -38,6 +38,7 @@ struct MouseState {
     ADBDevice parent_obj;
     /*< private >*/
 
+    QemuInputHandlerState *hs;
     int buttons_state, last_buttons_state;
     int dx, dy, dz;
 };
@@ -51,17 +52,52 @@ struct ADBMouseClass {
     DeviceRealize parent_realize;
 };
 
-static void adb_mouse_event(void *opaque,
-                            int dx1, int dy1, int dz1, int buttons_state)
+#define ADB_MOUSE_BUTTON_LEFT   0x01
+#define ADB_MOUSE_BUTTON_RIGHT  0x02
+
+static void adb_mouse_handle_event(DeviceState *dev, QemuConsole *src,
+                                   InputEvent *evt)
 {
-    MouseState *s = opaque;
+    MouseState *s = (MouseState *)dev;
+    InputMoveEvent *move;
+    InputBtnEvent *btn;
+    static const int bmap[INPUT_BUTTON__MAX] = {
+        [INPUT_BUTTON_LEFT]   = ADB_MOUSE_BUTTON_LEFT,
+        [INPUT_BUTTON_RIGHT]  = ADB_MOUSE_BUTTON_RIGHT,
+    };
+
+    switch (evt->type) {
+    case INPUT_EVENT_KIND_REL:
+        move = evt->u.rel.data;
+        if (move->axis == INPUT_AXIS_X) {
+            s->dx += move->value;
+        } else if (move->axis == INPUT_AXIS_Y) {
+            s->dy += move->value;
+        }
+        break;
+
+    case INPUT_EVENT_KIND_BTN:
+        btn = evt->u.btn.data;
+        if (bmap[btn->button]) {
+            if (btn->down) {
+                s->buttons_state |= bmap[btn->button];
+            } else {
+                s->buttons_state &= ~bmap[btn->button];
+            }
+        }
+        break;
 
-    s->dx += dx1;
-    s->dy += dy1;
-    s->dz += dz1;
-    s->buttons_state = buttons_state;
+    default:
+        /* keep gcc happy */
+        break;
+    }
 }
 
+static const QemuInputHandler adb_mouse_handler = {
+    .name  = "QEMU ADB Mouse",
+    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
+    .event = adb_mouse_handle_event,
+};
 
 static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
 {
@@ -94,10 +130,10 @@ static int adb_mouse_poll(ADBDevice *d, uint8_t *obuf)
     dx &= 0x7f;
     dy &= 0x7f;
 
-    if (!(s->buttons_state & MOUSE_EVENT_LBUTTON)) {
+    if (!(s->buttons_state & ADB_MOUSE_BUTTON_LEFT)) {
         dy |= 0x80;
     }
-    if (!(s->buttons_state & MOUSE_EVENT_RBUTTON)) {
+    if (!(s->buttons_state & ADB_MOUSE_BUTTON_RIGHT)) {
         dx |= 0x80;
     }
 
@@ -236,7 +272,7 @@ static void adb_mouse_realizefn(DeviceState *dev, Error **errp)
 
     amc->parent_realize(dev, errp);
 
-    qemu_add_mouse_event_handler(adb_mouse_event, s, 0, "QEMU ADB Mouse");
+    s->hs = qemu_input_handler_register(dev, &adb_mouse_handler);
 }
 
 static void adb_mouse_initfn(Object *obj)
-- 
2.39.2

Re: [PATCH v2] adb-mouse: convert to use QemuInputHandler

[Philippe Mathieu-Daudé]

Hi Mark,

On 7/9/24 19:37, Mark Cave-Ayland wrote:
> Update the ADB mouse implementation to use QemuInputHandler instead of the
> legacy qemu_add_mouse_event_handler() function.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 46 insertions(+), 10 deletions(-)
> 
> v2:
> - Rebase onto master
> 
> - Replace (DeviceState *)s with dev in adb_mouse_realize() as suggested by
>    Phil

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +static const QemuInputHandler adb_mouse_handler = {
> +    .name  = "QEMU ADB Mouse",
> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> +    .event = adb_mouse_handle_event,
> +};

Do you mind if you amend your comment from v1 for clarity?
I could squash the following and take in my next PR:

diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
index c0e0282fee..15e6e91804 100644
--- a/hw/input/adb-mouse.c
+++ b/hw/input/adb-mouse.c
@@ -97,6 +97,11 @@ static const QemuInputHandler adb_mouse_handler = {
      .name  = "QEMU ADB Mouse",
      .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
      .event = adb_mouse_handle_event,
+    /*
+     * We do not need the .sync handler because unlike e.g. PS/2 where 
async
+     * mouse events are sent over the serial port, an ADB mouse is 
constantly
+     * polled by the host via the adb_mouse_poll() callback.
+     */
  };

Regards,

Phil.

Re: [PATCH v2] adb-mouse: convert to use QemuInputHandler

[Mark Cave-Ayland]

On 10/09/2024 15:48, Philippe Mathieu-Daudé wrote:

> Hi Mark,
> 
> On 7/9/24 19:37, Mark Cave-Ayland wrote:
>> Update the ADB mouse implementation to use QemuInputHandler instead of the
>> legacy qemu_add_mouse_event_handler() function.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> v2:
>> - Rebase onto master
>>
>> - Replace (DeviceState *)s with dev in adb_mouse_realize() as suggested by
>>    Phil
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>> +static const QemuInputHandler adb_mouse_handler = {
>> +    .name  = "QEMU ADB Mouse",
>> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>> +    .event = adb_mouse_handle_event,
>> +};
> 
> Do you mind if you amend your comment from v1 for clarity?
> I could squash the following and take in my next PR:
> 
> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
> index c0e0282fee..15e6e91804 100644
> --- a/hw/input/adb-mouse.c
> +++ b/hw/input/adb-mouse.c
> @@ -97,6 +97,11 @@ static const QemuInputHandler adb_mouse_handler = {
>       .name  = "QEMU ADB Mouse",
>       .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>       .event = adb_mouse_handle_event,
> +    /*
> +     * We do not need the .sync handler because unlike e.g. PS/2 where async
> +     * mouse events are sent over the serial port, an ADB mouse is constantly
> +     * polled by the host via the adb_mouse_poll() callback.
> +     */
>   };
> 
> Regards,
> 
> Phil.

Sure! If you think it is useful to an external set of eyes, then feel free to add it in.


ATB,

Mark.

Re: [PATCH v2] adb-mouse: convert to use QemuInputHandler

[Philippe Mathieu-Daudé]

On 10/9/24 22:36, Mark Cave-Ayland wrote:
> On 10/09/2024 15:48, Philippe Mathieu-Daudé wrote:
> 
>> Hi Mark,
>>
>> On 7/9/24 19:37, Mark Cave-Ayland wrote:
>>> Update the ADB mouse implementation to use QemuInputHandler instead 
>>> of the
>>> legacy qemu_add_mouse_event_handler() function.
>>>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>   hw/input/adb-mouse.c | 56 ++++++++++++++++++++++++++++++++++++--------
>>>   1 file changed, 46 insertions(+), 10 deletions(-)
>>>
>>> v2:
>>> - Rebase onto master
>>>
>>> - Replace (DeviceState *)s with dev in adb_mouse_realize() as 
>>> suggested by
>>>    Phil
>>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>>> +static const QemuInputHandler adb_mouse_handler = {
>>> +    .name  = "QEMU ADB Mouse",
>>> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>>> +    .event = adb_mouse_handle_event,
>>> +};
>>
>> Do you mind if you amend your comment from v1 for clarity?
>> I could squash the following and take in my next PR:
>>
>> diff --git a/hw/input/adb-mouse.c b/hw/input/adb-mouse.c
>> index c0e0282fee..15e6e91804 100644
>> --- a/hw/input/adb-mouse.c
>> +++ b/hw/input/adb-mouse.c
>> @@ -97,6 +97,11 @@ static const QemuInputHandler adb_mouse_handler = {
>>       .name  = "QEMU ADB Mouse",
>>       .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
>>       .event = adb_mouse_handle_event,
>> +    /*
>> +     * We do not need the .sync handler because unlike e.g. PS/2 
>> where async
>> +     * mouse events are sent over the serial port, an ADB mouse is 
>> constantly
>> +     * polled by the host via the adb_mouse_poll() callback.
>> +     */
>>   };
>>
>> Regards,
>>
>> Phil.
> 
> Sure! If you think it is useful to an external set of eyes, then feel 
> free to add it in.

Thanks, patch queued then.