Menu

#59 what is LIRC_EOF?

1.0
closed
nobody
None
fixed,
2014-11-17
2014-10-16
No

Hi.

[I've posted that to ML but it seems to stuck waiting
for moderatorial]

I've found out that some keypresses from my remote
are not accepted by lirc. It turned out to be the problem
with LIRC_EOF: if it finds this bit set, the data is rejected.
I re-defined the LIRC_EOF const to another value and
things started to work.
What is the reason for LIRC_EOF? I googled up the NEC
protocol but can't find any EOF bit in it. What is it used for?

Discussion

  • Alec Leamas

    Alec Leamas - 2014-10-16

    Hi!

    First of all: the ML is not moderated, and your message should slip through if you have subscribed to it. Have you?

    That said, LIRC_EOF is new in current git. It was needed by the file driver, which obviously might be the first driver with this need.

    Here are many questions: why are these bits set in your case? (looks strange to me..) Have you requirements to encode something else in the spare bits?

    Another question is if I did this right. I thought about it, and my conclusion was that something like LIRC_EOF adds some meaningful semantics to the driver interface. This is still not released though, so it's certainly not set in stone.

    If anything else is affected by this it's kind of a bug - the LIRC_EOF bit was certainly undefined until I introduced it.

    EDIT: which driver are you using?

     

    Last edit: Alec Leamas 2014-10-16
  • Stas Sergeev

    Stas Sergeev - 2014-10-16

    No, I didn't subscribe. The notification I've
    got, was:


    Your mail to 'LIRC-list' with the subject

    what is LIRC_EOF?
    

    Is being held until the list moderator can review it for approval.

    So please double check if the list is not moderated
    for non-members.

    Here are many questions: why are these bits set in your case?

    I don't know. The string in my config file (that
    worked in the past) is this:
    FACE_POWEROFF 0x4AB53FC0
    Which definitely has this bit set.
    Could you please point me to some specs that
    prohibits that? I'll show it to our engineers
    then, maybe its a bug in the remote they did.
    Although it is very unlikely they'll fix it
    because it was in production for many years,
    but at least it would be good to know where
    the actual problem is.

    Have you requirements to encode something else in the spare bits?

    From what I've googled up about the NEC protocol,
    I can't answer this question. Where is the spec
    that mentions "spare bits"?

    EDIT: which driver are you using?

    "default". :)

     
  • Alec Leamas

    Alec Leamas - 2014-10-16

    The list is noderated for non-members. This is necessary these days (spam), sorry. Just subscribe to the list [1] to fix.

    LIRC is not famous for specs, for sure. However, there is the bundled kernel header file in include/media/lirc.h. This defines lirc_t, which basically says that the data from the driver is in the lower 24 bits of a 32-bit int while the upper 8 bits are reserved for bitflags.

    lirc_t (I didn't invent that name...) is used in a lot of places. However, the only place where it's actually used as such is in the data from the driver (function readdata()) into the receive buffer in receive.c. receive.c really wants this data to be at most 24 bits + flags.

    This is also described in the API docs( doc/api-docs/html), look in the driver API manual excerpt, function readdata().

    However, I'm not quite sure how this is handled by the default driver. I don't think the value returned form the remote is "wrong", but the decoding might be. We need more eyes on this (a ML message) and also your lircd.conf.

    So the default driver... on top of what? I presume /dev/lirc0, but what kernel driver is involved?

    Something is rotten here, for sure...

    EDIT: Silly me... Don't think too much about the value returned from the nec protocol, it's irrelevent. Each lirc_t value returned by readdata() is just a pulse or space duration, i. .e. a single lirc_t is not even a bit in the NEC protocol. This would only be a problem if there was a pulse/space longer than 24 bits (~16 s), and that's not happening.

    Bottom line: If the default driver returns a pulse/space width with undefined bits set in the upmost byte it's a bug.

    You should be able to pick up the log message defined in receive.c line 69 if there really is a LIRC_EOF bit set. Can you check this? This would nail this down.

    [1] https://sourceforge.net/p/lirc/mail

     

    Last edit: Alec Leamas 2014-10-16
  • Bengt Martensson

    1. The mailing list holds postings by non-subscribers in the way described. So, subscribe first, and make sure that you use the same sender address as the subscribed address. Q: who is "the list moderator"?

    2. It appears as the behavior described is a bona-fide bug, introduced by the activities described. (I have not studied the code or tried to reporoduce the bug.) There is definitely nothing like "EOF" in NEC signals, nor in any other IR protocol.

    3. One of the many shortcomings of Lirc is that there is no way for the user to force a close of the driver or the device -- except for a signal effectively terminating Lircd. Just recently, we introduced the "close" for the drivers, but, IIRC, it is only called by termination of the daemon. Possibly there should be a "command" in the "language" understood by the socket (Unix domain OR tcp) that say "close the driver"; problem is, what is the state or Lircd then? Closed driver, and no commmand to open anew?

     
  • Alec Leamas

    Alec Leamas - 2014-10-16

    Unforunately, I don't have not adminstratve access. I think the moderator basically is defunced(?).

     
  • Alec Leamas

    Alec Leamas - 2014-10-16

    It might be a bug in the new code, for sure. But ut might also be that this reveals some already existing bug. FWIW, my gut feeling is that this is what's happening.

     
  • Stas Sergeev

    Stas Sergeev - 2014-10-16

    The list is noderated for non-members. This is necessary
    these days (spam), sorry. Just subscribe to the list [1] to fix.

    OK, done, but this doesn't "fix" the already
    sent message. So I double the Q: who is "the list moderator"?
    Anyway, lets have the discussion in this ticket then. :)

    So the default driver... on top of what? I presume /dev/lirc0

    Yes.

    EDIT: Silly me... Don't think too much about the value
    returned from the nec protocol, it's irrelevent.

    No because you are applying the LIRC_EOF to the
    already decoded values, not to the values from
    readdata().

    Bottom line: If the default driver returns a pulse/space
    width with undefined bits set in the upmost byte it's a bug.

    It doesn't, I checked that.
    If you be only applying the LIRC_EOF to pulse periods,
    that would have probably worked well.

    I'll double check this tomorrow at work.
    Now from your answers I've got a clearer picture, so
    I'll be able to locate the bug.

     
  • Stas Sergeev

    Stas Sergeev - 2014-10-16

    get_code() is a suspect.
    (I am not at work now; whatever I say here is not verified yet)

     
  • Stas Sergeev

    Stas Sergeev - 2014-10-16

    Additionally, I didn't get the "receive: Got EOF"
    messages (the place where you apply LIRC_EOF to
    the values from readdata()), but instead I was getting
    "decode all: returning EOF" debug.

    I'll be testing the following patch tomorrow:

    diff --git a/lib/ir_remote.c b/lib/ir_remote.c
    index f11c4c3..c0ca292 100644
    --- a/lib/ir_remote.c
    +++ b/lib/ir_remote.c
    @@ -347,9 +347,6 @@ static struct ir_ncode *get_code(struct ir_remote *remote, i  
            int found_code, have_code;
            struct ir_ncode *codes, *found;
    
    -       if (code & LIRC_EOF)
    -               return &NCODE_EOF;
    -
            pre_mask = code_mask = post_mask = 0;
    
            if (has_toggle_bit_mask(remote)) {
    

    EDIT Indenting patch for readability --alec

     

    Last edit: Alec Leamas 2014-10-16
  • Stas Sergeev

    Stas Sergeev - 2014-10-16

    OK, patch got mangled...
    Will update it after testing.

     
  • Alec Leamas

    Alec Leamas - 2014-10-16

    Yes, anyway this line is broken. I actually test the decoded value, treating it as it was a plain duration which obviously is wrong. That patch will most likely solve your problem, but a real fix seems somewhat more complicated. I will look into it.

    Thanks for reporting this!

     
  • Alec Leamas

    Alec Leamas - 2014-10-22

    Yes, even the unit tests works.

    There are bugs and there are bugs. This make me blush. Thanks for patch! Applied in [e74abe], closing.

     

    Related

    Commit: [e74abe]

  • Alec Leamas

    Alec Leamas - 2014-10-22
    • status: open --> closed
     
  • Stas Sergeev

    Stas Sergeev - 2014-10-22

    Btw, thanks for making yaml optional!

     
  • Alec Leamas

    Alec Leamas - 2014-10-22

    You're welcome :)

     
  • Alec Leamas

    Alec Leamas - 2014-11-17
    • Resolution: --> fixed,
     

Log in to post a comment.