Skip to content
This repository was archived by the owner on Nov 19, 2020. It is now read-only.

Mark call to lirc_client.lirc_nextcode(...) as nogil - #17

Merged
tompreston merged 1 commit into
tompreston:masterfrom
jpstotz:master
Jul 9, 2018
Merged

tompreston merged 1 commit into
tompreston:masterfrom
jpstotz:master

Conversation

@jpstotz

@jpstotz jpstotz commented Feb 19, 2017

Copy link
Copy Markdown

I had some problems using python-lirc: every time I called lirc.nextcode() (with blocking mode activated, because without blocking it makes busy-waiting which is pretty bad) in my python3 script, my whole program stopped working, even if I did this in a separate thread.

While debugging the problem I learned about the "biggest problem of python" (the GIL) and how complicated python multi-threading is because of it.
As the problematic call is a native one from the lirc client library I assume that it is ok to mark it as nogil, when using only cdef (hence no-Python?) variables.

With the changes in this pull request I was able to create a separate python thread that calls lirc.nextcode() in a loop that effectively did not had any effect on the MainTread regarding blocking the GIL.

However I have to admit that my cython experience is very little. Hence that it works for me doesn't mean that it actually works without problems on all platforms. May be a more experienced cython programmer should look at the code and say if it is ok the way I changed it.

But as blocking the GIL has such a bad impact I thought that a non-GIL-blocking version of python-lirc would be useful for everybody.

…ng it would block all threads in our python program

@tompreston tompreston left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks good to me.

@tompreston

Copy link
Copy Markdown
Owner

Hey I don't have any IR hardware to hand at the minute. Can you write a test showing how this works for someone else to have a look? Otherwise I might be able to take a look tomorrow.

@jpstotz

jpstotz commented Oct 24, 2017

Copy link
Copy Markdown
Author

The following class is the simplified version of what I use in my project. It uses blocking wait in a separate thread. As lirc.nextcode() does not block the GIL it has no negative effect on other threads.
It also includes the logic for reconnecting e.g. if the lirc daemon has been restarted.

class LircThread (threading.Thread):
    def __init__(self):
        threading.Thread.__init__(self)
        self.daemon = True
        self.connect()
        
    def connect(self):
        try:
            lirc.deinit()
        except:
            pass
        self.lirc_client = lirc.init("mylirc", blocking=True)
        logger.info("New LIRC connection established")
        
        
    def run(self):
        logger.debug("LIRC thread started %s", threading.currentThread().getName())
        while 1:
            code = None
            try:
                code = lirc.nextcode()
            except:
                logger.exception('Exception thrown while waiting for next lirc code')
                time.sleep(5)
                self.connect()
                continue
            try:
                self.process_lirc_code(code)
            except:
                logger.exception('Exception thrown while processing lirc code')
                
                
    def process_lirc_code(self, code):
        if code is None:
            return
        if len(code)==0:
            return
        #print("code: "+str(code))
        if len(code)>0:
            code = code[0]

        if code == "PLAY":
            dothis
        else if code == "STOP":
            dothat
        else
            logger.warn("unknown code received: {}".format(code))

@tompreston
tompreston merged commit 3bb29b4 into tompreston:master Jul 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants