[PATCH] Docs: Input: initial uinput documentation

[PATCH] Docs: Input: initial uinput documentation

[Marcos Paulo de Souza]

Hi guys,

so, after taking some time to understand how to work with uinput, and to grab some examples
on web, I discovered that kernel doesn't have any "official" documentation about it.

So, here we are :)

I added some examples based in my tests, there are samples of EV_REL and EV_KEY, but
we can add more in the future to cover other cases.

Notes:
* I created the doc in rst format, following the trend to adapt all documentation using the
  new format (although input subsystem still have some work to do...)
* This documentation was converted to HTML, and it can be checked here:
  https://marcosps.github.io/
* Let me know if you find anything that can make the documentation more clear, or
  anything that can make the code better

Marcos Paulo de Souza (1):
  Documentation: Input: Add uinput documentation

 Documentation/input/uinput.rst | 158 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)
 create mode 100644 Documentation/input/uinput.rst

-- 
2.9.3

[PATCH] Documentation: Input: Add uinput documentation

[Marcos Paulo de Souza]

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
---
 Documentation/input/uinput.rst | 158 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)
 create mode 100644 Documentation/input/uinput.rst

diff --git a/Documentation/input/uinput.rst b/Documentation/input/uinput.rst
new file mode 100644
index 0000000..8d59c98
--- /dev/null
+++ b/Documentation/input/uinput.rst
@@ -0,0 +1,158 @@
+=============
+uinput module
+=============
+
+Introduction
+============
+
+uinput is a kernel module that makes possible create and handle input devices from userspace. By using /dev/uinput (or /dev/input/uinput), a process can create virtual devices and emit events like key pressing, mouse movements and joystick buttons.
+
+Interface
+=========
+
+::
+
+  linux/uinput.h
+
+The uinput header defines ioctl request keys to create, setup and destroy virtual devices, along with ioctls specific to uinput devices, like enabling events and keys to be send to the kernel.
+
+Examples
+========
+
+1.0 Keyboard events
+-------------------
+
+This first example shows how to create a new virtual device and how to send a key event as well as a physical keyboard. All default imports and error handlers were removed for the sake of simplicity.
+
+.. code-block:: c
+
+   #include <linux/uinput.h>
+
+   int fd;
+
+   void emit(int type, int code, int val)
+   {
+        struct input_event ie;
+        memset(&ie, 0, sizeof(ie));
+        ie.type = type;
+        ie.code = code;
+        ie.value = val;
+
+        if (write(fd, &ie, sizeof(ie)) < 0) {
+                perror("write2");
+                exit(1);
+        }
+   }
+
+   int main() {
+        struct input_id uid;
+        struct uinput_setup usetup;
+
+        fd = open("/dev/uinput", O_WRONLY | O_NONBLOCK);
+        ioctl(fd, UI_SET_EVBIT, EV_KEY);
+        ioctl(fd, UI_SET_KEYBIT, KEY_SPACE);
+
+        memset(&uid, 0, sizeof(iod));
+        memset(&usetup, 0, sizeof(usetup));
+        usetup.id = uid;
+        strcpy(usetup.name, "ex_device");
+
+        ioctl(fd, UI_DEV_SETUP, &usetup);
+        ioctl(fd, UI_DEV_CREATE);
+
+        /* wait some time until the Window Manager can get the reference for the
+         * new virtual device to receive data from
+         * */
+        sleep(1);
+
+        /* send key press, report the event, send key release, and report again */
+        emit(EV_KEY, KEY_SPACE, 1);
+        emit(EV_SYN, SYN_REPORT, 0);
+        emit(EV_KEY, KEY_SPACE, 0);
+        emit(EV_SYN, SYN_REPORT, 0);
+
+        close(fd);
+
+        return 0;
+   }
+
+2.0 Mouse movements
+-------------------
+
+This example shows how to create a virtual device who behaves like a physical mouse.
+
+.. code-block:: c
+
+    int i = 50;
+
+    /* emit function is the same of the example above */
+
+    void emit_rel(int code, int val, int syn)
+    {
+            emit(EV_REL, code, val);
+            if (syn)
+                    emit(EV_SYN, SYN_REPORT, 0);
+    }
+
+    /* ...open uinput file as shown in the previous example... */
+
+    /* enable mouse button left and relative events. This makes the Window Manager to interpret this
+     * device as a physical mouse
+     */
+    if (ioctl(fd, UI_SET_EVBIT, EV_KEY) == -1) {
+            perror("ioctl0");
+            exit(1);
+    }
+
+    if (ioctl(fd, UI_SET_KEYBIT, BTN_LEFT) == -1) {
+            perror("ioctl0.1");
+            exit(1);
+    }
+
+    if (ioctl(fd, UI_SET_EVBIT, EV_REL) == -1) {
+            perror("ioctl1");
+            exit(1);
+    }
+
+    if (ioctl(fd, UI_SET_RELBIT, REL_X) == -1) {
+            perror("ioctl2");
+            exit(1);
+    }
+
+    if (ioctl(fd, UI_SET_RELBIT, REL_Y) == -1) {
+            perror("ioctl3");
+            exit(1);
+    }
+
+    /* ...device setup, device create... */
+
+    /* Give some time for the Window Manager to get events of the new virtual device */
+    sleep(1);
+
+    /* moves the mouse diagonally, 5 units per axis */
+    while (i--) {
+                emit_rel(REL_X, 5, 0);
+                emit_rel(REL_Y, 5, 1);
+                usleep(15000);
+    }
+
+    /* device destroy, device close */
+    return 0;
+
+3.0 uinput old interface
+------------------------
+
+Before kernel 4.5, uinput didn't have an ioctl to setup a virtual device. When running a version prior to 4.5, the user needs to fill a different struct and call write on the uinput file descriptor.
+
+.. code-block:: c
+
+        /* add include of uinput header */
+        struct uinput_user_dev uud;
+
+        /* open uinput device, and set the proper events */
+
+        memset(&uud, 0 sizeof(uud));
+        snprintf(uud.name, UINPUT_MAX_NAME_SIZE, "uinput_old_style");
+        write(fd, &uud, sizeof(uud));
+
+        /* call DEV_CREATE ioctl, and emit the events */
-- 
2.9.3

