-
-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add switch platform for hp ilo integration #32209
Conversation
Codecov Report
@@ Coverage Diff @@
## dev #32209 +/- ##
==========================================
- Coverage 94.75% 94.68% -0.08%
==========================================
Files 775 773 -2
Lines 56154 55926 -228
==========================================
- Hits 53209 52951 -258
- Misses 2945 2975 +30
Continue to review full report at Codecov.
|
|
||
def setup_platform(hass, config, add_entities, discovery_info=None): | ||
"""Set up the HP iLO switch.""" | ||
hostname = config.get(CONF_HOST) |
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.
Please use dict[key] for required config keys as well.
"""Support for powering on/off server with HP iLO.""" | ||
import logging | ||
|
||
import hpilo |
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.
Only import the login function and maybe rename it to something more self-explanatory.
|
||
def __init__(self, hass, name, hp_ilo): | ||
"""Initialize the HP iLO switch.""" | ||
self._hass = hass |
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.
Self.hass is not used
"""Initialize the HP iLO switch.""" | ||
self._hass = hass | ||
self._name = name | ||
self.hp_ilo = hp_ilo |
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.
Please rename this to client or server since it resembles the name of the library too much IMHO.
So why add a separate platform to the integration? I rather would recommend converting it to an actual integration, having a single configuration entry to create both in one go. |
yes i agree, but not sure how to do this as I am quite new to developing on home assistant |
Most of the integrations we have, can serve as an example for this. What you basically need to do, on a high level overview, is process the configuration in https://developers.home-assistant.io/docs/creating_component_generic_discovery/ |
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. |
Proposed change
Adds switch platform to the HP ILO integration
Type of change
Example entry for
configuration.yaml
:Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale: