core
core copied to clipboard
Improvements for Habitica integration, adds icons and translations
Breaking change
Entities didn't have unique id before, so the sensors will be re-created.
Proposed change
This PR gives the Habitica integration some TLC in preparation for migrating the sensors with tasks to the todo platform.
SensorEntityDescriptions were added to the normal sensors, as well as translation strings and icon translations. Also sensors now have a unique id.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [x] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request:
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Ruff (
ruff format homeassistant tests
) - [ ] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [ ] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [ ] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest
. - [ ] New or updated dependencies have been added to
requirements_all.txt
.
Updated by runningpython3 -m script.gen_requirements_all
. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc
.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Hey there @asmfreak, @leikoilja, mind taking a look at this pull request as it has been labeled with an integration (habitica
) you are listed as a code owner for? Thanks!
Code owner commands
Code owners of habitica
can trigger bot actions by commenting:
-
@home-assistant close
Closes the pull request. -
@home-assistant rename Awesome new title
Renames the pull request. -
@home-assistant reopen
Reopen the pull request. -
@home-assistant unassign habitica
Removes the current integration label and assignees on the pull request, add the integration domain after the command. -
@home-assistant add-label needs-more-information
Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request. -
@home-assistant remove-label needs-more-information
Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.
It looks good to me, but I'm a bit rusty with this code, so it's better to wait for @leikoilja to comment.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Should we also delete the old SENSOR_TYPE?
I really wanted to delete it, but then realized it is imported from init and used in the service schema but i didn't want to touch the service yet, so i left it in for now. I think i might just remove the service, as it seems pretty complicated to use and it kept throwing exceptions.
Wait why is it used in the service?
Wait why is it used in the service?
i don't know, didn't want to explore further, that's something i postponed for later xD
Seems it was still in the schema but unused in the service and marked as deprecated, so i think it is save to remove SENSOR_TYPES
@joostlek There are also these Task sensors, that i haven't touched yet, because I'm planning to migrate them to todo entities. Should I remove them for now or with the PR that introduces the todo platform?
They should be deprecated for 6 months
Oh, well, then maybe i should add the max. health sensor back. I removed it because 50 is a fixed value for maximum health so that sensor is quite useless. What do I have to do for deprecating sensor entities?
@joostlek ok, i added deprecation warnings for the old task sensors, hope that does the job. :D Will be working now on the todo entities. Should i add them to this PR or in a new PR?
The PR is now starting to do too much, let's try to scope it down to its original scope
yeah, also think so, the deprecation warnings should be in the same PR as the addition of the todo entities