d0ugal / python-rfxcom

A Python 3.3+ library for working with your RFXTrx and automating your home.
BSD 3-Clause "New" or "Revised" License
8 stars 9 forks source link

Yield in setup #15

Closed kalfa closed 10 years ago

kalfa commented 10 years ago

Here's the change for having setup() a bit more readable. Unfortunately I had to split setup() and _setup().

A coroutine is a generator, which means that needs something to yield from it to generate something.

Simply doing: loop.call_later(setup)

def setup(self): yield from foo log(something) yield from bar

won't work, since when setup() is called back by the loop, will return a generator, which needs something to have it produce the results. The only way for the loop to have it done is to:

In other terms, setup now removes itself as writer and then calls asynchronously _setup() which eventually will add the default writer, after having setup everything.

It should not be a problem, flow-wise, i find just uglier than I though, to have to split it.

NOTE Please test it in a real HW env, before merging. I tested it on my env with a simple stub, which does not highlight any real HW problem.

Also, I updated some of the unittests you amended, for completeness:

d0ugal commented 10 years ago

Awesome, this looks great. I'll aim to find some time over the next few days to test it against hardware and review the code in detail then get back to you.

d0ugal commented 10 years ago

Looks like a few PEP8 errors snuck in. It's quite strict checking: https://travis-ci.org/d0ugal/python-rfxcom/jobs/27693397

Next I'll try this out on hardware.

kalfa commented 10 years ago

I'll fix the pep8 stuff, thanks. I don't recall python-mode showing those though, weird :/

d0ugal commented 10 years ago

If you use tox -e flake8 it will run the linting tests locally for you. I'll try to do the hardware test of this tomorrow.

kalfa commented 10 years ago

I repushed a fixed version for the branch, which should clear all the pep8/pyflake/pylint errors (my version of pep8 complained about the use of lambda, so I changed it, no change in functionality though). In theory it's clean. I'm using python-mode with vim, not tox ATM.

I also added a temporary sleep beteen STATUS & MODE. The way to get the answer and send the next pkt should be thought. Specifically we need to be able to read an answer, which at that point is already delegated to the reader consumer.

coveralls commented 10 years ago

Coverage Status

Coverage decreased (-2.02%) when pulling 871e6626356e975153c777e0bd62f838d0f066bc on kalfa:yield_in_setup into 35ba1ffdf7831ac21ae76b77f9fe0c7da7d626a1 on d0ugal:master.

coveralls commented 10 years ago

Coverage Status

Coverage decreased (-2.01%) when pulling 871e6626356e975153c777e0bd62f838d0f066bc on kalfa:yield_in_setup into 35ba1ffdf7831ac21ae76b77f9fe0c7da7d626a1 on d0ugal:master.

d0ugal commented 10 years ago

Sorry I've not gotten back to you about this, It's on my list for this week. If not before, I'll have time on Saturday - I plan to do some work on home automation stuff then. However, I hope to have a wee bit of time for this tonight.

d0ugal commented 10 years ago

Made a minor tweak in https://github.com/d0ugal/python-rfxcom/commit/ed69f61e11a8a3b832a85d17bfc58dc0850a9563 which seems to solve my startup issues. So, this is working really well now.

One thing that is now a little odd, we don't need to use blocking writes for the MODE or STATUS packets, but we do. I think we probably want to change that, but one of your objections originally was the confusion of some blocking and some non-blocking operations in the setup. Any ideas?

Thanks again!

d0ugal commented 10 years ago

You might find issue #16 interesting, it should summarise the issue with the Mode and what I want to do next.

kalfa commented 10 years ago

I opened an issue did the refactoring we mentioned time ago. This might use existing classes which do non blocking already.