From: Rene H. <re...@sa...> - 2008-11-10 21:15:51
Attachments:
lirc-CVS-imontouch.tar.bz2
|
Hi I saw that someone added the support for the iMON 15c2:0034 device lately. There is a problem, this device is not a regular IR/LCD. Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is the 4.3" version. (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46). I attached a patch which brings support for the touchscreen and the PAD on the remote as a regular kernel input device. I decided to split this driver completely from the lirc_imon driver because there are some issues which forced me to rewrite most of the lirc_imon code (combine both USB interfaces in only one lirc-device etc.) Rene |
From: Jarod W. <ja...@wi...> - 2008-11-10 22:06:51
|
On Mon, 2008-11-10 at 15:17 -0600, Rene Harder wrote: > Hi > > I saw that someone added the support for the iMON 15c2:0034 device > lately. There is a problem, this device is not a regular IR/LCD. > Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is > the 4.3" version. > (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46). > > I attached a patch which brings support for the touchscreen and the PAD > on the remote as a regular kernel input device. I decided to split this > driver completely from the lirc_imon driver because there are some > issues which forced me to rewrite most of the lirc_imon code (combine > both USB interfaces in only one lirc-device etc.) Ah! Very cool. I was actually curious to find someone w/the 0034 hardware to clarify exactly what it is. Not quite what I'd expected. I'll give your stuff a look when I have a chance... Combining both interfaces into one lirc device is actually something that I've discussed w/someone else, as you do get two devices w/the regular IR/LCD combos as well (and perhaps w/the IR/VFD, not sure), which is a bit of a pain to set up vs. if they were one lirc device... -- Jarod Wilson ja...@wi... |
From: Jarod W. <ja...@wi...> - 2009-06-11 14:54:42
|
On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: > I saw that someone added the support for the iMON 15c2:0034 device > lately. There is a problem, this device is not a regular IR/LCD. > Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is > the 4.3" version. > (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 > ). > > I attached a patch which brings support for the touchscreen and the > PAD > on the remote as a regular kernel input device. I decided to split > this > driver completely from the lirc_imon driver because there are some > issues which forced me to rewrite most of the lirc_imon code (combine > both USB interfaces in only one lirc-device etc.) I've been sitting on this, hoping to take a closer look at it for ages now. Well, the past few days, I finally did... Rather than add a separate driver though, I'm currently working on merging this into the current lirc_imon driver, as the part that combines both usb interfaces into a single lirc device benefits pretty much all recent imon devices, as does the setup of the PAD being toggle-able between up/down/left/right keys and actual mouse functionality. As it stands right now, I have both interfaces on my 0045 (Antec- branded iMON UltraBay) being combined into a single device and mouse functionality working. Still debugging a few things, but hoping to have a patch for folks to try out this weekend... -- Jarod Wilson ja...@wi... |
From: Xaero <kk...@gm...> - 2009-06-12 09:38:23
|
should work for 0038 device too? 2009/6/11 Jarod Wilson <ja...@wi...> > On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: > > > I saw that someone added the support for the iMON 15c2:0034 device > > lately. There is a problem, this device is not a regular IR/LCD. > > Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is > > the 4.3" version. > > ( > http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 > > ). > > > > I attached a patch which brings support for the touchscreen and the > > PAD > > on the remote as a regular kernel input device. I decided to split > > this > > driver completely from the lirc_imon driver because there are some > > issues which forced me to rewrite most of the lirc_imon code (combine > > both USB interfaces in only one lirc-device etc.) > > I've been sitting on this, hoping to take a closer look at it for ages > now. Well, the past few days, I finally did... Rather than add a > separate driver though, I'm currently working on merging this into the > current lirc_imon driver, as the part that combines both usb > interfaces into a single lirc device benefits pretty much all recent > imon devices, as does the setup of the PAD being toggle-able between > up/down/left/right keys and actual mouse functionality. > > As it stands right now, I have both interfaces on my 0045 (Antec- > branded iMON UltraBay) being combined into a single device and mouse > functionality working. Still debugging a few things, but hoping to > have a patch for folks to try out this weekend... > > -- > Jarod Wilson > ja...@wi... > > > > > > > ------------------------------------------------------------------------------ > Crystal Reports - New Free Runtime and 30 Day Trial > Check out the new simplified licensing option that enables unlimited > royalty-free distribution of the report engine for externally facing > server and web deployment. > http://p.sf.net/sfu/businessobjects > |
From: Jarod W. <ja...@wi...> - 2009-06-12 15:04:10
|
On Jun 12, 2009, at 5:38 AM, Xaero wrote: > should work for 0038 device too? Yep. Should apply to everything between 0034 and 0046. The mouse functionality should work for the ffdc devices too, I believe (need to double-check that). > 2009/6/11 Jarod Wilson <ja...@wi...> > On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: > > > I saw that someone added the support for the iMON 15c2:0034 device > > lately. There is a problem, this device is not a regular IR/LCD. > > Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 > is > > the 4.3" version. > > (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 > > ). > > > > I attached a patch which brings support for the touchscreen and the > > PAD > > on the remote as a regular kernel input device. I decided to split > > this > > driver completely from the lirc_imon driver because there are some > > issues which forced me to rewrite most of the lirc_imon code > (combine > > both USB interfaces in only one lirc-device etc.) > > I've been sitting on this, hoping to take a closer look at it for ages > now. Well, the past few days, I finally did... Rather than add a > separate driver though, I'm currently working on merging this into the > current lirc_imon driver, as the part that combines both usb > interfaces into a single lirc device benefits pretty much all recent > imon devices, as does the setup of the PAD being toggle-able between > up/down/left/right keys and actual mouse functionality. > > As it stands right now, I have both interfaces on my 0045 (Antec- > branded iMON UltraBay) being combined into a single device and mouse > functionality working. Still debugging a few things, but hoping to > have a patch for folks to try out this weekend... > > -- > Jarod Wilson > ja...@wi... > > > > > > ------------------------------------------------------------------------------ > Crystal Reports - New Free Runtime and 30 Day Trial > Check out the new simplified licensing option that enables unlimited > royalty-free distribution of the report engine for externally facing > server and web deployment. > http://p.sf.net/sfu/businessobjects > -- Jarod Wilson ja...@wi... |
From: Jeremy Y. <jy...@um...> - 2009-06-12 17:28:58
|
This sounds awesome! I'd vote for a point release when this is integrated and fully tested :) The number of "how to get your imon working" docs/posts out there right now is insane, and the 2 separate device issue doesn't help matters. You rock! Jeremy Jarod Wilson wrote the following on 6/12/2009 11:06 AM: > On Jun 12, 2009, at 5:38 AM, Xaero wrote: > > >> should work for 0038 device too? >> > > > Yep. Should apply to everything between 0034 and 0046. The mouse > functionality should work for the ffdc devices too, I believe (need to > double-check that). > > > > >> 2009/6/11 Jarod Wilson <ja...@wi...> >> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: >> >> >>> I saw that someone added the support for the iMON 15c2:0034 device >>> lately. There is a problem, this device is not a regular IR/LCD. >>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 >>> >> is >> >>> the 4.3" version. >>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 >>> ). >>> >>> I attached a patch which brings support for the touchscreen and the >>> PAD >>> on the remote as a regular kernel input device. I decided to split >>> this >>> driver completely from the lirc_imon driver because there are some >>> issues which forced me to rewrite most of the lirc_imon code >>> >> (combine >> >>> both USB interfaces in only one lirc-device etc.) >>> >> I've been sitting on this, hoping to take a closer look at it for ages >> now. Well, the past few days, I finally did... Rather than add a >> separate driver though, I'm currently working on merging this into the >> current lirc_imon driver, as the part that combines both usb >> interfaces into a single lirc device benefits pretty much all recent >> imon devices, as does the setup of the PAD being toggle-able between >> up/down/left/right keys and actual mouse functionality. >> >> As it stands right now, I have both interfaces on my 0045 (Antec- >> branded iMON UltraBay) being combined into a single device and mouse >> functionality working. Still debugging a few things, but hoping to >> have a patch for folks to try out this weekend... >> >> -- >> Jarod Wilson >> ja...@wi... >> >> >> >> >> >> ------------------------------------------------------------------------------ >> Crystal Reports - New Free Runtime and 30 Day Trial >> Check out the new simplified licensing option that enables unlimited >> royalty-free distribution of the report engine for externally facing >> server and web deployment. >> http://p.sf.net/sfu/businessobjects >> >> > > |
From: Jarod W. <ja...@wi...> - 2009-06-12 17:46:11
|
On Jun 12, 2009, at 1:28 PM, Jeremy Yoder wrote: > This sounds awesome! I'd vote for a point release when this is > integrated and fully tested :) > > The number of "how to get your imon working" docs/posts out there > right now is insane, and the 2 separate device issue doesn't help > matters. The two separate devices are damn near the *sole* reason there are so many 'how to get your imon working' bits out there, aside from devices that simply require newer lirc_imon drivers than available in distro X. But yeah, I think this will help hugely, no binding devices together, distro init scripts can start up a single lircd and it Just Works(tm). > You rock! Heh, I'm just recycling someone else's good work here. Rene is the one who deserves the bulk of the credit. :) Still on course to have something for folks to try out this weekend, just two more problems to sort out at the moment... > Jarod Wilson wrote the following on 6/12/2009 11:06 AM: >> >> On Jun 12, 2009, at 5:38 AM, Xaero wrote: >> >> >>> should work for 0038 device too? >>> >> >> >> Yep. Should apply to everything between 0034 and 0046. The mouse >> functionality should work for the ffdc devices too, I believe (need >> to >> double-check that). >> >> >> >> >>> 2009/6/11 Jarod Wilson <ja...@wi...> >>> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: >>> >>> >>>> I saw that someone added the support for the iMON 15c2:0034 device >>>> lately. There is a problem, this device is not a regular IR/LCD. >>>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 >>>> >>> is >>> >>>> the 4.3" version. >>>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 >>>> ). >>>> >>>> I attached a patch which brings support for the touchscreen and the >>>> PAD >>>> on the remote as a regular kernel input device. I decided to split >>>> this >>>> driver completely from the lirc_imon driver because there are some >>>> issues which forced me to rewrite most of the lirc_imon code >>>> >>> (combine >>> >>>> both USB interfaces in only one lirc-device etc.) >>>> >>> I've been sitting on this, hoping to take a closer look at it for >>> ages >>> now. Well, the past few days, I finally did... Rather than add a >>> separate driver though, I'm currently working on merging this into >>> the >>> current lirc_imon driver, as the part that combines both usb >>> interfaces into a single lirc device benefits pretty much all recent >>> imon devices, as does the setup of the PAD being toggle-able between >>> up/down/left/right keys and actual mouse functionality. >>> >>> As it stands right now, I have both interfaces on my 0045 (Antec- >>> branded iMON UltraBay) being combined into a single device and mouse >>> functionality working. Still debugging a few things, but hoping to >>> have a patch for folks to try out this weekend... >>> >>> -- >>> Jarod Wilson >>> ja...@wi... >>> >>> >>> >>> >>> >>> ------------------------------------------------------------------------------ >>> Crystal Reports - New Free Runtime and 30 Day Trial >>> Check out the new simplified licensing option that enables unlimited >>> royalty-free distribution of the report engine for externally facing >>> server and web deployment. >>> http://p.sf.net/sfu/businessobjects >>> >>> >> >> -- Jarod Wilson ja...@wi... |
From: Rene H. <re...@sa...> - 2009-06-15 21:11:44
|
Jarod Wilson wrote: > On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: > >> I saw that someone added the support for the iMON 15c2:0034 device >> lately. There is a problem, this device is not a regular IR/LCD. >> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is >> the 4.3" version. >> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46). >> >> >> I attached a patch which brings support for the touchscreen and the PAD >> on the remote as a regular kernel input device. I decided to split this >> driver completely from the lirc_imon driver because there are some >> issues which forced me to rewrite most of the lirc_imon code (combine >> both USB interfaces in only one lirc-device etc.) > > I've been sitting on this, hoping to take a closer look at it for ages > now. Well, the past few days, I finally did... Rather than add a > separate driver though, I'm currently working on merging this into the > current lirc_imon driver, as the part that combines both usb > interfaces into a single lirc device benefits pretty much all recent > imon devices, as does the setup of the PAD being toggle-able between > up/down/left/right keys and actual mouse functionality. > > As it stands right now, I have both interfaces on my 0045 > (Antec-branded iMON UltraBay) being combined into a single device and > mouse functionality working. Still debugging a few things, but hoping > to have a patch for folks to try out this weekend... > Well great news, now I can start getting the touchscreen support for the 15c2:0034 and 15c2:0035 into the imon driver. Thanks a lot Jarod, i guess it was quite some work to merge that into the imon driver without braking support for the old devices. Was quite a nightmare for me that's why i stopped and wrote a separate one. I'll test it later this evening and start modifying the touchscreen input support. Did you had some time to look over these code sections, is there anything you have to complain about? Rene |
From: Jarod W. <ja...@wi...> - 2009-06-13 06:27:43
|
On Friday 12 June 2009 13:47:58 Jarod Wilson wrote: > On Jun 12, 2009, at 1:28 PM, Jeremy Yoder wrote: > > > This sounds awesome! I'd vote for a point release when this is > > integrated and fully tested :) > > > > The number of "how to get your imon working" docs/posts out there > > right now is insane, and the 2 separate device issue doesn't help > > matters. > > The two separate devices are damn near the *sole* reason there are so > many 'how to get your imon working' bits out there, aside from devices > that simply require newer lirc_imon drivers than available in distro > X. But yeah, I think this will help hugely, no binding devices > together, distro init scripts can start up a single lircd and it Just > Works(tm). http://git.wilsonet.com/linux-2.6-lirc.git/?a=shortlog;h=HEAD The above is working pretty much flawlessly for me and my 0045 device now. I'm slightly exhausted now, so I'll deal with putting the changes into lirc cvs tomorrow, but the anxious are welcome to hit up the git tree and build from there. Oh, I'm pretty sure it the mouse input stuff won't work correctly with the older ffdc devices yet, as they appear to use a different value range for the decoded pad signals, but it shouldn't be too much effort to get them working either. Also on the docket for tomorrow is to merge the fix to re-enable front-panel buttons, which apparently get disabled on shutdown by the Windows driver... > > Jarod Wilson wrote the following on 6/12/2009 11:06 AM: > >> > >> On Jun 12, 2009, at 5:38 AM, Xaero wrote: > >> > >> > >>> should work for 0038 device too? > >>> > >> > >> > >> Yep. Should apply to everything between 0034 and 0046. The mouse > >> functionality should work for the ffdc devices too, I believe (need > >> to > >> double-check that). > >> > >> > >> > >> > >>> 2009/6/11 Jarod Wilson <ja...@wi...> > >>> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: > >>> > >>> > >>>> I saw that someone added the support for the iMON 15c2:0034 device > >>>> lately. There is a problem, this device is not a regular IR/LCD. > >>>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 > >>>> > >>> is > >>> > >>>> the 4.3" version. > >>>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 > >>>> ). > >>>> > >>>> I attached a patch which brings support for the touchscreen and the > >>>> PAD > >>>> on the remote as a regular kernel input device. I decided to split > >>>> this > >>>> driver completely from the lirc_imon driver because there are some > >>>> issues which forced me to rewrite most of the lirc_imon code > >>>> > >>> (combine > >>> > >>>> both USB interfaces in only one lirc-device etc.) > >>>> > >>> I've been sitting on this, hoping to take a closer look at it for > >>> ages > >>> now. Well, the past few days, I finally did... Rather than add a > >>> separate driver though, I'm currently working on merging this into > >>> the > >>> current lirc_imon driver, as the part that combines both usb > >>> interfaces into a single lirc device benefits pretty much all recent > >>> imon devices, as does the setup of the PAD being toggle-able between > >>> up/down/left/right keys and actual mouse functionality. > >>> > >>> As it stands right now, I have both interfaces on my 0045 (Antec- > >>> branded iMON UltraBay) being combined into a single device and mouse > >>> functionality working. Still debugging a few things, but hoping to > >>> have a patch for folks to try out this weekend... -- Jarod Wilson ja...@wi... |
From: Jarod W. <ja...@wi...> - 2009-06-13 20:15:08
|
On 06/13/2009 02:27 AM, Jarod Wilson wrote: > On Friday 12 June 2009 13:47:58 Jarod Wilson wrote: >> On Jun 12, 2009, at 1:28 PM, Jeremy Yoder wrote: >> >>> This sounds awesome! I'd vote for a point release when this is >>> integrated and fully tested :) >>> >>> The number of "how to get your imon working" docs/posts out there >>> right now is insane, and the 2 separate device issue doesn't help >>> matters. >> The two separate devices are damn near the *sole* reason there are so >> many 'how to get your imon working' bits out there, aside from devices >> that simply require newer lirc_imon drivers than available in distro >> X. But yeah, I think this will help hugely, no binding devices >> together, distro init scripts can start up a single lircd and it Just >> Works(tm). > > http://git.wilsonet.com/linux-2.6-lirc.git/?a=shortlog;h=HEAD > > The above is working pretty much flawlessly for me and my 0045 device > now. I'm slightly exhausted now, so I'll deal with putting the changes > into lirc cvs tomorrow, but the anxious are welcome to hit up the git > tree and build from there. Its in lirc cvs now. Have at it! I'm hopeful I didn't break anything, but I wouldn't be entirely surprised if I did, since this was around 700 lines of changes... It all works for my 0045 device, but its possible I've introduced problems for different devices. I'll try to resolve them as quickly as possible though. I'm particularly interested in how the single-interface (i.e. ffdc and older) devices behave. (Dammit, I left my iMON Knob down in NYC w/my brother... I can probably work on it a bit remotely w/his help though...) Oh, I should probably mention... While merging this, I found it was an unholy nightmare to try to support this as well as kernel 2.4 compatibility, so apologies to those still running 2.4, but as of now, the imon driver is now 2.6+ only. Really though, who in their right mind is still running a 2.4 kernel on anything equipped with an imon device?... :) Drastically simplifying imon setup for the vast majority of users outweighs supporting ancient kernels in my book. > Oh, I'm pretty sure it the mouse input stuff won't work correctly with > the older ffdc devices yet, as they appear to use a different value > range for the decoded pad signals, but it shouldn't be too much effort > to get them working either. Will work on this later tonight, hopefully... > Also on the docket for tomorrow is to merge the fix to re-enable > front-panel buttons, which apparently get disabled on shutdown by the > Windows driver... Also in lirc cvs now (and in my git tree), does the trick to re-enable the buttons on my device. Also, a wider range of devices, as extracted from the Windows iMON driver, are now "supported" by the driver. Since we don't have specific details on all of them though, they may not behave 100% correct, so we'll need user feedback to get 'em just right -- they're the devices in the imon_usb_id_table[] marked 'device specifics unknown'. --jarod >>> Jarod Wilson wrote the following on 6/12/2009 11:06 AM: >>>> On Jun 12, 2009, at 5:38 AM, Xaero wrote: >>>> >>>> >>>>> should work for 0038 device too? >>>>> >>>> >>>> Yep. Should apply to everything between 0034 and 0046. The mouse >>>> functionality should work for the ffdc devices too, I believe (need >>>> to >>>> double-check that). >>>> >>>> >>>> >>>> >>>>> 2009/6/11 Jarod Wilson<ja...@wi...> >>>>> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: >>>>> >>>>> >>>>>> I saw that someone added the support for the iMON 15c2:0034 device >>>>>> lately. There is a problem, this device is not a regular IR/LCD. >>>>>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 >>>>>> >>>>> is >>>>> >>>>>> the 4.3" version. >>>>>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46 >>>>>> ). >>>>>> >>>>>> I attached a patch which brings support for the touchscreen and the >>>>>> PAD >>>>>> on the remote as a regular kernel input device. I decided to split >>>>>> this >>>>>> driver completely from the lirc_imon driver because there are some >>>>>> issues which forced me to rewrite most of the lirc_imon code >>>>>> >>>>> (combine >>>>> >>>>>> both USB interfaces in only one lirc-device etc.) >>>>>> >>>>> I've been sitting on this, hoping to take a closer look at it for >>>>> ages >>>>> now. Well, the past few days, I finally did... Rather than add a >>>>> separate driver though, I'm currently working on merging this into >>>>> the >>>>> current lirc_imon driver, as the part that combines both usb >>>>> interfaces into a single lirc device benefits pretty much all recent >>>>> imon devices, as does the setup of the PAD being toggle-able between >>>>> up/down/left/right keys and actual mouse functionality. >>>>> >>>>> As it stands right now, I have both interfaces on my 0045 (Antec- >>>>> branded iMON UltraBay) being combined into a single device and mouse >>>>> functionality working. Still debugging a few things, but hoping to >>>>> have a patch for folks to try out this weekend... > |
From: Jarod W. <ja...@wi...> - 2009-06-15 20:39:18
|
On 06/15/2009 04:00 PM, Rene Harder wrote: > Jarod Wilson wrote: >> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: >> >>> I saw that someone added the support for the iMON 15c2:0034 device >>> lately. There is a problem, this device is not a regular IR/LCD. >>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is >>> the 4.3" version. >>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46). >>> >>> >>> I attached a patch which brings support for the touchscreen and the PAD >>> on the remote as a regular kernel input device. I decided to split this >>> driver completely from the lirc_imon driver because there are some >>> issues which forced me to rewrite most of the lirc_imon code (combine >>> both USB interfaces in only one lirc-device etc.) >> I've been sitting on this, hoping to take a closer look at it for ages >> now. Well, the past few days, I finally did... Rather than add a >> separate driver though, I'm currently working on merging this into the >> current lirc_imon driver, as the part that combines both usb >> interfaces into a single lirc device benefits pretty much all recent >> imon devices, as does the setup of the PAD being toggle-able between >> up/down/left/right keys and actual mouse functionality. >> >> As it stands right now, I have both interfaces on my 0045 >> (Antec-branded iMON UltraBay) being combined into a single device and >> mouse functionality working. Still debugging a few things, but hoping >> to have a patch for folks to try out this weekend... >> > Well great news, now I can start getting the touchscreen support for the > 15c2:0034 and 15c2:0035 into the imon driver. I *believe* everything that was in your standalone driver should be in lirc cvs now as well. I don't have a clue how a touchscreen is actually supposed to work though, so I wouldn't know one way or the other if there was more work to be done... :) > Thanks a lot Jarod, i guess it was quite some work to merge that into > the imon driver without braking support for the old devices. Was quite a > nightmare for me that's why i stopped and wrote a separate one. Yeah, it was a bit... challenging. Probably around 25 hours of work all told the past week and a half. Initial feedback for multiple devices looks good, but haven't heard definitively from any ffdc device users (nor had a chance to test w/my own, which is over at my brother's place atm). But yeah, got plenty done this weekend, because my wife took the kids out of town, so I had some peace and quiet for once to hack through it. :) > I'll test it later this evening and start modifying the touchscreen > input support. Excellent, thank you! > Did you had some time to look over these code sections, is there > anything you have to complain about? Most things I had any complaints about, I remedied whilst porting into cvs. Biggest thing would be coding style -- I try very hard to follow the canonical upstream linux kernel coding style -- so keeping that in mind for any additional patches would be appreciated. Thanks again, and sorry it took so damned long to get around to it... Nb: I've got some more minor additions coming shortly myself for the mouse input part, for scrollwheel emulation (courtesy of some patches floating around codeka.com). --jarod |
From: Jarod W. <ja...@wi...> - 2009-06-16 01:30:34
|
On 06/15/2009 04:38 PM, Jarod Wilson wrote: > On 06/15/2009 04:00 PM, Rene Harder wrote: >> Jarod Wilson wrote: >>> On Nov 10, 2008, at 4:17 PM, Rene Harder wrote: >>> >>>> I saw that someone added the support for the iMON 15c2:0034 device >>>> lately. There is a problem, this device is not a regular IR/LCD. >>>> Actually it's a 7” VGA/LCD with a touchscreen and I think the 0035 is >>>> the 4.3" version. >>>> (http://www.soundgraph.com/Eng_/Products/oem6.aspx?topMenu=2&subMenu=3&leftMenu=46). >>>> >>>> >>>> I attached a patch which brings support for the touchscreen and the PAD >>>> on the remote as a regular kernel input device. I decided to split this >>>> driver completely from the lirc_imon driver because there are some >>>> issues which forced me to rewrite most of the lirc_imon code (combine >>>> both USB interfaces in only one lirc-device etc.) >>> I've been sitting on this, hoping to take a closer look at it for ages >>> now. Well, the past few days, I finally did... Rather than add a >>> separate driver though, I'm currently working on merging this into the >>> current lirc_imon driver, as the part that combines both usb >>> interfaces into a single lirc device benefits pretty much all recent >>> imon devices, as does the setup of the PAD being toggle-able between >>> up/down/left/right keys and actual mouse functionality. >>> >>> As it stands right now, I have both interfaces on my 0045 >>> (Antec-branded iMON UltraBay) being combined into a single device and >>> mouse functionality working. Still debugging a few things, but hoping >>> to have a patch for folks to try out this weekend... >>> >> Well great news, now I can start getting the touchscreen support for the >> 15c2:0034 and 15c2:0035 into the imon driver. > > I *believe* everything that was in your standalone driver should be in > lirc cvs now as well. I don't have a clue how a touchscreen is actually > supposed to work though, so I wouldn't know one way or the other if > there was more work to be done... :) > >> Thanks a lot Jarod, i guess it was quite some work to merge that into >> the imon driver without braking support for the old devices. Was quite a >> nightmare for me that's why i stopped and wrote a separate one. > > Yeah, it was a bit... challenging. Probably around 25 hours of work all > told the past week and a half. Initial feedback for multiple devices > looks good, but haven't heard definitively from any ffdc device users > (nor had a chance to test w/my own, which is over at my brother's place > atm). But yeah, got plenty done this weekend, because my wife took the > kids out of town, so I had some peace and quiet for once to hack through > it. :) > >> I'll test it later this evening and start modifying the touchscreen >> input support. > > Excellent, thank you! > >> Did you had some time to look over these code sections, is there >> anything you have to complain about? > > Most things I had any complaints about, I remedied whilst porting into > cvs. Biggest thing would be coding style -- I try very hard to follow > the canonical upstream linux kernel coding style -- so keeping that in > mind for any additional patches would be appreciated. > > Thanks again, and sorry it took so damned long to get around to it... > > Nb: I've got some more minor additions coming shortly myself for the > mouse input part, for scrollwheel emulation (courtesy of some patches > floating around codeka.com). I just committed the scrollwheel emu bits as well as support for mouse input on the 0xffdc devices, which moved a fair amount of the input device code around a bit. Its all more or less the same, just located somewhere else to cut down on duplication... --jarod |
From: Rene H. <re...@sa...> - 2009-06-16 04:40:00
Attachments:
imon-touch-support-0035.patch
|
Hi Jarod, First of all, you did an outstanding job, i compiled the driver and the touchscreen was working quite fine. Well after i figured out that the lirc device needs to be opened before it's working. I've added a patch with two minor changes. The first on adds touchscreen support for the 4.3" (15c2:0035) model. Soundgraph changed one byte for this model. The second one solves a compile error which occurred after your last changes. There are still some minor bugs related to the touchscreen: The touchscreen and mouse input only works if the lirc device is open. I guess that should be solvable with moving the input event code into the usb_rx_callback_xxx functions. If you touch the screen before the lirc device is open. Once you open the device the X server gets confused. X probably does not receive the touch release bit (generated by the timer callback function) and sets the mouse button as clicked but not released, you need to unload the driver before it'll work normal again. If the input event generation is independent on the status of the lirc device this will be solved as well. For the iMONs with touchscreen there are at least 2 remotes with different mouse toggle button codes. One you have already implemented, for the other one the 3. byte is 0x35 instead of 0x15. Jarod Wilson wrote: > Yeah, it was a bit... challenging. Probably around 25 hours of work > all told the past week and a half. Initial feedback for multiple > devices looks good, but haven't heard definitively from any ffdc > device users (nor had a chance to test w/my own, which is over at my > brother's place atm). But yeah, got plenty done this weekend, because > my wife took the kids out of town, so I had some peace and quiet for > once to hack through it. :) That I believe right away, to combine both USB interfaces you had to change major parts of the code base. > Most things I had any complaints about, I remedied whilst porting into > cvs. Biggest thing would be coding style -- I try very hard to follow > the canonical upstream linux kernel coding style -- so keeping that in > mind for any additional patches would be appreciated. My apologies, I'm not a big kernel hacker but I'll try to get used to the kernel coding style before submitting more patches. > > Thanks again, and sorry it took so damned long to get around to it... No problem, I know that this was a huge task which took way more than couple hours and to do this right before an upcoming release is probably not a good idea either. |
From: Jarod W. <ja...@wi...> - 2009-06-16 19:52:42
|
On Jun 16, 2009, at 12:39 AM, Rene Harder wrote: > Hi Jarod, > > First of all, you did an outstanding job, i compiled the driver and > the > touchscreen was working quite fine. Woo! :) Unfortunately, it doesn't seem to be working quite as well on the ffdc devices yet... > Well after i figured out that the > lirc device needs to be opened before it's working. Yeah, I should probably mention that somewhere... > I've added a patch with two minor changes. > The first on adds touchscreen support for the 4.3" (15c2:0035) model. > Soundgraph changed one byte for this model. Figures... Applied (with some formatting differences -- trying to keep to 80 char or less lines). > The second one solves a compile error which occurred after your last > changes. Gah. I was wondering where this stray int err; came from, neglected to compile test after killing it off. I must have screwed up an insert in an earlier commit. Thanks for the fix. Applied, though by putting int err; back where it originally was. > There are still some minor bugs related to the touchscreen: > > The touchscreen and mouse input only works if the lirc device is > open. I > guess that should be solvable with moving the input event code into > the > usb_rx_callback_xxx functions. Nope, it fails just the same in there, at least right now. If IR isn't open, the urb's aren't set up and the callbacks won't fire (ir_open does usb_fill_int_urb() calls, ir_close does usb_kill_urb()). So I guess we'd have to move the urb setup into imon_probe() and the teardown into imon_disconnect() if we want mouse and touchscreen operable w/o IR open. Even then, it gets... messy. We'd need to check for all the mouse stuff in the intf0 callback, while its split between intf0 and intf1 for the newer devices. I'm thinking perhaps a new function that is designed specifically for handling input device packets, called from both intf0 and intf1 callbacks, to keep code duplication to a minimum... Okay, this might not actually be too bad... > If you touch the screen before the lirc device is open. Once you open > the device the X server gets confused. X probably does not receive the > touch release bit (generated by the timer callback function) and sets > the mouse button as clicked but not released, you need to unload the > driver before it'll work normal again. If the input event generation > is > independent on the status of the lirc device this will be solved as > well. Ick. I need me one of these devices, would make it much easier to work on the code... :) I'll be leaning heavily on you here. > For the iMONs with touchscreen there are at least 2 remotes with > different mouse toggle button codes. One you have already implemented, > for the other one the 3. byte is 0x35 instead of 0x15. I thought the 0x15 code was "button pressed" and the 0x35 code was "button released". But in looking back over my notes, I'm not so sure... I have a 'spews 0x0100007f between press and release' note here and '"no longer pressed" is key | 0x00004000'. Ugh. I'll poke more. In related news, I tried pinging soundgraph for some docs or something. Nothing but silence. Not exactly surprised though, I've heard they've been entirely uncooperative in the past too. -- Jarod Wilson ja...@wi... |
From: Rene H. <re...@sa...> - 2009-06-16 20:26:48
|
ups sorry forgot to reply to the list Jarod Wilson wrote: > On Jun 16, 2009, at 12:39 AM, Rene Harder wrote: > >> There are still some minor bugs related to the touchscreen: >> >> The touchscreen and mouse input only works if the lirc device is open. I >> guess that should be solvable with moving the input event code into the >> usb_rx_callback_xxx functions. > > Nope, it fails just the same in there, at least right now. If IR isn't > open, the urb's aren't set up and the callbacks won't fire (ir_open > does usb_fill_int_urb() calls, ir_close does usb_kill_urb()). So I > guess we'd have to move the urb setup into imon_probe() and the > teardown into imon_disconnect() if we want mouse and touchscreen > operable w/o IR open. > > Even then, it gets... messy. We'd need to check for all the mouse > stuff in the intf0 callback, while its split between intf0 and intf1 > for the newer devices. I'm thinking perhaps a new function that is > designed specifically for handling input device packets, called from > both intf0 and intf1 callbacks, to keep code duplication to a > minimum... Okay, this might not actually be too bad... > Yeah I'm working on it right now. I thought about a separate function as well but it's not as complex as i thought it'd be. Just need to find the right place in the imon_probe and imon_disconnect which is a bit challenging right now. I don't want to break anything for older devices so I need to understand the code a little bit more. > >> If you touch the screen before the lirc device is open. Once you open >> the device the X server gets confused. X probably does not receive the >> touch release bit (generated by the timer callback function) and sets >> the mouse button as clicked but not released, you need to unload the >> driver before it'll work normal again. If the input event generation is >> independent on the status of the lirc device this will be solved as >> well. > > Ick. I need me one of these devices, would make it much easier to work > on the code... :) I'll be leaning heavily on you here. > I think should not be a big problem i just need some time and will be solved with the first "bug" anyways. > >> For the iMONs with touchscreen there are at least 2 remotes with >> different mouse toggle button codes. One you have already implemented, >> for the other one the 3. byte is 0x35 instead of 0x15. > > I thought the 0x15 code was "button pressed" and the 0x35 code was > "button released". But in looking back over my notes, I'm not so > sure... I have a 'spews 0x0100007f between press and release' note > here and '"no longer pressed" is key | 0x00004000'. Ugh. I'll poke more. > Well i'm pretty sure released was 0x55 but i'll check that when i got home. But I do remember that in the beginning some people complaint that this key doesn't work and i added both to the imontouch driver. > In related news, I tried pinging soundgraph for some docs or > something. Nothing but silence. Not exactly surprised though, I've > heard they've been entirely uncooperative in the past too. Yes last year, i tried it too because i could not find the touch release bit in the touchscreen codes, the only response i got was. "Sorry, We do not support Linux" Afterwards they started ignoring my messages completely. > > |
From: Rene H. <re...@sa...> - 2009-06-16 20:26:58
|
ups sorry forgot to reply to the list Jarod Wilson wrote: > On Jun 16, 2009, at 12:39 AM, Rene Harder wrote: > >> There are still some minor bugs related to the touchscreen: >> >> The touchscreen and mouse input only works if the lirc device is open. I >> guess that should be solvable with moving the input event code into the >> usb_rx_callback_xxx functions. > > Nope, it fails just the same in there, at least right now. If IR isn't > open, the urb's aren't set up and the callbacks won't fire (ir_open > does usb_fill_int_urb() calls, ir_close does usb_kill_urb()). So I > guess we'd have to move the urb setup into imon_probe() and the > teardown into imon_disconnect() if we want mouse and touchscreen > operable w/o IR open. > > Even then, it gets... messy. We'd need to check for all the mouse > stuff in the intf0 callback, while its split between intf0 and intf1 > for the newer devices. I'm thinking perhaps a new function that is > designed specifically for handling input device packets, called from > both intf0 and intf1 callbacks, to keep code duplication to a > minimum... Okay, this might not actually be too bad... > Yeah I'm working on it right now. I thought about a separate function as well but it's not as complex as i thought it'd be. Just need to find the right place in the imon_probe and imon_disconnect which is a bit challenging right now. I don't want to break anything for older devices so I need to understand the code a little bit more. > >> If you touch the screen before the lirc device is open. Once you open >> the device the X server gets confused. X probably does not receive the >> touch release bit (generated by the timer callback function) and sets >> the mouse button as clicked but not released, you need to unload the >> driver before it'll work normal again. If the input event generation is >> independent on the status of the lirc device this will be solved as >> well. > > Ick. I need me one of these devices, would make it much easier to work > on the code... :) I'll be leaning heavily on you here. > I think should not be a big problem i just need some time and will be solved with the first "bug" anyways. > >> For the iMONs with touchscreen there are at least 2 remotes with >> different mouse toggle button codes. One you have already implemented, >> for the other one the 3. byte is 0x35 instead of 0x15. > > I thought the 0x15 code was "button pressed" and the 0x35 code was > "button released". But in looking back over my notes, I'm not so > sure... I have a 'spews 0x0100007f between press and release' note > here and '"no longer pressed" is key | 0x00004000'. Ugh. I'll poke more. > Well i'm pretty sure released was 0x55 but i'll check that when i got home. But I do remember that in the beginning some people complaint that this key doesn't work and i added both to the imontouch driver. > In related news, I tried pinging soundgraph for some docs or > something. Nothing but silence. Not exactly surprised though, I've > heard they've been entirely uncooperative in the past too. Yes last year, i tried it too because i could not find the touch release bit in the touchscreen codes, the only response i got was. "Sorry, We do not support Linux" Afterwards they started ignoring my messages completely. > > |
From: Jarod W. <ja...@wi...> - 2009-06-16 20:54:32
|
On Jun 16, 2009, at 4:26 PM, Rene Harder wrote: > Jarod Wilson wrote: >> On Jun 16, 2009, at 12:39 AM, Rene Harder wrote: >> >>> There are still some minor bugs related to the touchscreen: >>> >>> The touchscreen and mouse input only works if the lirc device is >>> open. I >>> guess that should be solvable with moving the input event code >>> into the >>> usb_rx_callback_xxx functions. >> >> Nope, it fails just the same in there, at least right now. If IR >> isn't >> open, the urb's aren't set up and the callbacks won't fire (ir_open >> does usb_fill_int_urb() calls, ir_close does usb_kill_urb()). So I >> guess we'd have to move the urb setup into imon_probe() and the >> teardown into imon_disconnect() if we want mouse and touchscreen >> operable w/o IR open. >> >> Even then, it gets... messy. We'd need to check for all the mouse >> stuff in the intf0 callback, while its split between intf0 and intf1 >> for the newer devices. I'm thinking perhaps a new function that is >> designed specifically for handling input device packets, called from >> both intf0 and intf1 callbacks, to keep code duplication to a >> minimum... Okay, this might not actually be too bad... >> > Yeah I'm working on it right now. I thought about a separate > function as > well but it's not as complex as i thought it'd be. Just need to find > the > right place in the imon_probe and imon_disconnect which is a bit > challenging right now. I don't want to break anything for older > devices > so I need to understand the code a little bit more. Looking over it again, it seems the input event handling can stay in imon_incoming_lirc_packet(), (which might better be renamed (again) to imon_incoming_packet()). Rather than having the check for ir_open in the callbacks, we run through the pad_mouse stuff first, then the touchscreen stuff, and then have a check for ir_open, and bail there if it isn't. imon_disconnect() seems to already have all the necessary bits wired up to kill off the urbs, duplicated from ir_close(), so it should be fine to just drop the urb kill bits from ir_close()... The urb setup stuff for each interface would probably work nested right under the respective input_register_device() calls in imon_probe(). >>> If you touch the screen before the lirc device is open. Once you >>> open >>> the device the X server gets confused. X probably does not receive >>> the >>> touch release bit (generated by the timer callback function) and >>> sets >>> the mouse button as clicked but not released, you need to unload the >>> driver before it'll work normal again. If the input event >>> generation is >>> independent on the status of the lirc device this will be solved as >>> well. >> >> Ick. I need me one of these devices, would make it much easier to >> work >> on the code... :) I'll be leaning heavily on you here. >> > > I think should not be a big problem i just need some time and will be > solved with the first "bug" anyways. Works for me. >>> For the iMONs with touchscreen there are at least 2 remotes with >>> different mouse toggle button codes. One you have already >>> implemented, >>> for the other one the 3. byte is 0x35 instead of 0x15. >> >> I thought the 0x15 code was "button pressed" and the 0x35 code was >> "button released". But in looking back over my notes, I'm not so >> sure... I have a 'spews 0x0100007f between press and release' note >> here and '"no longer pressed" is key | 0x00004000'. Ugh. I'll poke >> more. >> > Well i'm pretty sure released was 0x55 but i'll check that when i got > home. But I do remember that in the beginning some people complaint > that > this key doesn't work and i added both to the imontouch driver. Yeah, 0x55 makes sense based on my notes. No problem, we can add both. It makes the code slightly less clean (I was rather partial to the use of memcmp), but meh. >> In related news, I tried pinging soundgraph for some docs or >> something. Nothing but silence. Not exactly surprised though, I've >> heard they've been entirely uncooperative in the past too. > > Yes last year, i tried it too because i could not find the touch > release bit in the touchscreen codes, the only response i got was. > "Sorry, We do not support Linux" Afterwards they started ignoring my > messages completely. :( -- Jarod Wilson ja...@wi... |
From: Rene H. <re...@sa...> - 2009-06-17 03:35:51
Attachments:
touchscreen.patch
|
Hi Jarod, so here are some changes and bug fixes. I tried to follow the kernel coding style as good as possible, please let me know what you think. I've not tested the suspend and resume yet, but I think if you call usb_kill_urb() in suspend you need to run usb_fill_int_urb when resuming. patch summery: - renamed imon_incoming_lirc_packet() to imon_incoming_packet() - fixed down sampled touchscreen output (64 Button mode) and added support for 4.3" version. - added packet length check, to prevent interference between mouse and touchscreen input emulation. (Occurs only if your use the touchscreen right before the pad, in this case buf[6] still contains the previous value (0x14) same for buf[7].) - moved usb_fill_int_urb() into imon_probe()/imon_resume(), So the generic mouse and touchscreen input device emulation is working without prior opening the lirc device. Jarod Wilson wrote: > > Yeah, 0x55 makes sense based on my notes. No problem, we can add both. > It makes the code slightly less clean (I was rather partial to the use > of memcmp), but meh. > What's the benefit of using memcmp instead of comparing the individual bytes? |
From: Jarod W. <ja...@wi...> - 2009-06-17 04:33:19
|
On 06/16/2009 11:35 PM, Rene Harder wrote: > Hi Jarod, > > so here are some changes and bug fixes. I tried to follow the kernel > coding style as good as possible, please let me know what you think. Generally pretty good. A few minor things here and there, but for the most part, much easier to read than the original lirc_imontouch.c I was looking at. ;) > I've not tested the suspend and resume yet, but I think if you call > usb_kill_urb() in suspend you need to run usb_fill_int_urb when resuming. Yeah, that makes sense. I've never actually tested suspend/resume either, someone else submitted the patch... > patch summery: > > - renamed imon_incoming_lirc_packet() to imon_incoming_packet() > > - fixed down sampled touchscreen output (64 Button mode) and added > support for 4.3" version. Good and good. > - added packet length check, to prevent interference between mouse > and touchscreen input emulation. > (Occurs only if your use the touchscreen right before the pad, in this > case > buf[6] still contains the previous value (0x14) same for buf[7].) Hrm. I need to see how these len checks behave on an ffdc device. I'll put that on my TODO list for tomorrow. Oh! Wait... This is definitely a problem. Checking for len == 5 along with context->pad_mouse effectively neuters scroll wheel emulation using ch+/-. > - moved usb_fill_int_urb() into imon_probe()/imon_resume(), So the > generic mouse > and touchscreen input device emulation is working without prior > opening the > lirc device. Excellent. Yeah, that turned out not so messy at all, even looks more or less like what I'd envisioned in my prior mail. And I've now tested it locally w/my 0045 device, works great! Oh, except for one thing... Right-click never works for me in mouse mode, haven't yet cared enough to dig into it, but I thought I should mention it in case you have any ideas... Curious if yours works or not. > Jarod Wilson wrote: >> Yeah, 0x55 makes sense based on my notes. No problem, we can add both. >> It makes the code slightly less clean (I was rather partial to the use >> of memcmp), but meh. >> > > What's the benefit of using memcmp instead of comparing the individual > bytes? One comparison instead of four and its easier to read. No biggie. On to some specifics in the code... > @@ -1192,9 +1157,9 @@ > } > > /* send touchscreen events through input subsystem if touchpad data */ > - if (context->has_touchscreen && > + if (context->has_touchscreen && (len == 8) && > (buf[6] == 0x14 || buf[6] == 0x03) && buf[7] == 0x86) { > - if (mouse == NULL) { > + if (touch == NULL) { > printk(KERN_WARNING "%s: touchscreen input device is " > "NULL!\n", __func__); > return; That mouse/touch inversion was a bone-headed copy-n-paste error by me... :) > @@ -1206,9 +1171,11 @@ > input_report_abs(touch, ABS_Y, context->touch_y); > input_report_key(touch, BTN_TOUCH, 0x01); > input_sync(touch); > - return; I'd like to slip in a 'ts_input = 1;' right here, and then... > } > > + if (!context->ir_isopen) > + return; > + ...we can do if (!context->ir_isopen && !ts_input) return; here. Otherwise, if IR isn't open, your touch button presses are going to get thrown away, aren't they? > @@ -1274,7 +1241,8 @@ > buf[2] = dir & 0xFF; > buf[3] = (dir >> 8) & 0xFF; > > - } else if (buf[6] == 0x14 && buf[7] == 0x86 && intf == 1) { > + } else if ((buf[6] == 0x14 || buf[6] == 0x03) && > + buf[7] == 0x86 && intf == 1) { We can also then replace this with else if (ts_input), since we already made this nasty comparison earlier (and drop setting ts_input in here, of course). > @@ -1841,6 +1821,20 @@ > context->touch = NULL; > retval = 0; > } > + > + usb_fill_int_urb(context->rx_urb_intf1, context->usbdev_intf1, > + usb_rcvintpipe(context->usbdev_intf1, > + context->rx_endpoint_intf1->bEndpointAddress), > + context->usb_rx_buf, sizeof(context->usb_rx_buf), > + usb_rx_callback_intf1, context, > + context->rx_endpoint_intf1->bInterval); > + > + retval = usb_submit_urb(context->rx_urb_intf1, GFP_KERNEL); > + > + if (retval) { > + err("%s: usb_submit_urb failed for intf1 (%d)", > + __func__, retval); > + } In both the usb_submit_urb() cases here, we need to actually propagate the error upwards if it fails, not just warn. Stuff isn't going to work at all correctly if either of those failed on us. So I actually patched most of this into my local git tree for testing (I tend to work from git, then commit to cvs after I'm happy w/the way the changes are working), and fixed up the issues I raised above. I think all I've left out for the moment are the len checks, which will need some minor tweaks so we don't break scroll wheel emu, and need testing on ffdc devices... But if you're good with my alterations, hit me with a... Signed-off-by: Name <email> ...and I'll put this into the git tree and cvs tomorrow (its already past about my bed time ;) Ooops, oh yeah, the patch in my local git tree right now, so you can actually look at it: http://devel.wilsonet.com/misc/lirc_imon-mouse-without-ir-open.patch --jarod |
From: Rene H. <re...@sa...> - 2009-06-17 05:36:22
|
Jarod Wilson wrote: > On 06/16/2009 11:35 PM, Rene Harder wrote: >> Hi Jarod, >> >> so here are some changes and bug fixes. I tried to follow the kernel >> coding style as good as possible, please let me know what you think. > > Generally pretty good. A few minor things here and there, but for the > most part, much easier to read than the original lirc_imontouch.c I > was looking at. ;) > Well i hacked the imontouch driver within 48h didn't looked much on the style though. :-[ >> I've not tested the suspend and resume yet, but I think if you call >> usb_kill_urb() in suspend you need to run usb_fill_int_urb when >> resuming. > > Yeah, that makes sense. I've never actually tested suspend/resume > either, someone else submitted the patch... > > >> patch summery: >> >> - renamed imon_incoming_lirc_packet() to imon_incoming_packet() >> >> - fixed down sampled touchscreen output (64 Button mode) and added >> support for 4.3" version. > > Good and good. > >> - added packet length check, to prevent interference between mouse >> and touchscreen input emulation. >> (Occurs only if your use the touchscreen right before the pad, in >> this >> case >> buf[6] still contains the previous value (0x14) same for buf[7].) > > Hrm. I need to see how these len checks behave on an ffdc device. I'll > put that on my TODO list for tomorrow. Oh! Wait... This is definitely > a problem. Checking for len == 5 along with context->pad_mouse > effectively neuters scroll wheel emulation using ch+/-. Right, they have 8 bytes, but without length check the condition is true for the touchscreen as well if mouse input mode is activated. one way would be to move the touchscreen input stuff before the mouse condition and replace (len == 5) with !ts_input, that should block this condition if it is a touchscreen code. > > >> - moved usb_fill_int_urb() into imon_probe()/imon_resume(), So the >> generic mouse >> and touchscreen input device emulation is working without prior >> opening the >> lirc device. > > Excellent. Yeah, that turned out not so messy at all, even looks more > or less like what I'd envisioned in my prior mail. And I've now tested > it locally w/my 0045 device, works great! > > Oh, except for one thing... Right-click never works for me in mouse > mode, haven't yet cared enough to dig into it, but I thought I should > mention it in case you have any ideas... Curious if yours works or not. Your are right, right click is not working at all. I'll check that tomorrow. > > >> Jarod Wilson wrote: >>> Yeah, 0x55 makes sense based on my notes. No problem, we can add both. >>> It makes the code slightly less clean (I was rather partial to the use >>> of memcmp), but meh. >>> >> >> What's the benefit of using memcmp instead of comparing the individual >> bytes? > > One comparison instead of four and its easier to read. No biggie. Yeah that makes sens. > > > On to some specifics in the code... > >> @@ -1192,9 +1157,9 @@ >> } >> >> /* send touchscreen events through input subsystem if touchpad >> data */ >> - if (context->has_touchscreen && >> + if (context->has_touchscreen && (len == 8) && >> (buf[6] == 0x14 || buf[6] == 0x03) && buf[7] == 0x86) { >> - if (mouse == NULL) { >> + if (touch == NULL) { >> printk(KERN_WARNING "%s: touchscreen input device is " >> "NULL!\n", __func__); >> return; > > That mouse/touch inversion was a bone-headed copy-n-paste error by > me... :) > >> @@ -1206,9 +1171,11 @@ >> input_report_abs(touch, ABS_Y, context->touch_y); >> input_report_key(touch, BTN_TOUCH, 0x01); >> input_sync(touch); >> - return; > > I'd like to slip in a 'ts_input = 1;' right here, and then... > >> } >> >> + if (!context->ir_isopen) >> + return; >> + > > ...we can do if (!context->ir_isopen && !ts_input) return; here. > Otherwise, if IR isn't open, your touch button presses are going to > get thrown away, aren't they? If the lirc device is closed it doesn't not make any sense to try to feed the button matrix codes trough it. If the device is opened the coordinates are passed though lirc and the input subsystem. Some people use the button mode feature because you don't need a X-server to get coordinates from the touchscreen. > >> @@ -1274,7 +1241,8 @@ >> buf[2] = dir & 0xFF; >> buf[3] = (dir >> 8) & 0xFF; >> >> - } else if (buf[6] == 0x14 && buf[7] == 0x86 && intf == 1) { >> + } else if ((buf[6] == 0x14 || buf[6] == 0x03) && >> + buf[7] == 0x86 && intf == 1) { > > We can also then replace this with else if (ts_input), since we > already made this nasty comparison earlier (and drop setting ts_input > in here, of course). That's a good idea. > >> @@ -1841,6 +1821,20 @@ >> context->touch = NULL; >> retval = 0; >> } >> + >> + usb_fill_int_urb(context->rx_urb_intf1, context->usbdev_intf1, >> + usb_rcvintpipe(context->usbdev_intf1, >> + context->rx_endpoint_intf1->bEndpointAddress), >> + context->usb_rx_buf, sizeof(context->usb_rx_buf), >> + usb_rx_callback_intf1, context, >> + context->rx_endpoint_intf1->bInterval); >> + >> + retval = usb_submit_urb(context->rx_urb_intf1, GFP_KERNEL); >> + >> + if (retval) { >> + err("%s: usb_submit_urb failed for intf1 (%d)", >> + __func__, retval); >> + } > > In both the usb_submit_urb() cases here, we need to actually propagate > the error upwards if it fails, not just warn. Stuff isn't going to > work at all correctly if either of those failed on us. I removed this because I was not quite sure if a fail needs to result in an abort. I think you can remove the mutex_unlock before the goto exit; there's an unlock right before we exit the function. > > So I actually patched most of this into my local git tree for testing > (I tend to work from git, then commit to cvs after I'm happy w/the way > the changes are working), and fixed up the issues I raised above. I > think all I've left out for the moment are the len checks, which will > need some minor tweaks so we don't break scroll wheel emu, and need > testing on ffdc devices... But if you're good with my alterations, hit > me with a... > > Signed-off-by: Name <email> > > ...and I'll put this into the git tree and cvs tomorrow (its already > past about my bed time ;) > > Ooops, oh yeah, the patch in my local git tree right now, so you can > actually look at it: > > http://devel.wilsonet.com/misc/lirc_imon-mouse-without-ir-open.patch > > --jarod > |
From: Jarod W. <ja...@wi...> - 2009-06-17 15:46:42
|
On Jun 17, 2009, at 1:36 AM, Rene Harder wrote: > Jarod Wilson wrote: >> On 06/16/2009 11:35 PM, Rene Harder wrote: >>> - added packet length check, to prevent interference between mouse >>> and touchscreen input emulation. >>> (Occurs only if your use the touchscreen right before the pad, in >>> this >>> case >>> buf[6] still contains the previous value (0x14) same for buf[7].) >> >> Hrm. I need to see how these len checks behave on an ffdc device. >> I'll >> put that on my TODO list for tomorrow. Oh! Wait... This is definitely >> a problem. Checking for len == 5 along with context->pad_mouse >> effectively neuters scroll wheel emulation using ch+/-. > > Right, they have 8 bytes, but without length check the condition is > true > for the touchscreen as well if mouse input mode is activated. > > one way would be to move the touchscreen input stuff before the mouse > condition and replace (len == 5) with !ts_input, that should block > this > condition if it is a touchscreen code. Yeah, that looks like it'll do the trick, and there's no chance of it causing any regressions with the older devices, since none of them will have context->has_touchscreen true. I'm going to add one more check there: if ((context->pad_mouse || !context->ir_isopen) && !ts_input) With that, if IR isn't open, the mouse should always be functional, regardless of the keyboard/mouse toggle button status. >>> - moved usb_fill_int_urb() into imon_probe()/imon_resume(), So the >>> generic mouse >>> and touchscreen input device emulation is working without prior >>> opening the >>> lirc device. >> >> Excellent. Yeah, that turned out not so messy at all, even looks more >> or less like what I'd envisioned in my prior mail. And I've now >> tested >> it locally w/my 0045 device, works great! >> >> Oh, except for one thing... Right-click never works for me in mouse >> mode, haven't yet cared enough to dig into it, but I thought I should >> mention it in case you have any ideas... Curious if yours works or >> not. > > Your are right, right click is not working at all. I'll check that > tomorrow. Okay, so its not just me. Good, I think... ;) I can dig into it as well. >>> @@ -1206,9 +1171,11 @@ >>> input_report_abs(touch, ABS_Y, context->touch_y); >>> input_report_key(touch, BTN_TOUCH, 0x01); >>> input_sync(touch); >>> - return; >> >> I'd like to slip in a 'ts_input = 1;' right here, and then... >> >>> } >>> >>> + if (!context->ir_isopen) >>> + return; >>> + >> >> ...we can do if (!context->ir_isopen && !ts_input) return; here. >> Otherwise, if IR isn't open, your touch button presses are going to >> get thrown away, aren't they? > If the lirc device is closed it doesn't not make any sense to try to > feed the button matrix codes trough it. > If the device is opened the coordinates are passed though lirc and the > input subsystem. > Some people use the button mode feature because you don't need a > X-server to get coordinates from the touchscreen. Ah, I think I get it now, helps to read the code more carefully... The touch stuff has already been delivered via the input subsystem at this point, but it can *also* be delivered via lirc, if ir_isopen, and the touch data can then be processed from either input or lirc. So yeah, that check is correct. >>> @@ -1841,6 +1821,20 @@ >>> context->touch = NULL; >>> retval = 0; >>> } >>> + >>> + usb_fill_int_urb(context->rx_urb_intf1, context- >>> >usbdev_intf1, >>> + usb_rcvintpipe(context->usbdev_intf1, >>> + context->rx_endpoint_intf1->bEndpointAddress), >>> + context->usb_rx_buf, sizeof(context->usb_rx_buf), >>> + usb_rx_callback_intf1, context, >>> + context->rx_endpoint_intf1->bInterval); >>> + >>> + retval = usb_submit_urb(context->rx_urb_intf1, GFP_KERNEL); >>> + >>> + if (retval) { >>> + err("%s: usb_submit_urb failed for intf1 (%d)", >>> + __func__, retval); >>> + } >> >> In both the usb_submit_urb() cases here, we need to actually >> propagate >> the error upwards if it fails, not just warn. Stuff isn't going to >> work at all correctly if either of those failed on us. > > I removed this because I was not quite sure if a fail needs to > result in > an abort. I'm reasonably confident failure here means the usb_rx_callback_*() functions would never fire -- we're essentially in the same state as we are after usb_kill_urb() has been called. > I think you can remove the mutex_unlock before the goto exit; > there's an > unlock right before we exit the function. They're different locks, need to unlock both. :) I've gone ahead with some commits to my local git tree, which I think should be good to go, and if you don't see anything wrong w/them, I'll get them into lirc cvs too: http://devel.wilsonet.com/misc/0001-lirc_imon-touchscreen-handling-fixups.patch http://devel.wilsonet.com/misc/0002-lirc_imon-support-mouse-and-touch-input-devices-eve.patch I've tested these out successfully on both a 15c2:0045 and 15c2:ffdc (iMON Knob) device now. -- Jarod Wilson ja...@wi... |
From: Rene H. <re...@sa...> - 2009-06-17 16:25:28
|
Jarod Wilson wrote: > On Jun 17, 2009, at 1:36 AM, Rene Harder wrote: > >> Jarod Wilson wrote: >>> On 06/16/2009 11:35 PM, Rene Harder wrote: >>>> - added packet length check, to prevent interference between mouse >>>> and touchscreen input emulation. >>>> (Occurs only if your use the touchscreen right before the pad, in >>>> this >>>> case >>>> buf[6] still contains the previous value (0x14) same for buf[7].) >>> >>> Hrm. I need to see how these len checks behave on an ffdc device. I'll >>> put that on my TODO list for tomorrow. Oh! Wait... This is definitely >>> a problem. Checking for len == 5 along with context->pad_mouse >>> effectively neuters scroll wheel emulation using ch+/-. >> >> Right, they have 8 bytes, but without length check the condition is true >> for the touchscreen as well if mouse input mode is activated. >> >> one way would be to move the touchscreen input stuff before the mouse >> condition and replace (len == 5) with !ts_input, that should block this >> condition if it is a touchscreen code. > > Yeah, that looks like it'll do the trick, and there's no chance of it > causing any regressions with the older devices, since none of them > will have context->has_touchscreen true. I'm going to add one more > check there: > > if ((context->pad_mouse || !context->ir_isopen) && !ts_input) Or we use an else if (), so we can get rid of the !ts_input > > With that, if IR isn't open, the mouse should always be functional, > regardless of the keyboard/mouse toggle button status. Actually that's a great idea. > >>>> - moved usb_fill_int_urb() into imon_probe()/imon_resume(), So the >>>> generic mouse >>>> and touchscreen input device emulation is working without prior >>>> opening the >>>> lirc device. >>> >>> Excellent. Yeah, that turned out not so messy at all, even looks more >>> or less like what I'd envisioned in my prior mail. And I've now tested >>> it locally w/my 0045 device, works great! >>> >>> Oh, except for one thing... Right-click never works for me in mouse >>> mode, haven't yet cared enough to dig into it, but I thought I should >>> mention it in case you have any ideas... Curious if yours works or not. >> >> Your are right, right click is not working at all. I'll check that >> tomorrow. > > Okay, so its not just me. Good, I think... ;) I can dig into it as well. Well found it this morning. input_report_key(mouse, BTN_RIGHT, buf[1] >> 2 & 0x01); We shift to much here, it must be buf[1]>>1 &0x01 Was a bug in my first imontouch driver as well. > > >>>> @@ -1206,9 +1171,11 @@ >>>> input_report_abs(touch, ABS_Y, context->touch_y); >>>> input_report_key(touch, BTN_TOUCH, 0x01); >>>> input_sync(touch); >>>> - return; >>> >>> I'd like to slip in a 'ts_input = 1;' right here, and then... >>> >>>> } >>>> >>>> + if (!context->ir_isopen) >>>> + return; >>>> + >>> >>> ...we can do if (!context->ir_isopen && !ts_input) return; here. >>> Otherwise, if IR isn't open, your touch button presses are going to >>> get thrown away, aren't they? >> If the lirc device is closed it doesn't not make any sense to try to >> feed the button matrix codes trough it. >> If the device is opened the coordinates are passed though lirc and the >> input subsystem. >> Some people use the button mode feature because you don't need a >> X-server to get coordinates from the touchscreen. > > Ah, I think I get it now, helps to read the code more carefully... The > touch stuff has already been delivered via the input subsystem at this > point, but it can *also* be delivered via lirc, if ir_isopen, and the > touch data can then be processed from either input or lirc. So yeah, > that check is correct. > Yes exactly that's the way it works. > >>>> @@ -1841,6 +1821,20 @@ >>>> context->touch = NULL; >>>> retval = 0; >>>> } >>>> + >>>> + usb_fill_int_urb(context->rx_urb_intf1, >>>> context->usbdev_intf1, >>>> + usb_rcvintpipe(context->usbdev_intf1, >>>> + context->rx_endpoint_intf1->bEndpointAddress), >>>> + context->usb_rx_buf, sizeof(context->usb_rx_buf), >>>> + usb_rx_callback_intf1, context, >>>> + context->rx_endpoint_intf1->bInterval); >>>> + >>>> + retval = usb_submit_urb(context->rx_urb_intf1, GFP_KERNEL); >>>> + >>>> + if (retval) { >>>> + err("%s: usb_submit_urb failed for intf1 (%d)", >>>> + __func__, retval); >>>> + } >>> >>> In both the usb_submit_urb() cases here, we need to actually propagate >>> the error upwards if it fails, not just warn. Stuff isn't going to >>> work at all correctly if either of those failed on us. >> >> I removed this because I was not quite sure if a fail needs to result in >> an abort. > > I'm reasonably confident failure here means the usb_rx_callback_*() > functions would never fire -- we're essentially in the same state as > we are after usb_kill_urb() has been called. > > >> I think you can remove the mutex_unlock before the goto exit; there's an >> unlock right before we exit the function. > > They're different locks, need to unlock both. :) Well okay I just remembered that there was one right before the return. > > I've gone ahead with some commits to my local git tree, which I think > should be good to go, and if you don't see anything wrong w/them, I'll > get them into lirc cvs too: > > http://devel.wilsonet.com/misc/0001-lirc_imon-touchscreen-handling-fixups.patch > > http://devel.wilsonet.com/misc/0002-lirc_imon-support-mouse-and-touch-input-devices-eve.patch > > > I've tested these out successfully on both a 15c2:0045 and 15c2:ffdc > (iMON Knob) device now. > Yeah they look good so far, can't test it right know but will do it this evening. |
From: Jarod W. <ja...@wi...> - 2009-06-17 17:11:57
|
On Jun 17, 2009, at 12:24 PM, Rene Harder wrote: > Jarod Wilson wrote: >> On Jun 17, 2009, at 1:36 AM, Rene Harder wrote: >> >>> Jarod Wilson wrote: >>>> On 06/16/2009 11:35 PM, Rene Harder wrote: >>>>> - added packet length check, to prevent interference between mouse >>>>> and touchscreen input emulation. >>>>> (Occurs only if your use the touchscreen right before the pad, in >>>>> this >>>>> case >>>>> buf[6] still contains the previous value (0x14) same for buf[7].) >>>> >>>> Hrm. I need to see how these len checks behave on an ffdc device. >>>> I'll >>>> put that on my TODO list for tomorrow. Oh! Wait... This is >>>> definitely >>>> a problem. Checking for len == 5 along with context->pad_mouse >>>> effectively neuters scroll wheel emulation using ch+/-. >>> >>> Right, they have 8 bytes, but without length check the condition >>> is true >>> for the touchscreen as well if mouse input mode is activated. >>> >>> one way would be to move the touchscreen input stuff before the >>> mouse >>> condition and replace (len == 5) with !ts_input, that should block >>> this >>> condition if it is a touchscreen code. >> >> Yeah, that looks like it'll do the trick, and there's no chance of it >> causing any regressions with the older devices, since none of them >> will have context->has_touchscreen true. I'm going to add one more >> check there: >> >> if ((context->pad_mouse || !context->ir_isopen) && !ts_input) > Or we use an else if (), so we can get rid of the !ts_input Ah, good call, have done so locally. >>>> Oh, except for one thing... Right-click never works for me in mouse >>>> mode, ... > Well found it this morning. > > input_report_key(mouse, BTN_RIGHT, > buf[1] >> 2 & 0x01); > > > > We shift to much here, it must be buf[1]>>1 &0x01 > Was a bug in my first imontouch driver as well. I suspected it might be that. I've made that change locally now, as well as altering the bits to keep the ffdc devices working. >> I've gone ahead with some commits to my local git tree, which I think >> should be good to go, and if you don't see anything wrong w/them, >> I'll >> get them into lirc cvs too: >> >> http://devel.wilsonet.com/misc/0001-lirc_imon-touchscreen-handling-fixups.patch >> >> http://devel.wilsonet.com/misc/0002-lirc_imon-support-mouse-and-touch-input-devices-eve.patch >> >> >> I've tested these out successfully on both a 15c2:0045 and 15c2:ffdc >> (iMON Knob) device now. >> > > Yeah they look good so far, can't test it right know but will do it > this > evening. I'll hold off on committing to cvs until I hear from you. I've pushed these and the tweaks from your mail that I'm replying to here in my git tree: http://git.wilsonet.com/linux-2.6-lirc.git/?a=shortlog (Along with the as-yet-not-retested bits to set IR protocol) -- Jarod Wilson ja...@wi... |
From: Rene H. <re...@sa...> - 2009-06-17 22:58:18
|
Jarod Wilson wrote: > I'll hold off on committing to cvs until I hear from you. I've pushed > these and the tweaks from your mail that I'm replying to here in my > git tree: > > http://git.wilsonet.com/linux-2.6-lirc.git/?a=shortlog > > (Along with the as-yet-not-retested bits to set IR protocol) > just got home and tested the latest revision, so far most stuff is working but there is still a problem with the mouse input section. If i turn my Volume knob on the frontpanel to the left it will trigger an input event. I think this condition is not unique to the code of the pad, it is true for all buttons on the rc and frontpanel. Maybe we should add the length check and an exception for the old models. Do you have a better idea? |
From: Rene H. <re...@sa...> - 2009-06-18 01:16:26
Attachments:
false-mouse-input-gen-fix.patch
|
Hi Jarod, please check the attached patch, It fixes our false mouse event generation. It works for my device even the scroll emulation is not effected. Maybe we should add this for the old imon's as well but I'm not quite sure if they have the same packet length, do they? Rene |