sde1000 / python-dali

Library for controlling DALI lighting systems
Other
150 stars 73 forks source link

Unipi axon s605 driver #59

Closed mhemeryck closed 4 years ago

mhemeryck commented 4 years ago

Fixes #48

Adds driver implementation for Unipi Axon S605. My idea is to use this as part of my home automation setup. I would interface with this driver implementation by adding a mqtt wrapper around it, converting events to commands on my DALI bus.

Implementation taken from https://git.unipi.technology/UniPi/unipi-python-lighting/commit/0975401ba6358d475ef46532ff5271bee46d601a adapted with unit tests. I did check the original code, and it does mention your licence; see https://git.unipi.technology/UniPi/unipi-python-lighting/blob/0975401ba6358d475ef46532ff5271bee46d601a/UnipiDali/unipidali.py#L6

Implementation:

Tests:

Some further notes:

Note that this is a first proposal, looking forward to hear your remarks!

dgomes commented 4 years ago

Nice contribution!

I've come up with https://github.com/dgomes/dali2mqtt to integrate with Home Assistant through MQTT see if it's of any use to you

mhemeryck commented 4 years ago

The home assistant integration with MQTT is completely what I wanted to do! I'll have a look how it'd work with the unipi driver; will see if I can also create a PR for that. Thx!

dgomes commented 4 years ago

My code is rough and was built for personal use, would really love some PR's :)

(lets move discussion to that repo)

sde1000 commented 4 years ago

Thanks for this!

I'm happy to accept the driver code, but I'd like to keep the diff as clean as possible. Could you update this pull request to be just the driver and its tests, and drop the unrelated changes? So really just the files dali/driver/unipy.py, dali/tests/test_unipi.py, and the necessary changes to setup.py (i.e. drop the changes to quote style).

At the same time, please add yourself to the contributors list in README.rst and add a comment in the driver indicating its origin.

I'd be happy to accept a separate pull request for the .flake8 and .gitignore files if you would find that helpful.

I'm working on dali/bus.py at the moment so please just ignore it for now. (The plan there is to integrate it with the async device drivers, and add an example DALI<->MQTT bridge using gmqtt.)

mhemeryck commented 4 years ago

Thanks for this!

I'm happy to accept the driver code, but I'd like to keep the diff as clean as possible. Could you update this pull request to be just the driver and its tests, and drop the unrelated changes? So really just the files dali/driver/unipy.py, dali/tests/test_unipi.py, and the necessary changes to setup.py (i.e. drop the changes to quote style).

OK. This will probably break my py3-based setup for now, since some of the imports (e.g. import Sets) don't work for me.

At the same time, please add yourself to the contributors list in README.rst and add a comment in the driver indicating its origin.

Did this, thx :)

I'd be happy to accept a separate pull request for the .flake8 and .gitignore files if you would find that helpful.

Those are basically just some defaults I'd like to have in the project I work on, since it makes it for my editor to work in (vim). Also, I typically use these as part of linting checks in the CI.

I'm working on dali/bus.py at the moment so please just ignore it for now. (The plan there is to integrate it with the async device drivers, and add an example DALI<->MQTT bridge using gmqtt.)

That's really nice to hear! Looking forward to that.

sde1000 commented 4 years ago

Fails on the python 2.7 check, but I'm not going to worry about that since I'm removing it shortly.

mhemeryck commented 4 years ago

Fails on the python 2.7 check, but I'm not going to worry about that since I'm removing it shortly.

Just noticed it and fixed the import issue (just in time apparently, since you just merged it in :)).

Thx anyways, will certainly keep an eye on this repo and will look into how I can contribute in the future.

mhemeryck commented 4 years ago

btw, will this get its own tag / release / pypi artifact?

sde1000 commented 4 years ago

Next time a release happens, which will probably be when I have the asyncio stuff in a suitable state. Not too long, hopefully!