Re: [PATCH v3 1/7] rc: rc-ir-raw: Add scancode encoder callback

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2015-05-23 13:34, Antti Seppälä wrote:
On 22 May 2015 at 13:33, David Härdeman <david@xxxxxxxxxxx> wrote:
On 2015-05-22 07:27, Antti Seppälä wrote:
I think that approach too is far from perfect as it leaves us with
questions such as: How do we let the user know what variant of
protocol the label "rc-6" really means? If in nvt we hardcode it to
mean RC6-0-16 and a new driver cames along which chooses
RC_TYPE_RC6_6A_24 how do we tell the user that the implementations
differ? What if the scancode that was fed was really RC_TYPE_RC6_MCE?


I never claimed it was perfect.

For another (short-term, per-driver) solution, see the winbond-cir driver.


Heh, funny you should mention that... Back in late 2013/early 2014 I
submitted a patch for nuvoton which was modeled after winbond-cir. The
feedback from that could be summarized as:
 - Scancodes should be used instead of raw pulse/spaces (the initial
version of the patch worked without encoding)
 - Encoders should be generalized for others to use them too
 - Sysfs -api should be used instead of module parameters

So the suggestions were a pretty much the exact opposite of what
winbond-cir does.

Again, it was a short-term suggestion. A long-term "real" solution requires a definitive identification of the intended protocol.

One idea that I've had in the back of my head for a long time is to use the "flags" member of "struct rc_keymap_entry" in the new EVIOC[GS]KEYCODE_V2 ioctl variant (see http://www.spinics.net/lists/linux-media/msg88452.html).

If a RC_POWERON flag was defined, it could be used for that purpose...

It's entirely broken in the sense that a user has no idea of knowing if the hardware has been properly configured to wake the computer or not. It's just as broken as the connect() system call would be if it randomly rewrote the destination address passed by the user, optionally connected, and most of
the time returned zero independently of the result.


I think you may be misunderstanding the sysfs api or at least the
connect() analogue here as well as the userspace-kernelspace example
above are actually not how the api works. Remember that userspace does
not know about the protocol variants.

Userspace most definately does. Keymaps are a good example of that. Distributing keymaps for a certain remote should be possible.

Let me try to use your example and work-out how it really goes:
* Userspace: please set the hardware to wake up if the scancode
0x800f040c is received. I know this is RC6 scancode because it came
from the rc-6 decoder but I don't know the variant (and I don't really
care)

First of all...you assume that the only way of generating a valid wakeup scancode is by using the same remote on the same hardware first to see what it generates (which - thanks to things like boneheaded decisions on NEC scancode generation and the previously mentioned heuristics - may differ). So distributing a keymap from a central repository or just looking up scancodes from a vendor datasheet is no longer feasible with this API.

Second, you have absolutely nothing that ensures that the same heuristics are used in the decode/encode code paths and that the heuristics will remain in sync.

* Kernel: Ok (btw. based on the length of the scancode and the
bit-pattern in the front I've determined this to be rc6-mce type
scancode and encoded it accordingly)
* Userspace: Got it

The whole "btw" part won't be passed to userspace...so there's nothing to "get"

So no changing anything behind users back or not leading to
misconfigured hardware AFAICT.
...
I'm sorry that the encoding functionality clashes with your intentions
of improving the rc-core. I guess Mauro likes encoders more than
improving rc-core fundamentals :)
Kidding aside the fact that this series got merged might suggest that
you and Mauro don't necessarily share the same views about the future
and possible api breaks of rc-core.

If you've followed the development of rc-core during the last few years it should be pretty clear that Mauro has little to no long-term plan, understanding of the current issues or willingness to fix them. I wouldn't read too much into the fact that the code was merged.

Tell you what, I'll agree to reverting the series. In exchange I would
hope that you and Mauro mutually agree and let me know on:
 - What are the issues that need to be fixed in the encoding series
prefarably with how to fix them (e.g. module load order ambiquity,
whether a new api is needed, or switching to a more limited
functionality is desired like you suggested then so be it etc.)
 - When is a good chance to re-submit the series (e.g. after
ioctl/scancode/whatever api break is done or some pending series is
merged or some other core refactoring work is finished etc.)

Deal?

Mauro....wake up? I hope you're not planning to push the current code upstream???


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux