Closed pifou25 closed 4 years ago
Seems like a pretty reasonable idea! I have a few random questions about the design of where to put this mapping, etc.:
device_classes
would be helpful?DeviceInfo
class… maybe it should just be a module-level global?device_classes = { DeviceType.AC: ACDevice, ... }
. The only trick is that this may need to be in a separate module to avoid cyclic imports.I first imagined to add such method in the Client class next to get_device, something like a "get_device_class(self, device_id)" what do you t hink of that ? About "registration" : I didn't want to populate the dict mapping into client.py because it required to import every other files into client.py ... What I can try, is to define and import into the method:
def get_device_class(self, device_id):
from ac import ACDevice
...
device_classes = {DeviceType.AC: ACDevice, ... }
Is it a good practice ? Otherwise, I think the best way is to add a global registration function into util.py
Ah, got it, cool… such a method makes sense to have to get the specific class for a given DeviceInfo
, but maybe the mapping should be available outside of the class?
And yeah, that "local import" thing is not a bad idea. What do you think about this?
device_classes
that uses the local import
and returns the dictionary.get_device_class
method to DeviceInfo
, as you proposed, that just calls device_classes
and then looks up the right class to return.Does that seem simple enough?
Hello, I made it simple I hope so :) And I also modified the example.py to use the new method. I successfully tested it with an AC device.
Awesome; this seems great! Thanks again!!
This make it easy to get the relevant subclasse of Device according to the DeviceType of the model info