Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT plugin #5

Closed
Perforex opened this issue Jan 2, 2018 · 12 comments
Closed

MQTT plugin #5

Perforex opened this issue Jan 2, 2018 · 12 comments

Comments

@Perforex
Copy link

Perforex commented Jan 2, 2018

I'm considering writing an MQTT plugin for fauxmo but before I start I'd like to understand how popular this requirement might be. It's popularity won't dictate whether I write the plugin, since I need it for my own use, but rather the care, testing and functionality that I include. For example I don't need features such as user authentication or QoS since I'm only using MQTT on my internal network and very low volume but if these are features useful to a lot of other people, I'll consider it during the design phase.

Though I haven't been a professional code writer for many years, having looked at the existing plugins and the guidelines I don't think it will be an onerous task.

I'd welcome all feedback on this proposal/suggestion.

@SupraJames
Copy link

I am here looking for an MQTT plugin and my requirements are kinda like yours. I don't care (much) about security, if someone's on my internal network they are welcome to control my lights!

If you do hack something together, please let me know, and I'd be happy to test.

@n8henrie
Copy link
Owner

n8henrie commented Jan 2, 2018

I think an MQTT plugin would be sweet. I don't use it a whole lot, but I know the IOT community seems to love it overall.

FYI, for the few MQTT devices I do use, I have them integrated through Home Assistant by way of mosquitto, and Fauxmo works with them just fine through the Home Assistant plugin.

For people not interested in running things through Home Assistant, a direct MQTT plugin would probably be much appreciated.

@Perforex
Copy link
Author

Perforex commented Jan 2, 2018

@SupraJames - Thanks, I'll certainly keep you up to date on progress and testing would me much appreciated.
@n8henrie - Thanks, I'm sort of in the second boat, I already have devices that work well through MQTT and whilst I could use Home Assistant, I'd rather not go down that route.

@Perforex
Copy link
Author

Perforex commented Jan 6, 2018

@n8henrie
Code developed and tested, looking good so far but external testing is welcomed.

A couple of questions for you:

  • Do you need to add me as a contributor before I have write access to create a Pull Request or is the default write?
  • I've introduced a new module (paho-mqtt) and I'm not sure how that should be handled in terms of packaging. Advice and guidance welcomed.

Help with these two gratefully received.

@SupraJames
Copy link

SupraJames commented Jan 6, 2018 via email

@n8henrie
Copy link
Owner

n8henrie commented Jan 6, 2018

@Perforex awesome!

  1. No, like most GitHub repos, anyone can make a pull request. This allows everyone to review the changes before they are merged and for the continuous integration tests to run (if any).
  2. Dependencies (and pinned version numbers) should be added to the module-level docstring. Search the readme for dependenc, see my examples, and let me know how I can further clarify.

@Perforex
Copy link
Author

Perforex commented Jan 7, 2018

Thank you both for your help and advice. As you can probably tell I'm new to the collaborative use of git but it's a great learning experience. I've forked the plugin, added the new module (including the docstring for dependencies) and raised the pull request. #6

I hope this is all as per convention and I look forward to the next stage in the process. :-)

@n8henrie
Copy link
Owner

n8henrie commented Jan 7, 2018 via email

@SupraJames
Copy link

Perforex, just testing this and it is also working for me, so thanks!

I do see that your code is connecting, publishing, and disconnecting to the MQTT server for each call, which doesn't seem ideal, but I'm equally new to this.

Perhaps you can do self.client = mqtt.Client() and self.client.connect(self.qserver,self.qport,60) in init and just publish messages in the handlers for on and off.

I've tested locally that change and seems to work OK. I'm supposing that the MQTT library itself will take care of re-connecting to the broker as required.

@Perforex
Copy link
Author

Perforex commented Jan 7, 2018

@SupraJames
Thanks James, glad it's working. I've just had a look at your changes and I like your thinking, it does seem more efficient to do the connection once. I'll merge your changes so that it can be incorporated into the final pull request. (I think that's the way it's supposed to happen...)

@n8henrie
Please advise if we're doing this right, thanks.

@n8henrie
Copy link
Owner

n8henrie commented Jan 9, 2018

Doing what right? The PR?

I'm no expert on the MQTT part.

Will review your new PR when you submit.

@n8henrie
Copy link
Owner

Merged into dev with d2cfea3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants