[2/3] mceusb: fix up reporting of trailing space

  We were storing a bunch of spaces at the end of each signal, rather than
a single long space. The in-kernel decoders were actually okay with
this, but lirc isn't. Both are happy again with this change, which
starts accumulating data upon seeing an 0x7f space, and then stores it
when we see the next non-space, non-0x7f space, or an 0x80 end of signal
command. To get to that final 0x80 properly, we also need to support
proper parsing of 0x9f 0x01 commands, support for which is also added.

Also remove an obsolete include, some stray colon-space pairs from
prior to conversion to dev_foo printk macros and a magic number.

Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/media/IR/mceusb.c |   39 +++++++++++++++++++++++----------------
 1 files changed, 23 insertions(+), 16 deletions(-)
  

On Thu, Oct 28, 2010 at 11:08:10PM -0400, Jarod Wilson wrote:
> We were storing a bunch of spaces at the end of each signal, rather than
> a single long space. The in-kernel decoders were actually okay with
> this, but lirc isn't. Both are happy again with this change, which
> starts accumulating data upon seeing an 0x7f space, and then stores it
> when we see the next non-space, non-0x7f space, or an 0x80 end of signal
> command. To get to that final 0x80 properly, we also need to support
> proper parsing of 0x9f 0x01 commands, support for which is also added.

I think the driver could be further simplified by using 
ir_raw_event_store_with_filter(), right?
  

On Fri, Oct 29, 2010 at 09:21:21PM +0200, David Härdeman wrote:
> On Thu, Oct 28, 2010 at 11:08:10PM -0400, Jarod Wilson wrote:
> > We were storing a bunch of spaces at the end of each signal, rather than
> > a single long space. The in-kernel decoders were actually okay with
> > this, but lirc isn't. Both are happy again with this change, which
> > starts accumulating data upon seeing an 0x7f space, and then stores it
> > when we see the next non-space, non-0x7f space, or an 0x80 end of signal
> > command. To get to that final 0x80 properly, we also need to support
> > proper parsing of 0x9f 0x01 commands, support for which is also added.
> 
> I think the driver could be further simplified by using 
> ir_raw_event_store_with_filter(), right?

Hm, yeah, it probably would. Hadn't even thought to look at that. I'll
give that a closer look...
  

On Fri, Oct 29, 2010 at 09:21:21PM +0200, David Härdeman wrote:
> On Thu, Oct 28, 2010 at 11:08:10PM -0400, Jarod Wilson wrote:
> > We were storing a bunch of spaces at the end of each signal, rather than
> > a single long space. The in-kernel decoders were actually okay with
> > this, but lirc isn't. Both are happy again with this change, which
> > starts accumulating data upon seeing an 0x7f space, and then stores it
> > when we see the next non-space, non-0x7f space, or an 0x80 end of signal
> > command. To get to that final 0x80 properly, we also need to support
> > proper parsing of 0x9f 0x01 commands, support for which is also added.
> 
> I think the driver could be further simplified by using 
> ir_raw_event_store_with_filter(), right?

And in fact, it is. I've got a new series of patches redone atop your
rc-core patch series that includes usage of _with_filter in mceusb.

http://git.kernel.org/?p=linux/kernel/git/jarod/linux-2.6-ir.git;a=shortlog;h=refs/heads/staging

And more specifically, the update of this patch:

http://git.kernel.org/?p=linux/kernel/git/jarod/linux-2.6-ir.git;a=commitdiff;h=bc3d1300cd2d51dc8d877be343382d8932320dfc

Still a bit of testing to do before I send v2 off to the list, plus I'd
like to know where we're at w/your patches first wrt getting them
committed, just in case I need to rework them slightly.
  

On Tue, 2 Nov 2010 17:12:14 -0400, Jarod Wilson <jarod@redhat.com> wrote:
> On Fri, Oct 29, 2010 at 09:21:21PM +0200, David Härdeman wrote:
>> I think the driver could be further simplified by using 
>> ir_raw_event_store_with_filter(), right?
> 
> And in fact, it is. I've got a new series of patches redone atop your
> rc-core patch series that includes usage of _with_filter in mceusb.

From a quick check, I think the entire PARSE_IRDATA block in
mceusb_process_ir_data() could be simplified to:

case PARSE_IRDATA:
    ir->rem--;
    rawir.pulse = !!(ir->buf_in[i] & MCE_PULSE_BIT);
    rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK)
                      * MCE_TIME_UNIT * 1000;
    ir_raw_event_store_with_filter(ir->rc, &rawir);
    break;

And you can then remove "struct ir_raw_event rawir" from struct
mceusb_dev.
  

On Wed, Nov 03, 2010 at 01:15:30PM +0100, David Härdeman wrote:
> On Tue, 2 Nov 2010 17:12:14 -0400, Jarod Wilson <jarod@redhat.com> wrote:
> > On Fri, Oct 29, 2010 at 09:21:21PM +0200, David Härdeman wrote:
> >> I think the driver could be further simplified by using 
> >> ir_raw_event_store_with_filter(), right?
> > 
> > And in fact, it is. I've got a new series of patches redone atop your
> > rc-core patch series that includes usage of _with_filter in mceusb.
> 
> From a quick check, I think the entire PARSE_IRDATA block in
> mceusb_process_ir_data() could be simplified to:
> 
> case PARSE_IRDATA:
>     ir->rem--;
>     rawir.pulse = !!(ir->buf_in[i] & MCE_PULSE_BIT);
>     rawir.duration = (ir->buf_in[i] & MCE_PULSE_MASK)
>                       * MCE_TIME_UNIT * 1000;
>     ir_raw_event_store_with_filter(ir->rc, &rawir);
>     break;
> 
> And you can then remove "struct ir_raw_event rawir" from struct
> mceusb_dev.

Once again, I think you're correct. :) I'll give this a spin with a few
mceusb devices, but it does certainly look like that'll work, now that
we're using _with_filter.