MisterWil / abodepy

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

Fixes motion sensor support #40

Closed shred86 closed 5 years ago

shred86 commented 5 years ago
  1. Changing CONST.STATUS_OFFLINE to CONST.STATUS_ONLINE fixes motion sensors in Home Assistant (shows motion events).
  2. The motion and property decorators were not being used and are not required to get motion status from Abode.
  3. Removed constants not being used and were incorrect anyways (motion status is pulled from "statuses").
coveralls commented 5 years ago

Coverage Status

Coverage remained the same at ?% when pulling 6774b5cf1debd395f68116cb0d4ae024c6eba6fa on shred86:dev into 9ce1959a78896b052212a71aef65db2616bdb80e on MisterWil:master.

shred86 commented 5 years ago

Stupid question but wouldn't it be better to just return the actual status of the device instead of assuming it's offline or open? In the case where a sensor status is not known, I'd rather see something like "Unknown" instead of assuming offline or open.

I tried setting just return self.status without not in but many of the sensor states are incorrect. I assumed self.status was reading the actual status of the sensor from Abode.

Edit: I think I get it now. The component in Home Assistant returns True if the binary sensor is on, otherwise, False is returned. I can't think of any other way to do it besides what I submitted since some device states (like motion sensors) have a state of Online when nothing is being detected.

MisterWil commented 5 years ago

I tried to write the sensor states to default to the "unsafe" value. As in, if it isn't explicitly ONLINE then assume OFFLINE. If it isn't explicitly LOCKED then assume UNLOCKED.

shred86 commented 5 years ago

That makes sense. I guess for me, I'd rather know the state is unknown rather than just an assumed state. It seems like that's a limitation with Home Assistant though as it appears binary sensors either return True or False. I guess you could return None?

MisterWil commented 5 years ago

Returning None might be an option. I'm honestly not familiar off the top of my head how HASS reacts to NONE responses. Might be worth looking in to so it'll return UNKNOWN.

medic459 commented 5 years ago

I apologize in advance for my ignorance in well...everything. I just managed to get a home assistant instance with Abode integration working on my Pi and I noticed that my first alert (zwave) smoke detectors aren't showing up.

I was thinking that I could just buy a zwave usb dongle and work around it that way, but i've read that the devices can really only be attached to 1 zwave network...

So - is there a plan to add things like the smoke detector to abodepy? If not - is there a way for home assistant to see them by querying abode [api]?

Thanks in advance.

MisterWil commented 5 years ago

If they appear on the web or iPhone apps and you can see their statuses, and those statuses will change when they detect smoke, then probably its possible to add them to abodepy, yes.

shred86 commented 5 years ago

Been running this on my “production” Hassio setup for about 4 days now and so far, everything seems to work as expected.

va-lemon commented 5 years ago

I just got a water leak sensor and that too shows as ‘Wet’ at all times in Home Assistant. I’m assuming a fix similar to the motion sensors will be needed to differentiate between online status and moisture detected status?

medic459 commented 5 years ago

Can I make this a feature request? (sorry - i'm new to the whole GitHub thing and my python/coding skills are rudimentary).


From: Mister Wil notifications@github.com Sent: Wednesday, December 26, 2018 3:11 PM To: MisterWil/abodepy Cc: medic459; Comment Subject: Re: [MisterWil/abodepy] Fixes motion sensor support (#40)

If they appear on the web or iPhone apps and you can see their statuses, and those statuses will change when they detect smoke, then probably its possible to add them to abodepy, yes.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/MisterWil/abodepy/pull/40#issuecomment-450018987, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AdeuA1zJaDpqKnhcX_pGVs6v31GT2vCTks5u89fWgaJpZM4ZhMyP.

MisterWil commented 5 years ago

Merged and released in abodepy-0.15.0

shred86 commented 5 years ago

Awesome! Are you submitting an update in HA to bump the Abodepy version? Apologies if I missed it, I just didn't see a pull request for it.

shred86 commented 5 years ago

@medic459 I would just post the JSON data to the issue thread @MisterWil already has open. I'm not entirely sure what is even possible with the Smoke/CO2 sensors. Some of the devices paired to Abode don't really expose any usable data to use. Like the water leak sensor that @va-lemon is talking about, the JSON data doesn't differentiate between when it's "dry" versus "wet", so there's nothing we can do. If you can see the status change in the Abode web or mobile app, then it's something we can work with.

MisterWil commented 5 years ago

Awesome! Are you submitting an update in HA to bump the Abodepy version? Apologies if I missed it, I just didn't see a pull request for it.

I didn't, no. I wasn't sure if you had anything else you needed to change other than the version. I can do so if you want.

shred86 commented 5 years ago

Ah okay. Nope, I don't have anything else with the Abode component in HA. If you could make a pull request with the new version, that would be great! Thanks again for keeping Abodepy alive.

shred86 commented 5 years ago

I went ahead and made a pull request over at Home Assistant.