search

ateodorescu / home-assistant-ipmi

IPMI connector for Home Assistant
MIT License
36 stars 8 forks source link

Adding IPMI_SERVER_HOST as configurable property #23

Closed MNeverOff closed 5 months ago

MNeverOff commented 5 months ago

I've took the liberty to restructure the Integration slightly to allow for UI configuration of ipmi-server host.

This is necessary when HA is running within a docker container and the ipmi-server is set up as a separate docker container as well. In my setup they live on the same docker network, therefore I'm using hostname instead of localhost (i.e. http://ipmi_server with port 80), and I can achieve this by modifying the const.py but editing it in the UI feels apt.

The only concern is, since I'm not well versed with HA Integrations, is how to handle existing configs, maybe using https://developers.home-assistant.io/docs/core/platform/significant_change? I upped the version to 1.3.0 but if it'll be a breaking one it might be necessary to up to 2.0.0

Lastly, I took the liberty to cleanup some formatting to make it consistent, albeit it might not be "correct" as I don't have a linter, lmk if you'd like me to adjust it. SCR-20240202-bzbi-2

ateodorescu commented 5 months ago

@MNeverOff Thank you for your contribution!

czuares commented 5 months ago

This change does not appear to be backwards compatible. Any recommendations to get this working again without starting from scratch?

I reverted to a backup but I am happy to test out a new version if you'd like.

ateodorescu commented 5 months ago

@czuares I have fixed the issue and created a new release 1.3.1. Sorry for the invonvenience.

czuares commented 5 months ago

@ateodorescu I just upgraded without issue, thank you!

MNeverOff commented 5 months ago

Yeah, apologies, I suspected something like this might happen but was lacking the knowledge to handle it, thank you @ateodorescu for fixing that and sorry for adding that extra bit of headace!