Maybe remove Twisted dependency?

[zpfvo]

Hi,
this not a real issue rather than a question of design. I really like the interface you created for the gpio interface, but i think it is unnecessary to have the Twisted dependency.

Some people are are maybe using threads in a different way, for others its ok to wait for a pin change blocking.
I just forked and removed everything related to twisted and now call
events = Controller._poll_queue.poll(EPOLL_TIMEOUT)
Controller._poll_queue_event(events)
in my main loop which works nice. Everything is handled in a pin callback function.
At least it would be nice to have a public method to do a blocking read.

Either way,
nice module! Thanks for your work!

[derekstavis]

Hey @zpfvo. Thanks for your insights. They are very welcome 👍

Here's my considerations about your comments:

(...) but i think it is unnecessary to have the Twisted dependency.

In fact Twisted isn't a very necessary dependency, it's main duty is to handle two specific cases:

• Make the interaction between main thread and the poll queue thread. In a GUI application you probably don't want to touch the view hierarchy in a thread that's not the main one.
• Make callbacks happen in the next main loop tick, avoiding delays while processing pin changes. If callbacks were dispatched "inline" the surface for race conditions would be bigger.

I'd like to have a way to make the library compatible with other main loops (There's the Glib one that's used by GTK, for example), this would need a "manual" API, like you have been doing.

At least it would be nice to have a public method to do a blocking read.

This is the part where the main loop integration APIs would enter. I will be considering your proposal, but I still need to evaluate the impact of the changes in code that depends upon this module.

[thom-nic]

I thought this same thing, I was planning on coopting most of the code to make it work with Tornado's ioloop. I guess I will find out how 'compatible' they are once I try making the switch. I'll update if I get something working that I feel is worth sharing.

[derekstavis]

It should be completely compatible with Tornado, as Tornado is based on Twisted main loop!

[thom-nic]

awesome I didn't know that! So it seems like Controller could be updated to be initialized via a factory function that takes a reactor or ioloop argument...

[derekstavis]

Theoretically it should work out of the box without this need. At https://github.com/derekstavis/python-sysfs-gpio/blob/master/sysfs/gpio.py#L34 the module takes the reactor singleton, which should be initialised after Tornado. Or is ioloop different than reactor?

[thom-nic]

I guess what I'm thinking is I can remove the twisted import and hard dependency if the reactor is passed in from calling code... If Tornado has a compatible reactor.

[thom-nic]

From what I'm reading in the Tornado ioloop doc it essentially wraps select/ epoll so I think I would just pass a Pin instance to io_loop.add_handler(pin.fileno(), callback, io_loop.READ)

[thom-nic]

So to follow up, I was successful in swapping out twisted for tornado, of course I added a hard dependency on Tornado now and hacked it pretty bad to act just the way I want so there's no chance of doing a clean PR in the state that it's in. But I can at least share what I've got working...

The crux of what changed is here: https://github.com/thom-nic/chip-ioboard/blob/develop/sysfs/__init__.py#L174-L189
(sorry I renamed modules and moved Pin to its own file :/)

Usage can be seen here:
https://github.com/thom-nic/chip-ioboard/blob/develop/main.py#L124-L132
It's very clean, no additional epoll/select handling in the Gpio class. Tornado's internal epoll inside IOLoop watches the sysfs/gpio fds and calls my callback, which get passed the pin instance and new value which is also nice.

I tested this on a CHIP (getchip.com) using XIO-P7 which maps to pin # 415 in code. and it works very reliably.

Again, apologies I hacked the code into a state that can't be submitted back in a PR but hopefully this still proves useful.

[derekstavis]

So to follow up, I was successful in swapping out twisted for tornado

I'm glad you accomplished it! That's awesome! 🎉

Again, apologies I hacked the code into a state that can't be submitted back in a PR

You don't have to apologize. The freedoms of MIT license grants you the right to hack it in any way to suit your needs, as long as you keep the attribution. That's what open source is about: Hacking spreading the word 🔨 💬

Thanks for your interest in this module! I may fall into extending it to accomplish easy third-party integration, freeing it from explicit Twisted dependency to cover other main loops. I will ping you when I have something.

[thom-nic]

cool! Yeah I did leave the ability to pass in a (Tornado) ioloop but as it is I don't believe ioloop and twisted's reactor have compatible APIs so I imagine you'd end up creating a wrapper for one or the other or both.

I'll keep myself subscribed to this issue so if you update I'll see it. Thanks!