Add Compit integration #132164
Add Compit integration #132164
Conversation
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @Przemko92
It seems you haven't yet signed a CLA. Please do so here.
Once you do that we will be able to review and accept this pull request.
Thanks!
home-assistant
bot
added
cla-needed
has-tests
new-integration
integration: compit
labels
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| device.device_class, | ||
| device.type, | ||
| ) | ||
| state = await self.api.get_state(device.id) |
There was a problem hiding this comment.
Not strictly necessary, but we could have a get_devices() method on the library that returns the updated list of DeviceInstance (with state updated).
That way we could make the logic simpler here.
There was a problem hiding this comment.
I will refactor it in the future releases
| ) | ||
| except ValueError as e: | ||
| _LOGGER.warning("Value error: %s", e) | ||
| return False |
There was a problem hiding this comment.
raise ConfigEntryError. See https://developers.home-assistant.io/docs/core/integration-quality-scale/rules/test-before-setup
| return True | ||
|
|
||
| _LOGGER.error("Authentication API error") | ||
| return False |
There was a problem hiding this comment.
raise ConfigEntryError instead
| def current_temperature(self) -> float | None: | ||
| """Return the current temperature.""" | ||
| value = self.coordinator.data[self.device.id].state.get_parameter_value( | ||
| "__tpokojowa" |
There was a problem hiding this comment.
can we move this to a const instead? same for the other parameters
| device.type, | ||
| ) | ||
| continue | ||
| self.devices[device.id] = DeviceInstance(device_definition) |
There was a problem hiding this comment.
lets rename devices to device_instances or something, because otherwise it looks like it contains the actual devices instead of definitions
| self._hvac_mode = None | ||
|
|
||
| @property | ||
| def device_info(self) -> DeviceInfo | None: |
There was a problem hiding this comment.
since all the data is available on __init__ you can define _attr_device_info instead.
| self._attr_name = device.label | ||
| self._attr_has_entity_name = True | ||
| self._attr_temperature_unit = UnitOfTemperature.CELSIUS | ||
| self.parameters = { |
There was a problem hiding this comment.
| self.parameters = { | |
| parameters = { |
no need to store it as an instance variable
| "identifiers": {(DOMAIN, str(self.device.id))}, | ||
| "name": self.device.label, | ||
| "manufacturer": MANURFACER_NAME, | ||
| "model": self.device_name, |
There was a problem hiding this comment.
if you define _attr_device_info you can remove device_name from the class
| return [HVACMode.HEAT, HVACMode.COOL, HVACMode.OFF] | ||
|
|
||
| @property | ||
| def preset_mode(self) -> str | None: |
There was a problem hiding this comment.
can we cache the preset_mode, or does it change even if self._preset_mode remains?
There was a problem hiding this comment.
couldn't we just read/write _attr_preset_mode and avoid storing _preset_mode?
| self._hvac_mode: HVACMode | None = None | ||
| self.set_initial_values() | ||
|
|
||
| def set_initial_values(self) -> None: |
There was a problem hiding this comment.
since this is only called initially, how are the modes updated if a change is made from compit side and not from HA?
Breaking change
Proposed change
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests: