MisterWil / abodepy

A thin Python wrapper for the Abode alarm API
MIT License
50 stars 17 forks source link

Add support for CUE automations #63

Closed shred86 closed 4 years ago

shred86 commented 4 years ago

Abode legacy automations and quick actions are being retired on 28 Feb. This pull request adds support for CUE automations which resolves #41. I've tested this along some Home Assistant changes I've made and so far everything is working as expected. I'm having issues with the tests_automation_enable method in test_automation.py, but I'll comment directly in that part of the code in this pull request (it's all commented out currently). I'll definitely have more commits as I continue to test and revise the code but I was hoping to start getting some other eyes on.

coveralls commented 4 years ago

Coverage Status

Coverage decreased (-0.2%) to 85.312% when pulling fb40afc11ce9dc6d5433bcfc938a52963aacfd8f on shred86:cue into 32d687a30aca09aba5c44d4de7806fe94720eec6 on MisterWil:master.

shred86 commented 4 years ago

Coverage decreased because I deleted a test that wasn't needed anymore since quick actions will no longer be a thing (everything will become a CUE automation, to include manually triggered which is the replacement for quick actions). This part of abodepy init.py in the get_automations method that's not being covered.

if generic_type:
            automations = []
            for automation in self._automations.values():
                if (automation.generic_type is not None and
                        automation.generic_type in generic_type):
                    automations.append(automation)
            return automations

I'm not sure if this is even needed anymore since quick actions will be removed.

MisterWil commented 4 years ago

Coverage decreased because I deleted a test that wasn't needed anymore since quick actions will no longer be a thing (everything will become a CUE automation, to include manually triggered which is the replacement for quick actions). This part of abodepy init.py in the get_automations method that's not being covered.

if generic_type:
            automations = []
            for automation in self._automations.values():
                if (automation.generic_type is not None and
                        automation.generic_type in generic_type):
                    automations.append(automation)
            return automations

I'm not sure if this is even needed anymore since quick actions will be removed.

Well given that you removed the concept of a generic_type from the automations then you should go ahead and remove it from init.py. :-)

Otherwise everything looks good. Since both legacy automations and quick actions are going away anything that references them should be removed from the code.

shred86 commented 4 years ago

Removed the portion of the code mentioned above. Also removed the generic_type and sub_type properties from the automation class. The reason being for generic_type is because there's obviously only just automations now and the sub_type was returning a field that is an empty string in all the JSON data I've looked at for CUE automations (key is subType).

shred86 commented 4 years ago

Not sure what I changed caused the coverage to decrease. The stuff being highlighted as changed is in init.py for abodepy in the Abode class. Specifically:

    def refresh(self):
        """Do a full refresh of all devices and automations."""
        self.get_devices(refresh=True)
        self.get_automations(refresh=True)

Doesn't make any sense to me. This should be triggered based on both test_device.py and test_automation.py. There are test cases where a device or automation id and refresh=True being passed to get_device() and get_automation() which should call refresh().

@property
    def uuid(self):
        """Get the UUID."""
        return self._cache[CONST.UUID]

Not sure if this was tested in the past but I'm not even sure what this property is used for. I don't see it being called by anything in abodepy. I'm sure it's there for a reason, just can't figure it out.

if version and version.lower().startswith('minipir'):
        device_json['generic_type'] = CONST.TYPE_OCCUPANCY

This is within the _new_sensor method. I don't think it was ever tested before. I guess we just need to add another mocked device (device_type.pov I believe). I have one of these sensors so I can probably update the test for this at some point.

Edit: Quick look at my JSON data for the two devices I have classified as "Occupancy" and "device_type.pov" (keypad and multi sensor), both show the "version" field as "0002" and "0001" respectively. I wonder if there's been some changes in how Abode is classifying sensors.

Edit 2: After further looking at the _new_sensor, new_sensor and constants.py module, it seems like we could rework this since I think we have a good idea of what each device type_tag actually is now. Maybe something for another PR at some point...