Re: [PATCH] Documentation: Input: Add uinput documentation

[Peter Hutterer]

Thanks for this, I'm getting enough questions about this, so it's nice to
have a link :)

First comment: I don't think rst requires unwrapped lines, so please break
those up.

Second comment: I'd really like to have a link to libevdev here. It has a
uinput interface that's a bit more obvious and takes some of the guesswork
out. While it's good to have documentation for the kernel module,
application authors should really use libevdev's uinput interface.

On Tue, Mar 21, 2017 at 11:58:17PM -0300, Marcos Paulo de Souza wrote:
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
>  Documentation/input/uinput.rst | 158 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 158 insertions(+)
>  create mode 100644 Documentation/input/uinput.rst
> 
> diff --git a/Documentation/input/uinput.rst b/Documentation/input/uinput.rst
> new file mode 100644
> index 0000000..8d59c98
> --- /dev/null
> +++ b/Documentation/input/uinput.rst
> @@ -0,0 +1,158 @@
> +=============
> +uinput module
> +=============
> +
> +Introduction
> +============
> +
> +uinput is a kernel module that makes possible create and handle input

typo: "that makes it possible to ..."

> devices from userspace. By using /dev/uinput (or /dev/input/uinput), a
> process can create virtual devices and emit events like key pressing,
> mouse movements and joystick buttons.

I'd say something like this: "By writing to the module's /dev/uinput (or
/dev/input/uinput) file, a process can create a virtual device with specific
capabilities. Once created, the process can send events through that virtual
device."

> +
> +Interface
> +=========
> +
> +::
> +
> +  linux/uinput.h
> +
> +The uinput header defines ioctl request keys to create, setup and destroy virtual devices, along with ioctls specific to uinput devices, like enabling events and keys to be send to the kernel.

'request keys' - is this the official name for ioctl numbers? If not, let's
just use "define ioctls" or "ioctl numbers" or something, because the term
"keys" is heavily overloaded. And anything after "along with... " is
superfluous.

> +
> +Examples
> +========
> +
> +1.0 Keyboard events
> +-------------------
> +
> +This first example shows how to create a new virtual device and how to
> send a key event as well as a physical keyboard. All default imports and

"as well as" in english usually means "in addition". Just skip the part
after "send a key event".

> error handlers were removed for the sake of simplicity.
> +
> +.. code-block:: c
> +
> +   #include <linux/uinput.h>
> +
> +   int fd;
> +
> +   void emit(int type, int code, int val)
> +   {
> +        struct input_event ie;

empty line here.

> +        memset(&ie, 0, sizeof(ie));
> +        ie.type = type;
> +        ie.code = code;
> +        ie.value = val;
> +

memset followed by three out of five filled in seems strange. Just add
      ie.time.tv_sec = 0;
      ie.time.tv_usec = 0;

ideally, with a comment that states that the timestamp is ignored :)

> +        if (write(fd, &ie, sizeof(ie)) < 0) {
> +                perror("write2");
> +                exit(1);
> +        }
> +   }
> +
> +   int main() {
> +        struct input_id uid;
> +        struct uinput_setup usetup;
> +
> +        fd = open("/dev/uinput", O_WRONLY | O_NONBLOCK);

Empty line here to separate the open from the actual setup. And a comment
explaining what this does wouldn't go amiss.


> +        ioctl(fd, UI_SET_EVBIT, EV_KEY);
> +        ioctl(fd, UI_SET_KEYBIT, KEY_SPACE);
> +
> +        memset(&uid, 0, sizeof(iod));
> +        memset(&usetup, 0, sizeof(usetup));
> +        usetup.id = uid;

this is a bit strange - you're memsetting the id field anyway with the
usetup memset - it's superfluous. Given this is supposed to be example code,
something immediately obvious would help:
   usetup.id.bustype = BUS_USB;
   usetup.id.vendor = 0x1234; /* sample vendor */
   ... 

> +        strcpy(usetup.name, "ex_device");

Surely we have enough bytes to name this "Example device" for obviousness :)

> +
> +        ioctl(fd, UI_DEV_SETUP, &usetup);
> +        ioctl(fd, UI_DEV_CREATE);
> +
> +        /* wait some time until the Window Manager can get the reference for the
> +         * new virtual device to receive data from
> +         * */
> +        sleep(1);

This needs to be more generic, because the WM is the last thing that
actually cares about the device.

"UI_DEV_CREATE causes the kernel to create the device nodes for this device.
Insert a pause so that userspace has time to detect, initialize the new
device, and can start to listen to events from this device."

> +
> +        /* send key press, report the event, send key release, and report again */
> +        emit(EV_KEY, KEY_SPACE, 1);
> +        emit(EV_SYN, SYN_REPORT, 0);
> +        emit(EV_KEY, KEY_SPACE, 0);
> +        emit(EV_SYN, SYN_REPORT, 0);

UI_DEV_DESTROY is missing

> +
> +        close(fd);
> +
> +        return 0;
> +   }
> +
> +2.0 Mouse movements
> +-------------------
> +
> +This example shows how to create a virtual device who behaves like a physical mouse.

s/who/that/

> +
> +.. code-block:: c
> +
> +    int i = 50;
> +
> +    /* emit function is the same of the example above */

s/the same of/identical to/

> +
> +    void emit_rel(int code, int val, int syn)
> +    {
> +            emit(EV_REL, code, val);
> +            if (syn)
> +                    emit(EV_SYN, SYN_REPORT, 0);
> +    }

No to this bit, see below ***

> +
> +    /* ...open uinput file as shown in the previous example... */
> +
> +    /* enable mouse button left and relative events. This makes the Window Manager to interpret this
> +     * device as a physical mouse
> +     */

skip the second sentence, it's not accurate enough

> +    if (ioctl(fd, UI_SET_EVBIT, EV_KEY) == -1) {
> +            perror("ioctl0");
> +            exit(1);
> +    }

you didn't check for errors above, but you're doing so here. This is a tad
confusing, especially with the ioctl0.1 naming. Just skip the error checking
for this example as well.

> +
> +    if (ioctl(fd, UI_SET_KEYBIT, BTN_LEFT) == -1) {
> +            perror("ioctl0.1");
> +            exit(1);
> +    }
> +
> +    if (ioctl(fd, UI_SET_EVBIT, EV_REL) == -1) {
> +            perror("ioctl1");
> +            exit(1);
> +    }
> +
> +    if (ioctl(fd, UI_SET_RELBIT, REL_X) == -1) {
> +            perror("ioctl2");
> +            exit(1);
> +    }
> +
> +    if (ioctl(fd, UI_SET_RELBIT, REL_Y) == -1) {
> +            perror("ioctl3");
> +            exit(1);
> +    }
> +
> +    /* ...device setup, device create... */

you're skipping 5 lines or so here, at the cost of making the example not
self-contained...

> +
> +    /* Give some time for the Window Manager to get events of the new virtual device */
> +    sleep(1);

same as in the other example

> +
> +    /* moves the mouse diagonally, 5 units per axis */
> +    while (i--) {
> +                emit_rel(REL_X, 5, 0);
> +                emit_rel(REL_Y, 5, 1);

*** (continuation): just use emit three times for x/y and syn. the helper
function does almost nothing but make the code more convoluted.

> +                usleep(15000);
> +    }
> +
> +    /* device destroy, device close */

same here, this is 2 lines you're skipping here...

> +    return 0;
> +
> +3.0 uinput old interface
> +------------------------
> +
> +Before kernel 4.5, uinput didn't have an ioctl to setup a virtual device. When running a version prior to 4.5, the user needs to fill a different struct and call write on the uinput file descriptor.

this was merged in 4.5 (can't remember, didn't check) but the version
that matters is what's returned from UI_GET_VERSION (version 5). Though then
you should add that UI_GET_VERSION itself wasn't added until version 5 :)

> +
> +.. code-block:: c
> +
> +        /* add include of uinput header */
> +        struct uinput_user_dev uud;
> +
> +        /* open uinput device, and set the proper events */
> +
> +        memset(&uud, 0 sizeof(uud));
> +        snprintf(uud.name, UINPUT_MAX_NAME_SIZE, "uinput_old_style");
> +        write(fd, &uud, sizeof(uud));
> +
> +        /* call DEV_CREATE ioctl, and emit the events */

again, a complete example should be the minimum here, otherwise it's too
hard to piece things together.

Cheers,
   Peter

Re: [PATCH] Documentation: Input: Add uinput documentation

[Marcos Paulo de Souza]

Hi Peter,

first of all, thanks a lot for reading this patch so quickly and to
point a lot of things to make this doc way better.

See some notes below.

On Wed, Mar 22, 2017 at 02:03:31PM +1000, Peter Hutterer wrote:
> Thanks for this, I'm getting enough questions about this, so it's nice to
> have a link :)
> 
> First comment: I don't think rst requires unwrapped lines, so please break
> those up.
> 
> Second comment: I'd really like to have a link to libevdev here. It has a
> uinput interface that's a bit more obvious and takes some of the guesswork
> out. While it's good to have documentation for the kernel module,
> application authors should really use libevdev's uinput interface.
> 
> On Tue, Mar 21, 2017 at 11:58:17PM -0300, Marcos Paulo de Souza wrote:
> > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> > ---
> >  Documentation/input/uinput.rst | 158 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 158 insertions(+)
> >  create mode 100644 Documentation/input/uinput.rst
> > 
> > diff --git a/Documentation/input/uinput.rst b/Documentation/input/uinput.rst
> > new file mode 100644
> > index 0000000..8d59c98
> > --- /dev/null
> > +++ b/Documentation/input/uinput.rst
> > @@ -0,0 +1,158 @@
> > +=============
> > +uinput module
> > +=============
> > +
> > +Introduction
> > +============
> > +
> > +uinput is a kernel module that makes possible create and handle input
> 
> typo: "that makes it possible to ..."

Fixed here.

> 
> > devices from userspace. By using /dev/uinput (or /dev/input/uinput), a
> > process can create virtual devices and emit events like key pressing,
> > mouse movements and joystick buttons.
> 
> I'd say something like this: "By writing to the module's /dev/uinput (or
> /dev/input/uinput) file, a process can create a virtual device with specific
> capabilities. Once created, the process can send events through that virtual
> device."

Much better, fixed.

> 
> > +
> > +Interface
> > +=========
> > +
> > +::
> > +
> > +  linux/uinput.h
> > +
> > +The uinput header defines ioctl request keys to create, setup and destroy virtual devices, along with ioctls specific to uinput devices, like enabling events and keys to be send to the kernel.
> 
> 'request keys' - is this the official name for ioctl numbers? If not, let's
> just use "define ioctls" or "ioctl numbers" or something, because the term
> "keys" is heavily overloaded. And anything after "along with... " is
> superfluous.

I prefer the "define ioctls", I don't remember where I found that "keys"
definition tough...

> 
> > +
> > +Examples
> > +========
> > +
> > +1.0 Keyboard events
> > +-------------------
> > +
> > +This first example shows how to create a new virtual device and how to
> > send a key event as well as a physical keyboard. All default imports and
> 
> "as well as" in english usually means "in addition". Just skip the part
> after "send a key event".

I think you now know that I'm not an English native speaker :)
Fixed here.

> 
> > error handlers were removed for the sake of simplicity.
> > +
> > +.. code-block:: c
> > +
> > +   #include <linux/uinput.h>
> > +
> > +   int fd;
> > +
> > +   void emit(int type, int code, int val)
> > +   {
> > +        struct input_event ie;
> 
> empty line here.

Fixed.

> 
> > +        memset(&ie, 0, sizeof(ie));
> > +        ie.type = type;
> > +        ie.code = code;
> > +        ie.value = val;
> > +
> 
> memset followed by three out of five filled in seems strange. Just add
>       ie.time.tv_sec = 0;
>       ie.time.tv_usec = 0;
> 
> ideally, with a comment that states that the timestamp is ignored :)

All the code in this doc is the result of my tests using uinput, so
somethings were set in my code some time ago and were never touched
again. Yes, this makes things a way better :)

> 
> > +        if (write(fd, &ie, sizeof(ie)) < 0) {
> > +                perror("write2");
> > +                exit(1);
> > +        }
> > +   }
> > +
> > +   int main() {
> > +        struct input_id uid;
> > +        struct uinput_setup usetup;
> > +
> > +        fd = open("/dev/uinput", O_WRONLY | O_NONBLOCK);
> 
> Empty line here to separate the open from the actual setup. And a comment
> explaining what this does wouldn't go amiss.

Very good, fixed here.
> 
> 
> > +        ioctl(fd, UI_SET_EVBIT, EV_KEY);
> > +        ioctl(fd, UI_SET_KEYBIT, KEY_SPACE);
> > +
> > +        memset(&uid, 0, sizeof(iod));
> > +        memset(&usetup, 0, sizeof(usetup));
> > +        usetup.id = uid;
> 
> this is a bit strange - you're memsetting the id field anyway with the
> usetup memset - it's superfluous. Given this is supposed to be example code,
> something immediately obvious would help:
>    usetup.id.bustype = BUS_USB;
>    usetup.id.vendor = 0x1234; /* sample vendor */

Good. Fixed.
>    ... 
> 
> > +        strcpy(usetup.name, "ex_device");
> 
> Surely we have enough bytes to name this "Example device" for obviousness :)
Sure, fixed :)
> 
> > +
> > +        ioctl(fd, UI_DEV_SETUP, &usetup);
> > +        ioctl(fd, UI_DEV_CREATE);
> > +
> > +        /* wait some time until the Window Manager can get the reference for the
> > +         * new virtual device to receive data from
> > +         * */
> > +        sleep(1);
> 
> This needs to be more generic, because the WM is the last thing that
> actually cares about the device.
> 
> "UI_DEV_CREATE causes the kernel to create the device nodes for this device.
> Insert a pause so that userspace has time to detect, initialize the new
> device, and can start to listen to events from this device."

Fixed.

> 
> > +
> > +        /* send key press, report the event, send key release, and report again */
> > +        emit(EV_KEY, KEY_SPACE, 1);
> > +        emit(EV_SYN, SYN_REPORT, 0);
> > +        emit(EV_KEY, KEY_SPACE, 0);
> > +        emit(EV_SYN, SYN_REPORT, 0);
> 
> UI_DEV_DESTROY is missing
Fixed.

