ev3dev / ev3dev-lang

(deprecated) language bindings for ev3dev sensors, motors, LEDs, etc.
GNU General Public License v2.0
56 stars 39 forks source link

Make Button Interface `\sys` Device Based #121

Open rhempel opened 8 years ago

rhempel commented 8 years ago

While I appreciate the ability to use the standad Linux GPIO interface fo reading buttons, it is a total pain that there is not also a /sys interface to get button state.

Currently, we require an ioctl to read the button state from user space, which is fine for scripting languages like Python that have an ioctl library. But it does not help that the only peripheral on the EV3 that requires ioctl calls is the buttons, and for languages like Lua or even bash and awk we need to write a Language binding for ioctl and add it to the binding package.

I can write a button class for /sys that works pretty much like the attribute system we have today, where querying the state simply returns a list of all the pressed buttons.

Comments? Any reason why this is a particularly bad idea?

dlech commented 8 years ago

I don't see a problem with doing things this way. You should update http://www.ev3dev.org/docs/tutorials/using-ev3-buttons/ to explain how to accomplish the same thing without using an ioctl.

dlech commented 8 years ago

Wait a minute. I thought you were saying that there was already a way to do this from /sys. I say, no, don't write a button class because it will only work for the EV3 buttons. Any USB or Bluetooth keypads, keybords, gamepads, etc. will be different. Instead, you should focus on a solution that works for everything.

dlech commented 8 years ago

By the way, proper use of the LCD screen requires ioctls as well.

rhempel commented 8 years ago

Well, the LCD needs IOCTLs now, but maybe we can fix that :-)

Right now the Python API has a nice ButtonBase class that is extended to handle the remote control.

So I'm talking about fixing the EVIO Button class which is just the EV3 front panel.

The other way to do it is to write a little platform (or device) specific daemon than handles the ioctls that we care about through a sys file attribute.

I'm just throwing ideas out here - but the ioctl requirement is something I'd like to get rid of...

dlech commented 8 years ago

The linux input framework is really meant to be event based, which is why there is only an ioctl for this. You can read /dev/input/eventXand it will tell you whenever a key is pressed, no ioctls required. You can monitor this in a background thread and save key states in a variable whenever there is an event.

Another option is to use ev3dev kit. Since is uses gobject introspection, it is designed to be used by pretty much any language (except maybe bash).

rhempel commented 8 years ago

I am trying to make things as easy as possible for people getting into coding that don't think about event based frameworks as easily as "just give me a function call that returns the buttons that are pressed now" :-)

Is a platform specific daemon a reasonable approach for languages that don't support blocking file reads and multithreading?

The goal is to be able to read a file and get a list of buttons that are pressed.

dlech commented 8 years ago

You have a bit of conflict of interests here I think. To get something in /sys, you have to write a kernel driver. A system daemon is user space. But, the buttons/keys (and the framebuffer) are pretty much 100% mainline kernel code, and I know you don't want to touch that.

How many (reasonable) languages actually can't actually bind to C code or make syscalls? bash obviously, but that is borderline not a reasonable language and it is easy enough to write a utility program to check key states.

For example, a quick web search shows 2 different options that should work for Lua.

I'm totally with you that we need something simpler, but we are dealing with the most ancient parts of the linux kernel, so it is not making it easy for us.

dlech commented 8 years ago

Here's an idea. I was considering making a configfs sensor driver for userspace sensor drivers (I was mainly thinking of doing this for mindsensors.com BT sense android app). Basically you have to have a userspace program that sets up the driver via /sys/kernel/config/lego_sensor and then writes data to a sysfs attribute somewhere. This will create a sensor in the usual /sys/class/lego_sensor/.

This could be used for the EV3 buttons. We just create a program that listens to the "EV3 Buttons" input/event device and pushes the information to the "sensor". The users can read the buttons just like a touch sensor. The value0 - value5 attributes would be the 6 buttons.

What do you think?

rhempel commented 8 years ago

Yes, something like that could work - seems a bit roundabout but it's doable. Another option is to have a kernel module specifically for the EV3 buttons that does not interfere with the existing event driven button I/O. It creates a sys entry and responds to reads of the buttons_pressed attribute with a list of pressed buttons. Underneath the attribute read is a function that does the ioctl magic.

I know this is counter to the Linux input event handler, so I'm not suggesting we get rid of that - this is just another, hopefully easy way for button states to be read in a non-event capable language.

