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

preparation for multi device type support #307

Merged
merged 5 commits into from
May 3, 2023

Conversation

simon-schuster
Copy link
Contributor

Short explanation on what i did here:

  • PyViCareDevice is now a base class for all different types of devices
  • PyViCareHeatingDevice contains most of the functionality previously contained by the PyViCareDevice
  • All types of heating devices now inherit from PyViCareHeatingDevice
  • Sensors and radiators just inherit from PyViCareDevice
  • PyViCareGenericDevice is the new generic device which has to inherit all types of devices which will be added
  • added directories for better overview of the different device types

@woehrl01
Copy link
Collaborator

Hi @simon-schuster,

Thank you very much for your pull request. I'm not feeling that comfortable with the changes. If we change the root device name and introducing folders, we will introduce a breaking change for everyone, where I don't see a big benefit. I prefer just introducing a new class for heating devices and keep the structure as is. Please also remove the new generic device, and simply use the new heating device.

@simon-schuster
Copy link
Contributor Author

Hi @woehrl01,

Thanks for taking the time to review.
So you mean just copy the current device code and call it heating device, but leaving the current device as it is?
Wouldn't that be a lot duplicated code and also maybe hard to understand where to add new functions in the future?

@woehrl01
Copy link
Collaborator

No, the idea is to just add the HeatingDevices, move the stuff from Device to the new HeatingDevice, and use the HeatingDevice as the generic device.

@simon-schuster
Copy link
Contributor Author

@woehrl01 I have now reverted the folder structure to how it was before and move everything from device to heating device.
Should I also revert the recursive file search in tests/test_TestForMissingProperties.pyor is it ok to leave it as it is for possible folders in the future?

tests/test_TestForMissingProperties.py Outdated Show resolved Hide resolved
tests/test_TestForMissingProperties.py Outdated Show resolved Hide resolved
tests/test_TestForMissingProperties.py Outdated Show resolved Hide resolved
PyViCare/PyViCareRoomSensor.py Outdated Show resolved Hide resolved
@woehrl01
Copy link
Collaborator

@simon-schuster thank you. I left some comments, you can leave the changes if you make some additional ones 😊

@woehrl01 woehrl01 merged commit 7ced3ba into somm15:master May 3, 2023
CFenner pushed a commit to CFenner/PyViCare that referenced this pull request Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants