#84 Problems with mplay2 plugin

[Brian Moser]

The mplay2 plugin for the Moneual MonCaso 312/320 pc cases does not work correctly. Some keys (for example REC, code 0x11) are, as far as I understand it, recognized as control characters and then ignored. Also the volume knob is not supported.

There is a pretty old thread at ubuntuforums.org (http://ubuntuforums.org/showthread.php?t=1536934&p=12120799#post12120799) where the user Womiha provides a fix (which I tested and works), but it was never added to lirc.

[Alec Leamas]

Now, the first problem is that this patch does not apply. It's against 0.9.0, but current master is 0.9.2. There are huge changes in between: new filenames, dynamic drivers, modified driver API, build system changes.

That said, the core logic of what's now in plugins/mplay2.c is the same so it should be possible to rebase the patch without changing the logic. But it's certainly some work.

Note that there isn't any mplay.h any more; all required parts are included in mplay.c

You won't need to modify configure.ac for this. The specific libs required could (and should) be handled in plugins/Makefile.am.

Sorry for not spotting this at the first glance. Should have, for sure.

[Brian Moser]

The file lirc-0.9.0.patch.gz is the original patch by womiha. I attached an adapted file which works with the most recent git version of lirc. Like you said just one file had to be changed, the work wasn't all that bad.

[Alec Leamas]

Hm... now things looks more sane :)

Still: don't we need to patch plugins/Makefile.am so this is linked with -lpthread?

EDIT: All drivers are linked with -lphtread, my bad. No need to patch Makefile.am.

Otherwise, this basically looks fine.

[Alec Leamas]

Some comments needs a clean-up; I can fix that.

We try to conform to the kernel formatting rules (see CONTRIBUTE.md). Can you fix the long lines?

On a 64-bit host I get compilations warnings:

mplay.c: In function 'mplayfamily_listen_cleanup':
mplay.c:313:8: warning: cast from pointer to integer of different size [-Wpointer- to-int-cast]
close((int) arg);
    ^
In file included from mplay.c:51:0:
mplay.c: In function 'mplayfamily_listen':
mplay.c:536:51: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
pthread_cleanup_push(mplayfamily_listen_cleanup, (void*) fd);

[Brian Moser]

I changed the formatting so that no line is longer than 80 characters. I liked the previous version better, but OK.

I think I fixed the 64bit compilation warnings, but I don't have a 64bit system to check.

[Alec Leamas]

OK, I have pushed a feature branch mplay on sourceforge (please let me know if you need help to access it).

BTW, the formatting rules are certanly not my preferences. It's just what we use here. I really hate the 8 spaces/tab indent...

I have removed 135 lines with trailing whitespace (!), removed a large, bad comment block and made some minor mods to the other comments (which are really good, way better than almost all other drivers). If it's OK with you, I'm ready to merge it.

Absolutely no blocker, but if you could shed some light in the difference from a user perspective on the differences between the mplay and mplay2 drivers in the .info field it would be great.

EDIT: The 64-bit compilation is fine.

[Brian Moser]

I checked the file from the mplay branch, I think we are ready for the merge.

I did some research about the difference between mplay and mplay2, this is how I understand it:
The initial mplay driver is from Benoit Laurent, written in 2007 for a Zalman Hd135 case. In 2010 basic support was added for the Moneual Moncaso 312 (this is the case I own) by 123justme; see commit 06bbae8e74. It says in the commit message that the mplay2 device was added to avoid messing with already supported devices. The support for the Moncaso 312 wasn't perfect though. The volume knob and some buttons on the remote didn't work. Womiha fixed those problems, but his code never got added to lirc.
The main differences in the code between mplay and mplay2 are apparently the baud rate and an extra initialization step for mplay2.

[Alec Leamas]

I updated the .info fields with your findings about differences mplay/mplay2.

Patch merged as [288700].

If you want to check what the .info fields look like, lirc-lsplugins is the tool.