-
Notifications
You must be signed in to change notification settings - Fork 20
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
Draft: feat: make brand configurable #2
Conversation
Please not that I marked it as Draft because I'm not able to test this with a baxi thermostat, so it would be great if someone could help me with that. |
Hi Mart, thank you for your contribution. I found no issues in the code, thus approving the merge. I do not currently have time to develop more features or refactor the code, but any ideas are welcome for the future. |
Hi Freitdav, Thank you for the review. You approved the merge but did not merge it but closed it 🤔 Was that an mistake? |
Yup, sorry, missclicked. It should be fine now. Thank you for your work and greetings from the Czech Republic. |
|
||
|
||
SUPPORT_FLAGS = SUPPORT_TARGET_TEMPERATURE | SUPPORT_PRESET_MODE | ||
CLIMATE_SCHEMA = { | ||
vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string, | ||
vol.Required(CONF_USERNAME): cv.string, | ||
vol.Required(CONF_PASSWORD): cv.string, | ||
vol.Required(CONF_PAIR_CODE): cv.string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmoerdijk Did you mean to remove this line or was that a mistake when adding the new option to configure the brand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was not itentional indeed. The paringcode is required for this to work. It probably works for me because i had it configured before. I had it disabled during testing.
@@ -61,7 +61,8 @@ async def _store_token(self, token): | |||
self.token = token | |||
|
|||
async def _login(self): | |||
api_endpoint = self.endpoints["LOGIN"] | |||
api_endpoint = f"https://remoteapp.bdrthermea.com/user/{self._brand}/login", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something also seems wrong here: requests.exceptions.InvalidSchema: No connection adapters were found for "('https://remoteapp.bdrthermea.com/user/remeha/login',)"
.
Reverting to a fixed string with that same URL fixes that problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is strange it works fine for me. Currently travelling, could have a look in 2 weeks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed the , at the and of this line:
api_endpoint = f"https://remoteapp.bdrthermea.com/user/{self._brand}/login"
I have added the line:
vol.Required(CONF_PAIR_CODE): cv.string,
Restart HA and add integration
Via the eTwist app I have generated the pairing code: Home > Settings > Connect devices and services
Invite someone, this will show a pairing ID
For me it is working now, thanks for all the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey good that you found it. Could you make an PR including your fixes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Scratch that I made a PR for it, thanks for debugging: #6
Hey guys, im fairly new to the whole writing your own code thing. I have the intergration installed but i cant seem to work out how to actually configue my thermostat to it. Can u help? |
Goto settings > devices and Services and press the big add Integration Button. Search for BDR and it should pop up |
This pull request works like a charm with my Remeha Calenta 40c: Steps I've done to get this working in my HA (version 2022.9.7 / OS 8.3 / HACS 1.26.0)
|
Should be in the main repo now, so please use that. |
Works with my Remeha Avanta with eTwist, thanks! |
Hi, I used this integration and it worked fine with Home Assistant. However Remeha has moved me to the Remeha App (was Etwist). Forget this, I had to move to another plugin - see the issues page |
Hi,
Thanks for your work on this integration.
I have a remeha thermostat and was able to use this integration after modifying it a bit by changing baxi to remeha.
I decided to make the brand configurable and add it to the initial configuration screen. For now I only added baxi and remeha. I also fixed a small bug that was causing the state to be set incorrectly after changing it resulting in an error in lovelance.
There is still a bug in there that makes it in possible to set the mode to "frost" or "holiday" but i rather fix that in a separate MR ( If I get around fixing that)
Are you accepting PR's like this?