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

Refactored the code and added more documentation for API / MQTT #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

elad-bar
Copy link

thanks for the amazing integration!

Performed the following changes:

  • Seperate the code to different classes for easier maintainance
  • Added pre-commit to rearrenge the code according to the standards
  • Added support for Docker
  • Moved all documentation (except README.md) to docs directory
  • Exposed all parameters as environment variables (for docker)
  • Splitted & Modified endpoints to GET (status) and POST (arm) instead of just one over GET
  • Added error codes to better describe the issue
  • Added more log messages to understand the flow
  • MQTT - Simplify command message (without command)
  • API - Simplify command request (without command)
  • Added examples of requests, responses and MQTT messages
  • Moved content of HA YAML configuration to dedicated docs/homeassistant.md page

Created docker image in docker hub, based on the forked code, if you will agree to the pull request and create the image, I will remove it, please let me know

thanks

@deiger
Copy link
Owner

deiger commented Jan 4, 2021

Hi Elad,
Sorry for not handling this PR sooner. I'm not sure how I missed it.
I've done some changes recently, including support for MQTT auto-discovery, so I hope you can integrate them into your PR.

Thanks,
Dror

@Jens-Wymeersch
Copy link

@elad-bar I'm trying to get this integration to work. I have HP896, and I'm wondering if it will ever work with this version. I heard from PIMA itself that the chances that it will work are quite slim. Wondering which version you have and how you did the implementation.

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.

3 participants