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

Rest interface for miflora #86

Closed
wants to merge 1 commit into from

Conversation

jeroenterheerdt
Copy link

I created a simple REST interface for miflora.

@coveralls
Copy link

coveralls commented Feb 9, 2018

Coverage Status

Coverage remained the same at 92.727% when pulling 2bb3fe6 on jeroenterheerdt:master into 66c6202 on open-homeautomation:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.727% when pulling 2bb3fe6 on jeroenterheerdt:master into 66c6202 on open-homeautomation:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.727% when pulling 2bb3fe6 on jeroenterheerdt:master into 66c6202 on open-homeautomation:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.727% when pulling 2bb3fe6 on jeroenterheerdt:master into 66c6202 on open-homeautomation:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.727% when pulling 2bb3fe6 on jeroenterheerdt:master into 66c6202 on open-homeautomation:master.

@ChristianKuehnel
Copy link
Collaborator

@jeroenterheerdt thanks for the contribution.

Some questions:

  • Does it make sense to add all of that to the miflora library? Right now the focus is to offer a library, not an appliation. Would it make sense to provide a separate application as a separate instead (see https://github.com/open-homeautomation/miflora#projects-depending-on-miflora)?
  • What is the use case? Run miflora as deamon and allow others to poll data from it?
  • Shouldn't we put the MAC on the URL as well? Something like /api/v1.0/11:22:33:44:55:66/moisture?
  • If I understand the life cycle correctly, you instantiate a new poller object with every call. So that means with every call you directly talk to the sensor, right? Would it make sense to add some sort of caching?

@jeroenterheerdt
Copy link
Author

Great questions. I whipped this up quickly because (use case) : the reach of Bluetooth in my situation is not enough, so I build this rest api to run on a gateway. The gateway talks to the sensor and my consumer uses the rest api. It works fine this way. I was not planning to introduce caching etc. Thought the least I could do is share the code back to maybe serve as an extra demo or sample.

@ChristianKuehnel
Copy link
Collaborator

Are you aware of the existing solutions for this problem using MQTT (instead of REST)?

@jeroenterheerdt
Copy link
Author

Well yes, but why stand up MQTT on a limited chip when you can have a simple rest interface? Regardless, include the code or not. I do not mind, it serves my purpose. If you do not want it, fine by me.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Feb 9, 2018

I'm against the addition of such a feature here. "miflora" is a library for a pretty obvious reason - to decouple the hardware access from the use case specific integration.

To that end I've for example developed the MQTT daemon but a REST API based on the library is also a great idea! Your question if REST is better then MQTT can't be answered... Which is more useful solely depends on your use case, don't you agree?
May I suggest to push the specific code to it's own repository depending on miflora, then adding it to the list here: https://github.com/open-homeautomation/miflora#projects-depending-on-miflora
?

If you want to profit from my already existing systemd service unit implementation you are also invited to add your functionality in https://github.com/ThomDietrich/miflora-mqtt-daemon as a PR ;)

@ChristianKuehnel
Copy link
Collaborator

I agree with @ThomDietrich
Services on top if the library should be implemented in another project.

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

Successfully merging this pull request may close these issues.

4 participants