Skip to content
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

Reduce code duplication in entities #111

Merged
merged 4 commits into from
Mar 25, 2021
Merged

Conversation

KapJI
Copy link
Collaborator

@KapJI KapJI commented Mar 25, 2021

Simplify class structure.

  • One base class for common code, remove mixin.
  • All specialisation is done on sensor level.
  • Remove model with invalid VERSION for device_info. We don't really want to maintain version in different places.
  • Make GoogleHomeDeviceSensor constructor the same as for other sensors. If there is no such device in coordinator_data, set state to None, it should render as unknown in HA.
  • Rename GoogleHomeAlarmSensor to GoogleHomeAlarmsSensor, same for Timers.
  • Delete non existing device class for GoogleHomeDeviceSensor.
  • Update docstrings.

@KapJI KapJI added the refactoring Refactoring label Mar 25, 2021
@KapJI KapJI requested a review from leikoilja March 25, 2021 09:19
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, thank you 💥

The only thing I'm not really happy about is duplicating all the methods in sensors/alarms. But i think we can reduce that too :)

custom_components/google_home/entity.py Outdated Show resolved Hide resolved
custom_components/google_home/sensor.py Outdated Show resolved Hide resolved
custom_components/google_home/sensor.py Outdated Show resolved Hide resolved
custom_components/google_home/sensor.py Outdated Show resolved Hide resolved
custom_components/google_home/sensor.py Outdated Show resolved Hide resolved
custom_components/google_home/sensor.py Outdated Show resolved Hide resolved
@DurgNomis-drol
Copy link
Collaborator

Thanks @KapJI for taking the initiative to do this. 🚀😁

@leikoilja
Copy link
Owner

@KapJI, are you still working on it? Please re-request review when you want some of us to review :)

@KapJI KapJI requested a review from leikoilja March 25, 2021 15:10
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!
I'd be happy if we can try it locally just to make sure it doesn't break anything before merging it :)

Comment on lines +53 to +61
def get_device(self):
"""Return the device matched by device name
from the list of google devices in coordinator_data"""
matched_devices = [
device
for device in self.coordinator.data
if device.name == self.device_name
]
return matched_devices[0] if matched_devices else None
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it! Thanks for implementing it. Maybe not the most looking solution, but hopefully more robust

@KapJI
Copy link
Collaborator Author

KapJI commented Mar 25, 2021

I tested it locally, works as before :)

@KapJI KapJI merged commit f7813f8 into leikoilja:master Mar 25, 2021
@KapJI KapJI deleted the dedup-code branch March 25, 2021 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants