Re: [RFC PATCH 0/6] Send and receive decoded IR using lirc interface

["David Härdeman" <david@hardeman.nu>]

On Fri, Apr 03, 2015 at 11:11:30AM +0100, Sean Young wrote:
>On Thu, Apr 02, 2015 at 12:19:41AM +0200, David Härdeman wrote:
>> On Tue, Mar 31, 2015 at 08:47:16PM -0300, Mauro Carvalho Chehab wrote:
>> >Em Mon, 30 Mar 2015 23:18:19 +0200
>> >David Härdeman <david@hardeman.nu> escreveu:
>> >> And on a much more general level...I still think we should start from
>> >> scratch with a rc specific chardev with it's own API that is generic
>> >> enough to cover both scancode / raw ir / future / other protocols (not
>> >> that such an undertaking would preclude adding stuff to the lirc API of
>> >> course).
>> >
>> >Starting from scratch sounds a bad idea. We don't do that on Linux,
>> >except when we really screwed everything very badly.
>> 
>> LIRC...IR specific....rc-core....not IR specific...and the lirc IOCTL
>> API is pretty badly screwed. Have you had a closer look at it?
>
>LIRC is IR specific, yes. If something else comes along we can think
>about something new, but not before that.

Yeah, I'm not saying we should throw out lirc now or not improve on
it...not at all.

>> I'm not saying we should throw away the lirc module/device, it'll have
>> to stay around for a long time. But we should design a v2.
>
>What is wrong with lirc that requires a redesign?

I'll defer that discussion to a separate thread. It's really not related
to your patches (sorry).

>> If you've looked at my patches the idea is basically:
>> 
>> RC device is plugged in, /dev/rc/rc0 is created by udev
>> 
>> Applications wishing to muck about with RC devices do:
>> 
>> 	int fd;
>> 	int ver;
>> 	int type;
>> 
>> 	fd = open("/dev/rc/rc0")
>> 
>> 	ver = ioctl(fd, RCIOCGVERSION);
>> 	if (ver != 1)
>> 		warn("New RC API version");
>> 
>> 	type = ioctl(fd, RCIOCGVERSION);
>
>lirc uses feature bits. As we know from filesystems features are much
>better than versioning. I'm sure there are other examples.

My implementation supports feature bits (flags).

>> 	switch (type) {
>> 	case RC_DRIVER_SCANCODE:
>> 		debug("Scancode hardware");
>> 		break;
>> 	case RC_DRIVER_IR_RAW:
>> 		debug("Raw IR hardware");
>> 		break;
>> 	default:
>> 		debug("Unknown hardware type");
>> 		break;
>> 	}
>
>lirc_zilog can send both raw IR and scancodes (although I don't know how
>to do raw IR yet). So with lirc you would do:
>
>	unsigned features;
>
>	ioctl(fd, LIRC_GET_FEATURES, &features);
>
>	if (features & LIRC_CAN_SEND_MODE2) 
>		// can send raw IR
>
>	if (features & LIRC_CAN_SEND_SCANCODE) // needs my patches
>		// can send scancodes

That's something to keep in mind, thanks.

>> And then they can do further operations depending on the type of the
>> device. For example, for raw IR devices you can read() raw IR
>> pulse/space timings or (if the hardware supports TX) write() raw IR
>> timings.
>> 
>> Other examples of ioctls are (all four work using structs with all the
>> relevant parameters):
>> 
>> * RCIOCSIRRX
>> 	set all RX parameters in one go (and return the result since
>> 	the exact values requested might not be supported)
>
>Putting everything in one big struct isn't really future-proof, and it
>doesn't tell you which parts are supported by the hardware.

It's reasonably "future proof" for three reasons:

1) Microsoft has a CIR API...with RX/TX structs for setting the
parameters, that will in itself tend to make sure that hardware sticks
to what's supported by that API

2) If you look at the evolution of the LIRC API, it's pretty clear that
completely new hardware capabilities aren't really being developed at a
brisk pace. The capabilities and parameters that need to be set have
remained fairly static for a long time.

3) My proposed structs include explicit support for flags as well as
reserved struct members (which help future extensibility) and new
ioctls could be added as a fallback.

>> * RCIOCSIRTX
>> 	same as RCIOCSIRRX but for TX
>
>That would have to be a different struct for RX and TX.
>
>RX is different from TX. You want to set a carrier range for RX, and 
>a specific carrier for TX. You want a duty cycle for TX but not for RX,
>carrier reports for RX but that makes no sense for TX.

It is two different structs in the implementation that I've posted

>
>> * RCIOCGIRRX
>> 	get all RX parameters
>> * RCIOCGIRTX
>> 	get all TX parameters
>> 
>> These ioctls only work with RC_DRIVER_IR_RAW hardware. Others can be
>> defined for other kinds of hardware.
>
>The cec patches going round at the moment create their own character 
>devices. rc-core is only a side note in that system; IR and CEC are
>so widely different it doesn't really make sense to share character
>devices.

Maybe so. I haven't looked at the proposed CEC API in detail. But IIRC
CEC has IR passthrough capabilities so it's not obvious that they'll be
completely independent...?

>> Then there's one more thing, and that's multiple keytables per rc
>> device. Each keytable has one associated input device (so there's a
>> 1-to-N mapping between rc devices and input devices). Userspace can
>> create/destroy additional keytables and add/remove scancode<->keycode
>> mappings per keytable. The idea is that you'd be able to e.g. define one
>> keytable per physical remote control (the thing you hold in your hand,
>> not the receiver/transmitter), and each would get its own input device.
>> Those input devices can then be used by different applications (so you
>> could have that old VCR remote control the PVR software while the TV
>> remote controls your Kodi frontend). An idea I borrowed from Jon Smirl
>> (who posted a similar proof-of-concept based on debugfs back in the
>> days).
>
>That would be very nice thing to have, but that is a separate from this.

Agreed. And I kind of hijacked your thread. Sorry about that. I'm not
opposed to hacking on/improving the lirc interface, and I'm not opposed
to your patches. The takeaway from what I've said basically boils down to:

1) Dynamic scancode length
2) Need to sort out/agree on/standardize scancode representation before
exposing it further to userspace
3) Keeping lirc as separate from rc-core as possible (e.g. not littering
rc-core with lirc specifics) still makes sense

-- 
David Härdeman