dlech commented 8 years ago

I still don't like the idea of doing something "specifically for the EV3 buttons". If we are going to put the effort into it, I think we should do something that works for all linux input devices.

If you are absolutely set on having something in sysfs, I think the most practical (and least amount of work) would be to add a state folder to /sys/class/input/inputX/ that mirrors the existing /sys/class/input/inputX/capabilities/folder except that it returns the current state.

If you want something that is only for EV3, then copy the gpi-keys module and modify it so that it exports key states. However, this won't work for PiStorms or EVB because the buttons on those platforms are not GPIOs.

But, I like the pythonic principal that there should only be one obvious way to do a thing. All of the raspberry pi blogs out there are going to tell you to read input events from /dev/input/eventX, so it seems like we should find a way to make this work for us as well.

ddemidov commented 8 years ago

One point for some universal solution (and against making a kernel module just for EV3 buttons): anything with a USB port may have a USB keyboard connected. It would be nice to be able to work with such keyboards in a generic way. I am aware that our current python API only provides interface to EV3 buttons, but as I understand, the buttons use generic event interface that could work with any kind of input devices.

rhempel commented 8 years ago

I was doing some reading and I think that a userland program that uses libev3dev is the way to go - it does not interfere with the ability to do event driven or ioctls - and I think it gives us the ability to read the EV3 buttons from a file and yet allow the standard button API to still work.

dlech commented 8 years ago

libev3dev

Do you mean libevdev or ev3devKit?

rhempel commented 8 years ago

I mean libevdev :-)

rhempel commented 8 years ago

@dlech, I am coming around to the comment you made here - except that the sensor device should probably return a list of pressed button names instead of one button per value - I'm going to experiment with libevdev to manage processing events using the normal Linux event devices, and then have it write the current state to something in /sys/...

This userland program that runs in the background might also be a handy way to manage the display interface, but that's for later...

rhempel commented 8 years ago

This comment asks the reasonable question of how many languages cannot bind to a C library - my goal is to require NO language specific C library bindings.

At worst, a userspace program can be launched to provide what I might call a "universal" translation layer for the devices that need it.

What I have now is a userspace program that uses select() to block on the standard Linux input device stream until an event like a button press is ready. The trick now is to make those events available to any application that can read a file - so we might consider creating a new virtual sensor that accepts events and reports states vis the /sys/ filesystem.

Seems roundabout, but it really helps when your minimal scripting language has no support for doing event driven programming.

dlech commented 8 years ago

I still think using configfs is the way to do this (as in https://github.com/ev3dev/ev3dev-lang/issues/121#issuecomment-157945285). You setup the virtual sensor using configfs then in /sys/class/lego-sensor/..., the bin_dataattribute will be writable. So, your userspace program will have to do the setting up in configfs and write to bin_data and then the sensor device can be used as usual.

rhempel commented 8 years ago

I think we are in violent agreement :-)

dlech commented 8 years ago

violent? :laughing:

dlech commented 8 years ago

OK, so how about I work on the configfs stuff and you handle the userspace program.

rhempel commented 8 years ago

Sure - if that works for you ... I have the userspace side roughed out (testing on my desktop). Basically the program accepts a device name that is scanned here:

root@ev3dev:~# ls /dev/input/by-path/
platform-gpio-keys.0-event  platform-snd-legoev3-event

In this case platform-gpio-keys.0-event is a symlink to /dev/input/event0 and I can just wait for events using select(). When I get an event (or get a notificatoin of a dropped event) update the static button states.

I'm concerned about the sensor values being one per button, can we have a sensor return a string of all pressed buttons (it would make our button class handler a lot easier)

dlech commented 8 years ago

The current lego-sensor class does not know about strings. In other words, valueX can't return text.

The way I was envisioning it is that if you want to read the value of one button, you read valueX. If you want to read all of the buttons at the same time, you read bin_data. These are still all numeric values though.

rhempel commented 8 years ago

So does my userland driver write the total button state table to bin_data or to each individual valueX entry?

dlech commented 8 years ago

It will write the table to bin_data. My thinking is that you just copy the value fields from struct input_event to an array and that is what you write to bin_data.

rhempel commented 8 years ago

Each input_event is exactly one event, ie timestamp, value, and state - we'd need to somehow tell the userland program which button goes with valueX - by the way, are we using bin_data to hold more than the fixed number of valueX entries? ie if a sensor can have a total of 64 different values we must read bin_data to get all of them?

