-
Notifications
You must be signed in to change notification settings - Fork 4
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
Rearchitect to support specified IP addresses and temperature calculation method #17
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not removed from ignore as we may customise this and don't want it comitted.
Deleting this, adding a branch rather than my main so I can try other improvements. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support for manually entering IP address, addresses #9 & #11
A proper config entry rather than re-discover at start, if a device becomes available after HA startup it will now be enabled.
A config option to change the method of calculating the temperature, defaults to average, addresses #16
General tidy up as detailed below.
Version bump to 0.4.0
Tested on my own vertical mount heater with latest firmware, the High value seems to match the app/display for me but this may change for horizontal mount ones, hence allowing user choice on which method to use.
Important implications of changes
T-Smart devices need to have a fixed IP address since we are now storing this rather than discovering on start, the readme is updated to state this.
Users will have to remove/re-add devices so that the IP address is stored in the config, one time annoyance that should be mentioned in the release notes.
Notes on changes
Moved tsmart library into custom component to allow rearchitecture of both, can be moved back into library but it makes it tricky to make changes when there's that dependency.
Re-ordered params of TSmart class, mandatory IP but optional id & name to allow resolution later.
New async_get_configuration method within library to allow getting of device_id and name, don't get all fields as we won't need them, might be nice to decode the hw/firmware version as you can add them to HA devices view.
New request_successful property to show failure on init.
Exposed temperature high, low and average as attributes in climate sensor.
Coordinators now loaded at platform init and shared between entities of the same device.
Discovery now in config flow with timeout prompting for manual IP.
Config flow now allows re-configure to change IP.
Config entries store IP address, id and name of device and used at component load rather than discovery, allows device to come online after HA start and be recognised.
Added min HA version checking to be 2024.2 to cope with new climate requirements.
Updated readme with HACS easy install link, also specifying to use a fixed IP.
Add VS Code Dev Container allowing you to run a HA instance in docker, doesn't interfere with other dev workflow. UDP seems to not work with docker yet but having a container is still good for other areas.