Add Libre Hardware Monitor integration
Proposed change
The current Open Hardware Monitor legacy integration is for a software that has not been updated in more than 4 years. It's successor is Libre Hardware Monitor.
While the legacy integration technically also works for Libre Hardware Monitor, it has many issues and is not maintained.
This integration has been rewritten from scratch following the best practices from the current developer docs, providing:
- config flow
- ~configurable scan interval~
- ~add only user selected hardware devices~
- entities grouped by hardware device
- ~reconfigure anytime via UI reconfigure flow~
- robust against hardware changes on the host system
- much improved code complexity and readability
- extensive test coverage
- quality scale: silver
In my opinion, we should deprecate the legacy integration and redirect users to this one.
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] 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: n/a
- This PR is related to issue: n/a
- Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/37941
- Link to developer documentation pull request: n/a
- Link to frontend pull request: n/a
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) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [x] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest. - [x] 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.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Hi @tr4nt0r thanks for the feedback.
Regarding the reconfigure flow: sure I can take this out and submit it in a follow-up PR.
Regarding the configurable scan interval: could you point me to the developer docs where it is outlined that it cannot be configurable? I found the custom polling interval docs before, but it does not really say that a configurable scan interval is disallowed. I just want to note that I did set limitations to adhere to HA's minimum polling interval of 5 seconds. I feel like for a System Monitor it is vital to allow the user to specify their desired polling interval - ideally in a intuitive way during integration setup. But of course, if this is not desired then I can set a sensible default value and redirect to the custom polling interval docs on the integration docs.
Please note that someone is taking on the work on open hardware monitor and there's a PR for that at the moment.
Is the API of both applications the same?
Hi @joostlek wow, what are the odds after years of legacy status... Not sure what to make of this.
Here's my thought process when I started working on implementing this integration from scratch:
- Open Hardware Monitor is legacy software, nobody should use it anymore. Instead they should use Libre Hardware Monitor. The integration name should reflect this, guiding new users intuitively to the right integration.
- The new integration should reflect the current state of development i.e. fulfill at least all the bronze quality requirements.
After a quick look at the other PR for openhardwaremonitor, in comparison this new integration provides:
- re-implementation instead of re-factor, avoiding pitfalls that are present in the old integration (and still are there after the re-factor, e.g. if hardware exposed by LHM changes, it will break the integration as sensors are based on indices that will no longer be present/valid)
- allow users to select which devices they want to add to Home Assistant
- high unit test coverage
- updated documentation & higher quality icon
Regarding the API, Libre Hardware Monitor seems to be backwards compatible, which is why the current openhardwaremonitor integration does technically still work. However, I have not tested whether my implementation will work with Open Hardware Monitor installations, as I don't think this is a valid use case anymore. Presumably it should work though.
I am also not saying which one is better, PRs should be well scoped, so I don't think comparing PRs is fair here, as new integrations are always bigger and contain more feature than an integration that is trying to modernize.
cc @lazytarget just as an FYI.
We just discussed this in the architecture meeting, and we think its fine to have them co-exist.
Glad to hear that, thanks for clarifying and sorry for the hassle.
If someone could give me a quick hint why the tests on the CI are complaining about missing data_description translations when to my knowledge those are optional, I'd appreciate it. What am I missing there?
They are not optional for the quality_scale rule :)
wow, what are the odds after years of legacy status... Not sure what to make of this. @Sab44
What ARE the odds?! 😄
Cool to see that we both identify the need for an update, and interesting to see our different takes (even not all to different) Really great job! I see you also put some hours into it. I will see if can test yours out tomorrow
I am also not saying which one is better, PRs should be well scoped, so I don't think comparing PRs is fair here, as new integrations are always bigger and contain more feature than an integration that is trying to modernize.
We just discussed this in the architecture meeting, and we think its fine to have them co-exist.
@joostlek Sounds good. Should I revisit my PR a bit and make sure avoid breaking changes as much as possible? Like continue supporting the YAML configuration?
With the migration to the UI we should import the YAML config and that is an expected breaking change
@Sab44 What ARE the odds?! 😄
Yep, I guess it really was time to finally update this as LHM is such a nice tool and the legacy integration was increasingly becoming a pain point. Thank you too for your work and improvements! Let me know if you find any problems during your tests.
There were a couple of code coverage warnings which are now resolved, so this is now ready for review. If there is anything left to do from my side, please let me know.
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
@joostlek I have addressed all comments, did not resolve them yet so you can check them again. I structured all changes into hopefully easily reviewable commits, but I can of course squash them if desired. The docs PR is also updated to reflect changes made here.
Please send me a message on discord or put your username here so we can add you to the right groups
Add me: sab.44