> 
> > +
> > +        close(fd);
> > +
> > +        return 0;
> > +   }
> > +
> > +2.0 Mouse movements
> > +-------------------
> > +
> > +This example shows how to create a virtual device who behaves like a physical mouse.
> 
> s/who/that/
Fixed.

> 
> > +
> > +.. code-block:: c
> > +
> > +    int i = 50;
> > +
> > +    /* emit function is the same of the example above */
> 
> s/the same of/identical to/
Fixed.

> 
> > +
> > +    void emit_rel(int code, int val, int syn)
> > +    {
> > +            emit(EV_REL, code, val);
> > +            if (syn)
> > +                    emit(EV_SYN, SYN_REPORT, 0);
> > +    }
> 
> No to this bit, see below ***
> 
> > +
> > +    /* ...open uinput file as shown in the previous example... */
> > +
> > +    /* enable mouse button left and relative events. This makes the Window Manager to interpret this
> > +     * device as a physical mouse
> > +     */
> 
> skip the second sentence, it's not accurate enough
Fixed.

> 
> > +    if (ioctl(fd, UI_SET_EVBIT, EV_KEY) == -1) {
> > +            perror("ioctl0");
> > +            exit(1);
> > +    }
> 
> you didn't check for errors above, but you're doing so here. This is a tad
> confusing, especially with the ioctl0.1 naming. Just skip the error checking
> for this example as well.

I was in a hurry yesterday, so it went this way...
Fixed.

> 
> > +
> > +    if (ioctl(fd, UI_SET_KEYBIT, BTN_LEFT) == -1) {
> > +            perror("ioctl0.1");
> > +            exit(1);
> > +    }
> > +
> > +    if (ioctl(fd, UI_SET_EVBIT, EV_REL) == -1) {
> > +            perror("ioctl1");
> > +            exit(1);
> > +    }
> > +
> > +    if (ioctl(fd, UI_SET_RELBIT, REL_X) == -1) {
> > +            perror("ioctl2");
> > +            exit(1);
> > +    }
> > +
> > +    if (ioctl(fd, UI_SET_RELBIT, REL_Y) == -1) {
> > +            perror("ioctl3");
> > +            exit(1);
> > +    }
> > +
> > +    /* ...device setup, device create... */
> 
> you're skipping 5 lines or so here, at the cost of making the example not
> self-contained...
Yes, horrible. All examples are now self-contained, just mentioning the
emit function in the first example.
> 
> > +
> > +    /* Give some time for the Window Manager to get events of the new virtual device */
> > +    sleep(1);
> 
> same as in the other example
Fixed.

> 
> > +
> > +    /* moves the mouse diagonally, 5 units per axis */
> > +    while (i--) {
> > +                emit_rel(REL_X, 5, 0);
> > +                emit_rel(REL_Y, 5, 1);
> 
> *** (continuation): just use emit three times for x/y and syn. the helper
> function does almost nothing but make the code more convoluted.
yes, much more clear. Fixed.
> 
> > +                usleep(15000);
> > +    }
> > +
> > +    /* device destroy, device close */
> 
> same here, this is 2 lines you're skipping here...
> 
> > +    return 0;
> > +
> > +3.0 uinput old interface
> > +------------------------
> > +
> > +Before kernel 4.5, uinput didn't have an ioctl to setup a virtual device. When running a version prior to 4.5, the user needs to fill a different struct and call write on the uinput file descriptor.
> 
> this was merged in 4.5 (can't remember, didn't check) but the version
> that matters is what's returned from UI_GET_VERSION (version 5). Though then
> you should add that UI_GET_VERSION itself wasn't added until version 5 :)

Nice, I will add some code testng the uinput version. Thanks for the
suggestion!

> 
> > +
> > +.. code-block:: c
> > +
> > +        /* add include of uinput header */
> > +        struct uinput_user_dev uud;
> > +
> > +        /* open uinput device, and set the proper events */
> > +
> > +        memset(&uud, 0 sizeof(uud));
> > +        snprintf(uud.name, UINPUT_MAX_NAME_SIZE, "uinput_old_style");
> > +        write(fd, &uud, sizeof(uud));
> > +
> > +        /* call DEV_CREATE ioctl, and emit the events */
> 
> again, a complete example should be the minimum here, otherwise it's too
> hard to piece things together.
yes, much better this way.

> 
> Cheers,
>    Peter

I fixed a lot of things today, the things that are still missing are the
libevdev example, and the version check. I do think that I can send a
new version tomorrow.

So, thanks again for your time reviewing this doc!

-- 
Thanks,
	Marcos

Re: [PATCH] Documentation: Input: Add uinput documentation

[Peter Hutterer]

On Wed, Mar 22, 2017 at 11:54:48PM -0300, Marcos Paulo de Souza wrote:
> Hi Peter,
> 
> first of all, thanks a lot for reading this patch so quickly and to
> point a lot of things to make this doc way better.
> 
> See some notes below.

thanks for all the fixes, much appreciated.
just two comments below:

> On Wed, Mar 22, 2017 at 02:03:31PM +1000, Peter Hutterer wrote:
[...]
> > > +        memset(&ie, 0, sizeof(ie));
> > > +        ie.type = type;
> > > +        ie.code = code;
> > > +        ie.value = val;
> > > +
> > 
> > memset followed by three out of five filled in seems strange. Just add
> >       ie.time.tv_sec = 0;
> >       ie.time.tv_usec = 0;
> > 
> > ideally, with a comment that states that the timestamp is ignored :)
> 
> All the code in this doc is the result of my tests using uinput, so
> somethings were set in my code some time ago and were never touched
> again. Yes, this makes things a way better :)

note that if we ship this as documentation, these become the official
examples so they *have* to be correct. How many times have you copied
something from the examples of a library? Not ideal if there's a bug or just
messy code to begin with :)

> I fixed a lot of things today, the things that are still missing are the
> libevdev example, and the version check. I do think that I can send a
> new version tomorrow.

As for libevdev: just add a link to the documentation, don't add a libevdev
example. libevdev should (and does) provide the examples and you don't want
to ship example code that relies on some other library' API.

Cheers,
   Peter

Re: [PATCH] Documentation: Input: Add uinput documentation

[Marcos Paulo de Souza]

On Thu, Mar 23, 2017 at 01:23:56PM +1000, Peter Hutterer wrote:
> On Wed, Mar 22, 2017 at 11:54:48PM -0300, Marcos Paulo de Souza wrote:
> > Hi Peter,
> > 
> > first of all, thanks a lot for reading this patch so quickly and to
> > point a lot of things to make this doc way better.
> > 
> > See some notes below.
> 
> thanks for all the fixes, much appreciated.
> just two comments below:
> 
> > On Wed, Mar 22, 2017 at 02:03:31PM +1000, Peter Hutterer wrote:
> [...]
> > > > +        memset(&ie, 0, sizeof(ie));
> > > > +        ie.type = type;
> > > > +        ie.code = code;
> > > > +        ie.value = val;
> > > > +
> > > 
> > > memset followed by three out of five filled in seems strange. Just add
> > >       ie.time.tv_sec = 0;
> > >       ie.time.tv_usec = 0;
> > > 
> > > ideally, with a comment that states that the timestamp is ignored :)
> > 
> > All the code in this doc is the result of my tests using uinput, so
> > somethings were set in my code some time ago and were never touched
> > again. Yes, this makes things a way better :)
> 
> note that if we ship this as documentation, these become the official
> examples so they *have* to be correct. How many times have you copied
> something from the examples of a library? Not ideal if there's a bug or just
> messy code to begin with :)

I think there is a misunderstanding :)

I didn't just copied the code, I built it using other's code as
reference, and it's working. What I meant was, once the initialization
code worked, I just took it and used and other samples.

I do hope this code can help others like me who would like to test
uinput :)

> > I fixed a lot of things today, the things that are still missing are the
> > libevdev example, and the version check. I do think that I can send a
> > new version tomorrow.
> 
> As for libevdev: just add a link to the documentation, don't add a libevdev
> example. libevdev should (and does) provide the examples and you don't want
> to ship example code that relies on some other library' API.

OK, good enough. Will work on itand send a new version with the
suggested changes.

Thanks,
	Marcos

> 
> Cheers,
>    Peter