rhempel commented 8 years ago

@dlech - one thing we could do is pass to the generic userland program a file that tells it which input device name or path to look for, what the configfs entry will be, and a proposed event/value mapping. Or three parameters:

evdev_to_sensor input_device_path [sensor_path mapping_file]

If sensor_path or mapping_file are missing, then the userland program does an exhaustive enumeration of the EV_KEY, EV_ABS, and EV_REL capabilities of the input device and spits out a proposed event/value mapping.

dlech commented 8 years ago

Minor point: Since the device path is /dev/input/eventX, why not call it event2sensor. evdev is just too similar to ev3dev. Also, using "2" instead of "to" is also fairly common, e.g. dos2unix.

I'm thinking that the configfs path will be the basename of input_device_path, so it doesn't need an extra parameter. If there are going to be mapping files, they should be optional and there should be a naming convention so that they can be looked up automatically (e.g. /etc/event2sensor/map.d/xyz.map) without having to pass a parameter. This way we can have one generic udev rule and one generic systemd service that works for all devices.

http://www.freedesktop.org/software/systemd/man/udev.html lists the variables available from udev. I'm thinking $devpath will be your only parameter, but you can have a look for yourself.

rhempel commented 8 years ago

I think we are converging on the right solution!

rhempel commented 8 years ago

Mapping file is optional - without it we need to scan this many events to see if the device handles them:

Max EV_KEY is 767
Max EV_ABS is 63
Max EV_REL is 15
dlech commented 8 years ago

Have a look at /sys/class/input/event0/device/capabilities/. This enumerates what keys/axis/etc. are available. There are also ioctls that do the same thing, which might be better since they put the data in a nice struct for you.

rhempel commented 8 years ago

Nice pointer - I knew there had to be a better way! So on the EV3:

root@ev3dev:~# cat /sys/class/input/event0/device/capabilities/key
1680 0 0 10004000

correspond nicely with the values here. That gives us 4 64-bit values - and the rightmost bit of the rightmost value is 0 in the value field of the event type.

rhempel commented 8 years ago

OK, now that I'm looking at the actual current libevdev API, I can do this all without reading a bunch of files in /sys/class/input/eventX/device ... more to come

rhempel commented 8 years ago

One more idea - what about a text_data attribute and a config_data attribute for the sensor class?

The text_data for the button handler could simply be the list of pressed button names, and config_data could be the mapping of "analog" data from joysticks to the valueX attributes.

Or we could allow text data from the valueX attributes.

dlech commented 8 years ago

I'll have to do some thinking on how these would fit in with the 40+ other sensor drivers we already have.

rhempel commented 8 years ago

Or we could make a pure - configfs device that lives under /sys/class/misc/event2state.

I know we really don't want to add yet another class to the system, but this might avoid making changes to the sensor class.

dlech commented 8 years ago

This is sort of the direction I've been going. As I've been working on the, it has become clear that we need another device node in addition the the lego-sensor-class device. So, I'm making /sys/class/user-lego-sensor/sensorX. This is where the writeable bin_data attribute will be and anything else that we need in the future.

dlech commented 8 years ago

For future reference, this is how we callback from the kernel to userspace, for example if we need to implement more than one mode.

http://stackoverflow.com/questions/16067902/call-a-userspace-function-from-within-a-linux-kernel-module

rhempel commented 8 years ago

Essentially data transfer via the sys file system - the userland program should use some blocking wait mechanism like select() to avoid spinning in the read function though... Same guy as your link above ran into this :-) http://stackoverflow.com/questions/16442935/why-doesnt-this-call-to-poll-block-correctly-on-a-sysfs-device-attribute-file

dlech commented 8 years ago

I've pushed initial support for this in https://github.com/ev3dev/lego-linux-drivers/commit/af71025e2c105e6f5c336188711c7dfa8d036efc. Let's see how far you can get with it the way it is before adding any new attributes.

jippiee commented 5 years ago

Is there currently a way to read button states without the need to poll for events in the userland program?

BTW: On the EV3 /dev/input/by-path/platform-gpio-keys.0-event seems to have changed to /dev/input/by-path/platform-gpio_keys-event for me.

dlech commented 5 years ago

There is an ioctl for that.

Example: https://github.com/ev3dev/ev3dev-lang-python/blob/081ed9633b15e9c707a7638e1f570f87014b56d1/ev3dev2/button.py#L217-L270